Compare commits

...

11 Commits

Author SHA1 Message Date
Molecule AI · core-uiux 3fd38e6deb fix(canvas): keep "agent is working" indicator alive for external/poll-mode turns
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 4s
CI / Detect changes (pull_request) Successful in 5s
E2E API Smoke Test / detect-changes (pull_request) Successful in 5s
E2E Chat / detect-changes (pull_request) Successful in 5s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 7s
Harness Replays / detect-changes (pull_request) Successful in 6s
CI / Platform (Go) (pull_request) Successful in 4m58s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m0s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 4s
gate-check-v3 / gate-check (pull_request) Successful in 4s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 6s
qa-review / approved (pull_request) Successful in 4s
security-review / approved (pull_request) Successful in 4s
sop-tier-check / tier-check (pull_request) Successful in 6s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
CI / Python Lint & Test (pull_request) Successful in 2s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 2s
E2E Chat / E2E Chat (pull_request) Failing after 2s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2s
Harness Replays / Harness Replays (pull_request) Successful in 2s
CI / Canvas (Next.js) (pull_request) Successful in 6m49s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 1s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / all-required (pull_request) Successful in 2s
sop-checklist / all-items-acked (pull_request) [info tier:low] acked: 5/7 — missing: root-cause, no-backwards-compat
sop-checklist / na-declarations (pull_request) N/A: (none)
audit-force-merge / audit (pull_request) Successful in 6s
External / MCP-registered workspaces (delivery_mode=poll, e.g. the
local-Mac orchestrator) have no public URL: POST /workspaces/:id/a2a
short-circuits server-side (a2a_proxy.go:402) and returns a synthetic
{status:"queued", delivery_mode:"poll"} envelope IMMEDIATELY with no
reply. useChatSend treated that as a terminal response and called
releaseSendGuards() → `sending` went false the instant the POST
returned → the ChatTab thinking indicator vanished and the external
turn looked dead, even though the agent had not started. Internal
agents reply on the same synchronous POST so their indicator naturally
holds — that's the whole asymmetry; the transport is shared.

Fix (Tier A, minimal, client-only): detect the synthetic poll envelope
and KEEP `sending` true so the existing internal working-indicator
persists as a "received — agent is working" state. The eventual reply
already flows AgentMessageWriter → AGENT_MESSAGE WS push →
useChatSocket onAgentMessage/onSendComplete → releaseSendGuards, i.e.
the external reply now drives the indicator through the exact same path
an internal async reply uses — no parallel system. A generous 15-min
safety timer surfaces an honest, actionable error instead of an
infinite spinner if an offline poll agent never replies.

Tier B (lifecycle-driven interim progress) and Tier C (tool-call
parity — has an operator-privacy tradeoff, CTO decision required) are
designed in internal RFC external-workspace-canvas-progress-feedback,
not in this PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 11:12:16 -07:00
devops-engineer 283ebd5b47 Merge pull request 'fix(canvas): make Settings→Secrets reveal honest (value is write-only)' (#1421) from fix/secrets-reveal-anthropic-internal into staging
Block internal-flavored paths / Block forbidden paths (push) Successful in 5s
CI / Detect changes (push) Successful in 8s
E2E API Smoke Test / detect-changes (push) Successful in 12s
E2E Chat / detect-changes (push) Successful in 13s
Handlers Postgres Integration / detect-changes (push) Successful in 14s
Harness Replays / detect-changes (push) Successful in 7s
Runtime PR-Built Compatibility / detect-changes (push) Successful in 11s
Secret scan / Scan diff for credential-shaped strings (push) Successful in 10s
CI / Platform (Go) (push) Successful in 10m3s
CI / Canvas (Next.js) (push) Successful in 10m34s
CI / Shellcheck (E2E scripts) (push) Successful in 1s
CI / Python Lint & Test (push) Successful in 1s
E2E API Smoke Test / E2E API Smoke Test (push) Successful in 2s
E2E Chat / E2E Chat (push) Failing after 1s
Harness Replays / Harness Replays (push) Successful in 2s
Handlers Postgres Integration / Handlers Postgres Integration (push) Successful in 1m17s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (push) Failing after 36s
CI / Canvas Deploy Reminder (push) Successful in 1s
CI / all-required (push) Successful in 1s
2026-05-17 17:32:30 +00:00
devops-engineer 13073cdedd Merge pull request 'fix(registry): reconcile agent_card identity from trusted workspaces row (internal#492)' (#1427) from fix/agent-card-identity-reconcile-internal-492 into staging
CI / Shellcheck (E2E scripts) (push) Blocked by required conditions
CI / Canvas Deploy Reminder (push) Blocked by required conditions
CI / Python Lint & Test (push) Blocked by required conditions
CI / all-required (push) Blocked by required conditions
E2E API Smoke Test / E2E API Smoke Test (push) Blocked by required conditions
E2E Chat / E2E Chat (push) Blocked by required conditions
Handlers Postgres Integration / Handlers Postgres Integration (push) Blocked by required conditions
Harness Replays / Harness Replays (push) Blocked by required conditions
Runtime PR-Built Compatibility / PR-built wheel + import smoke (push) Blocked by required conditions
Block internal-flavored paths / Block forbidden paths (push) Successful in 4s
CI / Detect changes (push) Successful in 6s
CI / Platform (Go) (push) Successful in 6m16s
CI / Canvas (Next.js) (push) Successful in 6m59s
E2E API Smoke Test / detect-changes (push) Successful in 7s
E2E Chat / detect-changes (push) Successful in 6s
Handlers Postgres Integration / detect-changes (push) Successful in 7s
Harness Replays / detect-changes (push) Successful in 4s
Runtime PR-Built Compatibility / detect-changes (push) Successful in 7s
Secret scan / Scan diff for credential-shaped strings (push) Successful in 4s
2026-05-17 17:00:11 +00:00
devops-engineer d79f28ace0 Merge pull request 'fix(workspace-server): actionable error when EIC config.yaml write is deadline-killed' (#1426) from fix/eic-write-timeout-actionable-error into staging
Block internal-flavored paths / Block forbidden paths (push) Waiting to run
CI / Shellcheck (E2E scripts) (push) Blocked by required conditions
CI / Detect changes (push) Waiting to run
CI / Platform (Go) (push) Waiting to run
CI / Canvas (Next.js) (push) Waiting to run
CI / Canvas Deploy Reminder (push) Blocked by required conditions
CI / Python Lint & Test (push) Blocked by required conditions
CI / all-required (push) Blocked by required conditions
E2E API Smoke Test / detect-changes (push) Waiting to run
E2E API Smoke Test / E2E API Smoke Test (push) Blocked by required conditions
E2E Chat / detect-changes (push) Waiting to run
E2E Chat / E2E Chat (push) Blocked by required conditions
Handlers Postgres Integration / detect-changes (push) Waiting to run
Handlers Postgres Integration / Handlers Postgres Integration (push) Blocked by required conditions
Harness Replays / detect-changes (push) Waiting to run
Harness Replays / Harness Replays (push) Blocked by required conditions
Runtime PR-Built Compatibility / PR-built wheel + import smoke (push) Blocked by required conditions
Runtime PR-Built Compatibility / detect-changes (push) Waiting to run
Secret scan / Scan diff for credential-shaped strings (push) Waiting to run
2026-05-17 17:00:08 +00:00
devops-engineer 0655d5acf0 Merge pull request 'fix(canvas): Secrets "Test" reports honest error instead of fake "Connection timed out"' (#1424) from fix/secret-test-honest-error-internal-492 into staging
Block internal-flavored paths / Block forbidden paths (push) Waiting to run
CI / Detect changes (push) Waiting to run
CI / Platform (Go) (push) Waiting to run
CI / Canvas (Next.js) (push) Waiting to run
CI / Shellcheck (E2E scripts) (push) Blocked by required conditions
CI / Canvas Deploy Reminder (push) Blocked by required conditions
CI / Python Lint & Test (push) Blocked by required conditions
CI / all-required (push) Blocked by required conditions
E2E API Smoke Test / detect-changes (push) Waiting to run
E2E API Smoke Test / E2E API Smoke Test (push) Blocked by required conditions
E2E Chat / detect-changes (push) Waiting to run
E2E Chat / E2E Chat (push) Blocked by required conditions
Handlers Postgres Integration / detect-changes (push) Waiting to run
Handlers Postgres Integration / Handlers Postgres Integration (push) Blocked by required conditions
Harness Replays / detect-changes (push) Waiting to run
Harness Replays / Harness Replays (push) Blocked by required conditions
Runtime PR-Built Compatibility / detect-changes (push) Waiting to run
Runtime PR-Built Compatibility / PR-built wheel + import smoke (push) Blocked by required conditions
Secret scan / Scan diff for credential-shaped strings (push) Waiting to run
2026-05-17 17:00:05 +00:00
core-be 488018b156 fix(registry): reconcile agent_card identity from trusted workspaces row (internal#492)
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 7s
CI / Detect changes (pull_request) Successful in 10s
E2E API Smoke Test / detect-changes (pull_request) Successful in 15s
E2E Chat / detect-changes (pull_request) Successful in 12s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 13s
Harness Replays / detect-changes (pull_request) Successful in 10s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m7s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 14s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 10s
gate-check-v3 / gate-check (pull_request) Successful in 10s
qa-review / approved (pull_request) Successful in 11s
security-review / approved (pull_request) Successful in 11s
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request) Successful in 11s
sop-tier-check / tier-check (pull_request) Successful in 9s
CI / Platform (Go) (pull_request) Successful in 11m38s
CI / Canvas (Next.js) (pull_request) Successful in 11m41s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
CI / Python Lint & Test (pull_request) Successful in 2s
E2E Chat / E2E Chat (pull_request) Failing after 4s
Harness Replays / Harness Replays (pull_request) Successful in 3s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 5s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 2m26s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2m16s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / all-required (pull_request) Successful in 1s
audit-force-merge / audit (pull_request) Successful in 3s
The runtime builds its AgentCard from config.name, which the
CP-regenerated /configs/config.yaml sets to the raw workspace UUID — so
/registry/register stored (and /.well-known/agent-card.json + peer
agent_card_url served) a card with name=<uuid>, description="",
role=null, even though the operator-controlled workspaces.name DB
column holds the friendly name the canvas shows ("Claude Code Agent").
Fleet-wide; live registry confirmed name=UUID for ws 3b81321b while
workspaces.name="Claude Code Agent".

Server-side, platform-controlled repair at the register upsert: when the
runtime-supplied agent_card.name is empty or equals the workspace UUID,
substitute the trusted workspaces.name; default a blank description from
the reconciled name; default role from workspaces.role. Gaps are only
FILLED — a card already carrying a real friendly name (external channel
agents) is never downgraded; malformed/edge cards are stored verbatim
(no-worse-than-before). Identity stays platform-sourced from the
operator-controlled DB row — the agent gains no self-edit. Works for all
runtimes without touching every template or the CP generator. The
WORKSPACE_ONLINE broadcast now carries the reconciled card so the canvas
live-updates with the friendly name.

Pure helper (agent_card_reconcile.go) is exhaustively unit-tested
without DB/HTTP. Upstream CP config.yaml regeneration, the missing role
key in the runtime register payload, and an editable description/skills
surface are RFC-scoped in internal#492.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 07:50:14 -07:00
core-be 8f9b6a73f9 fix(workspace-server): actionable error when EIC config.yaml write is deadline-killed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 3s
CI / Detect changes (pull_request) Successful in 5s
E2E API Smoke Test / detect-changes (pull_request) Successful in 17s
E2E Chat / detect-changes (pull_request) Successful in 15s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 14s
Harness Replays / detect-changes (pull_request) Successful in 6s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 11s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 6s
gate-check-v3 / gate-check (pull_request) Successful in 4s
qa-review / approved (pull_request) Successful in 6s
security-review / approved (pull_request) Successful in 6s
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request) Successful in 6s
sop-tier-check / tier-check (pull_request) Successful in 6s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m6s
CI / Platform (Go) (pull_request) Successful in 10m20s
CI / Canvas (Next.js) (pull_request) Successful in 11m22s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
CI / Python Lint & Test (pull_request) Successful in 1s
E2E Chat / E2E Chat (pull_request) Failing after 2s
Harness Replays / Harness Replays (pull_request) Successful in 2s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 2s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 55s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2m30s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / all-required (pull_request) Successful in 2s
audit-force-merge / audit (pull_request) Successful in 4s
When the per-op context deadline (eicFileOpTimeout=30s) fires,
exec.CommandContext SIGKILLs the ssh subprocess and Run() returns the
bare "signal: killed" with empty stderr. That surfaced to the canvas
Settings/Config tab as an opaque
`500 {"error":"ssh install: signal: killed ()"}` — giving the operator
no signal that the workspace was simply mid-provision with a slow/unready
EIC tunnel (internal#423; recurred 2026-05-17 on claude-code ws
3b81321b, blocking config save).

Detect context abortion explicitly and return a message that names the
cause and points at the Settings -> Secrets encrypted-write path (which
does NOT use this EIC file-write path) as the unblock for applying
provider credentials. The EIC mechanism, timeout value, and success
path are unchanged — this only improves the error a stuck write emits.

Refs internal#423. Same Settings-area opaque-500 theme as #1420.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 07:45:14 -07:00
core-fe 3fc585b939 fix(canvas): Secrets "Test" reports honest error instead of fake "timed out"
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 3s
CI / Detect changes (pull_request) Successful in 5s
E2E API Smoke Test / detect-changes (pull_request) Successful in 8s
E2E Chat / detect-changes (pull_request) Successful in 6s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 6s
Harness Replays / detect-changes (pull_request) Successful in 7s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m2s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 9s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 4s
gate-check-v3 / gate-check (pull_request) Successful in 6s
qa-review / approved (pull_request) Successful in 5s
security-review / approved (pull_request) Successful in 7s
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request) Successful in 5s
sop-tier-check / tier-check (pull_request) Successful in 6s
CI / Platform (Go) (pull_request) Successful in 6m12s
CI / Canvas (Next.js) (pull_request) Successful in 8m16s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
CI / Python Lint & Test (pull_request) Successful in 2s
E2E Chat / E2E Chat (pull_request) Failing after 1s
Harness Replays / Harness Replays (pull_request) Successful in 2s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 1s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 50s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 1m39s
CI / all-required (pull_request) Successful in 1s
audit-force-merge / audit (pull_request) Successful in 5s
The Secrets test button calls POST ${PLATFORM_URL}/secrets/validate, a
route that has never been implemented on the workspace-server router
(router.go registers /secrets, /secrets/values, /settings/secrets,
/admin/secrets — no /secrets/validate) nor on the Next.js canvas. Live
probe: POST /secrets/validate → HTTP 404 in 0.28s (a fast 404, not a
network timeout).

request() throws ApiError(404); TestConnectionButton's bare `catch {}`
swallowed it and unconditionally rendered the hardcoded string
"Connection timed out. Service may be down." — factually wrong and
indistinguishable from a real outage or a token rejection.

Minimal fix (same "make the dead affordance honest" approach as the
reveal control, internal#490 / PR#1421): bind the caught error and
surface the real failure — distinguish "validation not available"
(404/501), a non-404 server error (with status), and a genuine
connectivity failure. No speculative server-side validate endpoint.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 07:38:29 -07:00
core-be 0d6b61bfff fix(canvas): make Settings→Secrets reveal honest (value is write-only)
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 3s
CI / Detect changes (pull_request) Successful in 5s
E2E API Smoke Test / detect-changes (pull_request) Successful in 12s
E2E Chat / detect-changes (pull_request) Successful in 14s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 10s
Harness Replays / detect-changes (pull_request) Successful in 6s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 7s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 4s
qa-review / approved (pull_request) Successful in 8s
security-review / approved (pull_request) Successful in 8s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m4s
CI / Platform (Go) (pull_request) Successful in 8m18s
CI / Canvas (Next.js) (pull_request) Successful in 9m14s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 1s
CI / Python Lint & Test (pull_request) Successful in 1s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 2s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 2s
Harness Replays / Harness Replays (pull_request) Successful in 1s
E2E Chat / E2E Chat (pull_request) Failing after 9s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 1s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / all-required (pull_request) Successful in 4s
gate-check-v3 / gate-check (pull_request) Successful in 3s
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request) Successful in 4s
sop-tier-check / tier-check (pull_request) Successful in 6s
audit-force-merge / audit (pull_request) Successful in 5s
The eye/RevealToggle in SecretRow was a dead affordance: it flipped a
local `revealed` boolean but the row always rendered `masked_value` and
never consumed it, so nothing was ever revealed. RevealToggle renders an
eye-WITH-SLASH when revealed=true, so a clicked row looked "active" while
showing nothing — read by users as "this doesnt work" (reported on
CLAUDE_CODE_OAUTH_TOKEN / Anthropic group).

Root cause is not Anthropic/OAuth/category-specific and not a server
4xx/5xx: secret values are write-only from the browser by design — the
server List handler "Never exposes values", there is no per-secret
decrypt route, and the only decrypted path (GET /secrets/values) is bulk
+ token-gated for remote agents and never called by canvas. The client
has no plaintext-fetch function. Reveal is architecturally impossible
without a deliberate security regression (out of scope).

Fix: remove the dead toggle (+ its local state / auto-hide effect) and
show a static write-only indicator (lock + explanatory title). Edit
(rotate/replace) and Delete are unaffected and independent of reveal.

Refs: internal#490; sibling Secrets/Tokens fixes PR #1415 + #1420
(referenced in triage as internal#210 / internal#211). Does not touch
the agent-error path (internal#212).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 07:25:23 -07:00
devops-engineer 330f54d281 Merge pull request 'fix(tokens): Workspace Tokens tab 500 on 'global' sentinel (no node selected)' (#1415) from fix/workspace-tokens-global-sentinel-500 into staging
Block internal-flavored paths / Block forbidden paths (push) Successful in 4s
CI / Detect changes (push) Successful in 7s
E2E API Smoke Test / detect-changes (push) Successful in 5s
E2E Chat / detect-changes (push) Successful in 4s
Handlers Postgres Integration / detect-changes (push) Successful in 5s
Harness Replays / detect-changes (push) Successful in 3s
Runtime PR-Built Compatibility / detect-changes (push) Successful in 5s
Secret scan / Scan diff for credential-shaped strings (push) Successful in 4s
CI / Shellcheck (E2E scripts) (push) Successful in 1s
CI / Python Lint & Test (push) Successful in 5s
CI / Platform (Go) (push) Successful in 4m27s
E2E API Smoke Test / E2E API Smoke Test (push) Successful in 35s
E2E Chat / E2E Chat (push) Failing after 10s
Handlers Postgres Integration / Handlers Postgres Integration (push) Successful in 1m10s
Harness Replays / Harness Replays (push) Successful in 1s
CI / Canvas (Next.js) (push) Successful in 6m2s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (push) Failing after 30s
CI / Canvas Deploy Reminder (push) Successful in 2s
CI / all-required (push) Successful in 2s
2026-05-17 13:46:55 +00:00
hongming 4fd6612272 fix(tokens): make Workspace Tokens tab sentinel-aware + reject non-UUID workspace id
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 2s
CI / Detect changes (pull_request) Successful in 4s
E2E API Smoke Test / detect-changes (pull_request) Successful in 8s
E2E Chat / detect-changes (pull_request) Successful in 10s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 8s
Harness Replays / detect-changes (pull_request) Successful in 5s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 6s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 5s
gate-check-v3 / gate-check (pull_request) Successful in 4s
qa-review / approved (pull_request) Successful in 4s
security-review / approved (pull_request) Successful in 4s
sop-checklist / na-declarations (pull_request) N/A: (none)
sop-checklist / all-items-acked (pull_request) Successful in 5s
sop-tier-check / tier-check (pull_request) Successful in 4s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m6s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 2s
CI / Python Lint & Test (pull_request) Successful in 2s
E2E Chat / E2E Chat (pull_request) Failing after 10s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1m37s
Harness Replays / Harness Replays (pull_request) Successful in 2s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 2s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 1m55s
CI / Platform (Go) (pull_request) Successful in 5m8s
CI / Canvas (Next.js) (pull_request) Successful in 6m20s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / all-required (pull_request) Successful in 0s
audit-force-merge / audit (pull_request) Successful in 3s
Settings → Workspace Tokens 500'd whenever opened with no canvas node
selected. SettingsPanel passes the literal sentinel "global" as the
workspace id; the backend queries the uuid `workspace_id` column with
it → Postgres `invalid input syntax for type uuid: "global"` → opaque
500 ("failed to list tokens"). Token create in that view broke the same
way. SecretsTab already handles the sentinel (api/secrets.ts reroutes
"global" → /settings/secrets); TokensTab did not — that asymmetry was
the bug. Pre-existing since 2026-04-13, NOT a regression.

Frontend (user-visible fix): TokensTab is now sentinel-aware like
SecretsTab. When workspaceId === "global" (no node selected) it no
longer calls /workspaces/global/tokens — it renders a clean state
pointing the user to the Org API Keys tab (the existing org-wide
surface). No 500, no scary error banner. The red account "Error" in
this view was just this 500 surfacing through TokensTab's local error
banner; it resolves with this guard (verified in code — no separate
widget).

Backend (defense-in-depth, same PR): List/Create/Revoke validate
c.Param("id") as a UUID up front and return 400 {"error":"invalid
workspace id"} instead of leaking a DB type error as a 500. Added the
missing log.Printf on the List query-error branch — it was the only
token handler silently swallowing the DB error, which is why this
incident had zero log trail. Mirrors the uuid.Parse guard already in
handlers/activity.go.

Workaround (pre-merge): select a workspace node before opening the
tab, or use the Org API Keys tab.

Product note for CTO: there is no /workspaces/global/tokens endpoint
(workspace tokens are inherently per-workspace; the org-wide
equivalent is the separate Org API Keys tab), so — unlike SecretsTab
which reroutes to a real global-secrets endpoint — the lowest-risk
safe behavior was a disabled state + pointer to Org API Keys rather
than a reroute. Flag if a different UX is wanted.

Tests: added TokensTab sentinel tests (no API call + Org-pointer) and
a backend table test asserting List/Create/Revoke 400 on non-UUID id
without hitting the DB. Updated existing token handler tests to use
valid UUIDs (they used "ws-1" etc.).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 06:22:00 -07:00
17 changed files with 1026 additions and 61 deletions
@@ -11,13 +11,21 @@ import { render, screen, fireEvent, cleanup, act } from "@testing-library/react"
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { TestConnectionButton } from "../ui/TestConnectionButton";
import type { SecretGroup } from "@/types/secrets";
import { validateSecret } from "@/lib/api/secrets";
import { validateSecret, ApiError } from "@/lib/api/secrets";
// ─── Mock validateSecret ──────────────────────────────────────────────────────
// vi.mock is hoisted, so validateSecret (imported above) refers to the mocked
// namespace value once vi.mock runs. Use vi.mocked() to access it in tests.
vi.mock("@/lib/api/secrets", () => ({
validateSecret: vi.fn(),
ApiError: class ApiError extends Error {
status: number;
constructor(status: number, message: string) {
super(message);
this.name = "ApiError";
this.status = status;
}
},
}));
// SecretGroup is a string literal type: 'github' | 'anthropic' | 'openrouter' | 'custom'
@@ -102,7 +110,7 @@ describe("TestConnectionButton — state machine", () => {
expect(screen.getByText("Permission denied")).toBeTruthy();
});
it("shows generic error message on unexpected exception", async () => {
it("shows a connectivity message on a genuine network exception", async () => {
vi.mocked(validateSecret).mockRejectedValue(new Error("timeout"));
render(<TestConnectionButton provider={toGroup("anthropic")} secretValue="sk-..." />);
@@ -110,8 +118,23 @@ describe("TestConnectionButton — state machine", () => {
await act(async () => { /* flush */ });
expect(screen.getByRole("alert")).toBeTruthy();
// The error detail is hardcoded to "Connection timed out. Service may be down."
expect(document.body.querySelector('[role="alert"]')?.textContent).toMatch(/timed out/i);
// A real thrown network error → honest connectivity message (not a
// fabricated "service down"); see internal#492.
expect(document.body.querySelector('[role="alert"]')?.textContent).toMatch(
/could not reach the validation service/i,
);
});
it("does not claim a timeout when the validate endpoint 404s (internal#492)", async () => {
vi.mocked(validateSecret).mockRejectedValue(new ApiError(404, "Not Found"));
render(<TestConnectionButton provider={toGroup("anthropic")} secretValue="sk-..." />);
fireEvent.click(screen.getByRole("button"));
await act(async () => { /* flush */ });
const alert = document.body.querySelector('[role="alert"]')?.textContent ?? "";
expect(alert).not.toMatch(/timed out/i);
expect(alert).toMatch(/not available/i);
});
});
+19 -23
View File
@@ -3,16 +3,24 @@ import { useState, useCallback, useRef, useEffect } from 'react';
import type { Secret, SecretGroup } from '@/types/secrets';
import { useSecretsStore } from '@/stores/secrets-store';
import { StatusBadge } from '@/components/ui/StatusBadge';
import { RevealToggle } from '@/components/ui/RevealToggle';
import { KeyValueField } from '@/components/ui/KeyValueField';
import { ValidationHint } from '@/components/ui/ValidationHint';
import { TestConnectionButton } from '@/components/ui/TestConnectionButton';
import { validateSecretValue } from '@/lib/validation/secret-formats';
import { SERVICES } from '@/lib/services';
const AUTO_HIDE_MS = 30_000;
const VALIDATION_DEBOUNCE_MS = 400;
// Secret values are write-only from the browser: the server List endpoint
// "Never exposes values", there is no per-secret decrypt route, and the
// only decrypted path (GET /secrets/values) is bulk + token-gated for
// remote agents. The old eye/RevealToggle was a dead affordance — it
// flipped its own icon but could never reveal anything, which read as
// "this doesn't work" (esp. once clicked → eye-with-slash). We show an
// honest static indicator instead; rotation is via Edit.
const WRITE_ONLY_TITLE =
'Value is write-only and cannot be revealed — use Edit to replace/rotate it';
interface SecretRowProps {
secret: Secret;
workspaceId: string;
@@ -31,28 +39,12 @@ export function SecretRow({ secret, workspaceId }: SecretRowProps) {
const setSecretStatus = useSecretsStore((s) => s.setSecretStatus);
const isEditing = editingKey === secret.name;
const [revealed, setRevealed] = useState(false);
const [editValue, setEditValue] = useState('');
const [validationError, setValidationError] = useState<string | null>(null);
const [isSaving, setIsSaving] = useState(false);
const [saveError, setSaveError] = useState<string | null>(null);
const debounceRef = useRef<ReturnType<typeof setTimeout>>(undefined);
const editBtnRef = useRef<HTMLButtonElement>(null);
const revealTimerRef = useRef<ReturnType<typeof setTimeout>>(undefined);
// Auto-hide revealed value after 30s
useEffect(() => {
if (revealed) {
clearTimeout(revealTimerRef.current);
revealTimerRef.current = setTimeout(() => setRevealed(false), AUTO_HIDE_MS);
return () => clearTimeout(revealTimerRef.current);
}
}, [revealed]);
// Reset revealed state when panel closes (session-only)
useEffect(() => {
return () => setRevealed(false);
}, []);
// Debounced validation
useEffect(() => {
@@ -133,11 +125,15 @@ export function SecretRow({ secret, workspaceId }: SecretRowProps) {
{secret.masked_value}
</span>
<div className="secret-row__actions">
<RevealToggle
revealed={revealed}
onToggle={() => setRevealed((r) => !r)}
label={`Toggle reveal ${secret.name}`}
/>
<span
data-testid="write-only-indicator"
className="secret-row__write-only"
role="img"
aria-label={`${secret.name} value is write-only and cannot be revealed; use Edit to replace it`}
title={WRITE_ONLY_TITLE}
>
🔒
</span>
<StatusBadge status={secret.status} />
<button
type="button"
@@ -16,7 +16,40 @@ interface TokensTabProps {
workspaceId: string;
}
// The settings panel passes the literal sentinel "global" when no canvas
// node is selected. Workspace tokens are inherently per-workspace — there
// is no /workspaces/global/tokens endpoint (querying the uuid column with
// "global" 500s on Postgres). The org-wide equivalent lives in the
// separate "Org API Keys" tab. Mirrors the sentinel-awareness that
// api/secrets.ts already has (workspaceId === 'global' → /settings/secrets).
const GLOBAL_WORKSPACE_ID = 'global';
export function TokensTab({ workspaceId }: TokensTabProps) {
if (workspaceId === GLOBAL_WORKSPACE_ID) {
return (
<div className="p-4 space-y-4">
<div>
<h3 className="text-sm font-semibold text-ink">API Tokens</h3>
<p className="text-[10px] text-ink-mid mt-0.5">
Bearer tokens for authenticating API calls to this workspace.
</p>
</div>
<div className="text-center py-6">
<p className="text-xs text-ink-mid">Select a workspace node first</p>
<p className="text-[10px] text-ink-mid mt-1">
Workspace tokens are scoped to a single workspace. Select a node
on the canvas to manage its tokens, or use the{' '}
<span className="text-accent font-medium">Org API Keys</span> tab
for org-wide API keys.
</p>
</div>
</div>
);
}
return <WorkspaceTokensTab workspaceId={workspaceId} />;
}
function WorkspaceTokensTab({ workspaceId }: TokensTabProps) {
const [tokens, setTokens] = useState<Token[]>([]);
const [loading, setLoading] = useState(true);
const [creating, setCreating] = useState(false);
@@ -138,14 +138,54 @@ describe("SecretRow — display mode", () => {
expect(document.querySelector('[role="row"]')).toBeTruthy();
});
it("has Reveal, Copy, Edit, Delete buttons", () => {
it("has Copy, Edit, Delete buttons", () => {
render(<SecretRow secret={GITHUB_SECRET} workspaceId="ws-1" />);
expect(screen.getByTestId("reveal-toggle")).toBeTruthy();
expect(screen.getByRole("button", { name: /copy/i })).toBeTruthy();
expect(screen.getByRole("button", { name: /edit/i })).toBeTruthy();
expect(screen.getByRole("button", { name: /delete/i })).toBeTruthy();
});
// Regression: the reveal/eye control was a dead affordance. Clicking it
// flipped its own icon (eye → eye-with-slash) but never revealed the value,
// because secret values are write-only from the browser (server List
// "Never exposes values"; there is no per-secret decrypt endpoint and the
// client has no plaintext-fetch function). The honest fix removes the
// toggle and shows a static "write-only / cannot be revealed" indicator.
// See internal tracking issue + internal#210/#211.
it("does NOT render a reveal/eye toggle (values are write-only)", () => {
render(<SecretRow secret={GITHUB_SECRET} workspaceId="ws-1" />);
expect(screen.queryByTestId("reveal-toggle")).toBeNull();
expect(
screen.queryByRole("button", { name: /toggle reveal/i }),
).toBeNull();
});
it("shows a write-only indicator explaining the value cannot be revealed", () => {
render(<SecretRow secret={ANTHROPIC_SECRET} workspaceId="ws-1" />);
const indicator = screen.getByTestId("write-only-indicator");
expect(indicator).toBeTruthy();
// Affordance must be honest: explain it cannot be revealed and that
// Edit is the rotate path. It must not be a clickable button.
const title = indicator.getAttribute("title") ?? "";
expect(title.toLowerCase()).toMatch(/write-only|cannot be revealed/);
expect(indicator.tagName).not.toBe("BUTTON");
});
it("write-only indicator is present for the Anthropic/OAuth-token row too", () => {
// The reported bug singled out CLAUDE_CODE_OAUTH_TOKEN (anthropic group);
// the fix is group-agnostic — every row gets the same honest affordance.
const OAUTH_SECRET = {
name: "CLAUDE_CODE_OAUTH_TOKEN",
masked_value: "••••••••••••••••9d2a",
group: "anthropic" as const,
status: "unverified" as const,
updated_at: "2024-01-04",
};
render(<SecretRow secret={OAUTH_SECRET} workspaceId="ws-1" />);
expect(screen.queryByTestId("reveal-toggle")).toBeNull();
expect(screen.getByTestId("write-only-indicator")).toBeTruthy();
});
it("shows invalid status correctly", () => {
render(<SecretRow secret={CUSTOM_SECRET} workspaceId="ws-1" />);
expect(screen.getByTestId("status-badge").getAttribute("data-status")).toBe("invalid");
@@ -302,3 +302,35 @@ describe("TokensTab — error", () => {
expect(document.querySelector('[role="status"]')).toBeNull();
});
});
// ─── "global" sentinel (no node selected) ────────────────────────────────────
//
// Regression: SettingsPanel passes the literal "global" when no canvas
// node is selected. workspace tokens are per-workspace and there is no
// /workspaces/global/tokens endpoint — calling it 500'd
// ("invalid input syntax for type uuid: global"). The tab must NOT call
// the API in that state and must point the user at the Org API Keys tab.
describe("TokensTab — global sentinel (no node selected)", () => {
beforeEach(() => {
mockApiGet.mockReset();
mockApiPost.mockReset();
mockApiGet.mockRejectedValue(new Error("should not be called"));
});
it("does not call the API and shows a pointer to Org API Keys", async () => {
render(<TokensTab workspaceId="global" />);
await flush();
expect(mockApiGet).not.toHaveBeenCalled();
expect(mockApiPost).not.toHaveBeenCalled();
expect(document.body.textContent).toContain("Select a workspace node");
expect(document.body.textContent).toContain("Org API Keys");
// No error banner, no scary 500 surfacing.
expect(document.querySelector(".text-bad")).toBeNull();
});
it("has no create button in the global state", async () => {
render(<TokensTab workspaceId="global" />);
await flush();
expect(document.body.textContent).not.toContain("New Token");
});
});
@@ -0,0 +1,209 @@
// @vitest-environment jsdom
/**
* Tests for useChatSend — the canvas user→agent send hook.
*
* Behavioural focus: the poll-mode ("queued") path. When the target
* workspace is an external / MCP-registered agent (delivery_mode=poll,
* e.g. an operator laptop running the molecule MCP channel), the
* platform's POST /workspaces/:id/a2a returns a synthetic
* {status:"queued", delivery_mode:"poll"} envelope IMMEDIATELY with no
* reply — the real reply arrives later over the AGENT_MESSAGE
* WebSocket push.
*
* Pre-fix the hook treated that synthetic envelope as a terminal
* response and called releaseSendGuards() → `sending` went false the
* instant the POST returned → the "agent is working" indicator
* vanished and the external turn looked dead. This suite pins the
* fixed contract:
*
* - a real reply still clears `sending` (regression guard)
* - a poll "queued" envelope KEEPS `sending` true (no terminal
* clear) so the existing thinking indicator persists
* - the eventual reply path (releaseSendGuards, the same call the
* AGENT_MESSAGE WS push makes via useChatSocket) clears it
* - an offline poll agent that never replies eventually surfaces an
* honest error instead of an infinite spinner
*
* Plus pure-function coverage for the poll-envelope detector.
*
* Root cause: workspace-server a2a_proxy.go:402 poll-mode
* short-circuit returns {status:"queued"} synchronously.
*/
import {
describe,
it,
expect,
vi,
beforeEach,
afterEach,
type Mock,
} from "vitest";
import { act, renderHook, cleanup } from "@testing-library/react";
const { mockApiPost } = vi.hoisted(() => ({ mockApiPost: vi.fn() }));
vi.mock("@/lib/api", () => ({
api: { post: mockApiPost },
}));
vi.mock("../uploads", () => ({
uploadChatFiles: vi.fn(),
}));
// Import AFTER mocks.
import {
useChatSend,
isPollQueuedResponse,
extractReplyText,
POLL_QUEUED_REPLY_TIMEOUT_MS,
} from "../useChatSend";
const flush = () => act(async () => { await Promise.resolve(); });
describe("isPollQueuedResponse", () => {
it("is true only for the synthetic poll-mode queued envelope", () => {
expect(isPollQueuedResponse({ status: "queued", delivery_mode: "poll" })).toBe(true);
});
it("is false for a real agent reply", () => {
expect(
isPollQueuedResponse({ result: { parts: [{ kind: "text", text: "hi" }] } }),
).toBe(false);
});
it("is false for null / undefined / partial shapes", () => {
expect(isPollQueuedResponse(null)).toBe(false);
expect(isPollQueuedResponse(undefined)).toBe(false);
// status=queued without delivery_mode=poll is NOT the poll envelope
// — don't accidentally swallow a real reply that happens to carry
// an unrelated status field.
expect(isPollQueuedResponse({ status: "queued" })).toBe(false);
expect(isPollQueuedResponse({ delivery_mode: "poll" })).toBe(false);
});
});
describe("extractReplyText (regression guard — unchanged by fix)", () => {
it("collects text parts from result", () => {
expect(
extractReplyText({ result: { parts: [{ kind: "text", text: "hello" }] } }),
).toBe("hello");
});
it("returns empty for the poll-queued envelope", () => {
expect(extractReplyText({ status: "queued", delivery_mode: "poll" })).toBe("");
});
});
describe("useChatSend — poll-mode in-progress state", () => {
beforeEach(() => {
vi.useFakeTimers();
mockApiPost.mockReset();
});
afterEach(() => {
vi.runOnlyPendingTimers();
vi.useRealTimers();
cleanup();
});
const setup = () => {
const onUserMessage = vi.fn();
const onAgentMessage = vi.fn();
const { result } = renderHook(() =>
useChatSend("ws-ext-1", {
getHistoryMessages: () => [],
onUserMessage,
onAgentMessage,
}),
);
return { result, onUserMessage, onAgentMessage };
};
it("a real reply clears `sending` (regression guard)", async () => {
mockApiPost.mockResolvedValue({
result: { parts: [{ kind: "text", text: "real reply" }] },
});
const { result, onAgentMessage } = setup();
await act(async () => {
void result.current.sendMessage("hi");
});
await flush();
expect(onAgentMessage).toHaveBeenCalledTimes(1);
expect(result.current.sending).toBe(false);
});
it("keeps `sending` true on a poll 'queued' envelope (no terminal clear)", async () => {
mockApiPost.mockResolvedValue({ status: "queued", delivery_mode: "poll" });
const { result, onAgentMessage } = setup();
await act(async () => {
void result.current.sendMessage("hi external agent");
});
await flush();
// The POST resolved, but it was only a queued ack — the indicator
// must stay up and no agent bubble should be rendered yet.
expect(result.current.sending).toBe(true);
expect(onAgentMessage).not.toHaveBeenCalled();
expect(result.current.error).toBeNull();
});
it("releaseSendGuards (the AGENT_MESSAGE-push path) clears the poll in-progress state", async () => {
mockApiPost.mockResolvedValue({ status: "queued", delivery_mode: "poll" });
const { result } = setup();
await act(async () => {
void result.current.sendMessage("hi");
});
await flush();
expect(result.current.sending).toBe(true);
// Simulate the terminal AGENT_MESSAGE WebSocket push arriving:
// useChatSocket's onAgentMessage / onSendComplete call
// releaseSendGuards. That must clear the in-progress state AND the
// safety timer (asserted by the next test).
act(() => {
result.current.releaseSendGuards();
});
expect(result.current.sending).toBe(false);
});
it("surfaces an honest error if a poll agent never replies (safety timeout)", async () => {
mockApiPost.mockResolvedValue({ status: "queued", delivery_mode: "poll" });
const { result } = setup();
await act(async () => {
void result.current.sendMessage("hi");
});
await flush();
expect(result.current.sending).toBe(true);
act(() => {
vi.advanceTimersByTime(POLL_QUEUED_REPLY_TIMEOUT_MS + 1000);
});
expect(result.current.sending).toBe(false);
expect(result.current.error).toMatch(/queued/i);
});
it("does NOT fire the safety error when the reply arrives before timeout", async () => {
mockApiPost.mockResolvedValue({ status: "queued", delivery_mode: "poll" });
const { result } = setup();
await act(async () => {
void result.current.sendMessage("hi");
});
await flush();
// Reply arrives (releaseSendGuards) well before the timeout.
act(() => {
result.current.releaseSendGuards();
});
act(() => {
vi.advanceTimersByTime(POLL_QUEUED_REPLY_TIMEOUT_MS + 1000);
});
expect(result.current.error).toBeNull();
expect(result.current.sending).toBe(false);
});
});
@@ -1,6 +1,6 @@
"use client";
import { useCallback, useRef, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import { api } from "@/lib/api";
import { uploadChatFiles } from "../uploads";
import { createMessage, type ChatMessage, type ChatAttachment } from "../types";
@@ -22,8 +22,42 @@ interface A2AResponse {
parts?: A2APart[];
artifacts?: Array<{ parts: A2APart[] }>;
};
/** Synthetic poll-mode envelope. The platform returns this
* immediately (HTTP 200) when the target workspace is registered
* delivery_mode=poll — an external / MCP-registered agent with no
* public URL (e.g. an operator's laptop running the molecule MCP
* channel). The request has only been QUEUED into activity_logs;
* the agent will pick it up on its next poll and the real reply
* arrives asynchronously over the AGENT_MESSAGE WebSocket push
* (consumed by useChatSocket). See workspace-server
* a2a_proxy.go:402 (poll-mode short-circuit) and
* a2a_proxy_helpers.go:516 (logA2AReceiveQueued). */
status?: string;
delivery_mode?: string;
}
/** True when `resp` is the platform's synthetic poll-mode "queued"
* envelope rather than a real agent reply. For these the send is
* acknowledged-but-pending: the user's message landed and the agent
* is working, but there is no reply yet — the terminal AGENT_MESSAGE
* push will arrive later over the WebSocket. Treating this as a
* terminal response (the pre-fix behaviour) cleared the "agent is
* working" indicator the instant the POST returned, so an external
* workspace turn looked dead even though work had not started. */
export function isPollQueuedResponse(resp: A2AResponse | null | undefined): boolean {
return !!resp && resp.status === "queued" && resp.delivery_mode === "poll";
}
/** Hard ceiling on how long the "agent is working" indicator stays up
* for a poll-mode turn with no reply. The terminal AGENT_MESSAGE push
* normally clears it well before this. The cap exists so a poll-mode
* workspace that is offline / never consumes its queue doesn't pin a
* spinner forever — at which point we surface an honest, actionable
* error instead of an opaque dead spinner. Generous because poll
* agents (an operator laptop) can legitimately take minutes to wake,
* poll, and respond; the goal is "eventually honest", not fail-fast. */
export const POLL_QUEUED_REPLY_TIMEOUT_MS = 15 * 60 * 1000;
export function extractReplyText(resp: A2AResponse): string {
const collect = (parts: A2APart[] | undefined): string => {
if (!parts) return "";
@@ -59,14 +93,29 @@ export function useChatSend(workspaceId: string, options: UseChatSendOptions) {
const sendInFlightRef = useRef(false);
const sendingFromAPIRef = useRef(false);
const sendTokenRef = useRef(0);
// Safety-net timer armed only for poll-mode ("queued") turns: the
// POST returns immediately with no reply, so the normal
// POST-resolves-→-clear-spinner path can't drive the indicator. The
// terminal AGENT_MESSAGE WebSocket push clears it via
// releaseSendGuards (which also clears this timer); the timer is the
// backstop for an offline poll agent that never consumes its queue.
const pollTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
const optionsRef = useRef(options);
optionsRef.current = options;
const clearPollTimeout = useCallback(() => {
if (pollTimeoutRef.current !== null) {
clearTimeout(pollTimeoutRef.current);
pollTimeoutRef.current = null;
}
}, []);
const releaseSendGuards = useCallback(() => {
clearPollTimeout();
setSending(false);
sendingFromAPIRef.current = false;
sendInFlightRef.current = false;
}, []);
}, [clearPollTimeout]);
const clearError = useCallback(() => setError(null), []);
@@ -146,6 +195,33 @@ export function useChatSend(workspaceId: string, options: UseChatSendOptions) {
sendInFlightRef.current = false;
return;
}
// Poll-mode ("queued") turn: the message landed and the
// external/MCP agent will pick it up on its next poll, but
// there is NO reply in this response. Pre-fix this fell
// through to releaseSendGuards() below and the "agent is
// working" indicator vanished the instant the POST returned —
// an external-workspace turn looked dead even though work had
// not started. Instead, keep `sending` true so the existing
// thinking indicator (the same one internal agents use)
// persists as a "received — agent is working" state; the
// terminal AGENT_MESSAGE WebSocket push (consumed by
// useChatSocket → onAgentMessage / onSendComplete →
// releaseSendGuards) clears it when the real reply arrives,
// exactly the path an internal async reply already uses.
if (isPollQueuedResponse(resp)) {
clearPollTimeout();
pollTimeoutRef.current = setTimeout(() => {
if (sendTokenRef.current !== myToken) return;
if (!sendingFromAPIRef.current) return;
releaseSendGuards();
setError(
"No response yet from this agent — it may be offline or " +
"busy. Your message was delivered and is queued; the " +
"reply will appear here if the agent picks it up.",
);
}, POLL_QUEUED_REPLY_TIMEOUT_MS);
return;
}
const replyText = extractReplyText(resp);
const replyFiles = extractFilesFromTask(
(resp?.result ?? {}) as Record<string, unknown>,
@@ -167,9 +243,15 @@ export function useChatSend(workspaceId: string, options: UseChatSendOptions) {
setError("Failed to send message — agent may be unreachable");
});
},
[workspaceId, sending, uploading],
[workspaceId, sending, uploading, clearPollTimeout],
);
// Drop the poll-mode safety timer on unmount / workspace switch so a
// stale timeout can't fire setError against a panel the user has
// already navigated away from. sendTokenRef guards correctness if it
// ever did fire; this just avoids the wasted timer + setState churn.
useEffect(() => clearPollTimeout, [clearPollTimeout]);
return {
sending,
uploading,
@@ -2,7 +2,7 @@
import { useState, useCallback, useRef, useEffect } from 'react';
import type { TestConnectionState, SecretGroup } from '@/types/secrets';
import { validateSecret } from '@/lib/api/secrets';
import { validateSecret, ApiError } from '@/lib/api/secrets';
interface TestConnectionButtonProps {
provider: SecretGroup;
@@ -55,9 +55,23 @@ export function TestConnectionButton({
}
onResult?.(result.valid);
resetTimerRef.current = setTimeout(() => setState('idle'), RESET_DELAYS[nextState]!);
} catch {
} catch (err) {
// Distinguish a real failure shape rather than always claiming a
// timeout. A reachable server that answered with an HTTP status
// (ApiError) did NOT time out — most commonly the validation route
// is not available (404/501), which must not masquerade as
// "service down". Only an actual thrown network/abort error is a
// connectivity failure.
setState('failure');
setErrorDetail('Connection timed out. Service may be down.');
if (err instanceof ApiError) {
setErrorDetail(
err.status === 404 || err.status === 501
? 'Key validation is not available for this service yet. The key was not tested.'
: `Could not verify key (server returned ${err.status}). Saving is unaffected.`,
);
} else {
setErrorDetail('Could not reach the validation service. Check your connection and try again.');
}
onResult?.(false);
resetTimerRef.current = setTimeout(() => setState('idle'), RESET_DELAYS.failure);
}
@@ -28,8 +28,20 @@ const mockValidateSecret = vi.fn();
vi.mock("@/lib/api/secrets", () => ({
validateSecret: (...args: unknown[]) => mockValidateSecret(...args),
ApiError: class ApiError extends Error {
status: number;
constructor(status: number, message: string) {
super(message);
this.name = "ApiError";
this.status = status;
}
},
}));
// Re-import the mocked ApiError so test cases construct the same class the
// component's `instanceof` check sees.
import { ApiError } from "@/lib/api/secrets";
beforeEach(() => {
vi.useFakeTimers();
vi.clearAllMocks();
@@ -201,8 +213,27 @@ describe("TestConnectionButton — failure path", () => {
});
describe("TestConnectionButton — catch path", () => {
it("shows 'Connection timed out' on network error", async () => {
mockValidateSecret.mockRejectedValue(new Error("timeout"));
it("does NOT claim a timeout when the validate endpoint 404s (regression: internal#492)", async () => {
// The validate route is unimplemented on the server and returns a fast
// 404. Before the fix this rendered the misleading hardcoded string
// "Connection timed out. Service may be down." It must instead state
// honestly that validation isn't available and the key was not tested.
mockValidateSecret.mockRejectedValue(new ApiError(404, "Not Found"));
render(
<TestConnectionButton provider="anthropic" secretValue="sk-ant-xxx" />,
);
fireEvent.click(document.querySelector('button[type="button"]')!);
await act(async () => {
await vi.advanceTimersByTimeAsync(0);
});
expect(document.body.textContent).not.toContain("Connection timed out");
expect(document.body.textContent).not.toContain("Service may be down");
expect(document.body.textContent).toContain("not available");
expect(document.body.textContent).toContain("not tested");
});
it("reports a non-404 server error with its status, not a timeout", async () => {
mockValidateSecret.mockRejectedValue(new ApiError(500, "Internal Server Error"));
render(
<TestConnectionButton provider="github" secretValue="ghp_xxx" />,
);
@@ -210,7 +241,20 @@ describe("TestConnectionButton — catch path", () => {
await act(async () => {
await vi.advanceTimersByTimeAsync(0);
});
expect(document.body.textContent).toContain("Connection timed out");
expect(document.body.textContent).toContain("500");
expect(document.body.textContent).not.toContain("Connection timed out");
});
it("shows a connectivity message on a genuine network error", async () => {
mockValidateSecret.mockRejectedValue(new Error("network down"));
render(
<TestConnectionButton provider="github" secretValue="ghp_xxx" />,
);
fireEvent.click(document.querySelector('button[type="button"]')!);
await act(async () => {
await vi.advanceTimersByTimeAsync(0);
});
expect(document.body.textContent).toContain("Could not reach the validation service");
});
it("calls onResult(false) on network error", async () => {
@@ -0,0 +1,113 @@
package handlers
import "encoding/json"
// agent_card_reconcile.go — server-side repair for the fleet-wide
// agent-card identity gap.
//
// Root cause: the runtime builds its AgentCard from config.name
// (workspace/main.py:198), and config.name is read from the
// CP-regenerated /configs/config.yaml whose `name:` field is the raw
// workspace UUID — NOT the friendly name the operator sees. The friendly
// name IS captured: POST /workspaces and PATCH /workspaces/:id (the
// canvas Details tab) write it to the trusted workspaces.name DB column.
// But /registry/register stores the runtime-supplied card verbatim
// (registry.go: `agent_card = EXCLUDED.agent_card`), so the stored card
// served at /.well-known/agent-card.json and returned to peers via
// agent_card_url ends up with name = UUID, description = "", role = null.
//
// Fix shape (deliberately minimal, no contract weakening): when the
// runtime-supplied card's `name` is empty or equals the workspace UUID
// (the placeholder the runtime had no better value for), the PLATFORM —
// not the agent — substitutes the friendly value from the trusted
// workspaces row. Identity stays platform-controlled: the agent never
// gains the ability to self-set its own name/role; the platform sources
// it from the operator-controlled DB column. We only ever FILL gaps
// (empty / UUID-placeholder); a card that already carries a real
// friendly name is never downgraded.
//
// list_peers / the /registry/:id/peers endpoint already resolve display
// names from workspaces.name directly (discovery.go / mcp_tools.go
// `SELECT w.id, w.name, ...`), so peer_name in delivered message tags
// was already correct — this fix closes the remaining surface: the
// agent_card blob itself (canvas Agent Card / Skills view, peer
// agent_card_url fetches, the well-known card).
//
// description / role degrade discovery the same way: an empty
// description and null role give peers nothing to reason about. We
// default description from the (now reconciled) name when blank and
// role from workspaces.role when the operator set one.
// reconcileAgentCardIdentity patches identity gaps in a runtime-supplied
// agent card from the trusted workspace DB row. It returns the
// (possibly rewritten) card bytes and whether anything changed. On any
// failure (malformed JSON, nothing to fill) it returns the input bytes
// unchanged with changed=false so the caller can store them verbatim —
// this is strictly no-worse-than-before, never a regression.
//
// Pure function: no DB / HTTP / globals, so it is exhaustively
// unit-testable (agent_card_reconcile_test.go) without booting the
// handler or a sqlmock.
func reconcileAgentCardIdentity(card json.RawMessage, workspaceID, dbName, dbRole string) (json.RawMessage, bool) {
var m map[string]any
if err := json.Unmarshal(card, &m); err != nil || m == nil {
// Malformed card — not this function's job to reject it (the
// upsert stores it as-is and downstream readers handle bad
// JSON). Return verbatim so byte-for-byte behaviour is
// preserved on the failure path.
return card, false
}
changed := false
// name: fill only when empty or the UUID placeholder. A dbName that
// is itself the UUID is a placeholder row (registry.go INSERT seeds
// name = id before the canvas sets a friendly one) — not a friendly
// name, so it is not an eligible source.
cardName, _ := m["name"].(string)
if (cardName == "" || cardName == workspaceID) &&
dbName != "" && dbName != workspaceID {
m["name"] = dbName
changed = true
}
// description: when blank, default to the (reconciled) name so peers
// and the canvas Agent Card view have a non-empty human label
// instead of "". Mirrors the runtime's own
// `config.description or config.name` fallback (main.py:199) but
// applied to the registry copy where the runtime's fallback was the
// UUID.
if desc, _ := m["description"].(string); desc == "" {
if n, _ := m["name"].(string); n != "" && n != workspaceID {
m["description"] = n
changed = true
}
}
// role: surface the operator-set workspaces.role when the card
// carries none. Discovery (peer_role) and the canvas Role row read
// workspaces.role directly; this just makes the standalone card
// self-describing too. Never overwrite a role the card already has.
if dbRole != "" {
if r, ok := m["role"].(string); !ok || r == "" {
m["role"] = dbRole
changed = true
}
}
if !changed {
// No-op: return the original bytes untouched so callers that
// compare/store get byte-identical input (re-marshalling would
// reorder keys for no reason).
return card, false
}
out, err := json.Marshal(m)
if err != nil {
// Re-marshal of a map we just unmarshalled should never fail;
// if it somehow does, fall back to the verbatim input rather
// than storing nothing.
return card, false
}
return out, true
}
@@ -0,0 +1,166 @@
package handlers
import (
"encoding/json"
"testing"
)
// TestReconcileAgentCardIdentity covers the server-side backfill that
// repairs the fleet-wide agent-card identity gap (internal#XXX): the
// runtime POSTs /registry/register with agent_card.name = the workspace
// UUID (because the CP-regenerated /configs/config.yaml sets name: <uuid>)
// while the trusted workspaces.name DB column — the value the canvas
// Details tab shows and lets the operator edit — holds the friendly
// name ("Claude Code Agent"). The platform reconciles them from the DB
// row (NOT from the agent — identity stays platform-controlled, not
// self-mutable).
func TestReconcileAgentCardIdentity(t *testing.T) {
const wsID = "3b81321b-1ec7-488c-96f7-72c42a968da6"
tests := []struct {
name string
card string
dbName string
dbRole string
wantName string
wantDesc string
wantRole string
wantChanged bool
}{
{
name: "name is the workspace UUID — backfill from DB",
card: `{"name":"3b81321b-1ec7-488c-96f7-72c42a968da6","description":"","capabilities":{"streaming":true}}`,
dbName: "Claude Code Agent",
dbRole: "",
wantName: "Claude Code Agent",
wantDesc: "Claude Code Agent",
wantRole: "",
wantChanged: true,
},
{
name: "empty name — backfill from DB",
card: `{"name":"","description":"x"}`,
dbName: "ops-agent",
dbRole: "sre",
wantName: "ops-agent",
wantDesc: "x",
wantRole: "sre",
wantChanged: true,
},
{
name: "role null in card, DB has role — backfill role only",
card: `{"name":"Reviewer","description":"Senior reviewer"}`,
dbName: "Reviewer",
dbRole: "code-reviewer",
wantName: "Reviewer",
wantDesc: "Senior reviewer",
wantRole: "code-reviewer",
wantChanged: true,
},
{
name: "card already has a real friendly name — do NOT clobber it",
// A richer card (e.g. an external channel agent) must win;
// the platform only fills gaps, never downgrades.
card: `{"name":"Claude Code (channel)","description":"Local Claude Code session bridged","role":"assistant"}`,
dbName: "hongming-pc",
dbRole: "operator",
wantName: "Claude Code (channel)",
wantDesc: "Local Claude Code session bridged",
wantRole: "assistant",
wantChanged: false,
},
{
name: "no DB name available — leave UUID name untouched (no worse than before)",
card: `{"name":"3b81321b-1ec7-488c-96f7-72c42a968da6","description":""}`,
dbName: "",
dbRole: "",
wantName: "3b81321b-1ec7-488c-96f7-72c42a968da6",
wantDesc: "",
wantRole: "",
wantChanged: false,
},
{
name: "dbName equals UUID (placeholder row) — not a friendly name, leave untouched",
card: `{"name":"3b81321b-1ec7-488c-96f7-72c42a968da6"}`,
dbName: "3b81321b-1ec7-488c-96f7-72c42a968da6",
dbRole: "",
wantName: "3b81321b-1ec7-488c-96f7-72c42a968da6",
wantDesc: "",
wantRole: "",
wantChanged: false,
},
{
name: "malformed card JSON — return unchanged, no panic",
card: `{not json`,
dbName: "Claude Code Agent",
dbRole: "",
wantChanged: false,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
out, changed := reconcileAgentCardIdentity(
json.RawMessage(tc.card), wsID, tc.dbName, tc.dbRole,
)
if changed != tc.wantChanged {
t.Fatalf("changed = %v, want %v", changed, tc.wantChanged)
}
if !tc.wantChanged {
// Unchanged path must return the input bytes verbatim.
if string(out) != tc.card {
t.Fatalf("unchanged path mutated bytes:\n got %s\n want %s", out, tc.card)
}
return
}
var got map[string]any
if err := json.Unmarshal(out, &got); err != nil {
t.Fatalf("output not valid JSON: %v (%s)", err, out)
}
if g, _ := got["name"].(string); g != tc.wantName {
t.Errorf("name = %q, want %q", g, tc.wantName)
}
if g, _ := got["description"].(string); g != tc.wantDesc {
t.Errorf("description = %q, want %q", g, tc.wantDesc)
}
if tc.wantRole != "" {
if g, _ := got["role"].(string); g != tc.wantRole {
t.Errorf("role = %q, want %q", g, tc.wantRole)
}
}
})
}
}
// TestReconcileAgentCardIdentity_PreservesOtherFields ensures the
// reconcile is a minimal in-place patch — capabilities, version,
// skills and any unknown future fields survive untouched.
func TestReconcileAgentCardIdentity_PreservesOtherFields(t *testing.T) {
card := `{"name":"ws-uuid","description":"","version":"1.0.0",` +
`"capabilities":{"streaming":true,"pushNotifications":true},` +
`"skills":[{"id":"a","name":"a"}],"configuration_status":"ready"}`
out, changed := reconcileAgentCardIdentity(
json.RawMessage(card), "ws-uuid", "Friendly Name", "",
)
if !changed {
t.Fatal("expected changed = true")
}
var got map[string]any
if err := json.Unmarshal(out, &got); err != nil {
t.Fatalf("invalid JSON: %v", err)
}
if got["version"] != "1.0.0" {
t.Errorf("version not preserved: %v", got["version"])
}
if got["configuration_status"] != "ready" {
t.Errorf("configuration_status not preserved: %v", got["configuration_status"])
}
caps, ok := got["capabilities"].(map[string]any)
if !ok || caps["streaming"] != true {
t.Errorf("capabilities not preserved: %v", got["capabilities"])
}
skills, ok := got["skills"].([]any)
if !ok || len(skills) != 1 {
t.Errorf("skills not preserved: %v", got["skills"])
}
}
+31 -3
View File
@@ -327,7 +327,33 @@ func (h *RegistryHandler) Register(c *gin.Context) {
}
}
agentCardStr := string(payload.AgentCard)
// Reconcile the runtime-supplied card's identity fields against the
// trusted workspaces row before storing. The runtime builds its card
// from config.name, which the CP-regenerated /configs/config.yaml
// sets to the workspace UUID — so without this the stored card
// served at /.well-known/agent-card.json and returned to peers via
// agent_card_url has name = UUID, description = "", role = null even
// though the operator-controlled workspaces.name holds the friendly
// name the canvas shows. We only FILL gaps from the DB (never
// downgrade a card that already carries a real name); identity stays
// platform-controlled — the agent cannot self-set these. Best-effort:
// a lookup failure leaves the card exactly as the runtime sent it
// (no-worse-than-before). See agent_card_reconcile.go.
reconciledCard := payload.AgentCard
{
var dbName, dbRole sql.NullString
if qErr := db.DB.QueryRowContext(ctx,
`SELECT name, role FROM workspaces WHERE id = $1`, payload.ID,
).Scan(&dbName, &dbRole); qErr == nil {
if rc, did := reconcileAgentCardIdentity(
payload.AgentCard, payload.ID, dbName.String, dbRole.String,
); did {
reconciledCard = rc
log.Printf("Registry register: reconciled agent_card identity for %s from workspaces row", payload.ID)
}
}
}
agentCardStr := string(reconciledCard)
// urlForUpsert: poll-mode workspaces don't need a URL. Empty input
// becomes NULL via sql.NullString so the row's URL stays clean (the
@@ -413,10 +439,12 @@ func (h *RegistryHandler) Register(c *gin.Context) {
}
}
// Broadcast WORKSPACE_ONLINE
// Broadcast WORKSPACE_ONLINE — use the reconciled card so the canvas
// Agent Card view live-updates with the friendly name, matching what
// was just persisted (not the runtime's raw UUID-name card).
if err := h.broadcaster.RecordAndBroadcast(ctx, string(events.EventWorkspaceOnline), payload.ID, map[string]interface{}{
"url": cachedURL,
"agent_card": payload.AgentCard,
"agent_card": reconciledCard,
"delivery_mode": effectiveMode,
}); err != nil {
log.Printf("Registry broadcast error: %v", err)
@@ -19,6 +19,7 @@ package handlers
import (
"bytes"
"context"
"errors"
"fmt"
"log"
"os"
@@ -357,6 +358,28 @@ func writeFileViaEIC(ctx context.Context, instanceID, runtime, root, relPath str
var stderr bytes.Buffer
sshCmd.Stderr = &stderr
if err := sshCmd.Run(); err != nil {
// When the per-op context deadline (eicFileOpTimeout) fires,
// exec.CommandContext SIGKILLs the ssh subprocess and Run()
// returns the bare "signal: killed" with empty stderr. That
// surfaced to the canvas as an opaque
// `500 {"error":"ssh install: signal: killed ()"}` which gave
// the operator no idea the workspace was simply mid-provision
// with a slow/unready EIC tunnel (internal#423). Detect the
// deadline explicitly and return an actionable message instead
// — the EIC mechanism, timeout value, and success path are all
// unchanged; this only improves the error a stuck write emits.
if cerr := ctx.Err(); cerr != nil {
reason := "timed out after " + eicFileOpTimeout.String()
if errors.Is(cerr, context.Canceled) && !errors.Is(cerr, context.DeadlineExceeded) {
reason = "was cancelled"
}
return fmt.Errorf(
"ssh install: EIC tunnel to workspace %s — "+
"the workspace may still be provisioning (slow/unready SSH); "+
"retry once it is online, or apply provider credentials via "+
"Settings → Secrets (encrypted, does not use this file-write path)",
reason)
}
return fmt.Errorf("ssh install: %w (%s)", err, strings.TrimSpace(stderr.String()))
}
log.Printf("writeFileViaEIC: ws instance=%s runtime=%s root=%s wrote %d bytes → %s",
@@ -0,0 +1,71 @@
package handlers
// template_files_eic_write_timeout_test.go — pins the actionable-error
// behavior added for internal#423.
//
// When the per-op context deadline (eicFileOpTimeout) fires,
// exec.CommandContext SIGKILLs the ssh subprocess and Run() returns the
// bare "signal: killed" with empty stderr. Before the fix that surfaced
// to the canvas as an opaque `500 {"error":"ssh install: signal:
// killed ()"}` — useless to an operator whose workspace was simply
// mid-provision with a slow/unready EIC tunnel. The fix detects the
// deadline explicitly (errors.Is(ctx.Err(), context.DeadlineExceeded))
// and returns a message that names the cause and the
// Settings → Secrets workaround.
import (
"context"
"strings"
"testing"
"time"
)
// TestWriteFileViaEIC_DeadlineExceeded_ActionableError stubs
// withEICTunnel so the *real* inner closure runs against a context that
// has already exceeded its deadline. The ssh subprocess fails (no real
// sshd on the fake port) and ctx.Err() == DeadlineExceeded, so the new
// branch must fire and produce an actionable message — NOT the opaque
// "signal: killed ()" string the canvas used to show.
func TestWriteFileViaEIC_DeadlineExceeded_ActionableError(t *testing.T) {
prev := withEICTunnel
withEICTunnel = func(_ context.Context, instanceID string, fn func(s eicSSHSession) error) error {
// Run the real inner closure. It closes over the ctx that
// writeFileViaEIC derived from our already-cancelled parent, so
// the ssh subprocess is killed immediately and ctx.Err()
// resolves — exactly the eicFileOpTimeout-expiry shape.
return fn(eicSSHSession{
instanceID: instanceID,
osUser: "ubuntu",
localPort: 1, // nothing listening → ssh fails fast
keyPath: "/nonexistent/key",
})
}
t.Cleanup(func() { withEICTunnel = prev })
// Drive the real writeFileViaEIC. Pass a parent whose deadline has
// already passed: the context.WithTimeout(ctx, eicFileOpTimeout)
// derived inside writeFileViaEIC inherits the expired parent
// deadline, so ctx.Err() == context.DeadlineExceeded by the time
// the killed ssh subprocess returns — the exact production shape
// (eicFileOpTimeout expiry), exercised deterministically.
parent, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second))
defer cancel()
err := writeFileViaEIC(parent, "i-test", "claude-code", "/configs", "config.yaml", []byte("model: sonnet\n"))
if err == nil {
t.Fatalf("expected an error from a killed ssh subprocess, got nil")
}
msg := err.Error()
// Must NOT leak the opaque bare-signal string to the operator.
if strings.Contains(msg, "signal: killed ()") {
t.Fatalf("error still surfaces the opaque %q form: %q", "signal: killed ()", msg)
}
// Must name the cause and the Secrets workaround so the canvas
// shows something actionable.
for _, want := range []string{"timed out", "provisioning", "Settings", "Secrets"} {
if !strings.Contains(msg, want) {
t.Errorf("actionable error missing %q; got: %q", want, msg)
}
}
}
@@ -10,8 +10,20 @@ import (
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth"
"github.com/gin-gonic/gin"
"github.com/google/uuid"
)
// validWorkspaceID returns true when id is a syntactically valid UUID.
// workspace_id is a `uuid` column; passing a non-UUID (e.g. the canvas
// "global" sentinel sent when no node is selected) makes Postgres raise
// `invalid input syntax for type uuid`, which previously leaked as an
// opaque 500. Reject up front with a clean 400 instead. Mirrors the
// uuid.Parse guard already used in handlers/activity.go.
func validWorkspaceID(id string) bool {
_, err := uuid.Parse(id)
return err == nil
}
// TokenHandler exposes user-facing token management for workspaces.
// Routes: GET/POST/DELETE /workspaces/:id/tokens (behind WorkspaceAuth).
type TokenHandler struct{}
@@ -31,6 +43,10 @@ type tokenListItem struct {
// never the plaintext or hash).
func (h *TokenHandler) List(c *gin.Context) {
workspaceID := c.Param("id")
if !validWorkspaceID(workspaceID) {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace id"})
return
}
limit := 50
if v := c.Query("limit"); v != "" {
@@ -53,6 +69,7 @@ func (h *TokenHandler) List(c *gin.Context) {
LIMIT $2 OFFSET $3
`, workspaceID, limit, offset)
if err != nil {
log.Printf("tokens: list query failed for workspace %s: %v", workspaceID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list tokens"})
return
}
@@ -85,6 +102,10 @@ const maxTokensPerWorkspace = 50
// exactly once in the response — it cannot be recovered afterwards.
func (h *TokenHandler) Create(c *gin.Context) {
workspaceID := c.Param("id")
if !validWorkspaceID(workspaceID) {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace id"})
return
}
// Rate limit: max active tokens per workspace
var count int
@@ -117,6 +138,10 @@ func (h *TokenHandler) Create(c *gin.Context) {
func (h *TokenHandler) Revoke(c *gin.Context) {
workspaceID := c.Param("id")
tokenID := c.Param("tokenId")
if !validWorkspaceID(workspaceID) {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace id"})
return
}
result, err := db.DB.ExecContext(c.Request.Context(), `
UPDATE workspace_auth_tokens
@@ -41,6 +41,15 @@ import (
func init() { gin.SetMode(gin.TestMode) }
// Workspace IDs are validated as UUIDs up front (tokens.go validWorkspaceID),
// so handler tests must pass syntactically valid UUIDs. Fixed values keep
// sqlmock WithArgs assertions deterministic.
const (
wsUUID1 = "11111111-1111-1111-1111-111111111111"
wsUUID2 = "22222222-2222-2222-2222-222222222222"
wsUUID3 = "33333333-3333-3333-3333-333333333333"
)
// withMockDB swaps `db.DB` for a sqlmock and returns the mock plus a
// restore func. Tests use this in place of setupTokenTestDB which
// skips on a missing real DB.
@@ -81,13 +90,13 @@ func TestTokenHandler_List_HappyPath(t *testing.T) {
created := time.Date(2026, 4, 1, 12, 0, 0, 0, time.UTC)
last := created.Add(time.Hour)
mock.ExpectQuery(`SELECT id, prefix, created_at, last_used_at\s+FROM workspace_auth_tokens`).
WithArgs("ws-1", 50, 0).
WithArgs(wsUUID1, 50, 0).
WillReturnRows(sqlmock.NewRows([]string{"id", "prefix", "created_at", "last_used_at"}).
AddRow("tok-1", "abc12345", created, last).
AddRow("tok-2", "def67890", created, nil))
w := makeReq(t, NewTokenHandler().List, "GET",
"/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: "ws-1"}})
"/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: wsUUID1}})
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
@@ -121,7 +130,7 @@ func TestTokenHandler_List_EmptyResult(t *testing.T) {
WillReturnRows(sqlmock.NewRows([]string{"id", "prefix", "created_at", "last_used_at"}))
w := makeReq(t, NewTokenHandler().List, "GET",
"/workspaces/ws-2/tokens", gin.Params{{Key: "id", Value: "ws-2"}})
"/workspaces/ws-2/tokens", gin.Params{{Key: "id", Value: wsUUID2}})
if w.Code != http.StatusOK {
t.Fatalf("expected 200 on empty list, got %d", w.Code)
@@ -146,7 +155,7 @@ func TestTokenHandler_List_QueryError(t *testing.T) {
WillReturnError(errors.New("connection refused"))
w := makeReq(t, NewTokenHandler().List, "GET",
"/workspaces/ws-3/tokens", gin.Params{{Key: "id", Value: "ws-3"}})
"/workspaces/ws-3/tokens", gin.Params{{Key: "id", Value: wsUUID3}})
if w.Code != http.StatusInternalServerError {
t.Errorf("query error must surface as 500, got %d", w.Code)
@@ -158,13 +167,13 @@ func TestTokenHandler_List_RespectsLimit(t *testing.T) {
defer cleanup()
mock.ExpectQuery(`SELECT id, prefix, created_at, last_used_at`).
WithArgs("ws-1", 10, 5).
WithArgs(wsUUID1, 10, 5).
WillReturnRows(sqlmock.NewRows([]string{"id", "prefix", "created_at", "last_used_at"}))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Request = httptest.NewRequest("GET", "/workspaces/ws-1/tokens?limit=10&offset=5", nil)
c.Params = gin.Params{{Key: "id", Value: "ws-1"}}
c.Params = gin.Params{{Key: "id", Value: wsUUID1}}
NewTokenHandler().List(c)
if w.Code != http.StatusOK {
@@ -186,7 +195,7 @@ func TestTokenHandler_List_ScanError(t *testing.T) {
AddRow("tok-1", "abc", "not-a-timestamp", nil))
w := makeReq(t, NewTokenHandler().List, "GET",
"/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: "ws-1"}})
"/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: wsUUID1}})
if w.Code != http.StatusInternalServerError {
t.Errorf("scan error must surface as 500, got %d: %s", w.Code, w.Body.String())
@@ -201,11 +210,11 @@ func TestTokenHandler_Create_RateLimited(t *testing.T) {
// Count query returns 50 (== max) → 429.
mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens`).
WithArgs("ws-1").
WithArgs(wsUUID1).
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(50))
w := makeReq(t, NewTokenHandler().Create, "POST",
"/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: "ws-1"}})
"/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: wsUUID1}})
if w.Code != http.StatusTooManyRequests {
t.Errorf("max active tokens should 429, got %d", w.Code)
@@ -225,7 +234,7 @@ func TestTokenHandler_Create_IssueFails(t *testing.T) {
WillReturnError(errors.New("disk full"))
w := makeReq(t, NewTokenHandler().Create, "POST",
"/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: "ws-1"}})
"/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: wsUUID1}})
if w.Code != http.StatusInternalServerError {
t.Errorf("IssueToken DB error must 500, got %d", w.Code)
@@ -242,7 +251,7 @@ func TestTokenHandler_Create_HappyPath(t *testing.T) {
WillReturnResult(sqlmock.NewResult(1, 1))
w := makeReq(t, NewTokenHandler().Create, "POST",
"/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: "ws-1"}})
"/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: wsUUID1}})
if w.Code != http.StatusCreated {
t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String())
@@ -257,7 +266,7 @@ func TestTokenHandler_Create_HappyPath(t *testing.T) {
if body.AuthToken == "" {
t.Errorf("auth_token must be present and non-empty in response")
}
if body.WorkspaceID != "ws-1" {
if body.WorkspaceID != wsUUID1 {
t.Errorf("workspace_id mismatch: %q", body.WorkspaceID)
}
}
@@ -269,12 +278,12 @@ func TestTokenHandler_Revoke_HappyPath(t *testing.T) {
defer cleanup()
mock.ExpectExec(`UPDATE workspace_auth_tokens\s+SET revoked_at = now\(\)`).
WithArgs("tok-1", "ws-1").
WithArgs("tok-1", wsUUID1).
WillReturnResult(sqlmock.NewResult(0, 1))
w := makeReq(t, NewTokenHandler().Revoke, "DELETE",
"/workspaces/ws-1/tokens/tok-1", gin.Params{
{Key: "id", Value: "ws-1"},
{Key: "id", Value: wsUUID1},
{Key: "tokenId", Value: "tok-1"},
})
@@ -289,12 +298,12 @@ func TestTokenHandler_Revoke_NotFound(t *testing.T) {
// 0 rows affected → token not found OR already revoked.
mock.ExpectExec(`UPDATE workspace_auth_tokens`).
WithArgs("tok-ghost", "ws-1").
WithArgs("tok-ghost", wsUUID1).
WillReturnResult(sqlmock.NewResult(0, 0))
w := makeReq(t, NewTokenHandler().Revoke, "DELETE",
"/workspaces/ws-1/tokens/tok-ghost", gin.Params{
{Key: "id", Value: "ws-1"},
{Key: "id", Value: wsUUID1},
{Key: "tokenId", Value: "tok-ghost"},
})
@@ -312,7 +321,7 @@ func TestTokenHandler_Revoke_DBError(t *testing.T) {
w := makeReq(t, NewTokenHandler().Revoke, "DELETE",
"/workspaces/ws-1/tokens/tok-1", gin.Params{
{Key: "id", Value: "ws-1"},
{Key: "id", Value: wsUUID1},
{Key: "tokenId", Value: "tok-1"},
})
@@ -321,6 +330,59 @@ func TestTokenHandler_Revoke_DBError(t *testing.T) {
}
}
// ---- UUID validation (regression: "global" sentinel 500) ------------
// The canvas Settings → Workspace Tokens tab sent the literal sentinel
// "global" as the workspace id when no node was selected. workspace_id
// is a `uuid` column, so the query raised
// `invalid input syntax for type uuid: "global"` which leaked as an
// opaque 500. List/Create/Revoke now reject any non-UUID id with a
// clean 400 before touching the DB. No DB expectation is set on the
// mock — a DB hit would fail ExpectationsWereMet, proving short-circuit.
func TestTokenHandler_RejectsNonUUIDWorkspaceID(t *testing.T) {
h := NewTokenHandler()
cases := []struct {
name string
run func(c *gin.Context)
method string
params gin.Params
}{
{"List", h.List, "GET", gin.Params{{Key: "id", Value: "global"}}},
{"Create", h.Create, "POST", gin.Params{{Key: "id", Value: "global"}}},
{"Revoke", h.Revoke, "DELETE", gin.Params{
{Key: "id", Value: "global"},
{Key: "tokenId", Value: "tok-1"},
}},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
mock, cleanup := withMockDB(t)
defer cleanup()
w := makeReq(t, tc.run, tc.method,
"/workspaces/global/tokens", tc.params)
if w.Code != http.StatusBadRequest {
t.Fatalf("%s with non-UUID id must 400, got %d: %s",
tc.name, w.Code, w.Body.String())
}
var body struct {
Error string `json:"error"`
}
_ = json.Unmarshal(w.Body.Bytes(), &body)
if body.Error != "invalid workspace id" {
t.Errorf("%s: want error=%q, got %q",
tc.name, "invalid workspace id", body.Error)
}
// No query/exec was expected → if the handler hit the DB
// this fails, proving the guard short-circuits before SQL.
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("%s leaked a DB call past the uuid guard: %v", tc.name, err)
}
})
}
}
// Compile-time noise removal: the imports list pulls in the sql /
// driver packages and the silenced ctx so a future scenario that
// needs them doesn't have to re-add the import. Documented here so
@@ -11,6 +11,7 @@ import (
"github.com/Molecule-AI/molecule-monorepo/platform/internal/db"
"github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth"
"github.com/gin-gonic/gin"
"github.com/google/uuid"
)
func init() { gin.SetMode(gin.TestMode) }
@@ -167,11 +168,14 @@ func TestTokenHandler_RevokeWrongWorkspace(t *testing.T) {
h := NewTokenHandler()
// Try to revoke with a different workspace ID — should 404
// Try to revoke with a different (valid-UUID) workspace ID that does
// not own the token — should 404. A valid UUID is required so this
// exercises the ownership branch, not the up-front uuid-shape 400.
otherWS := uuid.NewString()
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "wrong-workspace-id"}, {Key: "tokenId", Value: tokenID}}
c.Request = httptest.NewRequest("DELETE", "/workspaces/wrong/tokens/"+tokenID, nil)
c.Params = gin.Params{{Key: "id", Value: otherWS}, {Key: "tokenId", Value: tokenID}}
c.Request = httptest.NewRequest("DELETE", "/workspaces/"+otherWS+"/tokens/"+tokenID, nil)
h.Revoke(c)
if w.Code != http.StatusNotFound {