Skip to content

fix(runtime-host): copy user-uploaded attachments when branching a conversation - #4289

Merged
Astro-Han merged 2 commits into
apache:mainfrom
liuxiaocs7:fix/branch-copy-user-upload-attachments
Aug 31, 2026
Merged

fix(runtime-host): copy user-uploaded attachments when branching a conversation#4289
Astro-Han merged 2 commits into
apache:mainfrom
liuxiaocs7:fix/branch-copy-user-upload-attachments

Conversation

@liuxiaocs7

Copy link
Copy Markdown
Member

Summary

Forking/branching a conversation that contains a user-uploaded attachment (image / PDF / file) failed 100% of the time. The Desktop toast was the generic 操作失败 / 任务操作失败,请稍后重试; the Runtime Host log showed RuntimeHostOperationError: Conversation copy is missing Session file attachment-<id> (code: persistence_failed), and the whole copy was rolled back so no branch was created. Text-only conversations were unaffected. The same copy path backs revision and side-conversation copies, so they had the same gap.

Root cause: user uploads are committed as artifact records whose turnId is the upload id sentinel (artifact-coordinator.ts#commitIngest), not a conversation turn. copyConversationArtifacts selects which session files to copy strictly by conversation turn (turnIds.has(record.turnId)), so a user upload was never selected, its id never entered the artifactIds map, and rewriteStorageRef could not resolve the attachment's session_file ref → threw → rollback. (Tool-produced session_file images are turn-scoped, so they were already copied correctly — only user uploads were affected.)

Fix: collect the session_file attachment ids the copied slice actually references — a new collectConversationCopySessionFileRefs that walks exactly the ref sites rewriteStorageRef is reached from (user-message attachments, tool_result image refs, text runtime-event attachments, function_response / archived tool-result images) — and force-include them via a new same-Session includeArtifactIds input to copyConversationArtifacts, alongside the existing turn-scoped selection. The include branch is lenient (an id with no matching source record is a no-op) and de-dups against the turn selection to avoid a double-copy. Applied for all copy kinds (branch, revision, side conversation).

This is the same class of "a reference not carried through the conversation-copy path" bug as the already-fixed#2060 / #2061, #3806, and #3776.

Fixes#4288

Verification

  • New tests, each fails without the fix:
    • packages/storageincludes explicit same-Session Artifacts outside the copied turns: a user_upload record with a sentinel turnId is absent from a turn-scoped copy and present once includeArtifactIds names it; unknown ids are a no-op.
    • packages/runtimecollectConversationCopySessionFileRefs gathers source-Session refs across sites: collects source-session refs at all four ref sites and excludes a child-session ref.
    • packages/runtime-host — the two-client UDS branch E2E fixture is now production-faithful (upload turnId = sentinel, attachment ref.relativePath = record id). I confirmed it fails without the fix with the exact production error (Conversation copy is missing Session file source-artifact, persistence_failed) and passes with it.
  • Suites (local, test:dist):@maka/runtime 3090 pass / 0 fail; @maka/runtime-host 1427 pass / 0 fail; @maka/storage 1006 pass / 1 fail. The single storage failure is the pre-existing, environment-specific managed-dependency-environment-crash test (its spawned child's stderr picks up Node's ExperimentalWarning: SQLite …); it is untouched by this PR and fails the same way on a clean tree.
  • Lint / format / typecheck:biome check clean on all changed files; tsc builds clean for @maka/storage, @maka/runtime, and @maka/runtime-host.
  • Did not run: full Desktop/UI suites (no renderer or protocol changes).

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Claude Code (Anthropic) — root-caused the failure, implemented the collector + includeArtifactIds plumbing, and wrote the unit and E2E tests. Reviewed by the author. The commit carries a Generated-by: Claude Code trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

…nversation
Forking/branching a conversation (and the sibling revision / side-conversation
copies) that contained a user-uploaded attachment failed every time with
`persistence_failed: Conversation copy is missing Session file attachment-<id>`,
surfaced to the user as the generic "操作失败 / 任务操作失败,请稍后重试" toast.
User uploads are committed as artifact records whose `turnId` is the upload id
sentinel, not a conversation turn, so the turn-scoped artifact copy never
selected them and `rewriteStorageRef` could not resolve their `session_file`
refs — the copy threw and rolled back.
Collect the `session_file` attachment ids the copied slice actually references
(new `collectConversationCopySessionFileRefs`, walking the same ref sites as
`rewriteStorageRef`) and force-include them via a new same-Session
`includeArtifactIds` input to `copyConversationArtifacts`, alongside the
existing turn-scoped selection. Lenient: unknown ids are a no-op.
Fixesapache#4288
Generated-by: Claude Code
@github-actionsgithub-actionsBot added the effort/M Under 500 readable lines label Aug 30, 2026
Address non-blocking review feedback on apache#4289:
- collectConversationCopySessionFileRefs: add an archivedResults image case
(and a child-Session archived image that must be ignored), covering the
previously-untested archived tool-result branch.
- two-client UDS branch E2E: read back the copied attachment bytes and assert
the rewritten session_file ref for the branch, the admitted revision, and the
active-source side conversation — proving all three copy kinds carry the
user-uploaded attachment, not just branch.
Refs apache#4288
Generated-by: Claude Code
@liuxiaocs7

Copy link
Copy Markdown
MemberAuthor

Addressed the non-blocking test feedback in 3f7d3eb:

  • collectConversationCopySessionFileRefs: added an archivedResults image case (plus a child-Session archived image that must be ignored), covering the previously-untested archived tool-result branch.
  • Two-client UDS branch E2E: now reads back the copied attachment bytes and asserts the rewritten session_file ref for the branch, the admitted revision, and the active-source side conversation — proving all three copy kinds carry the user upload, not just branch.

Local: @maka/runtime conversation-copy 22/22 and the runtime-host branch E2E 1/1 green; biome check clean on both files.

Left the collector/linked-child "shared visitor" refactor out of this PR as a future cleanup, per the non-blocking note.

@Astro-HanAstro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing conversation branching through the existing Runtime Host and Artifact Store copy authority. I verified exact head 3f7d3eb: copied-slice reference discovery, target identity rewriting, source and target admission, deterministic retry, rollback, active-source handling, and content readback are all covered, with exact-head checks green. I found no P0–P3 issues. This was an AI-assisted review using Codex and independent reviewer agents; I independently verified the production, recovery, and test paths and own this approval.

@Astro-Han
Astro-Han merged commit ef94235 into apache:mainAug 31, 2026
2 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/MUnder 500 readable lines

Projects

None yet

2 participants

@liuxiaocs7@Astro-Han