feat(mothership): Chat-scoped outputs/ folder in VFS + Fork/Duplicate Chat#5401
feat(mothership): Chat-scoped outputs/ folder in VFS + Fork/Duplicate Chat#5401j15z wants to merge 30 commits into
Conversation
Adds Duplicate to the chat context menu. POST /api/mothership/chats/ [chatId]/duplicate clones the chat row, all messages, and the chat-owned files (uploads + outputs) under new ids/keys, rewriting every in-transcript file reference (attachment chips, embedded serve/view URLs, context chips, file resources) so the copy survives deletion of the original. Copied bytes are quota-checked up front and counted on success — deliberately diverging from the workspace-fork precedent (see comment at the increment site). Agent-side conversation state clones best-effort via the Go fork endpoint's new whole-chat mode. Duplicating navigates into the copy, titled "<name> (Copy)". Also contract-binds the vfs outputs route (surfaced by check:api-validation): listChatOutputsContract, storageContext enum + folderId alignment, and useChatOutputs upgraded from raw fetch to requestJson. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The sidebar Duplicate action is replaced by a Fork button on each assistant
reply (next to feedback). Forking copies the conversation up to and including
the clicked message, plus the chat's uploads born at-or-before that point:
each copy gets a fresh row id and storage key, the same message_id, physically
copied bytes counted against the storage quota, and every in-transcript file
reference re-pointed at the copies. Agent outputs/ stay behind.
- workspace_files gains a nullable message_id provenance column (drizzle
migration 0254); trackChatUpload stamps it from the sending user message
- fork route gains the quota gate + file copy + reference rewrite, reusing
the machinery built for duplicate (fork-chat-files.ts, rewrite helper)
- materialize_file nulls message_id alongside chatId
- duplicate route/contract/hook/tests removed; sidebar Duplicate reverted to
its pre-branch disabled state (showDuplicate={false})
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Clicking a #wsres-file outputs/ link minted a resource with an empty id (the outputs lookup ran with the route chatId, which stays undefined on the home surface), which was persisted and attached to the next send. The chat POST then 400'd on attachment validation before creating a run, and the send's catch "recovered" by reconnecting to its own never- registered stream id — 10 backoff retries against stream_not_found, ~3 minutes of stuck "running" UI with the real error swallowed. - resolve outputs/ file links with the stream-resolved chat id, and drop file resources that still have no id after resolution - reject empty-id resources in addResource, hydration merge, and the send's resourceAttachments - only retry-reconnect when the stream actually started; a failed POST now rolls back the optimistic send and surfaces the error - treat resume 404 (stream_not_found) as terminal instead of retrying - require min(1) resource ids in the add/reorder contracts (remove stays permissive so legacy empty-id rows can be deleted) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryHigh Risk Overview Fork and duplicate share one route: optional Security & reliability: Reviewed by Cursor Bugbot for commit 4a2333a. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThis PR adds two major capabilities to the chat/VFS surface: (1) a chat-scoped
Confidence Score: 5/5Safe to merge. The fork/duplicate logic, authorization changes, and outputs namespace are well-structured, consistently applied, and backed by thorough test coverage. The two migrations are additive and carefully constructed. No data-correctness or security defects were found. The authorization additions — ownership check for output files in both verifyWorkspaceFileAccess and verifyRegularFileAccess, WORKSPACE_FILE_LOOKUP_CONTEXTS filter, getPreviewableWorkspaceFile userId guard — are consistently applied and correct. The fork transaction correctly rolls back on any failure; post-commit blob copies are best-effort with proper dead-row cleanup and user-visible warnings. The rewrite functions correctly handle the ghost-resource drop and never leave cross-chat dangling references. The only findings are a missing 'output' entry in the schema comment and a missing isPending guard on the Duplicate context-menu action. No files require special attention. The schema.ts context comment omission and the sidebar double-click guard are the only items worth addressing before shipping. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[POST /fork chatId] --> B{upToMessageId set?}
B -- No --> C[Whole-chat duplicate\nAll files, title: name Copy]
B -- Yes --> D[Branch fork\nSlice messages to cut point\ntitle: Fork name]
C --> E[listDuplicableChatFiles\nALL chat-owned files\ncontext IN mothership,output]
D --> F[listDuplicableChatFiles then filterForkableChatFiles\nmessageId IN keptSet OR NULL]
E --> G[checkStorageQuota]
F --> G
G --> H[DB Transaction: INSERT copilot_chats]
H --> I[planChatFileCopies\nINSERT workspace_files copies\nfresh id + key per row]
I --> J[rewriteResourceFileRefs\nremap chips to new ids\ndrop ghost chat-owned refs]
J --> K[rewriteMessageFileRefs\nremap transcript URLs and attachments]
K --> L[appendCopilotChatMessages in tx]
L --> M[COMMIT]
M --> N[executeChatFileBlobCopies\nbounded concurrency 4\nbest-effort post-commit]
N --> O{Any failed?}
O -- Yes --> P[DELETE dead workspace_files rows\nremoveChatResources\nreturn failedFileCopies in response]
O -- No --> Q[Fork copilot-service Go endpoint\nbest-effort]
P --> Q
Q --> R[publishStatusChanged chatPubSub]
R --> S[Return id and optional failedFileCopies]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[POST /fork chatId] --> B{upToMessageId set?}
B -- No --> C[Whole-chat duplicate\nAll files, title: name Copy]
B -- Yes --> D[Branch fork\nSlice messages to cut point\ntitle: Fork name]
C --> E[listDuplicableChatFiles\nALL chat-owned files\ncontext IN mothership,output]
D --> F[listDuplicableChatFiles then filterForkableChatFiles\nmessageId IN keptSet OR NULL]
E --> G[checkStorageQuota]
F --> G
G --> H[DB Transaction: INSERT copilot_chats]
H --> I[planChatFileCopies\nINSERT workspace_files copies\nfresh id + key per row]
I --> J[rewriteResourceFileRefs\nremap chips to new ids\ndrop ghost chat-owned refs]
J --> K[rewriteMessageFileRefs\nremap transcript URLs and attachments]
K --> L[appendCopilotChatMessages in tx]
L --> M[COMMIT]
M --> N[executeChatFileBlobCopies\nbounded concurrency 4\nbest-effort post-commit]
N --> O{Any failed?}
O -- Yes --> P[DELETE dead workspace_files rows\nremoveChatResources\nreturn failedFileCopies in response]
O -- No --> Q[Fork copilot-service Go endpoint\nbest-effort]
P --> Q
Q --> R[publishStatusChanged chatPubSub]
R --> S[Return id and optional failedFileCopies]
Reviews (9): Last reviewed commit: "test(contracts): pin mothership↔copilot ..." | Re-trigger Greptile |
a03ff62 to
e4f59b3
Compare
|
@cursor review |
5615eff to
9d8756c
Compare
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 9d8756c. Configure here.
import only resolved chat uploads while save was extended to both namespaces — agent-generated workflow JSON under outputs/ always returned upload-not-found. Same routing as save: uploads//outputs/ prefixes target a namespace, bare-name collisions error as ambiguous. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The mothership resource routes are shims delegating to the copilot handlers, so the two contract families must describe the same boundary: - copilotResourceTypeSchema was missing filefolder/task/integration/ generic — valid MothershipResourceType rows of those types 400'd at the delegated parse, silently re-breaking the legacy empty-id cleanup (and reorder) for chats holding them. Enum now covers every member. - mothership add/reorder REQUEST items now require a non-empty id like the delegated copilot schemas (the client contract was advertising acceptance of payloads the server rejects); RESPONSE items stay permissive so replies containing not-yet-cleaned legacy rows still validate. Also records the confirmed _context-whitelist mechanism on ISSUES.md item 7 (sandbox-export chat threading). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The mothership resource routes delegate to the copilot handlers, so the two contract families describe one physical boundary. These tests turn the next one-sided schema edit into a red test instead of a silent client-side ZodError or delegated 400: the copilot resource-type enum must cover every MothershipResourceType, remove stays permissive on both sides (legacy empty-id deletion), add/reorder reject empty ids on both sides, and mothership responses stay permissive for not-yet-cleaned legacy rows. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
5 issues from previous reviews remain unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8afecd5. Configure here.
Round-4 review: the enum widening fully fixed remove/reorder (and made integration adds work), but POST keeps its own narrower VALID_RESOURCE_TYPES allow-list — scope the comment so it doesn't overclaim, ledger the pre-existing filefolder-add 400 as ISSUES.md item 11, and fix a stale 'from upload' log line in materialize import. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eviews Cursor auto-reviews each push; four findings landed between polling windows and surfaced via unresolved threads: - useChatOutputs no longer swallows list failures into a successful [] — errors throw so React Query retries instead of caching "no outputs" for the stale window (masked path-based output link resolution) - flushPendingResources and the post-flush reorder now use the shared isPersistedChatResource predicate (empty-id resources could reach the tightened API validation via the flush path) - fork quota gate only counts rows the plan will copy (workspaceId-less legacy rows are skipped by planChatFileCopies but their bytes could reject an otherwise-in-quota fork) - duplicate titles strip a leading "Fork | " like branch forks do: duplicating a forked chat yields "Name (Copy)", not "Fork | Name (Copy)" The fifth finding (copied-counter "race") is refuted on-thread: JS is single-threaded and the increment has no await between read and write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Optimistic duplicate titles strip "Fork | " like the server now does (no brief sidebar rename on refetch) - The hydration localOnly merge uses isPersistedChatResource — the last bare-predicate site that could readmit empty-id local resources Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4a2333a. Configure here.
| error: toError(err).message, | ||
| }) | ||
| return [] | ||
| } |
There was a problem hiding this comment.
Outputs API hides list failures
Medium Severity
The new chat-outputs HTTP route calls listChatOutputs, which catches database errors and returns an empty array. The route then responds with success: true and files: [], so transient DB failures look like “no outputs” instead of a 500. That undermines useChatOutputs throwing on failure, because the client never sees an error to retry.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4a2333a. Configure here.
| const isResolving = | ||
| isLoading || | ||
| (isFetching && !file) || | ||
| (listSettled && !listFile && (fallbackLoading || outputsLoading)) |
There was a problem hiding this comment.
Errored outputs query shows not found
Medium Severity
After useChatOutputs fails (network or 500), callers still default data to [] and never read isError. useResolvedEmbeddedFile then stops resolving once list and by-id lookups miss, so the embedded preview shows “File not found” instead of loading or error/retry—even when outputs exist.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4a2333a. Configure here.
…ixes Two fixes from the review rounds landed without tests — exactly the classes we can't let silently regress: - getToolCallTerminalData (f104cff): failed tool calls must surface their error in terminal data even when the handler ships a defined-but-empty output; verified these fail 5/10 against the pre-fix code. - resource contracts (834dc2e): add/reorder reject empty resource ids (the poison rows behind the phantom-stream reconnect wedge); remove stays permissive for legacy-row cleanup. The hook-internal half of the wedge fix (streamStarted gating, StreamNotFoundError terminal 404) has no testable seam in use-chat.ts and is tracked for extraction as a fast-follow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ip as fast-follows behind the flag Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… POSTs before rollback Max-effort review round 6 fixes. Root cause of the top cluster: by-id resolvers pin context='workspace', so chat-scoped output rows were invisible to paths not explicitly rewired for the third namespace. - resolveFileResource (model payload) falls back to the chat-scoped by-id resolver — output/upload tabs reach the model again - chatScopedOrWorkspacePath moves to vfs/path-utils as the single storageContext→prefix mapping (was module-private in resources.ts) - csv-preview route resolves via getPreviewableWorkspaceFile (owner-gated) and streams from the row's real storage context - resolveToolInputFile's chat by-id fallback drops its wf_ prefix gate: presign-flow uploads keep insert-time UUID ids - chat-scoped tabs render read-only: outputs are write-once and every editor save 404'd server-side, silently losing edits - send catch: a POST that dies without a response is probed (resume against its own stream id, RECONNECT_PROBE_ATTEMPTS) before the optimistic rollback — a blind rollback restored the queued item and double-sent a message the server was already answering Regression tests for each fix with a unit seam; the use-chat probe has none (extraction tracked in ISSUES.md item 9 fast-follow). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fixed, 30 fast-follows Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… thread chatId to tagged file contexts The fix-diff review (the round-6 gate) refuted the send-probe's premise: run registration happens at response-construction time, after the multi-second server prep, so the probe 404s terminally during exactly the ambiguous window it targeted — while the await it inserted before the rollback opened a new lost-message window on Stop. Reverted; the double-send is re-triaged as R0 in ISSUES.md with the server-side idempotency fix design. Also from the gate: biome import order in process-contents, and the tagged-@file context branch now passes chatId to resolveFileResource (same class as the active-tab fix — an @-tagged chat output resolved to null and was silently dropped). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…inst the working tree Every open item (11 original, R0-R7, 16 cleanup) confirmed still present with current file:line proof; all four round-6 'fixed on this branch' claims confirmed fixed; probe revert confirmed complete. Two entries sharpened: R0 (user row upserts on (chatId,messageId) — the duplicate is the second assistant run; upsert is the idempotency anchor) and R7 (needs a compileDoc signature change; chatId lives at the callers). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
39 ordered tests: outputs basics, fork/duplicate, explicit re-tests of every round-1-6 fixed bug, touched-surface regressions (tools, Files page, picker, VFS, chat core), and flag/permission edges. Known-issue tests are marked observe-don't-file against ISSUES.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…puts/ guard Found by manual test plan test 2: mothership's flag-on create_file schema advertises outputs/<name> while sim's server tool rejects outputs/ targets, so an explicit text-output request deterministically lands at files/<leaf>. Test 2 is now marked [KNOWN — R8] with the function_execute outputs.files workaround that unblocks tests 3-8/21/29. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>


Summary
Adds two related capabilities on the chat/VFS surface, plus the field fixes found while testing them. (1) A chat-scoped
outputs/namespace for one-off generated files (images, audio, video, ffmpeg results) — separate from workspacefiles/, living with the chat, saveable to the workspace on request; agent-side steering is behind the mothershipchat-scoped-outputsflag, so it dark-launches. (2) Fork and Duplicate — one engine, two gestures. The Fork button on each assistant reply copies the conversation up to and including that message into a fresh chat (Fork | <name>); the right-click Duplicate action is the same fork route with no cut point (upToMessageIdis now optional) — a whole-chat copy titled<name> (Copy)that keeps every message and copies agent outputs too. In both modes, copied files get fresh row ids and storage keys, bytes are physically copied and quota-charged, and every in-transcript file reference is re-pointed at the copies so the copy survives deletion of the original. Duplicates ask the mothership fork endpoint for its whole-chat clone mode (compacted working memory preserved verbatim); branch forks keep the truncate-and-rebuild path. Schema: two additive migrations —0254adds nullableworkspace_files.message_id, the provenance column that lets a branch fork copy only the chat-owned files (uploads AND outputs) born at-or-before the cut, and0255adds a partial unique index on output display names (see review-response changes below).Field fixes included (found via live testing of the above):
resourcesjsonb wholesale, keeping chips for files it doesn't have; file resources whose chat-owned file wasn't copied are now dropped.live-assistant:<streamId>id and forking it 400'd; the Fork button now waits for the persisted id.generate_image/video/audioandffmpegresolvedinputs.filesworkspace-only, souploads//outputs/references never loaded — andgenerate_imagesilently generated from the prompt alone. New sharedresolveToolInputFilecovers all three VFS namespaces, and unresolvable explicit references now fail the call instead of being silently skipped.outputs/link click on the home surface minted an empty-id resource that 400'd the next send and wedged the UI in "running" for ~3 minutes; empty-id resources are now rejected and a failed POST rolls back instead of retry-reconnecting.Review-response changes (
e43fe9c34)Addresses all 8 findings from code review:
outputs/+overwriteis now rejected in headless runs too — the files/ redirect can no longer silently replace a same-named workspace file.message_idat creation and join the same timeline cut as uploads, so inline embeds are re-pointed and forks fully survive source-chat deletion. The rule for every chat-owned file: it travels with the fork iff the user message that carried/requested it is kept.open_resource, table import, and knowledge add-document resolveoutputs//uploads/refs and barewf_ids (new chat-scoped by-id fallback inresolveToolInputFile); presigning and the background CSV import honor chat-scoped storage contexts (TableImportPayload.fileContext, backward-compatible).create_fileguard: checks the rawfileNamebeforefiles/prefixing, sooutputs/notes.mdcan no longer create a literalfiles/outputs/folder.0255: partial unique index on(chat_id, display_name) WHERE context='output'+ a 23505 retry inuploadChatOutput, closing the concurrent same-name generation race (mirrors the existing uploads index/retry; separate index so the two namespaces stay independent).upload-file-reader+output-file-readermerged into one context-parameterizedchat-file-reader(~200 duplicated lines removed, all public names preserved); sharedisOutputsPath/leaf helpers replace 5 inline prefix checks.headObjectreplay guard (no double-copy/double-charge on retry) + bounded concurrency (4) + a singleworkspace_filesread per fork with the branch cut applied in memory.EmbeddedFile/EmbeddedFileActionsshare one resolver hook (list → by-id → chat outputs by leaf name), so Download/Open work on path-referenced outputs.Deploy note: run a one-off duplicate check for
(chat_id, display_name)output rows before applying0255— the unique index build fails if race-produced dupes already exist (unlikely: young, flag-gated feature).Post-review rounds (after
e43fe9c34)Four more review/babysit rounds plus a max-effort adversarial pass landed on the branch; full detail lives in the two tracked handoff docs below.
importresolvesoutputs/likesave; the twin mothership↔copilot resource schema families are synced and parity-pinned by test; four round-5 Bugbot findings fixed; the two earlier review fixes that landed without tests are now pinned (tool-call-state.test.ts,contracts/copilot.test.ts).0255now builds its partial unique indexCONCURRENTLY(the deploy-note pre-check still applies).context='workspace', hidingoutputrows from every path not explicitly rewired for the third namespace. Fixed on-branch: model payload now includes output tabs; output tabs render read-only (no doomed editor); >5MB CSV outputs preview instead of 404ing; presign-flow UUID upload ids resolve as tool inputs. A client-side send-probe fix for the double-send was attempted and reverted after the gate re-review refuted its premise (see R0 in ISSUES.md).Handoff docs (tracked on this branch)
ISSUES.md— the live issue tracker: 11 triaged findings (quota ratchet, delete-path blob orphaning, fork crash window, …), review fast-follows R0–R8, the cleanup batch, and the accepted-behavior record. Newest is R8 (2026-07-06, found by manual testing): the flag-oncreate_fileschema advertisesoutputs/<name>while the sim server tool rejects it, so an explicit text-output request deterministically lands infiles/— fix touches mothershipcatalog/files/create_file.goplus a prompt/product call on how text outputs should be created at all.MANUAL-TESTING.md— a 39-test ordered manual plan (smoke → edge) with per-test expectations and [KNOWN — …] markers tying expected failures back to ISSUES.md entries. Test 2 documents the R8 behavior and the function_execute workaround that unblocks the tests depending onoutputs/notes.md.Type of Change
Testing
vitest run lib/copilot lib/table lib/uploads app/api/mothershipfromapps/sim): 107 files, 1067 tests passing, including fork route, fork-chat-files, rewrite-file-references, effective-transcript, resolve-input-file, chat-file-reader (the merged upload/output readers), vfs, materialize-file, create-file, resources, and track-chat-upload. New coverage from the review pass: write-once rejection in both interactive and headless modes, the headless files/ redirect, thecreate_fileraw-fileName guard, thewf_id chat-scoped fallback, outputs joining the fork timeline cut (pre-cut copied + re-pointed, post-cut ghosts dropped), the blob-copy replay guard, and the bounded concurrency pool — alongside the earlier whole-chat duplicate, ghost-resource, and resolver cases.tsc --noEmit✅ 0 errors ·biome check✅ clean ·check:api-validation✅ ·check:react-query✅. (apps/docstype-check noise is pre-existing — missing generated.sourceartifact; this branch touches no docs files.)<name> (Copy)with every message; reference-image generation actually uses the uploaded reference.fork/route.ts(isWholeChatDuplicatedrives the message cut, file listing, title, Go body, and analytics — now a singleworkspace_filesread cut in memory byfilterForkableChatFiles); the ghost-resource drop rule inrewriteResourceFileRefs(chat-owned ∧ not-copied ⇒ dropped; shared workspace files pass through); the deliberate quota divergence from the workspace-fork precedent (forked bytes ARE counted — see the comment at the increment site); the write-once check ordering inresource-writer.ts(rejection must precede the headless redirect); and the two migrations (0254: additive, no backfill, NULL = birth-unknown ⇒ included in every fork;0255: partial unique index, see deploy note).Checklist
Screenshots/Videos
Companion PR
Companion (mothership): https://github.com/simstudioai/mothership/pull/342