+ {session ? ([]);
const [loading, setLoading] = useState(true);
+ const [loadError, setLoadError] = useState(null);
// Dedup by timestamp+type+peer to handle API load + WebSocket race
const seenKeys = useRef(new Set());
const bottomRef = useRef(null);
+ // Mirrors the my-chat scroll behaviour from ChatTab (PR #2903) —
+ // smooth-scroll on a long history gets interrupted by concurrent
+ // renders and lands the panel mid-conversation. Switch the first
+ // arrival to instant; subsequent appends animate.
+ const hasInitialScrollRef = useRef(false);
- // Load history
- useEffect(() => {
+ // Load history. Extracted so the error-state retry button can
+ // re-invoke without remount. ChatTab uses the same shape
+ // (loadInitial → loadError state → retry button).
+ const loadInitial = useCallback(() => {
setLoading(true);
+ setLoadError(null);
+ seenKeys.current.clear();
api.get(`/workspaces/${workspaceId}/activity?source=agent&limit=50`)
.then((entries) => {
const filtered = (entries ?? [])
@@ -234,10 +244,15 @@ export function AgentCommsPanel({ workspaceId }: { workspaceId: string }) {
// the .then body) — the panel just sat on the empty state
// with zero signal.
console.warn("AgentCommsPanel: load activity failed", err);
+ setLoadError(err instanceof Error ? err.message : String(err));
setLoading(false);
});
}, [workspaceId]);
+ useEffect(() => {
+ loadInitial();
+ }, [loadInitial]);
+
// Live updates routed through the global ReconnectingSocket. The
// previous pattern of `new WebSocket(WS_URL)` per panel had no
// onclose / no reconnect, so any drop (idle timeout, browser
@@ -358,7 +373,18 @@ export function AgentCommsPanel({ workspaceId }: { workspaceId: string }) {
} catch { /* ignore */ }
});
- useEffect(() => {
+ // useLayoutEffect (not useEffect) so the scroll runs BEFORE paint —
+ // otherwise the user sees the panel jump for one frame on every
+ // append. Mirrors ChatTab's MyChatPanel scroll block.
+ useLayoutEffect(() => {
+ if (!hasInitialScrollRef.current && messages.length > 0) {
+ // Instant on first arrival — smooth-scroll on a long history
+ // gets interrupted by concurrent renders and lands the panel
+ // mid-conversation (the chat-opens-in-middle bug class).
+ hasInitialScrollRef.current = true;
+ bottomRef.current?.scrollIntoView({ behavior: "instant" as ScrollBehavior });
+ return;
+ }
bottomRef.current?.scrollIntoView({ behavior: "smooth" });
}, [messages]);
@@ -366,6 +392,27 @@ export function AgentCommsPanel({ workspaceId }: { workspaceId: string }) {
return
Your organizations
Each org is an isolated Molecule workspace. @@ -177,6 +187,40 @@ function Shell({ children }: { children: React.ReactNode }) { ); } +// AccountBar renders the signed-in email + a Sign-out button at the +// top of the page. Without this the user has no way to log out — the +// /cp/auth/signout endpoint exists on the control plane but no UI ever +// called it. Reported externally on 2026-05-05; this is the fix. +// +// Click → calls signOut() which POSTs /cp/auth/signout (clears the +// WorkOS session cookie + revokes at the provider) then bounces to +// /cp/auth/login. The signOut helper is best-effort — even on a 5xx +// or network failure the redirect fires so the user never gets stuck +// on an authed-looking page after they clicked Sign out. +function AccountBar({ session }: { session: Session }) { + const [signingOut, setSigningOut] = useState(false); + return ( +
+ {session.email}
+
+
+ );
+}
+
// DataResidencyNotice surfaces where workspace data lives so EU-based
// signups can make an informed choice (GDPR Art. 13 disclosure
// requirement). Plain text, no icon — the goal is clarity, not
diff --git a/canvas/src/components/tabs/chat/AgentCommsPanel.tsx b/canvas/src/components/tabs/chat/AgentCommsPanel.tsx
index fc327ea0..074d96fc 100644
--- a/canvas/src/components/tabs/chat/AgentCommsPanel.tsx
+++ b/canvas/src/components/tabs/chat/AgentCommsPanel.tsx
@@ -1,6 +1,6 @@
"use client";
-import { useState, useEffect, useMemo, useRef } from "react";
+import { useState, useEffect, useLayoutEffect, useMemo, useRef, useCallback } from "react";
import ReactMarkdown from "react-markdown";
import remarkGfm from "remark-gfm";
import { api } from "@/lib/api";
@@ -184,13 +184,23 @@ function unwrapErrorText(raw: string | null): string {
export function AgentCommsPanel({ workspaceId }: { workspaceId: string }) {
const [messages, setMessages] = useStateLoading agent communications...
;
}
+ if (loadError !== null && messages.length === 0) {
+ // Mirrors ChatTab my-chat error UI — surfaces the load failure
+ // with a retry button instead of silently rendering empty state.
+ return (
+
+
+ );
+ }
+
if (messages.length === 0) {
return (
+ Failed to load agent communications: {loadError} +
+ +
diff --git a/canvas/src/components/tabs/chat/__tests__/AgentCommsPanel.render.test.tsx b/canvas/src/components/tabs/chat/__tests__/AgentCommsPanel.render.test.tsx
new file mode 100644
index 00000000..80b37982
--- /dev/null
+++ b/canvas/src/components/tabs/chat/__tests__/AgentCommsPanel.render.test.tsx
@@ -0,0 +1,115 @@
+// @vitest-environment jsdom
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
+import { render, screen, fireEvent, waitFor } from "@testing-library/react";
+
+// API mock — tests can override per case via apiGetMock.mockImplementationOnce.
+const apiGetMock = vi.fn<(url: string) => Promise>();
+vi.mock("@/lib/api", () => ({
+ api: {
+ get: (url: string) => apiGetMock(url),
+ },
+}));
+
+// useSocketEvent — no-op for these render tests; live updates aren't
+// what we're verifying here.
+vi.mock("@/hooks/useSocketEvent", () => ({
+ useSocketEvent: () => {},
+}));
+
+// Canvas store — peer name resolution.
+vi.mock("@/store/canvas", () => ({
+ useCanvasStore: {
+ getState: () => ({
+ nodes: [
+ { id: "ws-self", data: { name: "Self" } },
+ { id: "ws-peer", data: { name: "Peer Agent" } },
+ ],
+ }),
+ },
+}));
+
+// Toaster shim — AgentCommsPanel imports showToast.
+vi.mock("../../Toaster", () => ({
+ showToast: vi.fn(),
+}));
+
+import { AgentCommsPanel } from "../AgentCommsPanel";
+
+// jsdom doesn't implement scrollIntoView. Tests that observe the call
+// install a spy here; tests that don't care still need a no-op stub
+// so the component doesn't throw.
+const scrollSpy = vi.fn<(opts?: ScrollIntoViewOptions | boolean) => void>();
+beforeEach(() => {
+ apiGetMock.mockReset();
+ scrollSpy.mockReset();
+ Element.prototype.scrollIntoView = scrollSpy as unknown as Element["scrollIntoView"];
+});
+
+afterEach(() => {
+ vi.clearAllMocks();
+});
+
+describe("AgentCommsPanel — initial-state parity with ChatTab my-chat", () => {
+ it("shows loading text while history fetch is in flight", () => {
+ apiGetMock.mockReturnValueOnce(new Promise(() => { /* never resolves */ }));
+ render( → AuthKit clears its
+ * session, then redirects the browser to the configured
+ * return_to (defaults to APP_URL/orgs).
+ *
+ * Best-effort by design: a 5xx, network failure, missing logout_url
+ * (DisabledProvider, dev), or stale cookie still results in the
+ * browser navigating away — leaving the user on a logged-in-looking
+ * page after they clicked "Sign out" is the worst possible UX. The
+ * fallback path navigates to /cp/auth/login on the auth origin, which
+ * works correctly in environments without a hosted logout flow (dev,
+ * tests, DisabledProvider).
+ *
+ * Throws nothing — callers can disable the button optimistically or
+ * await this and trust it returns. On a redirect-blocked test
+ * environment (jsdom under vitest) we still exit cleanly so unit tests
+ * can spy on the fetch call.
+ */
+export async function signOut(): Promise {
+ let logoutURL: string | undefined;
+ // Fire-and-tolerate the POST. credentials:include is mandatory cross-
+ // origin so the SaaS canvas (acme.moleculesai.app) can hit
+ // app.moleculesai.app/cp/auth/signout with the session cookie.
+ try {
+ const res = await fetch(`${getAuthOrigin()}${AUTH_BASE}/signout`, {
+ method: "POST",
+ credentials: "include",
+ });
+ if (res.ok) {
+ // Body shape: {"ok": true, "logout_url": "..."}. logout_url is
+ // empty for DisabledProvider (dev/local) — we fall back to
+ // /cp/auth/login below. Defensive parsing: a malformed body
+ // shouldn't strand the user on the authed page.
+ const body: unknown = await res.json().catch(() => null);
+ if (
+ body &&
+ typeof body === "object" &&
+ "logout_url" in body &&
+ typeof (body as { logout_url: unknown }).logout_url === "string" &&
+ (body as { logout_url: string }).logout_url
+ ) {
+ logoutURL = (body as { logout_url: string }).logout_url;
+ }
+ }
+ } catch {
+ // Ignore — we still redirect below.
+ }
+ if (typeof window === "undefined") return;
+ if (logoutURL) {
+ // Hosted logout: AuthKit clears its SSO cookie + redirects to
+ // return_to (configured server-side). This is the path that
+ // actually breaks the SSO re-auth loop.
+ window.location.href = logoutURL;
+ return;
+ }
+ // Fallback: no hosted logout (dev, DisabledProvider, network
+ // failure). Land on the login screen rather than the current URL:
+ // returning to a tenant URL after signout would just re-redirect
+ // through /cp/auth/login due to AuthGate. Send the user straight
+ // there with no return_to so they don't loop back into the org they
+ // just left.
+ const authOrigin = getAuthOrigin();
+ window.location.href = `${authOrigin}${AUTH_BASE}/login`;
+}
diff --git a/tests/e2e/test_poll_mode_chat_upload_e2e.sh b/tests/e2e/test_poll_mode_chat_upload_e2e.sh
new file mode 100755
index 00000000..fbed604f
--- /dev/null
+++ b/tests/e2e/test_poll_mode_chat_upload_e2e.sh
@@ -0,0 +1,295 @@
+#!/usr/bin/env bash
+# E2E for poll-mode chat upload (RFC #2891 phases 1-5b).
+#
+# Round-trip: register a workspace as poll-mode (no callback URL) → POST a
+# multi-file chat upload → verify each file becomes (a) one
+# `chat_upload_receive` activity row and (b) one /pending-uploads row → fetch
+# the bytes back via the poll endpoint → ack → verify the row 404s on
+# subsequent fetch. Also pins cross-workspace bleed protection: workspace B
+# cannot read workspace A's pending uploads even with its own valid bearer.
+#
+# Why this exists separately from test_chat_upload_e2e.sh: that script
+# covers the PUSH path (the workspace's own /internal/chat/uploads/ingest).
+# This script covers the POLL path: the same canvas-side request lands on
+# the platform's pendinguploads.Storage instead, and the workspace fetches
+# it later. The two paths share zero handler code on the platform side, so
+# both need their own E2E.
+#
+# Requires: platform running on localhost:8080 with migrations applied.
+# bash workspace-server/scripts/dev-start.sh
+# bash workspace-server/scripts/run-migrations.sh
+#
+# Idempotent: each run uses fresh per-script workspace UUIDs so reruns
+# don't collide. Best-effort cleanup on EXIT — does NOT call
+# e2e_cleanup_all_workspaces (see
+# `feedback_never_run_cluster_cleanup_tests_on_live_platform.md`).
+
+set -euo pipefail
+
+source "$(dirname "$0")/_lib.sh"
+
+PASS=0
+FAIL=0
+TIMEOUT="${A2A_TIMEOUT:-30}"
+
+gen_uuid() {
+ if command -v uuidgen >/dev/null 2>&1; then
+ uuidgen | tr '[:upper:]' '[:lower:]'
+ else
+ python3 -c 'import uuid; print(uuid.uuid4())'
+ fi
+}
+WS_A="$(gen_uuid)"
+WS_B="$(gen_uuid)"
+
+# Per-run scratch dir collected under one trap so every assertion-failure
+# path drops the temp files it made (see test_chat_attachments_e2e.sh).
+TMPDIR_E2E=$(mktemp -d -t poll-chat-upload-e2e-XXXXXX)
+
+cleanup() {
+ local rc=$?
+ curl -s -X DELETE "$BASE/workspaces/$WS_A?confirm=true" >/dev/null 2>&1 || true
+ curl -s -X DELETE "$BASE/workspaces/$WS_B?confirm=true" >/dev/null 2>&1 || true
+ rm -rf "$TMPDIR_E2E"
+ exit $rc
+}
+trap cleanup EXIT INT TERM
+
+check() {
+ local desc="$1" expected="$2" actual="$3"
+ if echo "$actual" | grep -qF -- "$expected"; then
+ echo "PASS: $desc"
+ PASS=$((PASS + 1))
+ else
+ echo "FAIL: $desc"
+ echo " expected to contain: $expected"
+ echo " got: $(echo "$actual" | head -10)"
+ FAIL=$((FAIL + 1))
+ fi
+}
+
+check_eq() {
+ local desc="$1" expected="$2" actual="$3"
+ if [ "$actual" = "$expected" ]; then
+ echo "PASS: $desc"
+ PASS=$((PASS + 1))
+ else
+ echo "FAIL: $desc"
+ echo " expected: $expected"
+ echo " got: $actual"
+ FAIL=$((FAIL + 1))
+ fi
+}
+
+echo "=== Poll-Mode Chat Upload E2E ==="
+echo " base: $BASE"
+echo " workspace A: $WS_A"
+echo " workspace B: $WS_B"
+echo ""
+
+# ---------- Phase 1: register poll-mode workspace ----------
+echo "--- Phase 1: Register poll-mode workspace A ---"
+
+REG_A=$(curl -s -X POST "$BASE/registry/register" \
+ -H "Content-Type: application/json" \
+ -d "{
+ \"id\": \"$WS_A\",
+ \"delivery_mode\": \"poll\",
+ \"agent_card\": {\"name\": \"poll-chat-upload-test-a\"}
+ }")
+check "register accepts poll mode without URL" '"status":"registered"' "$REG_A"
+TOK_A=$(echo "$REG_A" | e2e_extract_token || true)
+[ -n "$TOK_A" ] || { echo "FAIL: no auth_token in register response (ws A)"; FAIL=$((FAIL + 1)); exit 1; }
+
+# ---------- Phase 2: multi-file chat upload ----------
+echo ""
+echo "--- Phase 2: POST /chat/uploads with two files ---"
+
+FILE1="$TMPDIR_E2E/alpha.txt"
+FILE2="$TMPDIR_E2E/beta.txt"
+EXPECTED1="alpha-secret-$(openssl rand -hex 4)"
+EXPECTED2="beta-secret-$(openssl rand -hex 4)"
+printf '%s' "$EXPECTED1" > "$FILE1"
+printf '%s' "$EXPECTED2" > "$FILE2"
+
+UPLOAD=$(curl -s -X POST "$BASE/workspaces/$WS_A/chat/uploads" \
+ -H "Authorization: Bearer $TOK_A" \
+ -F "files=@$FILE1;filename=alpha.txt;type=text/plain" \
+ -F "files=@$FILE2;filename=beta.txt;type=text/plain" \
+ -w "\nHTTP_CODE=%{http_code}\n")
+UPLOAD_CODE=$(echo "$UPLOAD" | grep -oE 'HTTP_CODE=[0-9]+' | cut -d= -f2)
+UPLOAD_BODY=$(echo "$UPLOAD" | sed '/^HTTP_CODE=/,$d')
+
+check_eq "upload returns 200" "200" "$UPLOAD_CODE"
+check "upload response has files array" '"files":' "$UPLOAD_BODY"
+
+# Pull file_ids out of the URI in the response. URI shape is
+# `platform-pending:/` — proves the response came from the
+# poll-mode branch, not the push-mode internal-ingest branch.
+URI1=$(echo "$UPLOAD_BODY" | python3 -c 'import sys,json; d=json.load(sys.stdin); print(d["files"][0]["uri"])')
+URI2=$(echo "$UPLOAD_BODY" | python3 -c 'import sys,json; d=json.load(sys.stdin); print(d["files"][1]["uri"])')
+check "URI 1 has platform-pending: scheme" "platform-pending:$WS_A/" "$URI1"
+check "URI 2 has platform-pending: scheme" "platform-pending:$WS_A/" "$URI2"
+
+FID1="${URI1##*/}"
+FID2="${URI2##*/}"
+[ -n "$FID1" ] && [ -n "$FID2" ] || { echo "FAIL: could not extract file IDs"; FAIL=$((FAIL + 1)); exit 1; }
+echo " file_id 1: $FID1"
+echo " file_id 2: $FID2"
+
+# ---------- Phase 3: activity rows visible to the workspace ----------
+echo ""
+echo "--- Phase 3: /activity shows two chat_upload_receive rows ---"
+
+# activity_logs INSERTs run in a goroutine — give them a moment.
+sleep 1
+ACT=$(curl -s --max-time "$TIMEOUT" -H "Authorization: Bearer $TOK_A" \
+ "$BASE/workspaces/$WS_A/activity?type=a2a_receive&limit=20")
+check "activity feed has the alpha file" "$FID1" "$ACT"
+check "activity feed has the beta file" "$FID2" "$ACT"
+check "activity rows tagged chat_upload_receive" '"method":"chat_upload_receive"' "$ACT"
+check "activity rows record alpha mimetype" '"mimeType":"text/plain"' "$ACT"
+
+CHAT_UPLOAD_COUNT=$(echo "$ACT" | python3 -c '
+import json, sys
+rows = json.load(sys.stdin)
+n = sum(1 for r in rows if (r.get("method") or "") == "chat_upload_receive")
+print(n)
+')
+check_eq "exactly two chat_upload_receive rows" "2" "$CHAT_UPLOAD_COUNT"
+
+# ---------- Phase 4: GET /pending-uploads/:file_id/content ----------
+echo ""
+echo "--- Phase 4: Fetch content for each pending upload ---"
+
+GOT1=$(curl -s --max-time "$TIMEOUT" -H "Authorization: Bearer $TOK_A" \
+ "$BASE/workspaces/$WS_A/pending-uploads/$FID1/content")
+check_eq "alpha bytes round-trip" "$EXPECTED1" "$GOT1"
+
+GOT2=$(curl -s --max-time "$TIMEOUT" -H "Authorization: Bearer $TOK_A" \
+ "$BASE/workspaces/$WS_A/pending-uploads/$FID2/content")
+check_eq "beta bytes round-trip" "$EXPECTED2" "$GOT2"
+
+# Mimetype + Content-Disposition headers should match what was uploaded.
+HEAD1=$(curl -s -D - -o /dev/null --max-time "$TIMEOUT" -H "Authorization: Bearer $TOK_A" \
+ "$BASE/workspaces/$WS_A/pending-uploads/$FID1/content")
+check "alpha response carries text/plain Content-Type" "Content-Type: text/plain" "$HEAD1"
+check "alpha response carries Content-Disposition with filename" 'filename="alpha.txt"' "$HEAD1"
+
+# ---------- Phase 5: idempotent re-fetch (until ack) ----------
+echo ""
+echo "--- Phase 5: Re-fetch before ack returns the same bytes ---"
+
+RE_GOT1=$(curl -s --max-time "$TIMEOUT" -H "Authorization: Bearer $TOK_A" \
+ "$BASE/workspaces/$WS_A/pending-uploads/$FID1/content")
+check_eq "re-fetch returns same alpha bytes" "$EXPECTED1" "$RE_GOT1"
+
+# ---------- Phase 6: ack each row ----------
+echo ""
+echo "--- Phase 6: Ack each pending upload ---"
+
+ACK1=$(curl -s -X POST --max-time "$TIMEOUT" -H "Authorization: Bearer $TOK_A" \
+ "$BASE/workspaces/$WS_A/pending-uploads/$FID1/ack")
+check "alpha ack returns acked:true" '"acked":true' "$ACK1"
+
+ACK2=$(curl -s -X POST --max-time "$TIMEOUT" -H "Authorization: Bearer $TOK_A" \
+ "$BASE/workspaces/$WS_A/pending-uploads/$FID2/ack")
+check "beta ack returns acked:true" '"acked":true' "$ACK2"
+
+# Re-ack should still 200 (idempotent — the row's gone but the workspace's
+# at-least-once intent was already honored, and the second ack hits the
+# raced path which also returns 200).
+RE_ACK1=$(curl -s -w '\n%{http_code}' -X POST --max-time "$TIMEOUT" \
+ -H "Authorization: Bearer $TOK_A" \
+ "$BASE/workspaces/$WS_A/pending-uploads/$FID1/ack")
+RE_ACK1_CODE=$(printf '%s' "$RE_ACK1" | tail -n1)
+# Acked rows return 404 on Get-before-Ack (the row's still in the table
+# but Get filters acked_at IS NULL); workspace would not normally re-ack
+# since it already saw the success. Accept both 200 and 404 here so the
+# test pins the contract without being brittle on the inner ordering.
+case "$RE_ACK1_CODE" in
+ 200|404)
+ echo "PASS: re-ack returns 200 or 404 ($RE_ACK1_CODE)"
+ PASS=$((PASS + 1))
+ ;;
+ *)
+ echo "FAIL: re-ack returned unexpected $RE_ACK1_CODE"
+ FAIL=$((FAIL + 1))
+ ;;
+esac
+
+# ---------- Phase 7: GET content after ack returns 404 ----------
+echo ""
+echo "--- Phase 7: Acked file 404s on subsequent fetch ---"
+
+POST_ACK=$(curl -s -w '\n%{http_code}' --max-time "$TIMEOUT" -H "Authorization: Bearer $TOK_A" \
+ "$BASE/workspaces/$WS_A/pending-uploads/$FID1/content")
+POST_ACK_CODE=$(printf '%s' "$POST_ACK" | tail -n1)
+check_eq "acked alpha returns HTTP 404" "404" "$POST_ACK_CODE"
+
+# ---------- Phase 8: cross-workspace bleed protection ----------
+echo ""
+echo "--- Phase 8: Workspace B cannot read workspace A's pending uploads ---"
+
+# Stage a fresh upload on workspace A so we have an UN-acked row to probe.
+PROBE_FILE="$TMPDIR_E2E/probe.txt"
+printf '%s' "probe-bytes-$(openssl rand -hex 4)" > "$PROBE_FILE"
+PROBE_UP=$(curl -s -X POST "$BASE/workspaces/$WS_A/chat/uploads" \
+ -H "Authorization: Bearer $TOK_A" \
+ -F "files=@$PROBE_FILE;filename=probe.txt;type=text/plain")
+PROBE_FID=$(echo "$PROBE_UP" | python3 -c 'import sys,json; d=json.load(sys.stdin); print(d["files"][0]["uri"].split("/")[-1])')
+[ -n "$PROBE_FID" ] || { echo "FAIL: probe upload returned no file_id"; FAIL=$((FAIL + 1)); exit 1; }
+
+# Register a SECOND poll-mode workspace and capture its bearer.
+REG_B=$(curl -s -X POST "$BASE/registry/register" \
+ -H "Content-Type: application/json" \
+ -d "{
+ \"id\": \"$WS_B\",
+ \"delivery_mode\": \"poll\",
+ \"agent_card\": {\"name\": \"poll-chat-upload-test-b\"}
+ }")
+check "second workspace registers" '"status":"registered"' "$REG_B"
+TOK_B=$(echo "$REG_B" | e2e_extract_token || true)
+[ -n "$TOK_B" ] || { echo "FAIL: no auth_token (ws B)"; FAIL=$((FAIL + 1)); exit 1; }
+
+# B's bearer hitting B's URL with A's file_id → 404 (handler checks the row's
+# workspace_id matches the URL :id, not the bearer's workspace).
+CROSS_RESP=$(curl -s -w '\n%{http_code}' --max-time "$TIMEOUT" \
+ -H "Authorization: Bearer $TOK_B" \
+ "$BASE/workspaces/$WS_B/pending-uploads/$PROBE_FID/content")
+CROSS_CODE=$(printf '%s' "$CROSS_RESP" | tail -n1)
+check_eq "B's URL with A's file_id returns 404" "404" "$CROSS_CODE"
+
+# B's bearer hitting A's URL → 401 (wsAuth pins bearer to :id). This is the
+# strictest cross-workspace check: a presented-but-wrong bearer is rejected
+# in EVERY platform posture (dev-mode fail-open only triggers when no bearer
+# is presented at all — invalid tokens always 401).
+WRONG_BEARER=$(curl -s -w '\n%{http_code}' --max-time "$TIMEOUT" \
+ -H "Authorization: Bearer $TOK_B" \
+ "$BASE/workspaces/$WS_A/pending-uploads/$PROBE_FID/content")
+WRONG_CODE=$(printf '%s' "$WRONG_BEARER" | tail -n1)
+check_eq "B's bearer on A's URL returns 401" "401" "$WRONG_CODE"
+
+# NB: a fully bearerless request to /pending-uploads/:fid/content returns
+# 401 ONLY when the platform has MOLECULE_ENV != development (production /
+# staging). On local-dev with MOLECULE_ENV=development the wsauth middleware
+# fail-opens for bearerless requests so the canvas at :3000 can talk to the
+# platform at :8080 without per-call token plumbing — see middleware/
+# devmode.go. The strict bearerless-401 contract is covered by the wsauth
+# unit + middleware tests; we don't reassert it here because the result
+# depends on platform posture, not the poll-mode upload contract.
+
+# ---------- Phase 9: invalid file_id rejected at the URL parser ----------
+echo ""
+echo "--- Phase 9: Invalid file_id returns 400 ---"
+
+BAD_FID=$(curl -s -w '\n%{http_code}' --max-time "$TIMEOUT" \
+ -H "Authorization: Bearer $TOK_A" \
+ "$BASE/workspaces/$WS_A/pending-uploads/not-a-uuid/content")
+BAD_FID_CODE=$(printf '%s' "$BAD_FID" | tail -n1)
+check_eq "invalid file_id UUID returns 400" "400" "$BAD_FID_CODE"
+
+# ---------- Results ----------
+echo ""
+echo "=== Results: $PASS passed, $FAIL failed ==="
+[ "$FAIL" -eq 0 ]
diff --git a/workspace-server/internal/handlers/class1_ast_gate_test.go b/workspace-server/internal/handlers/class1_ast_gate_test.go
new file mode 100644
index 00000000..bb362364
--- /dev/null
+++ b/workspace-server/internal/handlers/class1_ast_gate_test.go
@@ -0,0 +1,468 @@
+package handlers
+
+// class1_ast_gate_test.go — generic Class 1 leak gate per #2867 PR-A.
+//
+// What this gate prevents:
+// The tenant-hongming leak class — a handler iterates a YAML-derived
+// slice (ws.Children, sub_workspaces, etc.) and calls
+// `INSERT INTO workspaces` inside the loop body without first
+// checking whether a workspace with the same (parent_id, name) is
+// already there. Each call to such a handler doubles the tree.
+//
+// Why this is broader than TestCreateWorkspaceTree_CallsLookupBeforeInsert:
+// The existing gate is hard-coded to org_import.go's createWorkspaceTree.
+// That catches the specific function that triggered the original
+// incident — but a future handler written from scratch in a different
+// file would not be covered. This gate walks every production handler
+// .go file and applies a structural rule that does not depend on
+// function or file names.
+//
+// The rule (verbatim from #2867 PR-A):
+//
+// "No handler in handlers/ may iterate a slice (any RangeStmt) AND
+// call INSERT INTO workspaces inside the loop body without a
+// preceding SELECT id FROM workspaces WHERE name=$1 AND parent_id IS
+// NOT DISTINCT FROM $2 in the same function (== a lookupExistingChild
+// call, OR an ON CONFLICT clause baked into the same INSERT, OR an
+// explicit allowlist annotation)."
+//
+// Allowlist mechanism: a function whose body contains the exact comment
+// string `// class1-gate: idempotent-by-design` is treated as safe.
+// Use this only after writing a unit test that pins WHY the function
+// is safe. The annotation is intentionally awkward to type — it should
+// be rare.
+
+import (
+ "go/ast"
+ "go/parser"
+ "go/token"
+ "os"
+ "path/filepath"
+ "regexp"
+ "sort"
+ "strings"
+ "testing"
+)
+
+// reINSERTWorkspaces matches the exact statement shape we care about.
+// Tightened (vs bytes.Index "INSERT INTO workspaces") so the audit
+// table `workspaces_audit` literal — or any other lookalike — does not
+// false-positive trigger this gate. The same regex is used in the
+// existing createWorkspaceTree gate (workspaces_insert_allowlist_test.go)
+// — keep them in sync if either changes.
+var reINSERTWorkspaces = regexp.MustCompile(`(?m)^\s*INSERT INTO workspaces\s*\(`)
+
+// reONCONFLICT matches ON CONFLICT clauses anywhere in the same SQL
+// literal. An UPSERT (INSERT ... ON CONFLICT ... DO UPDATE) is
+// idempotent by definition, so the gate exempts it.
+var reONCONFLICT = regexp.MustCompile(`(?i)\bON CONFLICT\b`)
+
+// gateAllowlistComment is the magic comment a function author writes
+// to opt out of this gate. Forces an explicit decision.
+const gateAllowlistComment = "// class1-gate: idempotent-by-design"
+
+// preflightCallNames are function names whose presence in a function
+// body counts as "did a SELECT-by-(parent_id, name) preflight". Add
+// new names here as new preflight helpers are introduced. Keep the
+// list TIGHT — any sloppy addition weakens the gate.
+var preflightCallNames = map[string]bool{
+ "lookupExistingChild": true,
+}
+
+// TestClass1_NoUnpreflightedInsertInsideRange walks every production
+// .go file in this package, parses the AST, and fails the test if any
+// FuncDecl violates the rule above.
+//
+// Failure message must include: file path, function name, line of
+// the offending INSERT, line of the enclosing range, and a hint at
+// the three escape hatches (preflight call, ON CONFLICT, allowlist
+// comment).
+func TestClass1_NoUnpreflightedInsertInsideRange(t *testing.T) {
+ wd, err := os.Getwd()
+ if err != nil {
+ t.Fatalf("getwd: %v", err)
+ }
+
+ entries, err := os.ReadDir(wd)
+ if err != nil {
+ t.Fatalf("readdir %s: %v", wd, err)
+ }
+
+ type violation struct {
+ file string
+ fn string
+ insertLine int
+ rangeLine int
+ }
+ var violations []violation
+ scanned := 0
+
+ for _, e := range entries {
+ name := e.Name()
+ if e.IsDir() || !strings.HasSuffix(name, ".go") {
+ continue
+ }
+ if strings.HasSuffix(name, "_test.go") {
+ continue
+ }
+ path := filepath.Join(wd, name)
+ src, err := os.ReadFile(path)
+ if err != nil {
+ t.Fatalf("read %s: %v", path, err)
+ }
+ fset := token.NewFileSet()
+ file, err := parser.ParseFile(fset, name, src, parser.ParseComments)
+ if err != nil {
+ t.Fatalf("parse %s: %v", path, err)
+ }
+ scanned++
+
+ // Walk every function declaration and apply the rule.
+ for _, decl := range file.Decls {
+ fd, ok := decl.(*ast.FuncDecl)
+ if !ok || fd.Body == nil {
+ continue
+ }
+
+ // Allowlist: skip if the function body contains the magic
+ // comment. We check via the source range of the function
+ // — comments inside the body are in file.Comments and
+ // must overlap the function's Pos/End range.
+ if functionHasAllowlistComment(file, fd) {
+ continue
+ }
+
+ // First pass: locate every INSERT INTO workspaces literal
+ // in this function. We treat each such literal as a
+ // candidate violation and try to clear it via the rules.
+ candidates := findInsertWorkspacesLiterals(fd, src, fset)
+ if len(candidates) == 0 {
+ continue
+ }
+
+ // Has the function called a preflight helper? Single
+ // pass — if any preflight name appears, every INSERT in
+ // the function is considered preflighted. This is more
+ // permissive than position-aware (preflight could be
+ // AFTER the INSERT and still satisfy the gate), but the
+ // existing org_import.go gate already pins the position
+ // invariant for createWorkspaceTree, and a function that
+ // preflights AFTER inserting would fail the position
+ // gate in a separate test.
+ hasPreflight := functionCallsAny(fd, preflightCallNames)
+
+ for _, c := range candidates {
+ if c.hasONCONFLICT {
+ continue
+ }
+ if hasPreflight {
+ continue
+ }
+ if c.enclosingRangeLine == 0 {
+ // INSERT not inside any RangeStmt — single-shot,
+ // not the bug pattern.
+ continue
+ }
+ violations = append(violations, violation{
+ file: name,
+ fn: fd.Name.Name,
+ insertLine: c.insertLine,
+ rangeLine: c.enclosingRangeLine,
+ })
+ }
+ }
+ }
+
+ if scanned == 0 {
+ t.Fatal("scanned 0 .go files — wrong working directory? gate would always pass")
+ }
+
+ if len(violations) > 0 {
+ // Stable sort so the failure message is deterministic across
+ // reruns.
+ sort.Slice(violations, func(i, j int) bool {
+ if violations[i].file != violations[j].file {
+ return violations[i].file < violations[j].file
+ }
+ return violations[i].insertLine < violations[j].insertLine
+ })
+ var b strings.Builder
+ b.WriteString("Class 1 leak gate (#2867 PR-A) — these handler functions iterate a slice and INSERT INTO workspaces inside the loop body without a (parent_id, name) preflight.\n\n")
+ b.WriteString("This is the bug shape that triggered the tenant-hongming leak (TeamHandler.Expand re-inserting the entire sub_workspaces tree on every call). To fix any reported violation, choose ONE of:\n")
+ b.WriteString(" 1. Call h.lookupExistingChild(ctx, name, parentID) before the INSERT and skip the INSERT when it returns existing=true. (preferred)\n")
+ b.WriteString(" 2. Use INSERT ... ON CONFLICT ... DO ... (idempotent UPSERT, like registry.go).\n")
+ b.WriteString(" 3. Annotate the function with a `// class1-gate: idempotent-by-design` comment AND a unit test that pins why the function is structurally idempotent. (rare; require code review)\n\n")
+ b.WriteString("Violations:\n")
+ for _, v := range violations {
+ b.WriteString(" - ")
+ b.WriteString(v.file)
+ b.WriteString(":")
+ b.WriteString(itoa(v.insertLine))
+ b.WriteString(" — function ")
+ b.WriteString(v.fn)
+ b.WriteString("() INSERTs inside RangeStmt at line ")
+ b.WriteString(itoa(v.rangeLine))
+ b.WriteString("\n")
+ }
+ t.Fatal(b.String())
+ }
+}
+
+func itoa(n int) string {
+ // Avoid strconv import for one call site — keeps the test focused.
+ if n == 0 {
+ return "0"
+ }
+ neg := n < 0
+ if neg {
+ n = -n
+ }
+ var buf [20]byte
+ i := len(buf)
+ for n > 0 {
+ i--
+ buf[i] = byte('0' + n%10)
+ n /= 10
+ }
+ if neg {
+ i--
+ buf[i] = '-'
+ }
+ return string(buf[i:])
+}
+
+// candidateInsert holds the per-INSERT facts needed to decide whether
+// the gate fires.
+type candidateInsert struct {
+ insertLine int
+ hasONCONFLICT bool
+ enclosingRangeLine int // 0 means not inside any range
+}
+
+// findInsertWorkspacesLiterals walks fd's body and returns one
+// candidateInsert per INSERT INTO workspaces string literal.
+//
+// Position-based detection: collect every RangeStmt's body span first,
+// then for each INSERT literal check if its position is inside any
+// span. ast.Inspect's nil-call ordering does NOT give per-node pop
+// semantics, so a stack-based approach against ast.Inspect would
+// silently miscount. Position spans are deterministic and easy to
+// reason about.
+func findInsertWorkspacesLiterals(fd *ast.FuncDecl, src []byte, fset *token.FileSet) []candidateInsert {
+ var out []candidateInsert
+
+ type span struct{ start, end token.Pos }
+ var ranges []span
+ ast.Inspect(fd.Body, func(n ast.Node) bool {
+ rs, ok := n.(*ast.RangeStmt)
+ if !ok || rs.Body == nil {
+ return true
+ }
+ ranges = append(ranges, span{rs.Body.Lbrace, rs.Body.Rbrace})
+ return true
+ })
+
+ enclosingRangeLineFor := func(p token.Pos) int {
+ // Pick the innermost enclosing range — i.e., the one with the
+ // largest start that still covers p. Innermost is the one
+ // whose body actually contains the INSERT, which is the line
+ // most useful in a violation message.
+ bestStart := token.NoPos
+ bestLine := 0
+ for _, s := range ranges {
+ if p > s.start && p < s.end && s.start > bestStart {
+ bestStart = s.start
+ bestLine = fset.Position(s.start).Line
+ }
+ }
+ return bestLine
+ }
+
+ ast.Inspect(fd.Body, func(n ast.Node) bool {
+ bl, ok := n.(*ast.BasicLit)
+ if !ok || bl.Kind != token.STRING {
+ return true
+ }
+ // Strip surrounding backticks/quotes — value includes them.
+ lit := bl.Value
+ if len(lit) >= 2 {
+ lit = lit[1 : len(lit)-1]
+ }
+ if !reINSERTWorkspaces.MatchString(lit) {
+ return true
+ }
+ out = append(out, candidateInsert{
+ insertLine: fset.Position(bl.Pos()).Line,
+ hasONCONFLICT: reONCONFLICT.MatchString(lit),
+ enclosingRangeLine: enclosingRangeLineFor(bl.Pos()),
+ })
+ return true
+ })
+ return out
+}
+
+// functionCallsAny returns true if any CallExpr in fd's body has a
+// function name (either a SelectorExpr Sel.Name or an Ident name)
+// matching a key in names.
+func functionCallsAny(fd *ast.FuncDecl, names map[string]bool) bool {
+ found := false
+ ast.Inspect(fd.Body, func(n ast.Node) bool {
+ if found {
+ return false
+ }
+ ce, ok := n.(*ast.CallExpr)
+ if !ok {
+ return true
+ }
+ switch fun := ce.Fun.(type) {
+ case *ast.Ident:
+ if names[fun.Name] {
+ found = true
+ return false
+ }
+ case *ast.SelectorExpr:
+ if names[fun.Sel.Name] {
+ found = true
+ return false
+ }
+ }
+ return true
+ })
+ return found
+}
+
+// functionHasAllowlistComment returns true if the function body
+// (between fd.Body.Lbrace and fd.Body.Rbrace) contains a comment
+// equal to gateAllowlistComment.
+func functionHasAllowlistComment(file *ast.File, fd *ast.FuncDecl) bool {
+ if fd.Body == nil {
+ return false
+ }
+ start := fd.Body.Lbrace
+ end := fd.Body.Rbrace
+ for _, cg := range file.Comments {
+ for _, c := range cg.List {
+ if c.Pos() < start || c.Pos() > end {
+ continue
+ }
+ if strings.TrimSpace(c.Text) == gateAllowlistComment {
+ return true
+ }
+ }
+ }
+ return false
+}
+
+// TestClass1_GateFiresOnSyntheticBuggySource — proves the gate actually
+// catches the bug shape it's named after. Without this, a regression
+// to "always pass" would not be noticed until the leak shipped again.
+// Per memory feedback_assert_exact_not_substring.md: tighten the test
+// + verify it FAILS on old-shape source before merging.
+func TestClass1_GateFiresOnSyntheticBuggySource(t *testing.T) {
+ const buggySrc = `package handlers
+
+import "context"
+
+type fakeDB struct{}
+func (fakeDB) ExecContext(ctx context.Context, sql string, args ...interface{}) {}
+
+func buggyExpand(db fakeDB, ctx context.Context, children []string) {
+ for _, child := range children {
+ // Bug shape: INSERT inside the range body, no preflight.
+ db.ExecContext(ctx, ` + "`INSERT INTO workspaces (id, name) VALUES ($1, $2)`" + `, "x", child)
+ }
+}
+`
+ fset := token.NewFileSet()
+ file, err := parser.ParseFile(fset, "buggy.go", buggySrc, parser.ParseComments)
+ if err != nil {
+ t.Fatalf("parse synthetic source: %v", err)
+ }
+ for _, decl := range file.Decls {
+ fd, ok := decl.(*ast.FuncDecl)
+ if !ok || fd.Name.Name != "buggyExpand" {
+ continue
+ }
+ candidates := findInsertWorkspacesLiterals(fd, []byte(buggySrc), fset)
+ if len(candidates) != 1 {
+ t.Fatalf("expected 1 INSERT literal, got %d", len(candidates))
+ }
+ c := candidates[0]
+ if c.enclosingRangeLine == 0 {
+ t.Errorf("synthetic INSERT inside `for _, child := range` should be detected as enclosed by range, got enclosingRangeLine=0 — gate would miss the bug shape")
+ }
+ if c.hasONCONFLICT {
+ t.Errorf("synthetic INSERT has no ON CONFLICT, gate falsely treated it as idempotent")
+ }
+ if functionCallsAny(fd, preflightCallNames) {
+ t.Errorf("synthetic function does not call lookupExistingChild — gate falsely treated it as preflighted")
+ }
+ // All three guards say the gate WOULD fire. Pass.
+ return
+ }
+ t.Fatal("buggyExpand FuncDecl not found in synthetic source")
+}
+
+// TestClass1_GateAllowsONCONFLICT — pins that an INSERT with ON
+// CONFLICT inside a range body is NOT flagged. registry.go's
+// upsert pattern is the prod example.
+func TestClass1_GateAllowsONCONFLICT(t *testing.T) {
+ const safeSrc = `package handlers
+
+import "context"
+
+type fakeDB struct{}
+func (fakeDB) ExecContext(ctx context.Context, sql string, args ...interface{}) {}
+
+func upsertLoop(db fakeDB, ctx context.Context, children []string) {
+ for _, child := range children {
+ db.ExecContext(ctx, ` + "`INSERT INTO workspaces (id, name) VALUES ($1, $2) ON CONFLICT (id) DO UPDATE SET name = $2`" + `, "x", child)
+ }
+}
+`
+ fset := token.NewFileSet()
+ file, _ := parser.ParseFile(fset, "safe.go", safeSrc, parser.ParseComments)
+ for _, decl := range file.Decls {
+ fd, ok := decl.(*ast.FuncDecl)
+ if !ok || fd.Name.Name != "upsertLoop" {
+ continue
+ }
+ candidates := findInsertWorkspacesLiterals(fd, []byte(safeSrc), fset)
+ if len(candidates) != 1 {
+ t.Fatalf("expected 1 candidate, got %d", len(candidates))
+ }
+ if !candidates[0].hasONCONFLICT {
+ t.Errorf("ON CONFLICT clause should be detected, was missed — gate would falsely flag idempotent UPSERTs")
+ }
+ }
+}
+
+// TestClass1_GateAllowsAllowlistAnnotation — pins the escape hatch
+// works. Annotated functions are skipped at the FuncDecl level.
+func TestClass1_GateAllowsAllowlistAnnotation(t *testing.T) {
+ const annotatedSrc = `package handlers
+
+import "context"
+
+type fakeDB struct{}
+func (fakeDB) ExecContext(ctx context.Context, sql string, args ...interface{}) {}
+
+func intentionallyUnpreflighted(db fakeDB, ctx context.Context, children []string) {
+ // class1-gate: idempotent-by-design
+ for _, child := range children {
+ db.ExecContext(ctx, ` + "`INSERT INTO workspaces (id, name) VALUES ($1, $2)`" + `, "x", child)
+ }
+}
+`
+ fset := token.NewFileSet()
+ file, _ := parser.ParseFile(fset, "annotated.go", annotatedSrc, parser.ParseComments)
+ for _, decl := range file.Decls {
+ fd, ok := decl.(*ast.FuncDecl)
+ if !ok || fd.Name.Name != "intentionallyUnpreflighted" {
+ continue
+ }
+ if !functionHasAllowlistComment(file, fd) {
+ t.Error("allowlist comment should be detected for the intentionallyUnpreflighted function — escape hatch not working")
+ }
+ }
+}