refactor(chat_files): extract streamWorkspaceResponse helper for Upload+Download - #2385
Merged
HongmingWang-Rabbit merged 1 commit intoApr 30, 2026
Merged
Conversation
…ad+Download
The "do request → check err → defer close → forward headers → set
status → io.Copy → log mid-stream errors" tail was duplicated between
Upload and Download. Each handler had ~12 lines that differed only in:
- the op label in log messages ("upload" vs "download")
- the set of response headers to forward verbatim
(Upload: Content-Type only; Download: Content-Type +
Content-Length + Content-Disposition)
Hoist into ChatFilesHandler.streamWorkspaceResponse(c, op,
workspaceID, forwardURL, req, forwardHeaders). Each call site
reduces to one line. Future changes — request-id forwarding,
observability metric, response-size cap, bytes-streamed log —
go in ONE place rather than two.
Same drift-prevention rationale as resolveWorkspaceForwardCreds
(#2372) and readOrLazyHealInboundSecret (#2376), applied to the
response-streaming layer of the same handlers.
Behavior preserved: existing TestChatUpload_* and TestChatDownload_*
integration tests (8 across both handlers) all pass unchanged. The
log message format is consistent across both handlers now (single
"chat_files {op}: ..." string template) — operators can grep one
prefix for both features instead of separate prefixes per handler.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
requested a review
from hongmingwang-moleculeai
as a code owner
April 30, 2026 15:28
HongmingWang-Rabbit
enabled auto-merge
April 30, 2026 15:28
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…org root (priv-esc) Independent security review of #2385 found a privilege-escalation path: POST /registry/register is bootstrap-allowed for a fresh workspace id and wrote the caller-supplied kind, while workspaces_platform_root_check only enforces 'platform => parent_id IS NULL' (NOT a single root). So an ordinary in-VPC workspace could register a fresh UUID as {"kind":"platform"}, mint a second org root, and POST /workspaces/:id/restart it — the shared provision path then injects MOLECULE_API_KEY=ADMIN_TOKEN (tenant-wide org-admin credential) into any kind='platform' workspace, on self-host AND SaaS. That breaks the invariant that only the concierge gets the org MCP + admin token. Defense in depth: - migration 20260607000000_one_platform_root: partial UNIQUE index (kind) WHERE kind='platform' — at most one platform root per (single-org) tenant DB. isPlatformRootViolation now also maps the 23505 to a friendly 409. - registry.go Register: app-layer guard refusing to CREATE or PROMOTE a row to kind='platform' via the public path (reserve that for the AdminAuth/boot-gated install paths); a platform agent re-registering its already-platform row is unaffected. Placed after the token check to avoid side-channeling row existence. - corrected the false 'CHECK structurally guarantees one per org' claims in the 20260606 migration + integration-test header. Tests: - registry_test.go: rejects fresh kind=platform (403), rejects workspace->platform promotion (403), allows already-platform re-register (200). - kind_platform_root_integration_test.go: real-PG test that a SECOND platform root is rejected by the unique index (the CHECK alone accepts it). - canvas-topology-pure.test.ts: cover stripPlatformRootForMap (QA HIGH gap) — abs-position reparent math, platform-edge drop, grandchild preservation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…rom the Canvas gate Two pre-existing Canvas-gate fragilities (both on main, surfaced by #2385's CI) that blocked the required CI / all-required gate on resource/timing, not on a real test result: 1. DisplayTab.test.tsx "forwards browser paste events into the noVNC clipboard" raced: it fired paste as soon as the "Workspace desktop" title rendered, but the component sets rfbRef.current synchronously after new RFB() INSIDE the async connect() (which awaits a lease/token first). When the race lost under CI runner load, the window paste handler's rfbRef.current?.clipboardPasteFrom no-op'd -> 0 calls. Wait for mockRFBConstructor before pasting -> deterministic. 2. The Canvas gate ran "npx vitest run --coverage" as the pass/fail step. v8 coverage + JSDOM under vitest maxWorkers:1 accumulates memory across all 228 files and OS-OOM-killed the run mid-suite on the shared runner. Split: the GATE is now plain "npx vitest run" (light, deterministic); coverage moves to a separate continue-on-error artifact step (no threshold gate per #1815, so it was never a real gate). Removes the OOM from the required path. Verified: DisplayTab 13/13 (5x); full canvas suite 3358/0; coverage run still produces the artifact when memory allows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…lf-host platform-agent backend (BYOK · user-tasks · boot-provision)' (#2385) from feat/canvas-concierge-ui into main
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The "do request → check err → defer close → forward headers → set status → io.Copy → log mid-stream errors" tail was duplicated between Upload and Download. Each handler had ~12 lines that differed only in:
Changes
Hoist into `ChatFilesHandler.streamWorkspaceResponse(c, op, workspaceID, forwardURL, req, forwardHeaders)`. Each call site reduces to one line. Future changes — request-id forwarding, observability metric, response-size cap, bytes-streamed log — go in ONE place rather than two.
Same drift-prevention rationale as `resolveWorkspaceForwardCreds` (#2372) and `readOrLazyHealInboundSecret` (#2376), applied to the response-streaming layer of the same handlers.
Behavior preservation
Test plan
🤖 Generated with Claude Code