Same shape as molecule-controlplane#29: per-job GITHUB_TOKEN
doesn't have the Gitea API permissions to open PRs / push branches
the auto-sync flow needs. AUTO_SYNC_TOKEN is the devops-engineer
persona PAT (per saved memory feedback_per_agent_gitea_identity_default).
Companion prod ops (already done):
- devops-engineer added as collaborator on molecule-core (write)
- devops-engineer added to staging branch protection push_whitelist
- AUTO_SYNC_TOKEN registered as Actions secret on molecule-core
All current core/staging reds ran 12:14-12:33 BEFORE the runner
image swap (cloudflared bake + GOPROXY pipe-separator at 12:55).
This empty commit forces a fresh CI run under the post-fix
runner image so we can categorize:
- REAL fails (need targeted fix)
- STALE-cleared (was a runner-image issue, now fixed)
- Genuinely unrelated (Auto-sync, CodeQL — Hongming-parked)
Per feedback_orchestrator_must_verify_before_declaring_fixed,
don't mass-mark stale — wait for fresh run, verify each context.
Two coupled cleanups for the post-2026-05-06 stack:
#157 — drop molecule-ai-plugin-github-app-auth
============================================
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.
#161 — swap GHCR→ECR for publish-workspace-server-image
=======================================================
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.
The README hadn't been refreshed since the v0 wave. Several major
shipped surfaces weren't called out (Canvas v4 warm-paper theme,
Memory v2 with pgvector, RFC #2967 typed-SSOT A2A response path,
the SaaS control plane, the molecule-mcp-claude-channel plugin we
just shipped via v0.4.0/0.4.1/0.4.2). The runtime list still said
"6" when 8 are in production. The icon was a 1.3 MB PNG with no
light-mode variant.
- New `docs/assets/branding/molecule-icon.svg` matches the landing
page's `public/favicon.svg` shape (5-spoke molecular graph) but
carries `prefers-color-scheme` styles so it adapts to GitHub's
light/dark modes. The PNG stays for back-compat with anything
that hotlinks it.
- `docs/assets/branding/molecule-logo.svg` adds a wordmark variant
for places that want the brand name alongside the icon.
- README hero replaces the PNG `<img>` with the SVG so contributors
reading on GitHub light see a tinted version that doesn't blow
out the page background.
- **8 production runtimes** named explicitly throughout: Claude
Code, Hermes, Gemini CLI, LangGraph, DeepAgents, CrewAI, AutoGen,
OpenClaw. Comparison table grew Hermes 4 + Gemini CLI rows with
the integration mechanism (Option B upstream hook, A2A bridge,
multi-provider derivation).
- **Canvas v4** — warm-paper theme system (light / dark / follow-
system) called out alongside the existing Next.js 15 / React Flow /
Zustand stack.
- **Memory v2 backed by pgvector** — semantic recall callout in
both the "memory model" pitch line and the runtime stack section.
- **RFC #2967 typed-SSOT A2A response path** named in the platform
ship list + architecture diagram.
- **SaaS surface section** added — multi-tenant EC2 + Neon +
Cloudflare Tunnels, WorkOS + Stripe, KMS envelope, tenant_resources
audit + 30-min reconciler. Cross-links to molecule-controlplane.
- **molecule-mcp-claude-channel plugin** added — entry point for
Claude Code users to bridge A2A traffic into a local session via
MCP. Documents the standard marketplace install flow + multi-
tenant config.
- **Architecture diagram** redrawn with Canvas → Platform → Postgres
+ Provisioner (Docker | EC2+SSM) layout, plus a SaaS control plane
block.
- **Quick Start** repo URL fixed (`molecule-monorepo` → `molecule-core`),
Go version bumped to 1.25, Python ≥3.11 noted.
- Deploy buttons + Quick Start URL all bump from the old
`molecule-monorepo` name to the current `molecule-core`. Pre-fix
these clicked through to a 404.
The provisioner refactor (`registry.go` deletion + RegistryPrefix
env-driven changes) that lived alongside an earlier draft of this
README on the `docs/readme-refresh-2026-05-06` branch is OUT of
this PR — that work shipped separately via #6. This branch is
docs-only so the review surface is small and the merge is reversible.
- `git diff staging --stat`:
```
README.md | 75 +++++++++++++++++++++++-----------
docs/assets/branding/molecule-icon.svg | 28 +++++++++++++
docs/assets/branding/molecule-logo.svg | 17 ++++++++
3 files changed, 97 insertions(+), 23 deletions(-)
```
- SVGs validated in a browser at light + dark `prefers-color-scheme`.
- All linked docs (./docs/index.md, ./docs/quickstart.md, ./docs/
architecture/architecture.md, ./docs/api-protocol/platform-api.md,
./docs/agent-runtime/workspace-runtime.md, ./LICENSE, etc.) verified
to exist on staging.
- README.zh-CN.md mirror — non-trivial translation work; file as
separate issue if mirror is wanted.
- molecule-ai/.github org-profile README — Gitea has no equivalent
to GitHub's org-profile surface, and the GitHub org is suspended.
Skipped.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
The v2 dropped codex from TEMPLATES on the basis of "no
publish-image.yml = not part of cascade today." That was correct
about the immediate behavior but tripped cascade-list-drift-gate.yml
because manifest.json still declares codex (it IS a live runtime —
referenced from workspace/config.py and cloned into dev envs by
clone-manifest.sh; only the image-publish path is missing).
Restore codex to TEMPLATES (matching manifest) and add a runtime
soft-skip: probe each repo for .github/workflows/publish-image.yml
via the Gitea contents API and skip cleanly if 404. Final job log
distinguishes "complete across all" vs "complete with soft-skips".
This preserves the drift gate's invariant (TEMPLATES == manifest)
while honoring the empirical fact that codex has no publish-image
workflow yet. If codex later gains the workflow, no change here is
needed — the probe will see 200 and the cascade will fan out to it
naturally.
Refs molecule-core#14, molecule-core#20.
Empirical blocker on v1: Gitea 1.22.6 has no repository_dispatch /
workflow_dispatch trigger API (verified across 6 candidate paths in
issuecomment-913). v1's curl-POST loop would always exit-1.
v2 pivots to push-mode: each template repo got a small companion PR
(merged 2026-05-07) adding a `.runtime-version` file at root + a
`resolve-version` job in publish-image.yml that reads the file and
forwards the value to the reusable build workflow. publish-runtime
now updates that file via git-clone + commit + push, which trips
each template's existing `on: push: branches: [main]` trigger.
Behaviour changes vs v1:
- Templates list dropped from 9 → 8 (codex has no publish-image.yml
so was never part of the cascade in practice).
- 3-retry pull-rebase loop per template (handles concurrent-push
races without force-push). Failures collected, job exits 1 with
the failed-template list at the end.
- Idempotency: when re-run with the same version, templates already
pinned to that version contribute zero commits — operator can
safely re-run to retry partial failures.
- Author line: "publish-runtime cascade <publish-runtime@moleculesai
.app>" trailer makes it clear the commit is workflow-driven, not
human (per memory feedback_github_botring_fingerprint).
DISPATCH_TOKEN secret name unchanged (still consumed at
secrets.DISPATCH_TOKEN per 569df259).
Refs molecule-core#14, builds on molecule-core#20 issuecomment-923
(Phase 2 design).
The cascade workflow was reading from `secrets.TEMPLATE_DISPATCH_TOKEN`
but the plumbed secret name is `DISPATCH_TOKEN` (verified just now via
GET /repos/molecule-ai/molecule-core/actions/secrets — only DISPATCH_TOKEN
is set). Without this rename the cascade would always evaluate "secret
missing" and exit 1 on the next push to staging, defeating the entire
point of grant-role-access.sh --apply that just landed.
Three references updated:
- env mapping (`secrets.X` → `secrets.DISPATCH_TOKEN`)
- workflow_dispatch warning text
- push-trigger error text
The bash-side variable name is unchanged (still `DISPATCH_TOKEN`) so
the curl invocation at line 372 is unaffected. YAML round-trip parses
clean.
## Symptom
`publish-runtime.yml::cascade` fired a `repository_dispatch` to 10 workspace-template
repos via direct curl to `https://api.github.com/repos/...`. Post-2026-05-06 the
org's GitHub presence is suspended; every invocation 404s. The job's
`:⚠️:` posture meant the failure didn't propagate, leaving the runtime
PyPI publish → template image rebuild pipeline silently broken.
## Why Option A (rewrite) and not Option B (delete)
Verified 2026-05-07 by devops-engineer (molecule-core#14 thread):
- The cron-poll mechanism (/etc/cron.d/molecule-deploy-poll) tracks ONLY the
Vercel/Railway-deployed repos (landingpage/docs/molecule-app/molecules-market
/molecule-controlplane). It does NOT track workspace-template-* repos.
- Each of the 9 template `publish-image.yml` workflows has
`repository_dispatch: types: [runtime-published]` as a load-bearing trigger.
Without the cascade, when the runtime ships a new PyPI version, templates
don't auto-rebuild.
So Option B (delete) would silently break the runtime → template fan-out.
Option A (rewrite to Gitea's API shape) is the right call. Security-auditor
agreed after seeing the cron-poll TRACKED list.
## API surface change
| Concern | Pre-fix (GitHub) | Post-fix (Gitea) |
|---|---|---|
| URL | `https://api.github.com/repos/$REPO/dispatches` | `${GITEA_URL}/api/v1/repos/$REPO/dispatches` |
| Owner case | `Molecule-AI/...` | `molecule-ai/...` (lowercase, Gitea is case-sensitive) |
| Auth header | `Authorization: Bearer $DISPATCH_TOKEN` | `Authorization: token $DISPATCH_TOKEN` |
| Body shape | `{event_type, client_payload}` | UNCHANGED — Gitea is GitHub-compatible here |
| Success code | `204 No Content` | `204 No Content` (unchanged) |
`GITEA_URL` defaults to `https://git.moleculesai.app`; overridable via job env.
## Out-of-band: DISPATCH_TOKEN secret rotation
The DISPATCH_TOKEN secret was a GitHub PAT. It must be re-minted as a Gitea
PAT for the new API to authenticate. Per saved memory
`feedback_per_agent_gitea_identity_default`, this should be a dedicated
`publish-runtime-bot` persona token with `write:repository` scope on the
9 target repos — NOT the founder PAT.
This PR ships the workflow change. Token rotation is the operator-host
follow-up (security-auditor's lane) — coordinate the merge so the token
is in place before the next runtime release fires.
## Backwards compatibility
The workflow ran silently-broken since 2026-05-06 (every invocation 404
+ :⚠️: but no failure). So there is no functional regression from
"silently broken" to "actually working". Any in-progress operator-managed
manual dispatch path is unaffected; the Gitea API parallel path doesn't
require operator intervention.
## Test plan
- [x] YAML parse OK on the modified workflow file
- [ ] Smoke test: trigger a runtime publish (or simulate via dispatching to one
template) post-merge; verify HTTP 204 + the template's publish-image
workflow fires + the template's image gets re-pushed against the new
runtime version. Phase 4 verification belongs to internal#46 follow-up.
## Hostile self-review (3 weakest spots)
1. The fan-out remains all-or-nothing: a single template failure surfaces as
a `:⚠️:` but PyPI publish proceeds. With 9 templates this is a
~10% per-template chance of stale-image-on-runtime-bump if any one fails.
Defense: the warning shows up in the workflow summary; operators retry.
Future hardening: requeue-on-fail with bounded retry, or a separate
reconcile cron that detects template/runtime version drift and re-dispatches.
2. `DISPATCH_TOKEN` validity is enforced by the Gitea API (401 on stale)
but the workflow doesn't differentiate 401 from 404. Either way the
warning fires. Future hardening: explicit token-shape check at the start
of the cascade job (curl `/api/v1/user` once, fail-fast if 401).
3. Owner-case lowercase is right today but couples the workflow to the
current Gitea org slug. If the org is ever renamed, this workflow
breaks silently. Less fragile alternative: derive REPO from a
canonical config (e.g. `gh repo list molecule-ai`) instead of
string-concatenating. Acceptable today; filed as the same future
hardening pass as item 1.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mechanical pin: 4 `actions/upload-artifact@v4.6.2/v7.0.1` uses → `@v3`. v4+/v7+
rely on a runtime API shape that Gitea's act_runner v0.6.x doesn't fully
support. v3 uses the legacy server protocol act_runner ships end-to-end.
Files (4 uses):
- .github/workflows/ci.yml:238 (v4.6.2 → v3)
- .github/workflows/codeql.yml:124 (v7.0.1 → v3)
- .github/workflows/e2e-staging-canvas.yml:142 (v7.0.1 → v3)
- .github/workflows/e2e-staging-canvas.yml:150 (v7.0.1 → v3)
YAML parse green on all 3 files.
Sister PRs land for `molecule-controlplane` and `codex-channel-molecule`.
Per internal#46 Phase 2 audit; tracked under that umbrella.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per documentation-specialist's grep agent (2026-05-07T07:30, see
internal#46): runtime-breaking ghcr.io references in shell scripts +
docker-compose + the slip-past-workflow lint_secret_pattern_drift.py
all need migration. These were missed by security-auditor's
workflow-only audit.
Files (6):
- .github/scripts/lint_secret_pattern_drift.py:40 — workspace-runtime
pre-commit-checks.sh consumer URL: raw.githubusercontent.com →
Gitea raw URL (https://git.moleculesai.app/molecule-ai/.../raw/
branch/main/...). The lint job runs in CI and would 404 today.
- scripts/refresh-workspace-images.sh:54 — workspace-template image
pull URL: ghcr.io → ECR (153263036946.dkr.ecr.us-east-2.amazonaws.com).
- scripts/rollback-latest.sh — full rewrite of header + auth flow:
* ghcr.io/molecule-ai/{platform,platform-tenant} → ECR
* GITHUB_TOKEN with write:packages → AWS ECR auth
(aws ecr get-login-password). Per saved memory
reference_post_suspension_pipeline, prod cutover is to ECR.
* Updated header docs to match new auth flow + prereqs.
- scripts/demo-freeze.sh:13,17 — comment-only ghcr → ECR
(the script doesn't currently exec these URLs, but the comments
describe the cascade and need to match reality).
- docker-compose.yml:215-216 — canvas image: ghcr.io → ECR + updated
the auth comment to describe `aws ecr get-login-password` flow.
- tools/check-template-parity.sh:21 — inline curl install instructions:
raw.githubusercontent.com → Gitea raw URL.
Hostile self-review:
1. rollback-latest.sh's GITHUB_TOKEN→aws-cli auth swap is a behavior
change. Operators using this script now need aws CLI
authenticated for region us-east-2 with ECR pull/push perms.
Documented in updated header. Operators who don't have aws CLI
will get 'aws: command not installed' which is a clear failure
mode (not silent).
2. The Gitea raw URL shape (/raw/branch/main/) differs from GitHub's
raw.githubusercontent.com structure. Verified pattern by
inspecting other Gitea raw URLs in the codebase. If Gitea's URL
changes (1.23+), update via the same one-line edit.
3. Doesn't touch packer/scripts/install-base.sh which has a similar
ghcr.io ref per the grep agent's findings — that's bigger-scope
(packer build pipeline) and lives in molecule-controlplane-ish
territory; filing as parked follow-up under #46 if not already.
Refs: molecule-ai/internal#46, molecule-ai/internal#37,
molecule-ai/internal#38, saved memory reference_post_suspension_pipeline
The molecule-ai-workspace-runtime mirror is regenerated on every
runtime-v* tag from this monorepo's workspace/. Per saved memory
reference_runtime_repo_is_mirror_only, mirror-guard rejects direct
PRs to the mirror; edit at source.
Source-side files that propagate to the mirror's published README +
read by users of the in-monorepo workspace-runtime docs:
- scripts/build_runtime_package.py (the README generator):
* line 281 README_TEMPLATE: 'Shared workspace runtime for Molecule
AI' link → Gitea
* line 399 doc-link to workspace-runtime-package.md → Gitea path
(with /src/branch/main/ shape)
LEFT AS-IS (per Q3 audit-trail decision):
* lines 379, 392 historical issue cross-refs (#2936, #2937)
- workspace/build-all.sh:5 — comment block linking to template-*
repos. Migrated to Gitea path-shape.
- docs/workspace-runtime-package.md:
* lines 101-108 adapter→repo table (8 templates, all PUBLIC on
Gitea) — Gitea URLs
* line 247 starter-repo link — substituted host + added inline
note that starter doesn't survive the suspension migration
(recreation pending; cross-link to this issue)
* line 259 generic git clone command for new templates → Gitea
* line 289 second starter mention — same handling as 247
Files NOT touched in this PR:
- workspace/ Python source code (.py files) — those use github
paths in docstrings + a few log strings; fix bundled with the
cross-repo Go-module-style migration (per #37 Q5 + parked
follow-ups).
- 'Writing a new adapter' section's `gh repo create` command (line
254-256) — gh CLI doesn't talk to Gitea (per #45 parked follow-up).
- 'Writing a new adapter' section's ghcr.io image ref (line 276) —
per #46 ghcr→ECR migration (separate concern).
After this PR merges to staging + a runtime-v* tag is pushed, the
mirror's published README will inherit the Gitea link. Until then
the mirror's README continues to reference github.com/Molecule-AI
(stale but historical-marker-correct since the mirror existed
pre-suspension).
Refs: molecule-ai/internal#41, molecule-ai/internal#37,
molecule-ai/internal#38, molecule-ai/internal#42,
molecule-ai/internal#45, molecule-ai/internal#46
## Symptom
Canvas detail-panel "config + filesystem load" took ~20s. Reported on
production hongming tenant, workspace c7c28c0b-... (Claude Code Agent T2).
## Two stacked latency sources
### 1. Server-side: per-call EIC tunnel setup (~80% of the win)
`workspace-server/internal/handlers/template_files_eic.go::realWithEICTunnel`
performed ssh-keygen + SendSSHPublicKey + open-tunnel + waitForPort PER call.
4 callers (read/write/list/delete) each paid the full ~3-5s setup cost even
when fired back-to-back on the same workspace EC2.
Fix: refcounted pool keyed on instanceID with TTL ≤ 50s (under the 60s
SendSSHPublicKey grant). One tunnel serves N file ops; concurrent acquires
for the same instance share the slot via a pendingSetups gate; LRU eviction
caps simultaneous tracked instances at 32. Poisons entries on tunnel-fatal
errors (connection refused, broken pipe, auth failed) so the next acquire
builds fresh. Cleanup on panic via defer-release pattern (added after
self-review caught a refcount-leak hazard).
Public API unchanged — `var withEICTunnel` rebinds to `pooledWithEICTunnel`
at package init, so all 4 callers inherit pooling for free.
10 unit tests pin: 4-ops-amortise (1 setup), different-instances-do-not-share,
TTL eviction, poison invalidates, concurrent-acquire-single-setup,
TTL=0 escape hatch, LRU eviction at cap, error classification heuristic,
refcount blocks expired eviction, panic poisons entry. All green.
### 2. Canvas-side: serial fan-out + duplicate fetch (~20% of the win)
`canvas/src/components/tabs/ConfigTab.tsx::loadConfig` awaited 3 independent
metadata GETs (`/workspaces/{id}`, `/model`, `/provider`) serially.
`AgentCardSection` fired a SECOND `/workspaces/{id}` from its own useEffect.
Fix: Promise.all over the 3 metadata GETs (each leg keeps its existing
.catch fallback semantics). AgentCardSection now reads `agentCard` from
the canvas store (`useCanvasStore`) instead of refetching — the canvas
already hydrates `node.data.agentCard` from the platform event stream.
Defensive selector handles test mocks without a `nodes` array.
## Verification
- `go test ./internal/handlers/` 5.07s green (full handlers package, including
10 new pool tests)
- `go vet ./internal/handlers/` clean
- `npx vitest run` — 1380/1380 canvas unit tests pass (2 test FILES fail on
a pre-existing xyflow CSS-load issue in vitest config, unrelated to this
change)
- `npx tsc --noEmit` clean
Live wall-time verification deferred to Phase 4 / E2E (canvas browser session
required; external probe blocked by 403 since the canvas auth chain is
session-cookie + Origin header, not a bearer token I can fabricate).
## Backwards compatibility
API surface unchanged. All 4 EIC handler callers use the rebound var; no
caller migration. Pool defaults to enabled (TTL=50s); tests can disable by
setting poolTTL=0 or by overwriting withEICTunnel directly (existing stub
pattern in template_files_eic_dispatch_test.go preserved).
## Hostile self-review (3 weakest spots)
1. `fnErrIndicatesTunnelFault` is a substring grep on err.Error() — the
marker list is hand-curated and ssh client error formats vary across
OpenSSH versions. A future ssh that reports a tunnel failure via a
phrasing not in the list would NOT poison the entry → next callers reuse
a dead tunnel until TTL evicts. Acceptable: TTL bounds the impact (≤50s
of bad reuse), and the heuristic covers every tunnel-error shape that
appears in the existing test fixtures and known incidents.
2. `acquire`'s for-loop has unbounded retry potential under pathological
churn (signal closed → new acquirer → setup fails → repeat). No bounded
retry counter. Today there is no test exercise for "flaky setup that
succeeds-then-fails-then-succeeds"; if observability ever shows this
shape, add a max-retry guard. Filed as a known limitation, not blocking.
3. The substring assertion `strings.Contains` style I used for tunnel-fault
classification could false-positive on app-level error messages that
happen to contain "permission denied" or "broken pipe" verbatim. The
classification test covers the discriminator but only against the
error shapes we know today. Acceptable: poisoning errs on the side of
building fresh, which is correct-but-slightly-slow rather than incorrect.
## Phase 4 / E2E plan
- Live timing of the canvas detail-panel open against a real workspace
(browser session, not external probe).
- Target: perceived latency under 2s on warm pool. Cold open still pays
one tunnel setup (~3-5s) — the pool buys you the SECOND through Nth
panel-open within the TTL window.
- Memory `feedback_chase_verification_to_staging` applies — will not
declare done at PR-merge; will follow through to user-visible behavior
on staging.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the SSOT story shipped in PR-C/D: canvas now consumes the typed
/chat-history endpoint instead of /activity?type=a2a_receive, and the
server emits messages in display-ready chronological order so the
client doesn't have to re-order them.
## Canvas (consumer migration)
- loadMessagesFromDB swaps from /activity to /chat-history.
- Drops type=a2a_receive + source=canvas params (server applies the
filter centrally now).
- Drops [...activities].reverse() — wire is already display-ready.
- Drops the local INTERNAL_SELF_MESSAGE_PREFIXES constant +
isInternalSelfMessage helper. Server-side IsInternalSelfMessage
applies the same predicate before emitting rows.
- Drops the activityRowToMessages + ActivityRowForHydration imports
from historyHydration.ts. The TS parser stays in tree because
message-parser.ts is still load-bearing for live A2A WebSocket
messages (ChatTab.tsx:805, AgentCommsPanel.tsx, canvas-events.ts).
## Server (row-aware wire-order fix)
The pre-PR-C-2 client did `[...activities].reverse()` over ROWS, then
flattened each row into [user, agent] messages. The reversal was
ROW-aware. After PR-C/D, the server returned a flat ChatMessage slice
in `ORDER BY created_at DESC` order, with [user, agent] within each
row. A naive client-side flat reverse would FLIP each pair (agent
before user at same timestamp).
Two ways to fix it:
A) Server emits oldest-first within page; canvas does NOT reverse.
B) Canvas does row-aware reversal (group by timestamp, reverse).
Option A is cleaner — server owns the wire-order responsibility, every
client trusts `for m of messages` to render chronologically. Server
adds reverseRowChunks() that:
1. Groups consecutive same-Timestamp messages into row chunks
(1-2 messages per row).
2. Reverses the chunk order (newest-row-first → oldest-row-first).
3. Flattens. Within-chunk [user, agent] order is preserved.
Single-message rows (agent reply not yet recorded, attachments-only
user upload) collapse to 1-element chunks and reverse correctly too.
## Tests
Server: 3 new unit tests on reverseRowChunks (paired across rows,
single-message rows, empty input) + 1 sqlmock integration test on
List() that drives the full SQL → reverse → wire path. Mutation-tested:
removed `messages = reverseRowChunks(messages)` from List(), confirmed
the integration test fires red with all 4 misordered indices flagged.
Restored, all 25 messagestore tests + 9 chat-history handler tests
green.
Canvas: 8 lazyHistory pagination tests refactored to mock
/chat-history (not /activity) and assert against the new wire shape
({messages, reached_end} not raw activity rows). All 1389/1389 vitest
tests green; tsc --noEmit clean.
## Three weakest spots (hostile-reviewer self-pass)
1. reverseRowChunks groups by Timestamp string equality. If two
distinct rows had the SAME timestamp (legitimately possible at sub-
millisecond granularity), the algorithm would treat them as one
chunk and not reverse them relative to each other. Mitigated:
activity_logs.created_at uses microsecond resolution; concurrent
inserts at exact-same microsecond are vanishingly rare. If a
collision happens, the within-chunk order is whatever the SQL
returned — both rows render at the same timestamp, no user-visible
misordering.
2. The pre-existing TS parser files (historyHydration.ts +
message-parser.ts) stay in tree. historyHydration.ts is now dead
code (no consumers post-migration); deletion is parked as a follow-
up after a one-week observation window confirms no live-message
consumer reaches it.
3. canvas's loadMessagesFromDB returns `resp.messages ?? []`. If the
server were ever to return `null` instead of `[]` (it currently
doesn't — handler defensively coerces nil to []), the nullish coalesce
keeps the canvas from crashing. A stricter wire schema would assert
the never-null invariant; for today's pragmatic safety, the ?? is
enough.
## Security review
- Untrusted input? Same as PR-C — agent JSON parsed defensively in
the messagestore parser. No new exposure.
- Trust boundary? Same. Canvas → /chat-history → wsAuth → messagestore.
- Output sanitization? Plain text + opaque attachment URIs as before.
No security-relevant changes beyond what /chat-history already
exposes via PR-C. Considered, not skipped.
## Versioning / backwards compat
- /activity endpoint unchanged.
- /chat-history endpoint shape unchanged (still {messages, reached_end});
only the wire ORDER within a page changed (newest-first row → oldest-
first row). Canvas is the only consumer in tree; no API consumers
depend on the previous order.
- canvas's loadMessagesFromDB call signature unchanged — internal
refactor.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
The deprovision path marks `workspaces.status='removed'` BEFORE calling
the controlplane DELETE. If that CP call fails (transient 5xx, network
hiccup, AWS provider error), the DB row stays at 'removed' with
`instance_id` populated and there's no retry — the EC2 lives forever.
9 prod orphans accumulated over 3 days under this bug.
Adds a SaaS-mode counterpart to the existing Docker `orphan_sweeper`:
- 60s tick (matches the Docker sweeper cadence)
- LIMIT 100 per cycle so a sustained CP outage drains over multiple
cycles without blowing the request timeout
- Re-issues `cpProv.Stop` for any workspace at status='removed' with a
non-NULL `instance_id`. Stop is idempotent (AWS terminate on
already-terminated is a no-op; CP's Deprovision tolerates already-
deleted DNS) so retries are safe.
- On Stop success, NULLs `instance_id` so the next cycle skips the row.
- On Stop failure, leaves `instance_id` populated for next cycle.
The existing Docker sweeper is gated on `prov != nil`; the new sweeper
is gated on `cpProv != nil`. SaaS tenants get exactly one of the two,
self-hosted tenants get the Docker one — no overlap.
Why this shape over option A (CP-first ordering) or B (durable outbox):
the existing inline path already returns a loud 500 to the user when
CP fails — the only missing piece is automatic retry, which a 60s
sweeper provides without protocol changes, new tables, or new workers.
~30 LOC of production code vs. ~400 for an outbox. RFC discussion in
#2989 comment chain.
Tests:
- 9 unit tests covering happy path, Stop failure, UPDATE failure,
multiple orphans (one-fails-others-still-process), DB query error,
nil-DB defense, nil-reaper short-circuit, and the boot-immediate-then-
tick cadence contract.
- Mutation-tested: status='running' substitution and removed-UPDATE-
block both fail at least one test.
Out of scope:
- Backfilling the 9 named orphans — they'll heal automatically on the
first sweep cycle after this lands; no manual cleanup needed.
- Long-term durable-outbox architecture — separate RFC.
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.
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>
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)
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>
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>
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>
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>
## 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>
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>
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>
