4615 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
claude-ceo-assistant	85b09659e6	Merge pull request 'fix(ci): add scripts/** to publish-workspace-server-image path filter' (#32) from fix/publish-path-filter-add-scripts into main	2026-05-07 15:19:12 +00:00
devops-engineer	6de3c1ccd2	fix(ci): add scripts/** to publish-workspace-server-image path filter ... scripts/clone-manifest.sh runs inside the platform Dockerfile build, so a change to that script needs to retrigger publish. Without it, the prior fix (clone via Gitea + lowercase org) didn't trigger this workflow because scripts/ wasn't in the path filter. Also serves as the file change to satisfy the path filter for THIS push, retriggering publish-workspace-server-image now.	2026-05-07 08:18:53 -07:00
claude-ceo-assistant	d4256b9d83	Merge pull request 'fix(scripts): clone-manifest.sh — use Gitea + lowercase org slug (Class G)' (#31) from fix/clone-manifest-gitea into main	2026-05-07 15:18:09 +00:00
devops-engineer	8313b2a7a7	fix(scripts): clone-manifest.sh — use Gitea + lowercase org slug ... Post-2026-05-06 GitHub-org suspension: scripts/clone-manifest.sh was still pointing at https://github.com/${repo}.git, so the Docker build for workspace-server'\''s platform image fails at: fatal: could not read Username for 'https://github.com': No such device or address with no credentials available in the build container. Fix: clone from https://git.moleculesai.app/${repo}.git instead. manifest.json'\''s repo paths still read 'Molecule-AI/...' (the historic GitHub slug, mixed-case); Gitea lowercases the org component to 'molecule-ai/...'. Lowercase the org segment on the fly with awk so we don'\''t need to rewrite every manifest entry. Local verify: bash -n passes, lowercase transform produces correct Gitea paths, anonymous git clone of one of the manifest plugins over HTTPS to git.moleculesai.app succeeds. Class G in the prod-ship CI sweep — same shape as the github.com ref Harness Replays hits, this is the second instance found.	2026-05-07 08:17:58 -07:00
claude-ceo-assistant	566c095571	Merge pull request 'chore(ci): trigger publish-workspace-server-image (path-filter satisfaction)' (#30) from chore/touch-publish-workflow-to-trigger into main	2026-05-07 15:12:22 +00:00
devops-engineer	694a036a7f	chore(ci): trailing newline to retrigger publish-workspace-server-image (path-filter requires workflow file change)	2026-05-07 08:12:10 -07:00
claude-ceo-assistant	8c1dbc6ba5	Merge pull request 'chore(ci): retrigger publish-workspace-server-image post AWS secrets registration' (#29) from chore/retrigger-publish-post-aws-secrets into main	2026-05-07 15:08:03 +00:00
devops-engineer	72d0d4b44e	chore(ci): retrigger publish-workspace-server-image post AWS secrets registration	2026-05-07 08:07:46 -07:00
claude-ceo-assistant	52e61d4704	fix(ci): cherry-pick PR#23 — drop github-app-auth plugin checkout (#28)	2026-05-07 14:52:47 +00:00
devops-engineer	10e510f50c	chore: drop github-app-auth + swap GHCR→ECR (closes #157, #161) ... Two coupled cleanups for the post-2026-05-06 stack: ============================================ The plugin injected GITHUB_TOKEN/GH_TOKEN via the App's installation-access flow (~hourly rotation). Per-agent Gitea identities replaced this approach after the 2026-05-06 suspension — workspaces now provision with a per-persona Gitea PAT from .env instead of an App-rotated token. The plugin code itself lived on github.com/Molecule-AI/molecule-ai-plugin-github-app-auth which is also unreachable post-suspension; checking it out at CI build time was already failing. Removed: - workspace-server/cmd/server/main.go: githubappauth import + the `if os.Getenv("GITHUB_APP_ID") != ""` block that called BuildRegistry. gh-identity remains as the active mutator. - workspace-server/Dockerfile + Dockerfile.tenant: COPY of the sibling repo + the `replace github.com/Molecule-AI/molecule-ai- plugin-github-app-auth => /plugin` directive injection. - workspace-server/go.mod + go.sum: github-app-auth dep entry (cleaned up by `go mod tidy`). - 3 workflows: actions/checkout steps for the sibling plugin repo: - .github/workflows/codeql.yml (Go matrix path) - .github/workflows/harness-replays.yml - .github/workflows/publish-workspace-server-image.yml Verified `go build ./cmd/server` + `go vet ./...` pass post-removal. ======================================================= Same workflow used to push to ghcr.io/molecule-ai/platform + platform-tenant. ghcr.io/molecule-ai is gone post-suspension. The operator's ECR org (153263036946.dkr.ecr.us-east-2.amazonaws.com/ molecule-ai/) already hosts platform-tenant + workspace-template-* + runner-base images and is the post-suspension SSOT for container images. This PR aligns publish-workspace-server-image with that stack. - env.IMAGE_NAME + env.TENANT_IMAGE_NAME repointed to ECR URL. - docker/login-action swapped for aws-actions/configure-aws- credentials@v4 + aws-actions/amazon-ecr-login@v2 chain (the standard ECR auth pattern; uses AWS_ACCESS_KEY_ID/SECRET secrets bound to the molecule-cp IAM user). The :staging-<sha> + :staging-latest tag policy is unchanged — staging-CP's TENANT_IMAGE pin still points at :staging-latest, just with the new registry prefix. Refs molecule-core#157, #161; parallel to org-wide CI-green sweep.	2026-05-07 07:48:51 -07:00
claude-ceo-assistant	6fac24e3de	Merge pull request 'fix(workspace-server): SSOT-route container check + 422 on external runtimes (closes #10)' (#12) from fix/issue10-runtime-aware-plugin-install into main	2026-05-07 11:27:52 +00:00
claude-ceo-assistant	f51722411b	Merge branch 'main' into fix/issue10-runtime-aware-plugin-install	2026-05-07 11:26:14 +00:00
claude-ceo-assistant	f0015bff81	Merge pull request 'fix(workspace-server): default-bind to 127.0.0.1 in dev-mode fail-open (closes #7)' (#8) from fix/s8-bind-loopback-dev into main	2026-05-07 11:25:48 +00:00
claude-ceo-assistant	b72d1d3f26	Merge branch 'main' into fix/issue10-runtime-aware-plugin-install	2026-05-07 11:25:24 +00:00
claude-ceo-assistant	a674a6547e	Merge branch 'main' into fix/s8-bind-loopback-dev	2026-05-07 11:25:20 +00:00
claude-ceo-assistant	f2f5338183	Merge pull request 'fix(ci): lowercase 'molecule-ai/' in cross-repo workflow refs' (#17) from fix/lowercase-org-slug into main	2026-05-07 10:38:12 +00:00
security-auditor	e01077be38	fix(ci): lowercase 'molecule-ai/' in cross-repo workflow refs ... Gitea is case-sensitive on owner slugs; canonical is lowercase `molecule-ai/...`. Mixed-case `Molecule-AI/...` refs fail-at-0s when the runner tries to resolve the cross-repo workflow / checkout. Same fix as molecule-controlplane#12. Mechanical case-correction; no behavior change beyond making CI resolve again. Refs: internal#46 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-07 01:00:10 -07:00
security-auditor	c1de2287fd	fix(workspace-server): SSOT-route container check + 422 on external runtimes ... Two coupled fixes for molecule-core#10 (plugin install 503 vs status=online split-state): 1. SSOT for "is this workspace's container running" — `findRunningContainer` in plugins.go used to carry its own copy of `cli.ContainerInspect`, which collapsed transient daemon errors into the same `""` return as a genuinely-stopped container. Healthsweep's `Provisioner.IsRunning` handled the same input correctly (defensive). Promote the inspect logic to `provisioner.RunningContainerName`, route both consumers through it. Transient errors get a distinct log line on the plugins side so triage doesn't confuse a flaky daemon with a stopped container. 2. Runtime-aware Install/Uninstall — `runtime='external'` workspaces have no local container; push-install via docker exec is meaningless. They pull plugins via the download endpoint instead (Phase 30.3). Without a guard they fell through to `findRunningContainer` and 503'd with a misleading "container not running." Add an early 422 with a hint pointing at the download endpoint. The two fixes are independent: (1) preserves correctness when the SSOT helper is later modified; (2) eliminates the persistent split-state on the 5 external persona-agent workspaces in this DB (and on tenant deployments hitting the same shape). * `internal/provisioner/provisioner.go` — new `RunningContainerName(ctx, cli, id) (string, error)` with three documented outcomes (running / stopped / transient). `Provisioner.IsRunning` now wraps it; behavior preserved. * `internal/handlers/plugins.go` — `findRunningContainer` shimmed onto `RunningContainerName`; new `isExternalRuntime(id)` predicate. * `internal/handlers/plugins_install.go` — Install + Uninstall reject external runtimes with 422 + hint, before the source-fetch step. * `internal/handlers/plugins_install_external_test.go` — 5 cases: external→422, uninstall-external→422, container-backed-falls-through, no-runtime-lookup-fails-open, lookup-error-fails-open. * `internal/handlers/plugins_findrunning_ssot_test.go` — two AST gates pin the SSOT routing so future PRs can't silently re-introduce the parallel impl. Mutation-tested: reverting either consumer to a direct `ContainerInspect` makes the gate fail. Refs: molecule-core#10 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-06 22:58:20 -07:00
security-auditor	f3187ea0c1	fix(workspace-server): default-bind to 127.0.0.1 in dev-mode fail-open ... In dev mode (`MOLECULE_ENV=dev|development`, `ADMIN_TOKEN` unset) the AdminAuth chain fails open by design so canvas at :3000 can call workspace-server at :8080 without a bearer token. Combined with the existing wildcard bind on `:8080`, that exposed unauthenticated `POST /workspaces` to any same-LAN peer (S-8 in the audit RFC v1). Couple the bind narrowness to the same signal that drives the auth fail-open: when `middleware.IsDevModeFailOpen()` returns true, default the listener to `127.0.0.1`. Production (`ADMIN_TOKEN` set) keeps binding to all interfaces — its auth chain is doing the work. Operators who need LAN exposure set `BIND_ADDR=<host>` explicitly. * `cmd/server/main.go` — `resolveBindHost()` precedence: BIND_ADDR explicit > IsDevModeFailOpen() loopback > "" (all interfaces). Startup log line now includes the resolved bind + dev-mode-fail-open state for post-deploy auditing. * `cmd/server/bind_test.go` — 8 t.Setenv table cases covering precedence, explicit overrides, dev/prod env words. Mutation-tested: removing the `IsDevModeFailOpen()` branch makes the dev-mode cases fail with "" vs "127.0.0.1". Refs: molecule-core#7 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-06 22:29:24 -07:00
claude-ceo-assistant	f92ba492de	Merge pull request 'test(org_import): tighten sqlmock regex on lookupExistingChild (#2872 PR-B)' (#3) from fix/2872-sqlmock-regex-tightening into staging	2026-05-07 00:19:40 +00:00
Hongming Wang	00cfe51df7	test(org_import): tighten sqlmock regex on lookupExistingChild (#2872 PR-B) ... The five `mock.ExpectQuery(\`SELECT id FROM workspaces\`)` sites used a loose substring regex that silent-passed three regression shapes #2872 called out: 1. `WHERE parent_id = $2` (drops `IS NOT DISTINCT FROM` — breaks NULL-parent root matching) 2. `WHERE name = $1` only (drops parent_id check entirely — hijacks siblings of the same name across different parents) 3. Drops `AND status != 'removed'` (blocks re-import after Collapse) Extracts a `lookupChildSQLRE` const that anchors all four load-bearing tokens (the SELECT/FROM, the name predicate, the IS NOT DISTINCT FROM predicate, and the status filter). All five ExpectQuery sites now use the same const so a future schema/predicate change fails one place. Mutation-tested per memory feedback_assert_exact_not_substring.md: - Replacing `IS NOT DISTINCT FROM` with `=` fails TestLookupExistingChild_NilParent_MatchesRoot. - Dropping `AND status != 'removed'` fails TestLookupExistingChild_Found_ReturnsIDAndTrue. Note: #2872 PR-A (AST gate strengthening) is already addressed inline — findWorkspacesInsertSQL + TestCreateWorkspaceTree_InsertUsesOnConflictDoNothing pin the ON CONFLICT DO NOTHING shape, which is a strictly stronger gate than the original lookup-before-insert ordering check.	2026-05-06 16:43:42 -07:00
claude-ceo-assistant	55ef3176ed	feat(provisioner): env-driven RegistryPrefix() for workspace template images (#6) ... Allows MOLECULE_IMAGE_REGISTRY env override on the tenant workspace-server. Used to flip from ghcr.io/molecule-ai → private ECR mirror after the GitHub org suspension on 2026-05-06. Default unchanged for OSS users. Closes #6.	2026-05-06 22:51:53 +00:00
claude-ceo-assistant (Claude Opus 4.7 on Hongming's MacBook)	4b074f631b	feat(provisioner): env-driven RegistryPrefix() for workspace template images (#6) ... Add MOLECULE_IMAGE_REGISTRY env var to override the registry prefix used by all workspace-template image references. Defaults to ghcr.io/molecule-ai (unchanged for OSS users); set to an ECR URI in production tenants when mirroring to AWS. Why this matters: GitHub suspended the Molecule-AI org on 2026-05-06 with no warning. Production tenants kept running because they had images cached locally, but any tenant restart (AWS health event, redeploy, OS reboot) would have failed at `docker pull ghcr.io/molecule-ai/...` because GHCR returned 401. This change introduces the seam needed to point new pulls at a registry we control (AWS ECR) by flipping a single env var on Railway. Design (RFC: molecule-ai/internal#6): - New `RegistryPrefix()` function in `provisioner/registry.go` reads MOLECULE_IMAGE_REGISTRY, falls back to "ghcr.io/molecule-ai". - New `RuntimeImage(runtime)` returns the canonical ref using the prefix. - `RuntimeImages` map computed at init via `computeRuntimeImages()` so existing callers that range over it still work. - `DefaultImage` likewise computed via `RuntimeImage(defaultRuntime)`. - `handlers.TemplateImageRef()` switched from hardcoded format string to `provisioner.RegistryPrefix()`. - `runtime_image_pin.go::resolveRuntimeImage()` automatically inherits the prefix change because it reads from `provisioner.RuntimeImages[]` and only re-formats the tag suffix to a digest pin. Alternatives rejected (see RFC): - Multi-registry fallback chain (try ECR, fall back to GHCR): GHCR is locked from outbound for our org, so the fallback never works for us. Adds code complexity for no benefit. - Hardcoded ECR-only switch: couples production code to a specific deployment environment. OSS users self-hosting Molecule would need the upstream GHCR. - Self-hosted Harbor / registry-on-Hetzner: adds a component to operate. Not justified at 3-tenant scale; AWS ECR is mature and IAM-integrated. Auth — deliberately NOT changed in this commit: - For GHCR, the existing `ghcrAuthHeader()` reads GHCR_USER/GHCR_TOKEN. - For ECR, EC2 user-data installs `amazon-ecr-credential-helper` and adds a `credHelpers` entry in `~/.docker/config.json` so the daemon resolves ECR credentials via the EC2 instance role on every pull. The Go code needs no auth change. This keeps the diff minimal. Backwards compatibility: - Additive: env unset → identical behavior to today (GHCR). - Existing tests reference literal `ghcr.io/molecule-ai/...` strings; they continue to pass under the default prefix. - `RuntimeImages` map preserved for callers that iterate it. - No interface, schema, API, or migration version bump needed. Security review: - No untrusted input: MOLECULE_IMAGE_REGISTRY is set at deploy time (Railway env, EC2 user-data), not by users. - No expanded data collection or logging changes. - No new permissions: ECR pull permission is a future user-data + IAM role change, separate from this code change. - Worst-case: an attacker who already compromises Railway can swap the registry prefix to a malicious URI — same blast radius as compromising Railway today, no expansion. Tests: - 9 new unit tests in `registry_test.go` covering: default fallback, env override, empty env, all 9 known runtimes, unknown runtime, override-applies-to-all, computeRuntimeImages map population, env reflection, alphabetical ordering pin. - All existing provisioner + handlers tests continue to pass. - Mutation-tested mentally: deleting `if v := os.Getenv(...)` makes TestRegistryPrefix_RespectsEnv fail. Deleting `for _, r := range knownRuntimes` makes TestRuntimeImage_AllKnownRuntimes fail. The test suite would catch a regression of the original failure mode. Rollout plan: this PR is safe to merge with no env change. Production cutover happens by setting MOLECULE_IMAGE_REGISTRY on Railway after the AWS ECR mirror is populated (separate ops change, tracked in issue #6 phases 3b–3f). Tracking: - RFC: molecule-ai/internal#6 - Tasks: #97 (ECR setup), #98 (CP fallback) - Tech debt: runbooks/hetzner-rollout-tech-debt-2026-05-06.md item 7 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-06 14:23:01 -07:00
Hongming Wang	50c3bdfd6c	Merge pull request #3028 from Molecule-AI/rfc-2945-pr-d-message-store ... feat(messagestore): MessageStore interface + Postgres impl (RFC #2945 PR-D)	2026-05-06 06:42:13 +00:00
Hongming Wang	a33c879017	feat(messagestore): MessageStore interface + Postgres impl (RFC #2945 PR-D) ... Closes #3026. Final piece of RFC #2945. ## What's new New package internal/messagestore/ holds: - MessageStore interface — single read-side contract operators implement to plug in alternative chat-history backends. - ChatMessage / ChatAttachment / ListOptions types — canonical data shapes returned by any impl, mirrors canvas's TS ChatMessage. - PostgresMessageStore — platform-default impl wrapping the activity_logs query + A2A-envelope parser ported in PR-C. Behavior is byte-identical to the pre-PR-D handler. ## What moves The activity_logs query, the parser (activityRowToChatMessages, extractRequestText, extractChatResponseText, extractFilesFromTask, etc.), and the internal-self-message predicate all migrate from internal/handlers/chat_history.go into the new package. handlers/ chat_history.go becomes a thin HTTP-shape adapter: parse query params → store.List(ctx, workspaceID, opts) → emit JSON Compile-time interface assertion in postgres_store.go catches future drift if the interface evolves and the impl falls behind. ## Why this PR OSS operators wanting to: - Tier hot/warm/cold storage (recent in Postgres, archival in S3) - Use a vector store with hybrid search (Pinecone, Weaviate) - Run an in-memory store for ephemeral test environments - Federate history across regions …had no extension point — they'd have to fork the handler. This PR makes that a constructor swap at router.go. ## Tests Parser-level (22 tests, MOVED to internal/messagestore/postgres_ store_test.go): every TS test case in canvas/src/components/tabs/chat/__tests__/historyHydration.test.ts has a Go counterpart. Timestamp preservation, user/agent extraction, internal-self filter, role decision (status=error vs agent-error prefix), v0/v1 file shapes, malformed JSON resilience. Handler-level (9 NEW tests in internal/handlers/chat_history_test.go): thin adapter coverage using a fake MessageStore. UUID validation, before_ts RFC3339 validation, default limit, max-limit clamp, invalid-limit fallback, before_ts passthrough, empty-array (not null) JSON shape, attachment shape preservation, store-error → 502 mapping. Compile-time interface conformance: PostgresMessageStore satisfies MessageStore, fakeStore (test fake) satisfies MessageStore. Mutation-tested. Removed UUID validation in the handler; confirmed TestChatHistoryHandler_RejectsNonUUIDWorkspaceID fires red (status 200 instead of 400, non-UUID reaches the store). Restored, all green. Full handlers + messagestore + router test runs green; full repo go test ./... green. ## SSOT decision ChatMessage / ChatAttachment / parser / DB query all live in internal/messagestore/ ONLY. handlers/chat_history.go imports the package and uses the types via messagestore.ChatMessage etc. — no re-declaration anywhere. ## Three weakest spots (hostile-reviewer self-pass) 1. The internal-self prefix list (Delegation results are ready...) is a package var in messagestore/postgres_store.go. A future impl that wants to override the predicate must reach into the package to use IsInternalSelfMessage or define its own. Acceptable: the predicate is part of the contract; if an impl wants different semantics it owns that decision explicitly. 2. ListOptions has Limit + BeforeTS + HasBefore; future paging needs (after_ts, peer_id filter, role filter) require additive struct field additions, which is a soft API break for any impl that handles ListOptions positionally. Mitigated by Go's struct-literal convention (named fields by default); also flagged in the interface comment for impl authors. 3. The handler does NOT log when a store returns an error — it just maps to 502. An impl that wants to surface its error class up the stack can't, today. If/when an impl needs that, the interface can add a typed-error contract in a follow-up. Today's coverage is sufficient: most ops issues land in the store impl's own logs. ## Security review - Untrusted input? Same as PR-C — agent-emitted JSON parsed defensively. New fakeStore in tests can't reach production. - Trust boundary? Same. Interface lives BEHIND wsAuth; impls only see workspace IDs already authenticated. - Auth/authz? Inherited from handler; the interface doesn't authenticate. - PII / secrets in logs? Documented in the interface contract: impls MUST NOT log full message bodies / attachment URIs. The Postgres impl logs nothing on the happy path. - Output sanitization? Same plain-text + opaque-URI surface as PR-C. Canvas validates attachment-URI schemes. No security-relevant changes beyond what /chat-history already exposes via PR-C. Considered, not skipped. ## Versioning / backwards compat - New internal package. Zero public API change. - Single caller site in router.go updated (one-line constructor change). NewChatHistoryHandler() → NewChatHistoryHandler(store). - No schema change, no migration. - Existing /chat-history endpoint unchanged on the wire — clients don't notice the refactor. ## Phasing This is the final RFC #2945 piece. Follow-ups parked: - PR-C-2 (canvas migration): swap canvas loadMessagesFromDB to call /chat-history instead of /activity. Independent of this PR; blocked only by canvas team's calendar. - Sample alternative impls (S3, in-memory) for OSS docs: separate PR when the first OSS consumer materializes; demonstration code untested against a real workload is anti-pattern. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-05-05 23:38:14 -07:00
Hongming Wang	e91186c4bf	Merge pull request #3020 from Molecule-AI/rfc-2945-pr-c-chat-history ... feat(workspace-server): server-side chat-history endpoint (RFC #2945 PR-C)	2026-05-06 06:23:12 +00:00
Hongming Wang	089be695a9	Merge staging into rfc-2945-pr-c-chat-history	2026-05-05 23:18:52 -07:00
Hongming Wang	dcc870a6b7	feat(workspace-server): server-side chat-history endpoint (RFC #2945 PR-C) ... Closes the SSOT gap for chat-history hydration: today every consumer (canvas TS) re-implements an A2A-envelope walk to map activity_logs rows into rendered ChatMessage objects. This PR moves that walk into the server. ## What's added GET /workspaces/:id/chat-history?limit=N&before_ts=T Returns: { "messages": [ {"id": "<uuid>", "role": "user"|"agent"|"system", "content": "...", "attachments": [...], "timestamp": "<RFC3339>"} ], "reached_end": false } Auth chain: same wsAuth as /workspaces/:id/activity (tenant ADMIN_TOKEN + X-Molecule-Org-Id). No new trust boundary. Filter: a2a_receive rows with source_id IS NULL — same canvas-source filter the canvas applies via /activity?type=a2a_receive&source=canvas, centralized so future API consumers don't need to know it. ## What's mirrored from canvas TS Direct port of canvas/src/components/tabs/chat/historyHydration.ts + message-parser.ts: - extractRequestText / extractFilesFromUserMessage — user-side parts walk through request_body.params.message.parts[] - extractChatResponseText — agent-side response_body collector across the four shapes (string, A2A JSON-RPC parts, older nested parts.root.text, task artifacts) joined with "\n" (matches canvas multi-source collector — claude-code emits multiple text parts; hermes emits summary+artifacts) - extractFilesFromResponse / extractFilesFromTask — file walk across parts[] + artifacts[].parts[] + status.message.parts[] + message.parts[] - v0 hot path ({kind:"file", file:{...}}) AND v1 protobuf flat shape ({url, filename, mediaType}) both supported - Role decision: status='error' OR text starts with "agent error" (case-insensitive) → "system", else "agent" - isInternalSelfMessage prefix filter (Delegation results are ready...) - Timestamp pinned to row.created_at (regression cover for 2026-04-25 bubble-collapse bug) ## Tests 22 unit tests in chat_history_test.go, every TS test case in historyHydration.test.ts has a Go counterpart: Timestamp preservation (3): user/agent pin to created_at, two-rows produce two distinct timestamps. User-message extraction (5): text-only, internal-self skip, null body, attachments hydrated, attachments-only-when-text-empty, internal-self suppresses even with attachments. Agent-message extraction (4): result-string, status=error→system, agent-error-prefix→system, response_body.parts attachments, null body, no-text-no-files-no-bubble. End-to-end (1): paired user+agent same timestamp. Go-specific (5): malformed JSON returns empty (no panic), v1 protobuf flat shape extraction, task-artifacts extraction, older nested root.text shape, basename helper edge cases. isInternalSelfMessage predicate (1): prefix match, non-prefix non- match, empty-text non-match. Mutation-tested. Removed the role-promotion branch (status=error + agent-error prefix → system); confirmed both TestChatHistory_RoleSystemWhenStatusError and TestChatHistory_RoleSystemWhenAgentErrorPrefix fire red. Restored. Both green. Full handlers test suite (4.3s) green; full repo `go test ./...` green. ## SSOT decision Parsing logic lives in workspace-server/internal/handlers/chat_history.go ONLY. Canvas keeps historyHydration.ts + message-parser.ts during the transition because: - PR-C-2 (follow-up): canvas loadMessagesFromDB swaps to new endpoint. Today's canvas still calls /activity for backward compatibility. - The TS parsers are still load-bearing for LIVE message handling (WebSocket A2A_RESPONSE events) until RFC #2945 PR-B-2 mirrors the typed event payloads to canvas consumers. Canvas's TS path will be deleted in a separate PR after a one-week observation window confirms no live-message consumers depend on it. ## Security review - Untrusted input? YES — request_body and response_body come from agents (potentially OSS / third-party). Defensive: any malformed JSON returns empty content + no attachments, no panic. Tested via TestChatHistory_MalformedJSONInRequestBodyReturnsEmpty. - Trust boundary? Same as today: agent → workspace-server. No new boundary; reuses existing wsAuth middleware. - Auth/authz? Inherits wsAuth chain. Cross-workspace access blocked by existing TenantGuard middleware. - PII / secrets in logs? None. The handler logs nothing on the happy path; errors log 502 without body content. - Output sanitization? ChatMessage.content is plain text returned as-is; canvas already sanitizes via ReactMarkdown. Attachment URIs are agent-provided (workspace: / platform-pending: / https:); canvas's existing scheme allow-list still applies. ## Versioning / backwards compatibility - New endpoint /chat-history. /activity unchanged. - Canvas historyHydration.ts + message-parser.ts intact during transition (will be removed in PR-C-2 follow-up). - No public API consumer of /activity is broken — added route is additive. - No semver bump (server is internal versioning). ## Three weakest spots (hostile-reviewer self-pass) 1. extractRequestText returns ONLY parts[0].text. If a user message contains multiple text parts (uncommon — canvas only ever emits one), we lose later parts. Matches canvas exactly today, but a future change that emits multi-text user messages needs both parsers updated. Documented in code; covered by test if/when added. 2. activityRowToChatMessages rebuilds ChatMessage IDs every call (no caching). Each chat reload mints fresh UUIDs. This is fine because canvas dedupes by (role, content, timestamp window) not id, but a future API consumer that DID rely on id stability would break. Documented in the ChatMessage struct comment. 3. The handler scopes to source_id IS NULL only (canvas-source rows). A future "show all messages, including agent-to-agent" mode would need a new endpoint or a parameter. Out of scope for PR-C; canvas's /activity?source=canvas already enforces the same filter. Closes #3017. Unblocks RFC #2945 PR-D (MessageStore interface) which returns []ChatMessage typed values. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 23:17:26 -07:00
Hongming Wang	d144dcc700	Merge pull request #3016 from Molecule-AI/fix/textutil-ssot-truncate-2962 ... fix(textutil): SSOT for rune-safe string truncation, fix 3 audit-gap bugs (#2962)	2026-05-06 06:05:00 +00:00
Hongming Wang	656a02fae4	fix(textutil): SSOT for rune-safe string truncation, fix 3 audit-gap bugs ... Closes #2962. ## Why Six per-package `truncate` helpers had drifted into independent re-implementations of the same idea. Three of them (delegation.go, memory/client/client.go, memory-backfill/verify.go) used `s[:max] + "…"` byte-slice form, which on a multi-byte codepoint at byte `max` produces invalid UTF-8 → Postgres `text`/`jsonb` rejects the INSERT silently → `delegation` / `activity_logs` row never lands → audit gap. Three other helpers (delegation_ledger.go #2962, agent_message_writer.go #2959, scheduler.go #2026) had each been fixed in isolation with three slightly different rune-safe shapes — confirming this is a class of bug, not a single instance. ## What New package `internal/textutil` with three rune-safe functions: - `TruncateBytes(s, maxBytes)` — byte-cap, "…" marker. Used by 5 callers writing into byte-bounded columns / log lines. - `TruncateBytesNoMarker(s, maxBytes)` — byte-cap, no marker. Used by delegation_ledger.go where the storage already conveys "preview" and an extra ellipsis would push the result over the column cap. - `TruncateRunes(s, maxRunes)` — rune-cap, "…" marker. Used by agent_message_writer.go where the cap is in display chars (UI summary), not bytes. All three guarantee `utf8.ValidString(out)` for any `utf8.ValidString(in)`. Inputs already invalid go through `sanitizeUTF8` at the call site boundary (scheduler.go preserved this defense-in-depth). ## Migration map | Old | New | Behavior change | |---|---|---| | `delegation_ledger.truncatePreview` | `textutil.TruncateBytesNoMarker(s, 4096)` | none | | `agent_message_writer.truncatePreviewRunes` | `textutil.TruncateRunes(s, n)` | none | | `scheduler.truncate` | `textutil.TruncateBytes(s, n)` | "..." → "…" (3 bytes either way; single-glyph display) | | `delegation.truncate` | `textutil.TruncateBytes(s, n)` | bug fix + ellipsis swap | | `memory/client.truncate` | `textutil.TruncateBytes(s, n)` | bug fix | | `memory-backfill.truncate` | `textutil.TruncateBytes(s, n)` | bug fix | Five separate `truncate*` helpers + their per-package tests removed. Net: 12 files / +427 / -255. ## Tests - `internal/textutil/truncate_test.go` — 27 table-test cases + 145 fuzz-invariant cases asserting `utf8.ValidString` and byte-cap invariants on every output. - `delegation_ledger_test.go TestLedgerInsert_TruncatesOversizedPreview` strengthened with `capValidUTF8Matcher` so the SQL-write argument is asserted to be valid UTF-8 + within cap (not just `AnyArg()`). Mutation-tested: replacing the SSOT call with byte-slice form makes this test fail loud. ## Compatibility - All callers internal; no external API surface change. - Ellipsis swap "..." → "…": same byte budget (3 bytes), single-glyph display. No alerting/grep on either marker in this codebase (verified). Canvas renders both correctly. - DB column widths unchanged (4096 / 80 / 200 / 256 / 300 — all preserved in the migrations). ## Security Fixes a silent INSERT-failure mode that hid `activity_logs` / `delegations` rows containing peer-controlled text. The class of input that triggered it (CJK, emoji, accented Latin) is normal user content, not malicious — but the symptom (audit gap) makes incident reconstruction harder. Helper is pure-function over `string`; no secrets / PII / auth handling involved. Untrusted input is handled identically to before, just rune-aligned now. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 23:01:21 -07:00
Hongming Wang	c53155ec5f	Merge pull request #3014 from Molecule-AI/test/cross-table-atomicity-integ-149-followup ... test(chat-uploads): integration test for cross-table atomicity (#149 follow-up)	2026-05-06 05:05:49 +00:00
Hongming Wang	debe29c889	ci(handlers-postgres-integration): apply legacy *.sql migrations too ... The migration-replay step globbed only *.up.sql, silently skipping the older flat-naming migrations (001_workspaces.sql, 009_activity_logs.sql, etc.). Fine while no integration test depended on those tables; broke when the #149 cross-table atomicity test came in needing both workspaces (FK target for activity_logs) and activity_logs themselves. Switch to globbing *.sql + sorted lex-order, excluding *.down.sql so up/down pairs don't undo themselves mid-run. Add a sanity check for workspaces + activity_logs + pending_uploads alongside the existing delegations gate so a future migration drift fails loud instead of silently skipping the regressed test. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 22:02:24 -07:00
Hongming Wang	7a39a08837	test(chat-uploads): integration test for cross-table atomicity (#149 follow-up) ... Adds two real-Postgres tests under //go:build integration: - TestIntegration_PollUpload_AtomicRollback_AcrossBothTables exercises the helpers in the same Tx shape uploadPollMode does (PutBatchTx + LogActivityTx + Rollback) and asserts COUNT(*)=0 on BOTH pending_uploads AND activity_logs after the rollback. Failure injection: NUL byte in `summary` triggers lib/pq protocol rejection on the second activity insert — same trick the existing PutBatch AtomicRollback test uses. - TestIntegration_PollUpload_HappyPath_AcrossBothTables is the positive counterpart — Commit lands N rows in both tables. Coverage rationale (post-PR-3010 review): - sqlmock unit test (TestPollUpload_AtomicRollbackOnActivityInsertFailure) proved the handler calls Begin/Exec/Exec-fail/Rollback in order. - Existing PutBatch integration test proved Postgres honors rollback for pending_uploads alone. - New tests close the cross-table gap: prove LogActivityTx + PutBatchTx + real Postgres MVCC compose correctly under rollback. A regression that made LogActivityTx silently route through db.DB instead of the passed tx would still pass the sqlmock test (the Begin/Commit/Rollback shape would look right) but would fail this integration test (the activity_logs row would survive the rollback). Verified locally: postgres:15-alpine + all migrations applied, both tests pass in 0.1s. Skips cleanly without INTEGRATION_DB_URL — CI already runs this file via the Handlers Postgres Integration job. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 21:57:56 -07:00
Hongming Wang	bb9bf85dbd	Merge pull request #3011 from Molecule-AI/rfc-2872-workspaces-uniq-toctou ... fix(workspace-server): close TOCTOU race on workspaces(parent_id, name) (#2872 Critical 1)	2026-05-06 04:51:01 +00:00
Hongming Wang	ff21bbb876	Merge staging into rfc-2872-workspaces-uniq-toctou to clear BEHIND	2026-05-05 21:46:33 -07:00
Hongming Wang	da3cb4c098	fix(workspace-server): close TOCTOU race on workspaces(parent_id, name) (#2872 Critical 1) ... ## Bug `/org/import` had no per-tenant mutex, advisory lock, or DB-level uniqueness on (parent_id, name). The pattern was lookup-then-insert: existingID, existing, err := h.lookupExistingChild(...) // SELECT if existing { return /* skip */ } db.DB.ExecContext(ctx, `INSERT INTO workspaces ...`) // INSERT Two concurrent admin POSTs (rapid double-click in canvas, retry-after- timeout, two operators on the same template) both saw "not found" in the SELECT and both INSERT'd the same (parent_id, name). Captured impact: tenant-hongming accumulated 72 stale child workspaces in 4 days from repeated org-template spawns of the same template (see #2857 phase 4 sweeper for the cleanup; #2872 for the prevention RFC). ## Fix Two-layer fix — DB-level backstop AND application-level happy path: 1. **Migration** `20260506000000_workspaces_unique_parent_name.up.sql` ```sql CREATE UNIQUE INDEX CONCURRENTLY IF NOT EXISTS workspaces_parent_name_uniq ON workspaces ( COALESCE(parent_id, '00000000-0000-0000-0000-000000000000'::uuid), name ) WHERE status != 'removed'; ``` * COALESCE(parent_id, sentinel) collapses NULLs so root workspaces also collide pairwise. * `WHERE status != 'removed'` lets a tombstoned row be replaced by a same-named re-import (preserves existing org-import semantics). * CONCURRENTLY avoids ACCESS EXCLUSIVE on production tenants under live traffic; IF NOT EXISTS makes the migration resumable. * Down migration drops CONCURRENTLY symmetrically. 2. **`org_import.go` swap** Replace lookup-then-insert with `INSERT ... ON CONFLICT DO NOTHING RETURNING id`. On the skip path (RETURNING returns 0 rows → sql.ErrNoRows), re-select the existing id to recurse children: INSERT INTO workspaces (...) VALUES (...) ON CONFLICT (COALESCE(parent_id, ...), name) WHERE status != 'removed' DO NOTHING RETURNING id; The ON CONFLICT target predicate matches the partial-index predicate exactly — required for Postgres to consider the index applicable. Existing `lookupExistingChild` helper kept (still used on the skip path); semantics unchanged. ## Test coverage * AST gate refreshed to assert the workspaces INSERT contains the ON CONFLICT pattern (`onConflictDoNothingRE`) instead of the now-obsolete "lookup-before-insert" ordering. Per behavior-based gating (memory: feedback_behavior_based_ast_gates.md), the new gate pins the actual TOCTOU-resolution behavior. * Companion `TestGate_FailsWhenInsertOmitsOnConflict` proves the gate catches the bug shape on synthetic source. * All existing `lookupExistingChild` unit tests (no-rows, found, nil-parent, DB error, wrapped no-rows) still pass — helper is unchanged and still load-bearing on the skip path. * Live Postgres E2E coverage runs via the existing "Handlers Postgres Integration" CI job, which applies migrations to a real PG and exercises the INSERT path. ## Why ship the migration + swap together (not stacked) The migration alone provides a DB-level backstop, but without the handler swap a UNIQUE-violation surfaces as a 500 to the user. The handler swap alone has no enforceable target until the migration applies. Shipped together they give graceful skip + atomic backstop. Migration is CONCURRENTLY + IF NOT EXISTS, safe to apply even on tenants where the sweeper (#2860) hasn't run yet — the index just declines to build until conflicting rows are reconciled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 21:43:49 -07:00
Hongming Wang	ef9bd1e0e2	Merge pull request #3010 from Molecule-AI/fix/activity-row-tx-atomicity-149 ... fix(chat-uploads): activity rows commit atomically with PutBatch (#149)	2026-05-06 04:37:55 +00:00
Hongming Wang	b759548822	fix(chat-uploads): activity rows commit atomically with PutBatch ... Closes #149. uploadPollMode for poll-mode chat uploads previously committed N pending_uploads rows in one Tx (PutBatch), then wrote N activity_logs rows individually outside any Tx. A per-row failure on activity row K left rows 1..K-1 committed and pending_uploads orphaned until the 24h TTL — not data-loss because the platform's fetcher handled the half-state cleanly, but the user never saw file K in the canvas and the inconsistency surfaced as an "uploaded but invisible" complaint class. Thread one Tx through PutBatchTx + N × LogActivityTx + Commit so all or none commit. Broadcasts are deferred until after Commit — emitting an ACTIVITY_LOGGED event for a row that ends up rolled back would paint a ghost message into the canvas's optimistic UI. A new LogActivityTx returns a commitHook the caller invokes post-Commit; the existing fire-and-forget LogActivity is unchanged for the 4 other production callers (a2a_proxy_helpers + activity.go report path). Storage interface gains PutBatchTx; PostgresStorage.PutBatch is refactored to share the validation + insert path. inMemStorage and fakeSweepStorage delegate or no-op for PutBatchTx (the in-mem fake can't model Tx state — DB-level atomicity is verified by the existing real-Postgres integration test for PutBatch + the new unit test asserting the Go handler calls Rollback on activity-insert failure). Tests: - TestPollUpload_AtomicRollbackOnActivityInsertFailure pins the new contract via sqlmock — second activity insert errors → Rollback expected, Commit must NOT be called. - TestLogActivityTx_DefersBroadcastUntilCommitHook + _InsertError_NoHook_NoBroadcast + _NilTx_Errors cover the new API. - TestPutBatchTx_HappyPath / _EmptyItems / _ValidationFails / _PerRowErrorPropagates cover Tx-aware storage layer. - 7 existing TestPollUpload_* tests updated to mock Begin + Commit (or Begin + Rollback for failure paths) since the handler now opens a Tx around PutBatch + activity inserts. All workspace-server tests pass; integration tag also clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 21:34:28 -07:00
Hongming Wang	cce2050b6a	Merge pull request #2997 from Molecule-AI/rfc-2991-pr-1-image-preview-lightbox ... feat(canvas/chat): inline image preview + fullscreen lightbox (RFC #2991 PR-1)	2026-05-06 04:28:03 +00:00
Hongming Wang	e87df906bd	Merge staging into rfc-2991-pr-1 to clear BEHIND (post PR-2993 + PR-3005)	2026-05-05 21:24:20 -07:00
Hongming Wang	c60e2b5fa2	chore(canvas/chat): drop unused downloadChatFile import in AttachmentImage ... github-code-quality bot flagged this as the last unresolved review thread blocking the merge queue. The function is referenced in comments but never called from this file (download is dispatched via the lightbox / AttachmentChip path). Removing the import resolves the bot thread and clears the staging branch-protection 'all conversations resolved' gate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 21:23:46 -07:00
Hongming Wang	143fbb91ff	Merge pull request #3005 from Molecule-AI/ux/files-tab-drag-drop-upload ... ux(canvas/files): drag-drop upload to target folder (#2999 PR-D)	2026-05-06 03:52:10 +00:00
Hongming Wang	1b29b24e83	Merge staging into rfc-2991-pr-1 to clear BEHIND state	2026-05-05 20:50:55 -07:00
Hongming Wang	6033179f48	Merge pull request #3006 from Molecule-AI/rfc-2991-pr-3-pdf-text-preview ... feat(canvas/chat): inline PDF + text/code preview (RFC #2991 PR-3)	2026-05-05 20:49:53 -07:00
Hongming Wang	ab1acff2d2	ux(canvas/files): drag-drop upload to target folder (#2999 PR-D) ... User asked for VSCode-style drag-drop upload (#2999): "drag local to upload to target folder just like vscode does". Today the only upload path is the toolbar's Upload button (folder picker). Drag-drop lets users grab files from Finder/Explorer and drop them directly on a specific subdirectory in the tree. 1. New `uploadDataTransferItems(items, targetDir)` in `useFilesApi` — walks the HTML5 DataTransferItemList via `webkitGetAsEntry()`, recursing folders to a flat (relativePath, file) list, then PUTs each via the existing /files/<path> endpoint. The walker (also exported via `__testables`) calls `readEntries()` in a loop until empty so multi-batch folders (browsers cap each call at ~100 entries) aren't silently truncated. 2. `uploadFiles` (folder-picker path) gained an optional `targetDir` parameter. Same prefixing semantics so future surfaces (e.g. an "upload here" toolbar button on a row) can reuse it. 3. `FileTree` directory rows gained `onDragOver` / `onDragEnter` / `onDragLeave` / `onDrop` handlers + a hover-target highlight (accent-tinted background + outline). dragLeave uses `currentTarget.contains(relatedTarget)` to suppress the flicker that fires when the cursor crosses any child of the row (icon, label, ✕ button) — without this the highlight strobes on every sub-element transition. 4. `FilesTab` wraps the tree column in an outer drop zone for "drop on root" — drops outside any specific subdir row land at root. The empty-state placeholder copy now includes a "drag files here to upload" hint when the active root is /configs (the only writable root today). 5. Both the row drop and the root drop are gated on `root === "/configs"` (the same gate that already blocks the toolbar's New / Upload / Clear). Other roots ignore the drag entirely (no highlight, no drop), so the user doesn't get a misleading drag affordance followed by a "switch root" toast. `dragDropUpload.test.tsx` (9 tests, two layers): Walker tests (pure function, no DOM): - `walkEntry` collects a single dropped file with correct relpath. - `walkEntry` walks a folder + preserves folder name in the path. - **Multi-batch loop**: a fake reader that emits two batches of 2 + an empty terminator must yield 4 files. A walker that called readEntries once would see only 2 — this is the load-bearing assertion against silent folder truncation. - Nested directories: outer/inner/file.md → "outer/inner/file.md". FileTree drag-drop wiring (DOM): - `dragover` on a directory row preventDefault's (load-bearing — without it the drop event never fires). - `drop` on a directory row fires `onDropToTarget(path, items)`. - `drop` on a FILE row does NOT fire (only directories are valid drop targets). - `drop` with no DataTransferItems does NOT fire (defensive guard against text-only drags). - `dragenter` adds the highlight class to the directory row. 1. The 1MB per-file size cap is inherited from the existing `uploadFiles`. A user dropping a 5MB skill bundle silently skips the file (the loop's `continue` on `file.size > 1_000_000`). Same behavior as the toolbar Upload, so consistent if not great. Surfacing skipped-files would be a UX improvement tracked separately — not load-bearing for this PR. 2. Drop-zone highlight on the column wrapper uses an outline that sits inside the column's overflow-y-auto scroll container. If the user drags onto a row that's mid-scroll, the highlight may clip slightly at the scroll boundary. Cosmetic only; the drop still works. 3. The `?root=` query is NOT passed on the underlying writeFile call (matches the existing uploadFiles behavior). On a backend without #2999 PR-A, this means uploads always land in /configs regardless of selected root — but we already gated drop on `root === "/configs"` so the practical effect is nil today. Once PR-A merges and the canvas threads ?root= through writes (separate follow-up), drops on /home etc. would be enableable by lifting the canDelete-style gate. - `npx tsc --noEmit` clean - 177/177 canvas tab tests pass - Manual on local dev: drag a file from Finder onto /configs/skills row → file appears under /configs/skills/<name>. Drag a folder of 3 files onto root area → 3 files uploaded with folder structure preserved. Drag onto /home tree → no highlight, no drop. Refs #2999. Pairs with PR-A (backend EIC) — without PR-A the tree is empty on SaaS and there's nothing to drop ONTO; PR-D still works on self-hosted today. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-05-05 20:47:47 -07:00
Hongming Wang	19df43e3da	Merge pull request #2993 from Molecule-AI/rfc-2945-pr-b-1-migrate-bare-event-strings ... refactor(events): migrate 18 producers to typed EventType constants (RFC #2945 PR-B-1)	2026-05-06 03:45:47 +00:00
Hongming Wang	dcece2762b	feat(canvas/chat): inline PDF + text/code preview (RFC #2991 PR-3) ... Adds two new arms to the AttachmentPreview kind dispatcher: * PDF — chip in the bubble, click opens the shared AttachmentLightbox with a browser-native <embed type="application/pdf"> at 95vw/90vh. Fetch+Blob+ObjectURL auth path matches AttachmentImage / Video. PDF.js not pulled in; browser viewer is good enough for the desktop chat MVP (Slack/Linear/Notion all gate full-page PDF behind a click for the same reason). Falls back to AttachmentChip on fetch error. * Text/code/JSON/YAML — first 10 lines in monospace <pre><code> right in the bubble, "Show all N lines" expands to full content, with a filename + ⬇ download header. Streams up to 256 KB then marks truncated and offers a download chip; large logs don't crash the bubble. No syntax highlighting in v1 — shiki adds 200-500 KB and is pure polish. Coverage: 5 new dispatch tests (PDF success → embed in lightbox, PDF fetch fail → chip fallback, text inline render, text long content → Show-all-N-lines expand button, text fetch fail → chip fallback). All 19 AttachmentPreview tests pass; tsc --noEmit clean. Stacked on rfc-2991-pr-1-image-preview-lightbox (PR-2 already merged into PR-1's branch). PR-1 ships first; this rebases onto staging once it lands. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 20:43:46 -07:00
Hongming Wang	57bfa40990	Merge pull request #3004 from Molecule-AI/ux/files-tab-context-menu ... ux(canvas/files): right-click context menu — Open / Download / Delete (#2999 PR-C)	2026-05-06 03:37:16 +00:00
Hongming Wang	d88fbb90fb	ux(canvas/files): right-click context menu — Open / Download / Delete (#2999 PR-C) ... ## Why User asked for a VSCode-style right-click menu on file rows (#2999): "right click to have a menu to download". Today the only download affordance is the toolbar's Export-all (bulk JSON dump), and the inline ✕ button is the only delete UX (small click target, easy to miss). ## Fix 1. New `FileTreeContextMenu` component — fixed-position popover with Open / Download / Delete items composed per-row (files get all three; directories get Delete only since "open a directory in the editor" doesn't apply). Esc + outside-click + Tab + scroll dismiss. ↓/↑ arrow keys rove focus between menu items. role=menu + role=menuitem + autofocus on first item for a11y. 2. Menu state lifted to the top-level `FileTree` (not per-row) so opening a second row's menu auto-closes the first — only one menu open at a time, matching VSCode/Theia. Pinned by the `replaces the first` test. 3. New `downloadFileByPath(path)` in `useFilesApi` — fetches via the existing GET /workspaces/<id>/files/<path>?root= endpoint and triggers a browser download. Distinct from the existing `handleDownloadFile` which downloads the in-editor buffer (round-trips unsaved edits to disk); the context-menu download targets arbitrary tree rows the user hasn't opened. 4. `canDelete` prop threaded from FilesTab → FileTree → menu → item. Same gate as the toolbar (Clear/New/Upload all gated to /configs); context menu's Delete renders as disabled with a muted background on other roots, matching the "feature exists but isn't applicable here" pattern. ## Test coverage `FileTreeContextMenu.test.tsx` (8 tests): - File row → menu opens with Open + Download + Delete. - Directory row → menu opens with Delete only. - Click Download → onDownload(path) fires + menu closes. - Click Delete (canDelete=true) → onDelete(path) fires. - Click Delete (canDelete=false) → onDelete NOT called + menu stays open (disabled-state UX). - Esc dismisses. - Outside-click (mousedown on document.body) dismisses. - Opening second context menu replaces the first (only-one-open invariant). Each test uses fireEvent + screen.getByRole, so they fail on a deleted-code regression — none would pass on the pre-PR shape. ## Three weakest spots (hostile self-review) 1. The menu is positioned at `clientX/clientY` without viewport clamping. If the user right-clicks at the very bottom-right of the panel, part of the menu may overflow off-screen. VSCode handles this by flipping the anchor; we don't yet. Acceptable v1 because the FilesTab is fixed-width (≤ side-panel width) and the menu is small (140×~80px); the overflow would be a few pixels of one item. Filed as a follow-up. 2. Auto-focus on the first item shifts keyboard focus away from the row that opened the menu. Closing with Esc returns focus to the body, not the row. Same behavior as TerminalTab's placeholder + the canvas's other context menus; consistent isn't ideal but at least uniform. Documented inline. 3. The download request reuses the API client's 15s default timeout — large config files (multi-MB skill bundles) on a slow connection could time out. Same risk applies to the existing toolbar Export. If we see real download failures we can add a `timeoutMs` override at the call site without touching the menu. ## Verification - `npx tsc --noEmit` clean - 176/176 canvas tab tests pass - Manual on local dev: right-click a config.yaml row → menu opens → click Download → file lands in Downloads. Right-click on /home root → Delete renders disabled. Refs #2999. Pairs with PR-A (backend EIC) — without PR-A the tree is empty and there's nothing to right-click on a SaaS workspace. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-05-05 20:26:04 -07:00
Hongming Wang	2e6bed71b9	Merge pull request #3003 from Molecule-AI/ux/files-tab-external-not-available ... ux(canvas/files): "Files not available" banner for external runtimes (#2999 PR-B)	2026-05-06 03:24:45 +00:00