4601 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
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
Hongming Wang	030377bb84	Merge pull request #3002 from Molecule-AI/fix/files-eic-list-delete-symmetry ... fix(workspace files API): EIC parity for ListFiles + DeleteFile (#2999 PR-A)	2026-05-06 03:22:45 +00:00
Hongming Wang	f93957e982	ux(canvas/files): "Files not available" banner for external runtimes (#2999 PR-B) ... ## Why Reported by user (issue #2999): external workspaces (mac laptop, mac mini, hermes-on-home-server — runtime="external") render the FilesTab identically to the SaaS empty-listing bug, showing "0 files / No config files yet" even though the platform doesn't actually own the filesystem of these workspaces. Visually indistinguishable from the broken state, reads as a bug. ## Fix Mirror the affordance TerminalTab adopted in PR #2830 for runtimes without a TTY: 1. New `NotAvailablePanel` in `canvas/src/components/tabs/FilesTab/` — folder-with-slash icon + "Files not available" headline + body text that names the runtime and points the user at Chat. 2. `FilesTab` now takes optional `data?: WorkspaceNodeData`. When `data.runtime` is in `RUNTIMES_WITHOUT_FILES` (currently just "external"), early-return the placeholder before mounting the useFilesApi hook. Mirrors TerminalTab's prop shape exactly so the review pattern is uniform across tabs. 3. SidePanel passes `node.data` to FilesTab (matches existing pattern for ChatTab / TerminalTab). ## Test coverage `FilesTab.notAvailable.test.tsx` (4 tests): - external runtime → banner renders with runtime name + Chat-tab guidance copy. - external runtime → NO `/files` API request fires (asserted by inspecting the mocked api.get call log). - claude-code runtime → no banner, normal mount proceeds (toolbar's root selector is the discriminator). - data prop omitted → falls through to normal mount (back-compat with any caller that doesn't thread data through, e.g. legacy tests). Each branch is independent and discriminating — none would pass on a code-deleted version of the early-return. ## Three weakest spots (hostile self-review) 1. `RUNTIMES_WITHOUT_FILES` is a hardcoded set in this file. If a future runtime joins (e.g. a "byok-claude" that runs on user hardware), someone has to remember to add it here. Reviewed alternatives: pull from a runtime-capabilities registry — same shape as `RUNTIMES_WITHOUT_TERMINAL` already in TerminalTab. We chose the parallel pattern over a new abstraction; consolidating into a shared registry can land if/when a third tab grows the same gate (rule of three). Documented inline. 2. The placeholder is a static panel — no retry, no "report bug" link. Same as TerminalTab's. Acceptable because the absence is intentional, not transient. 3. Chat-tab guidance is hardcoded English. No i18n in canvas yet; matches the rest of the codebase. Will move with the i18n migration when that lands. ## Verification - `npx tsc --noEmit` clean - 54/54 canvas tab + SidePanel tests pass - Will be live-verified on staging post-merge: open Files tab on an external workspace (mac laptop) → expect placeholder; open on a platform-owned workspace (Hongming Personal Brand Agent) → expect normal tree (assuming PR-A also lands). Refs #2999. Pairs with PR-A (backend EIC fix) — without PR-A the platform-owned path still shows "0 files" because the backend never returns rows. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-05-05 20:21:45 -07:00
Hongming Wang	b530c147de	Merge pull request #3000 from Molecule-AI/rfc-2991-pr-2-video-audio-preview ... feat(canvas/chat): inline video + audio HTML5 players (RFC #2991 PR-2)	2026-05-05 20:18:36 -07:00
Hongming Wang	f39b595a9c	fix(workspace files API): EIC parity for ListFiles + DeleteFile (closes #2999 PR-A) ... ## User-visible bug Canvas Files tab returns "0 files / No config files yet" for every SaaS workspace, every root (/configs, /home, /workspace, /plugins). Reported by user (canvas screenshot, hongming.moleculesai.app, Hongming Personal Brand Agent — claude-code, T4, online). ## Root cause `ListFiles` (templates.go) was missing the SSH-via-EIC branch that ReadFile (PR #2785) and WriteFile (PR #1702) already have. On SaaS, dockerCli is nil → findContainer returns "" → falls through to host-side resolveTemplateDir which only matches baked-in template names. For a user-named workspace it matches nothing, so the handler silently returns []fileEntry{}. DeleteFile had the same gap — right-click delete (introduced in PR-C of this issue) would silently no-op once #1 was fixed. ## Fix 1. Extracted shared EIC plumbing into `withEICTunnel` (closure-based, single SSOT for keypair → key push → tunnel → port-wait → cleanup). Refactored writeFileViaEIC + readFileViaEIC to use it. Added listFilesViaEIC + deleteFileViaEIC on the same scaffold. The `LogLevel=ERROR` shim from PR #2822 now lives in one `eicSSHSession.sshArgs()` helper instead of being duplicated per helper — the next time we need to tweak ssh options, one place. 2. Factored remote shell strings into pure functions (buildInstallShell / buildCatShell / buildRmShell / buildFindShell + parseFindOutput) so the wire shape can be pinned without booting a real EIC tunnel. 3. Refactored `resolveWorkspaceFilePath(runtime, root, relPath)` to honor `?root=`. New rule: `/configs` (or empty / unrecognized) → runtime managed-config dir via workspaceFilePathPrefix (preserves the v1 ReadFile/WriteFile behaviour where canvas's Config tab GETs/PUTs config.yaml without specifying a root and lands in the right per-runtime dir); `/home`, `/workspace`, `/plugins` → literal absolute path on the EC2 host. List/Read/Write/Delete now agree on what file a tree row points to — pre-fix List would say "/home contents" but Read/Write would route to /configs. 4. ListFiles + DeleteFile dispatch on instance_id != "" → EIC helper. Errors from the EIC path produce 500 (not silent fall-through to local-Docker, which would mask the failure as "0 files" — the exact user-visible symptom). 5. Added ?root= validation gate to WriteFile + DeleteFile so an out-of-allowlist root is rejected before the resolver runs. ## Test coverage - TestResolveWorkspaceFilePath_RuntimeIndirection — pins the /configs → runtime prefix translation per-runtime (hermes, claude-code, langgraph, external, unknown). Catches the regression where a future edit accidentally drops the runtime indirection. - TestResolveWorkspaceFilePath_LiteralRoots — pins /home, /workspace, /plugins as literal pass-through regardless of runtime. Catches the symmetric regression where the literal roots start getting rewritten to the runtime prefix (which would mean the FilesTab "/home" selector silently routes to /configs on hermes). - TestResolveWorkspaceRootPath — directory-only translation used by listFilesViaEIC, same indirection rules. - TestSSHArgs_HardenedFlags — pins the centralised ssh option set (LogLevel=ERROR + hardening). Catches drift in the one-place-where-ssh-flags-live. - TestEicSSHSessionSingleSourceForSSHFlags — behaviour-based AST gate (per memory). Counts s.sshArgs() callers (must be ≥4 — list/read/write/delete) and asserts LogLevel=ERROR appears exactly once in the source. Fires if anyone copy-pastes a raw ssh args slice instead of going through the helper. - TestBuildInstallShell / TestBuildCatShell / TestBuildRmShell / TestBuildFindShell — pure-function tests pinning the remote command shape. Catches regression like "rm -f silently becomes rm -rf" or "find loses node_modules pruning" without needing a real EC2. - TestBuildFindShell_DepthForwarding — catches a regression where the helper hard-codes a depth instead of using the caller's value. - TestParseFindOutput / TestParseFindOutput_EmptyInput — pin the TYPE|SIZE|REL parser. Empty-input case explicitly returns [] not nil so the JSON wire shape stays a list. - TestListFiles_EICDispatch_Success / Error — sqlmock-driven handler test. Verifies instance_id != "" routes to listFilesViaEIC and surfaces errors as 500 (does NOT silently fall through to local-Docker, which is the exact regression-mode of the original bug). - TestListFiles_EICBranch_NotTakenForSelfHosted — back-compat guard: instance_id == "" must NOT enter the EIC branch (would break self-hosted operators). - TestDeleteFile_EICDispatch_Success / Error — same shape for DeleteFile. - TestListFiles_RootValidation / TestDeleteFile_RootValidation — ?root=/etc must 400 before any DB query or EIC call. ## Verification - `go build ./...` clean - `go test ./...` clean (full workspace-server suite) - Will be live-verified against staging on hongming.moleculesai.app after merge: open Files tab → expect populated /home + /configs + /workspace listings (not "0 files"); right-click delete on /configs/old.yaml → expect file removed on the EC2 host. ## Three weakest spots (hostile self-review) 1. The LogLevel=ERROR drift gate counts source occurrences. A future refactor that intentionally moves the literal somewhere else (e.g. into a constant) would trigger a false positive. The gate's failure message points to the load-bearing constraint (must appear in sshArgs); operator can adjust. 2. `eicFileWriteTimeout` constant kept as an alias for back-compat with prior tests. Documented as intentional + safe to remove on the next pass. 3. The resolver tests pin the runtime → prefix map values (`/home/ubuntu/.hermes`, `/configs`, etc.). A future runtime addition that ships a new prefix needs the test updated. This is intentional — silent prefix changes orphan saved files, so a test failure on map edit IS the right signal. ## Follow-up (RFC #2312 subtask 2) Long-term the right fix is to drop EIC entirely and HTTP-forward to the workspace's own URL (RFC #2312). That's a substantially larger refactor across 5 surfaces (chat upload, files, templates, plugins, terminal) and out of scope for this bug-fix PR. Tracked separately under that RFC. Refs #2999. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 20:18:05 -07:00
Hongming Wang	95fdf86187	feat(canvas/chat): inline video + audio HTML5 native players (RFC #2991 PR-2) ... Second specialized renderer pair landing under RFC #2991. Stacks on PR-1 (#2997) — extends the AttachmentPreview dispatcher with video/ audio cases. Why HTML5-native (not custom JS player) --------------------------------------- - Browser vendors ship hardware-accelerated decoders, captions, pinch + scrub UX, and fullscreen UI. We get all of it for free. - Native fullscreen via the <video> control bar — no AttachmentLightbox needed for video (the browser's built-in fullscreen handles it). - Mobile-friendly without us writing the touch handlers. Auth model ---------- Identical to AttachmentImage (PR-1): platform-auth URIs need our cookie/token, so we fetch the bytes, wrap in a Blob, hand the browser an ObjectURL via <video src=> / <audio src=>. External http(s) URIs skip the fetch. Memory caveat: a Blob holds the entire media in JS memory until the bubble unmounts. The server's 25MB single-file cap (chat_files.go) bounds this; v2 can switch to MediaSource + streaming if larger files become a real shape. Failure modes ------------- - Fetch failure (404, 403, network) → AttachmentChip fallback. - Bytes that aren't valid media (corrupt, wrong Content-Type) → <video onError> / <audio onError> swap to chip. Tests ----- 5 new component tests in AttachmentPreview.test.tsx (now 14 total): - kind=video → <video controls> with blob URL src - kind=video fetch fails → falls back to chip - kind=video extension fallback (no mime) → routes to video path - kind=audio → <audio controls> + filename label visible - kind=audio fetch fails → falls back to chip The preview-kind unit tests from PR-1 (49 cases) already cover the MIME → video / audio dispatch logic; this PR's component tests pin the rendered DOM shape (controls attribute, blob URL src, fallback behavior). Hostile self-review ------------------- 1. Memory bound: 25MB cap protects us today; documented future migration path (MediaSource). 2. iOS Safari autoplay: playsInline pinned on <video> so mobile doesn't auto-fullscreen on play. 3. Captions accessibility: <track kind="captions" /> placeholder so the element is tagged correctly even though we don't have caption files yet (forward-compatible). Verified - tsc --noEmit clean - 173 chat tests green (49 unit + 14 component + 110 pre-existing) Stacks on PR-1 (#2997). PR-3 (PDF + text/code) is the final piece. Refs RFC #2991, PR #2997 (PR-1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 20:10:19 -07:00
Hongming Wang	04f7a07add	feat(canvas/chat): inline image preview + fullscreen lightbox (RFC #2991 PR-1) ... First specialized renderer landing under RFC #2991 — chat attachment preview. Adds the dispatch infrastructure that PR-2 (video/audio) and PR-3 (PDF/text) will extend. Architecture (RFC #2991 Phase 2 design) --------------------------------------- - preview-kind.ts: pure helper that maps mimeType (+ extension fallback for missing/generic MIME) to one of: image | video | audio | pdf | text | file. Single source of truth; the dispatch axis for every attachment renderer. - AttachmentPreview.tsx: SSOT dispatch component. ChatTab no longer imports kind-specific components — it imports AttachmentPreview, which switches on the kind and renders the right child. - AttachmentImage.tsx: inline thumbnail (max 240×180) + click → lightbox. Auth-aware: for platform URIs (workspace: / platform-pending: / etc) the bytes are fetched via JS-injected headers, wrapped in a Blob, served as ObjectURL — bare <img src> would not include the cookie/token. - AttachmentLightbox.tsx: shared fullscreen modal (image now; PDF will use it in PR-3). Esc / backdrop click / X button to close, focus trap on close button, focus restoration on close. - AttachmentChip retained as the kind=file fallback. No breaking change for existing renderable shapes. External-workspace coverage --------------------------- The wire shape (ChatAttachment.mimeType + uri) is identical for internal + external workspaces — both go through AgentMessageWriter (PR #2949). External claude-code agents that attach images via send_message_to_user automatically get the new preview surface; no runtime-side change needed. Failure modes ------------- - Fetch failure (404, 403, network) → AttachmentChip fallback so the user still gets a working download. Pinned by tests. - Decoded as non-image (corrupt bytes, wrong Content-Type) → onError on the <img> swaps to AttachmentChip. Pinned by tests. - Non-platform URIs (http/https external image hosts) → skip the auth-fetch flow, use the raw URL via resolveAttachmentHref. Pinned by extension-fallback tests. Tests ----- preview-kind.test.ts (49 cases): - Strict MIME match across image/video/audio/pdf/text/unknown - Extension fallback when MIME is missing or application/octet-stream - URL with query string + fragment → strip before parsing - MIME wins over extension (regression: don't render image-named zip) - SVG is image (not text) despite being XML - Non-canonical MIME like application/javascript → text AttachmentPreview.test.tsx (9 component tests): - Dispatch: kind=file → chip, kind=image → image path - Loading state shows placeholder, NOT chip (proves dispatch routed) - Extension fallback (no mimeType) routes to image path - Fetch fail (404) and network error → fall back to chip - Image success: <img> renders ObjectURL, click opens lightbox - Lightbox: Esc closes, backdrop click closes, content click doesn't - Universal fallback: unknown MIME → chip even when extension hints at a renderable kind Hostile self-review (3 weakest spots, addressed) ------------------------------------------------ 1. <img> auth: bare <img src="/chat/download?..."> would NOT include our auth headers. Resolved via fetch+Blob+ObjectURL pattern. Pinned by the image-success test (asserts src === "blob:test-url"). 2. Server-side allowed-roots mismatch: pre-fix tests used /tmp/ paths which the server doesn't allow. Caught when the dispatch test fell into the non-platform path. Updated tests to use /workspace/ subpaths matching templates.go's allowedRoots. 3. Bundle size creep: each kind component adds bytes. Lightbox is currently always-bundled. Lazy-loading is plausible but defer until measured-needed. Verified - tsc --noEmit clean - 168 chat tests green (49 unit + 9 component + 110 pre-existing) PR-2 (video + audio) and PR-3 (PDF + text) extend the dispatch in AttachmentPreview.tsx with their own kind-specific components. Refs RFC #2991. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 19:39:37 -07:00
hongmingwang-moleculeai	3dfeb180ab	Merge pull request #2995 from Molecule-AI/fix/sweep-add-orphan-tunnel-cleanup-2987 ... chore(sweep): add orphan-tunnel cleanup step (#2987 / #340)	2026-05-06 02:38:39 +00:00
Hongming Wang	88ff0d770b	chore(sweep): add orphan-tunnel cleanup step (#2987 / #340) ... The 15-min sweeper has been deleting stale e2e orgs but not the orphan tunnels left behind when the org-delete cascade half-fails (CP transient 5xx after the org row is gone but before the CF tunnel delete completes). Result: tunnels accumulate in CF until manual operator cleanup. Add a final step that POSTs `/cp/admin/orphan-tunnels/cleanup` every tick. Best-effort — failure doesn't fail the workflow; next tick re-attempts. Output reports deleted_count + failed count for ops visibility. This is the catch-all for the orphan-tunnel class. The proper upstream fix (transactional org delete) lives in CP and tracks as issue #2989. Until that lands, the sweeper bounded-time-to-cleanup keeps the leak from escalating. Note: PR #492 (cf-tunnel silent-success fix) makes this step actually effective — pre-fix DeleteTunnel silent-succeeded on 1022, so the cleanup endpoint reported success without deleting. Post-fix the cleanup chains CleanupTunnelConnections + retry on 1022, which actually clears stuck-connector orphans. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-05-05 19:36:20 -07:00
Hongming Wang	86b8d8d744	Merge pull request #2982 from Molecule-AI/fix-config-skip-yaml-for-external-runtime ... fix(canvas/config): skip config.yaml fetch for external/hermes runtimes	2026-05-06 02:22:14 +00:00
Hongming Wang	9b9419ad5e	Merge pull request #2992 from Molecule-AI/chore/ssot-pointer-sweep-workflow ... chore(sweep): note SSOT for ephemeral prefixes lives in CP	2026-05-06 02:20:35 +00:00
Hongming Wang	a19ee90556	chore(sweep): note SSOT for ephemeral prefixes lives in CP ... Mirrors molecule-controlplane#494: the canonical EPHEMERAL_PREFIXES list now lives in molecule-controlplane/internal/slugs/ephemeral.go, where redeploy-fleet reads it to skip in-flight test tenants. The sweep workflow keeps a Python copy because GHA Python can't import Go, but a comment now points engineers updating the list to update both files. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 19:18:13 -07:00
hongmingwang-moleculeai	bd0580f4af	Merge pull request #2990 from Molecule-AI/fix/memory-v2-namespace-labels-2988 ... fix(memory-v2): namespace labels use display names not UUID prefixes (#2988)	2026-05-06 02:13:30 +00:00
Hongming Wang	64e58fb390	test(memory-v2-e2e): update expectChainQueryRoot for new name column ... PR #2990 root cause: the resolver SQL added `name` to the SELECT for DisplayName plumbing, but the e2e test's sqlmock fixture (expectChainQueryRoot at swap_test.go:216) still scripts the 3-column shape. Three e2e tests fail with: sql: expected 3 destination arguments in Scan, not 4 Fix: bump the fixture to 4 columns (id, name, parent_id, depth) and pass an empty name. The e2e tests don't assert on label rendering — they pin the namespace string flow ("workspace:root-1" etc), which is unchanged. Empty name is fine: ReadableNamespaces still emits the correct namespace strings; only DisplayName is empty. Caught by CI's Platform (Go) check on PR #2990 — would have been a silent missed-coverage case in the resolver_test.go run because that package doesn't import the e2e package. 🤖 Generated with [Claude Code](https://claude.com/claude-code)	2026-05-05 19:10:18 -07:00
Hongming Wang	9ceda9d81f	refactor(events): migrate 18 files to typed EventType constants (RFC #2945 PR-B-1) ... Mechanical migration of bare event-name strings in BroadcastOnly / RecordAndBroadcast call sites to the typed constants from internal/events/types.go (RFC #2945 PR-B). Wire format unchanged (both shapes serialize to identical WSMessage.Event literals); pinned by TestAllEventTypes_IsSnapshot in #2965. Migrated (18 files, scope: handlers/, scheduler/, registry/, bundle/, channels/): - handlers/{approvals,a2a_proxy_helpers,a2a_queue,activity,agent, delegation,external_rotate,org_import,registry,workspace, workspace_bootstrap,workspace_crud,workspace_provision_shared, workspace_restart}.go - channels/manager.go (caught by hostile-reviewer pass — initial scope missed channels/, found via grep on the post-migration tree) - scheduler/scheduler.go - registry/provisiontimeout.go - bundle/importer.go Hostile self-review (3 weakest spots, addressed) ------------------------------------------------ 1. Missed call sites — initial scope omitted channels/. Post-migration `grep -rEn 'BroadcastOnly\([^,]+,[^,]*"[A-Z_]+"|RecordAndBroadcast\([^,]+,[^,]*"[A-Z_]+"' internal/` found 2 stragglers in channels/manager.go. Migrated. Final grep on the same pattern returns only the docstring example in types.go (intentional). 2. gofmt drift — auto-import injection produced non-canonical import ordering. `gofmt -w` applied ONLY to the 18 modified files (NOT the whole tree, to avoid sweeping unrelated pre-existing drift into this PR's diff). Three pre-existing un-gofmt'd files in handlers/ (a2a_proxy.go, a2a_proxy_test.go, a2a_queue_test.go) left as-is — they're unchanged by this PR and their drift predates it. 3. Wire format — paranoia check: do the constants serialize to the exact strings consumers (canvas TS, hermes plugin, anything parsing WSMessage.Event) expect? Yes. Pinned by the snapshot test. The migration is name-only; not a single character of wire output changes. Verified - go build ./... clean - go vet ./internal/... clean - gofmt -l on the 5 migrated package dirs: only pre-existing files - Full tests: handlers/, channels/, scheduler/, registry/, events/, bundle/ all green (5 ok, 0 fail) PR-B-2 (canvas TS mirror + cross-language parity gate) remains as the final piece of RFC #2945 PR-B. Tracked separately so this PR stays mechanical + reviewable. Refs RFC #2945, PR #2965 (PR-B types). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>	2026-05-05 19:05:03 -07:00