From 2d2e9fa4589d71843881888b3c4d2376ed7d8de2 Mon Sep 17 00:00:00 2001 From: ggbdpq Date: Fri, 23 Oct 2026 04:59:21 +0800 Subject: [PATCH 1/8] feat(desktop,ui): restage a selected message's attachments on edit-and-resend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Desktop edit-and-resend refused any selected message carrying attachments: the copy excludes the revised turn, so no target-owned refs existed client-side and the edit failed closed (#5109). The Host can now stage them: a revision copy carries the edited turn's user-uploaded artifacts into the new Session (the include path built for user uploads, whose sentinel turnId the turn-scoped selection misses) and returns the rewritten refs on the committed response. The shared lifecycle restages the source attachments as retained pending items at edit start, swaps them for the copy's target-owned refs when the prepare succeeds, and re-keys the plate onto the branch child — the same flow quotes already follow (#5274). A rewrite missing from the response keeps its source ref, so the Host's session-owned guard fails that submit loudly instead of sending a dangling reference. Cancel drops the edit's own attachments and keeps the user's. Generated-by: GLM-5.3-Flash (ZCode) --- apps/desktop/src/main/runtime-host-client.ts | 7 +- ...runtime-host-session-execution-ipc-main.ts | 5 +- .../controller/use-composer-attachments.ts | 2 + .../src/shared/desktop-session-projection.ts | 3 + .../session-revision-protocol.test.ts | 68 ++++++++ .../src/protocol/session-revision.ts | 30 +++- .../server/session-revision-coordinator.ts | 49 +++++- .../__tests__/revision-staged-context.test.ts | 148 ++++++++++++++---- packages/ui/src/revision-staged-context.ts | 105 ++++++++++--- packages/ui/src/use-composer-attachments.ts | 9 ++ 10 files changed, 367 insertions(+), 59 deletions(-) diff --git a/apps/desktop/src/main/runtime-host-client.ts b/apps/desktop/src/main/runtime-host-client.ts index 7b456bcedfd..da9ad8db4af 100644 --- a/apps/desktop/src/main/runtime-host-client.ts +++ b/apps/desktop/src/main/runtime-host-client.ts @@ -1245,7 +1245,7 @@ export class DesktopRuntimeHostClient { async copySession( kind: "branch" | "revision", input: Omit, - ): Promise { + ): Promise { for (let attempt = 0; attempt < MAX_OPTIMISTIC_ATTEMPTS; attempt += 1) { const source = await this.#requireSession(input.sourceSessionId); const request = { ...input, expectedSourceRevision: source.revision }; @@ -1254,7 +1254,10 @@ export class DesktopRuntimeHostClient { ? await this.request("session.branch.create", request) : await this.request("session.revision.create", request); if (result.kind === "committed") - return requireSessionProjection(result.session); + return { + ...requireSessionProjection(result.session), + ...(result.stagedAttachments ? { stagedAttachments: result.stagedAttachments } : {}), + }; } throw revisionConflict(`${kind} copy`, input.sourceSessionId); } diff --git a/apps/desktop/src/main/runtime-host-session-execution-ipc-main.ts b/apps/desktop/src/main/runtime-host-session-execution-ipc-main.ts index 1c47fd362f9..3457a886555 100644 --- a/apps/desktop/src/main/runtime-host-session-execution-ipc-main.ts +++ b/apps/desktop/src/main/runtime-host-session-execution-ipc-main.ts @@ -866,7 +866,10 @@ export function registerRuntimeHostSessionExecutionIpc( sourceTurnId: normalized.sourceTurnId, }); deps.emitSessionsChanged("created", revision.id); - return toDesktopHostSessionSummary(revision); + return { + ...toDesktopHostSessionSummary(revision), + ...(revision.stagedAttachments ? { stagedAttachments: revision.stagedAttachments } : {}), + }; }, ); return async (sessionId) => { diff --git a/apps/desktop/src/renderer/features/conversation/controller/use-composer-attachments.ts b/apps/desktop/src/renderer/features/conversation/controller/use-composer-attachments.ts index 22f9dd28bd7..7431734c64f 100644 --- a/apps/desktop/src/renderer/features/conversation/controller/use-composer-attachments.ts +++ b/apps/desktop/src/renderer/features/conversation/controller/use-composer-attachments.ts @@ -55,6 +55,8 @@ export function useComposerAttachments(options: Omit { ); }); + test('carries the edited turn staged attachments on a committed revision', () => { + const attachment = { + kind: 'code', + name: 'source.txt', + mimeType: 'text/plain', + bytes: 14, + ref: { kind: 'session_file', sessionId: 'revision-target', relativePath: 'artifacts/x' }, + }; + assert.deepEqual( + decodeHostFrame({ + requestId: 'request-staged', + operation: 'session.revision.create', + ok: true, + result: { + kind: 'committed', + session: sessionProjection('target-session'), + stagedAttachments: [attachment], + }, + }), + { + requestId: 'request-staged', + operation: 'session.revision.create', + ok: true, + result: { + kind: 'committed', + session: sessionProjection('target-session'), + stagedAttachments: [attachment], + }, + }, + ); + // Optional: a committed revision without staged attachments decodes bare. + assert.deepEqual( + decodeHostFrame({ + requestId: 'request-bare', + operation: 'session.revision.create', + ok: true, + result: { + kind: 'committed', + session: sessionProjection('target-session'), + }, + }), + { + requestId: 'request-bare', + operation: 'session.revision.create', + ok: true, + result: { + kind: 'committed', + session: sessionProjection('target-session'), + }, + }, + ); + // A malformed attachment shape is rejected, not silently dropped. + assert.throws( + () => + decodeHostFrame({ + requestId: 'request-bad', + operation: 'session.revision.create', + ok: true, + result: { + kind: 'committed', + session: sessionProjection('target-session'), + stagedAttachments: [{ kind: 'code', name: 'x' }], + }, + }), + isInvalidFrame, + ); + }); + test('keeps single-target Revision abandonment identity exact', () => { const frame = decodeClientFrame({ requestId: 'request-abandon', diff --git a/packages/runtime-host/src/protocol/session-revision.ts b/packages/runtime-host/src/protocol/session-revision.ts index 79e970219b2..4e668d50490 100644 --- a/packages/runtime-host/src/protocol/session-revision.ts +++ b/packages/runtime-host/src/protocol/session-revision.ts @@ -24,6 +24,7 @@ import { requireRecord, requireShapedRecord, } from './codec.js'; +import { isAttachmentRef, type AttachmentRef } from '@maka/core/events'; import { invalidProtocolFrame } from './errors.js'; import { defineOperation } from './operation-spec.js'; import { decodeSessionCatalogItem, type SessionCatalogItem } from './session-catalog.js'; @@ -58,6 +59,13 @@ export type SessionConversationCopyResult = | { readonly kind: 'committed'; readonly session: SessionCatalogItem; + /** + * Revision copies only: the edited turn's own attachments, copied into + * the new Session with their refs rewritten, so the client can re-attach + * them to the replacement submit (#5109). Absent when the turn (or the + * copy kind) carries none. + */ + readonly stagedAttachments?: readonly AttachmentRef[]; } | { readonly kind: 'source_revision_conflict'; @@ -183,13 +191,20 @@ export function decodeSessionConversationCopyInput(value: unknown): SessionConve export function decodeSessionConversationCopyResult(value: unknown): SessionConversationCopyResult { const result = requireRecord(value, 'Session conversation-copy result'); if (result.kind === 'committed') { - const exact = requireExactRecord(result, 'committed Session conversation-copy result', [ - 'kind', - 'session', - ]); + const exact = requireShapedRecord( + result, + 'committed Session conversation-copy result', + ['kind', 'session'], + ['stagedAttachments'], + ); return { kind: 'committed', session: decodeSessionCatalogItem(exact.session), + ...(exact.stagedAttachments !== undefined + ? { + stagedAttachments: decodeStagedAttachments(exact.stagedAttachments), + } + : {}), }; } if (result.kind === 'source_revision_conflict') { @@ -222,6 +237,13 @@ function assertConversationCopyOutput( } } +function decodeStagedAttachments(value: unknown): readonly AttachmentRef[] { + if (!Array.isArray(value) || !value.every((item) => isAttachmentRef(item))) { + throw invalidProtocolFrame('Invalid staged attachments'); + } + return value; +} + function positiveRevision(value: unknown, label: string): number { const revision = requireCount(value, label); if (revision < 1) throw invalidProtocolFrame(`Invalid ${label}`); diff --git a/packages/runtime-host/src/server/session-revision-coordinator.ts b/packages/runtime-host/src/server/session-revision-coordinator.ts index 5dd4db49b51..86f4c9504fd 100644 --- a/packages/runtime-host/src/server/session-revision-coordinator.ts +++ b/packages/runtime-host/src/server/session-revision-coordinator.ts @@ -424,6 +424,25 @@ export class HostSessionRevisionCoordinator { messages: slice.messages, runtimeEvents, }); + // The edited turn sits outside the copied slice, so its own attachments + // are not carried by the transcript copy. A revision stages them into the + // new Session anyway (#5109): the client re-attaches the rewritten refs to + // the replacement submit. + const editedTurnAttachments = + kind === 'revision' && input.sourceTurnId !== undefined + ? source.messages.flatMap((message) => + message.turnId === input.sourceTurnId && message.type === 'user' + ? (message.attachments ?? []) + : [], + ) + : []; + const editedTurnArtifactIds = new Set( + editedTurnAttachments.flatMap((attachment) => + attachment.ref.kind === 'session_file' && attachment.ref.sessionId === input.sourceSessionId + ? [attachment.ref.relativePath] + : [], + ), + ); const missingGraphChildSessionIds = linkedChildCopyAdmissionSessionIds({ sourceSessionId: input.sourceSessionId, sessionHeaders, @@ -541,8 +560,10 @@ export class HostSessionRevisionCoordinator { sourceSessionId: input.sourceSessionId, targetSessionId: input.targetSessionId, turnIds: copyTurnIds, - ...(referencedSessionFileIds.size > 0 - ? { includeArtifactIds: [...referencedSessionFileIds] } + ...(referencedSessionFileIds.size > 0 || editedTurnArtifactIds.size > 0 + ? { + includeArtifactIds: [...referencedSessionFileIds, ...editedTurnArtifactIds], + } : {}), ...(linkedReferences.snapshots.size > 0 ? { @@ -593,11 +614,35 @@ export class HostSessionRevisionCoordinator { copiedMessages.some((message) => message.type === 'user'), }); await this.options.continuity.refreshCanonical(input.targetSessionId, lease); + // Only refs that actually landed in the target are staged; a missed + // rewrite stays off the list and the client keeps its fail-closed gate + // for that attachment instead of submitting a dangling reference. + const stagedAttachments = editedTurnAttachments.flatMap((attachment) => { + if ( + attachment.ref.kind !== 'session_file' || + attachment.ref.sessionId !== input.sourceSessionId + ) { + return []; + } + const relativePath = artifactCopy.relativePaths.get(attachment.ref.relativePath); + if (!relativePath) return []; + return [ + { + ...attachment, + ref: { + kind: 'session_file' as const, + sessionId: input.targetSessionId, + relativePath, + }, + }, + ]; + }); return copySuccess({ kind: 'committed', session: projectSessionCatalogRecord( await this.#stores.sessionStore.readCatalogRecord(input.targetSessionId), ), + ...(stagedAttachments.length > 0 ? { stagedAttachments } : {}), }); } catch (error) { console.error( diff --git a/packages/ui/src/__tests__/revision-staged-context.test.ts b/packages/ui/src/__tests__/revision-staged-context.test.ts index 7780f358b49..8720dcd5d80 100644 --- a/packages/ui/src/__tests__/revision-staged-context.test.ts +++ b/packages/ui/src/__tests__/revision-staged-context.test.ts @@ -21,6 +21,7 @@ import { strict as assert } from 'node:assert/strict'; import { describe, it } from 'node:test'; import type { AttachmentRef, QuoteRef } from '@maka/core/events'; +import type { PendingAttachment } from '../composer-attachments.js'; import type { StoredMessage } from '@maka/core/session'; import { clearRevisionStagedContext, @@ -61,13 +62,16 @@ type StagedLog = { quotes: QuoteRef[]; /** Per-owner buckets, so a clear only ever empties the key it names. */ buckets: Map; + /** The live attachment plate and its per-owner buckets (#5109). */ + attachments: PendingAttachment[]; + attachmentBuckets: Map; }; function emptyStagedLog(): StagedLog { const buckets = new Map(); const quotes: QuoteRef[] = []; buckets.set('session-1', quotes); - return { restored: [], cleared: [], quotes, buckets }; + return { restored: [], cleared: [], quotes, buckets, attachments: [], attachmentBuckets: new Map() }; } function fakeStaged(log: StagedLog): RevisionStagedContext { @@ -79,9 +83,17 @@ function fakeStaged(log: StagedLog): RevisionStagedContext { } return bucket; }; + const attachmentBucketOf = (ownerKey: string): PendingAttachment[] => { + let bucket = log.attachmentBuckets.get(ownerKey); + if (!bucket) { + bucket = []; + log.attachmentBuckets.set(ownerKey, bucket); + } + return bucket; + }; return { quotes: log.quotes, - attachments: [], + attachments: log.attachments, restoreQuotes: (ownerKey, quotes) => { if (quotes.length === 0) return; log.restored.push({ ownerKey, quotes: [...quotes] }); @@ -91,10 +103,27 @@ function fakeStaged(log: StagedLog): RevisionStagedContext { log.cleared.push(ownerKey); return bucketOf(ownerKey).splice(0); }, + restoreAttachments: (ownerKey, attachments) => { + if (attachments.length === 0) return; + attachmentBucketOf(ownerKey).push(...attachments); + log.attachments.push(...attachments); + }, + clearAttachments: (ownerKey) => { + const removed = attachmentBucketOf(ownerKey).splice(0); + for (const item of removed) { + const live = log.attachments.indexOf(item); + if (live >= 0) log.attachments.splice(live, 1); + } + return removed; + }, }; } -function createEnv(input: { messages: StoredMessage[]; staged: StagedLog }) { +function createEnv(input: { + messages: StoredMessage[]; + staged: StagedLog; + stagedAttachments?: readonly AttachmentRef[]; +}) { const activeIdRef = { current: 'session-1' }; const revisionDraftRef: { current: TurnRevisionDraftBase | null } = { current: null }; const toasts: Array<{ kind: 'info' | 'error'; title: string; description?: string }> = []; @@ -133,7 +162,10 @@ function createEnv(input: { messages: StoredMessage[]; staged: StagedLog }) { error: (title, description) => toasts.push({ kind: 'error', title, description }), }, copy, - reviseBeforeTurn: async () => ({ id: 'session-2' }), + reviseBeforeTurn: async () => ({ + id: 'session-2', + ...(input.stagedAttachments ? { stagedAttachments: input.stagedAttachments } : {}), + }), abandonSessionCopy: async () => {}, localizedShellErrorMessage: (_error, fallback) => fallback, reportSessionWorkspaceUnavailable: () => false, @@ -229,37 +261,35 @@ describe('revision lifecycle (#5109)', () => { ); }); - it('refuses to edit a message that carries attachments (#5274 review)', () => { - // Attachment ownership does not follow a revision copy — the copied - // transcript stops before the selected turn, so no target-owned refs - // exist client-side to restage. The edit refuses rather than silently - // dropping the files. + it('restages a carrying message as retained attachments instead of refusing (#5109)', () => { + // The Host now stages the edited turn's artifacts into the copy and hands + // back target-owned refs on the copy response, so the edit restages the + // attachment (source-owned until prepare swaps the ref) instead of + // refusing the edit outright. + const attachment: AttachmentRef = { + kind: 'image', + name: 'chart.png', + mimeType: 'image/png', + bytes: 10, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'a.png' }, + }; + const staged = emptyStagedLog(); const h = createEnv({ - messages: [ - userMessage('turn-1', 'with image', { - attachments: [ - { - kind: 'image', - name: 'chart.png', - mimeType: 'image/png', - bytes: 10, - ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'a.png' }, - }, - ], - }), - ], - staged: emptyStagedLog(), + messages: [userMessage('turn-1', 'with image', { attachments: [attachment] })], + staged, }); const actions = createRevisionActions(h.env); actions.beginEditUserMessage('turn-1'); - assert.equal(h.revisionDraftRef.current, null, 'no draft is committed'); - assert.deepEqual(h.toasts.at(-1), { - kind: 'info', - title: 'unavailable', - description: 'source-attachments-unsupported', - }); + assert.ok(h.revisionDraftRef.current, 'the edit commits a draft'); + assert.deepEqual(staged.attachments.map((item) => item.displayName), ['chart.png']); + assert.deepEqual(h.revisionDraftRef.current?.originalAttachments, [attachment]); + assert.deepEqual( + staged.attachments[0]?.source, + { type: 'retained', attachment }, + 'the pending item carries the source-owned ref until prepare', + ); }); it('lets an unchanged replacement through (#5815)', async () => { @@ -402,7 +432,10 @@ describe('revision staged-context helpers', () => { it('stages the source quotes under the owner key and records them', () => { const restored: Array<{ ownerKey: string; quotes: readonly QuoteRef[] }> = []; const snapshot = stageRevisionSourceContext( - { restoreQuotes: (ownerKey, quotes) => restored.push({ ownerKey, quotes }) }, + { + restoreQuotes: (ownerKey, quotes) => restored.push({ ownerKey, quotes }), + restoreAttachments: () => {}, + }, 'session-1', { quotes: [quotedQuote] }, ); @@ -427,6 +460,8 @@ describe('revision staged-context helpers', () => { cleared.push(ownerKey); return ownerKey === 'session-1' ? [restoredByEdit] : [addedByUser]; }, + restoreAttachments: () => {}, + clearAttachments: () => [], }, ['session-1', 'session-2', 'session-1'], [restoredByEdit], @@ -450,6 +485,8 @@ describe('revision staged-context helpers', () => { { restoreQuotes: (ownerKey, quotes) => restored.push({ ownerKey, quotes }), clearQuotes: (ownerKey) => (ownerKey === 'session-1' ? [identical, identical, identical] : []), + restoreAttachments: () => {}, + clearAttachments: () => [], }, ['session-1'], [identical], @@ -460,4 +497,55 @@ describe('revision staged-context helpers', () => { 'only the edit-owned entry is dropped; both user copies survive', ); }); + + it('restages a source attachment and swaps its ref for the copy target on prepare (#5109)', async () => { + const sourceRef: AttachmentRef = { + kind: 'code', + name: 'source.txt', + mimeType: 'text/plain', + bytes: 14, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'a/src' }, + }; + const targetRef: AttachmentRef = { + ...sourceRef, + ref: { kind: 'session_file', sessionId: 'session-2', relativePath: 'a/tgt' }, + }; + const staged = emptyStagedLog(); + const h = createEnv({ + messages: [ + { + type: 'user', + turnId: 'turn-1', + text: 'with attachment', + attachments: [sourceRef], + } as unknown as StoredMessage, + ], + staged, + stagedAttachments: [targetRef], + }); + const actions = createRevisionActions(h.env); + actions.beginEditUserMessage('turn-1'); + assert.equal( + staged.attachments.length, + 1, + 'the edit restages the source attachment as a retained pending item', + ); + assert.equal(h.toasts.some((toast) => toast.title === 'unavailable'), false); + const ready = await actions.prepareRevisionSend('with attachment'); + assert.equal(ready, true); + const live = staged.attachments[0]; + assert.equal(live?.source.type, 'retained'); + if (live?.source.type === 'retained') { + assert.deepEqual( + live.source.attachment.ref, + targetRef.ref, + 'the plate now carries the target-owned ref', + ); + } + assert.deepEqual( + staged.attachmentBuckets.get('session-2')?.length ?? 0, + 1, + 'the plate re-keyed onto the branch child', + ); + }); }); diff --git a/packages/ui/src/revision-staged-context.ts b/packages/ui/src/revision-staged-context.ts index 241dd49bddf..c763ca694b8 100644 --- a/packages/ui/src/revision-staged-context.ts +++ b/packages/ui/src/revision-staged-context.ts @@ -28,9 +28,9 @@ import type { PendingAttachment } from './composer-attachments.js'; * staging hooks bind their mutators to the active session's draft key, which * moves across the revision commit (source → branch child). Restoring and * clearing take an explicit owner key so the lifecycle can re-key the staged - * quotes across that commit; attachments do not restage — a revision copy - * excludes the revised turn and no target-owned refs exist client-side, so - * an edit of a message carrying them is refused outright (#5109 review). + * quotes and attachments across that commit; restaged attachments carry + * source-owned refs until prepare swaps them for the copy's target-owned + * ones (#5109). */ export type RevisionStagedContext = { quotes: readonly QuoteRef[]; @@ -39,6 +39,9 @@ export type RevisionStagedContext = { /** Removes and returns what an owner key holds, so the cancel path can * re-stage the entries the edit did not stage itself (#5274 review). */ clearQuotes(ownerKey: string): readonly QuoteRef[]; + restoreAttachments(ownerKey: string, attachments: readonly PendingAttachment[]): void; + /** Removes and returns what an owner key holds, mirroring clearQuotes. */ + clearAttachments(ownerKey: string): readonly PendingAttachment[]; }; /** The edit-and-resend source context a staged plate must match verbatim. */ @@ -64,6 +67,20 @@ export type TurnRevisionDraftBase = { }; +/** The retained pending form of a source message attachment: it carries the + * source-owned ref verbatim until prepare swaps in the copy's target-owned + * one (#5109). */ +export function retainedAttachmentToPending(attachment: AttachmentRef): PendingAttachment { + return { + stagingKey: crypto.randomUUID(), + displayName: attachment.name, + mimeType: attachment.mimeType, + kind: attachment.kind, + size: attachment.bytes, + source: { type: 'retained', attachment: structuredClone(attachment) }, + }; +} + function quoteKey(quote: QuoteRef): string { // Every field participates: two snapshots can share text, label, and turn // while differing in which Session they were captured from, when, or how @@ -85,17 +102,24 @@ function quoteKey(quote: QuoteRef): string { /** * Snapshot the source message's context at edit start and restage its quotes - * under the edit's owner key, so the gate can later tell the edit's own - * staging from the user's. + * and attachments under the edit's owner key, so the gate can later tell the + * edit's own staging from the user's. */ export function stageRevisionSourceContext( - staged: Pick, + staged: Pick, ownerKey: string, - message: { quotes?: readonly QuoteRef[] }, + message: { quotes?: readonly QuoteRef[]; attachments?: readonly AttachmentRef[] }, ): RevisionStagedSource { const sourceQuotes = [...(message.quotes ?? [])]; if (sourceQuotes.length > 0) staged.restoreQuotes(ownerKey, sourceQuotes); - return { originalQuotes: sourceQuotes, originalAttachments: [] }; + const sourceAttachments = [...(message.attachments ?? [])]; + if (sourceAttachments.length > 0) { + staged.restoreAttachments( + ownerKey, + sourceAttachments.map(retainedAttachmentToPending), + ); + } + return { originalQuotes: sourceQuotes, originalAttachments: sourceAttachments }; } /** @@ -128,9 +152,13 @@ export function revisionSendGate( * Session the cancel returns to (#5274 review). */ export function clearRevisionStagedContext( - staged: Pick, + staged: Pick< + RevisionStagedContext, + 'clearQuotes' | 'restoreQuotes' | 'clearAttachments' | 'restoreAttachments' + >, ownerKeys: readonly string[], editOwnedQuotes: readonly QuoteRef[], + editOwnedAttachments: readonly AttachmentRef[] = [], ): void { const keys = [...new Set(ownerKeys)]; const owned = new Map(); @@ -151,6 +179,21 @@ export function clearRevisionStagedContext( if (ownedCount <= 0) kept.push(quote); } if (keys.length > 0 && kept.length > 0) staged.restoreQuotes(keys[0], kept); + // Attachments: the edit's own entries (matched by their source ref) are + // dropped; anything else on the plates was staged by the user during the + // edit and survives on the first owner key. ponytail: count-based survival + // can mirror the quote logic if duplicated user additions matter. + const editOwnedRefs = new Set(editOwnedAttachments.map((ref) => JSON.stringify(ref))); + const dropped: PendingAttachment[] = []; + for (const ownerKey of keys) dropped.push(...staged.clearAttachments(ownerKey)); + const keptAttachments = dropped.filter( + (item) => + item.source.type !== 'retained' || + !editOwnedRefs.has(JSON.stringify(item.source.attachment.ref)), + ); + if (keys.length > 0 && keptAttachments.length > 0) { + staged.restoreAttachments(keys[0], keptAttachments); + } } /** Localized strings an edit-and-resend surface needs from its own catalog. */ @@ -219,7 +262,7 @@ export interface RevisionActionsEnv< reviseBeforeTurn( sourceSessionId: string, input: { sourceTurnId: string; copyId: string }, - ): Promise<{ id: string }>; + ): Promise<{ id: string; stagedAttachments?: readonly AttachmentRef[] }>; abandonSessionCopy(sourceSessionId: string, copyId: string): Promise; localizedShellErrorMessage(error: unknown, fallback: string, locale: UiLocale): string; /** True when the error is the workspace-unavailable class, having toasted. */ @@ -303,25 +346,23 @@ export function createRevisionActions< }); return; } - if ((userMessage.attachments?.length ?? 0) > 0) { - // Host-side attachment ownership does not follow a revision copy — the - // copied transcript stops before the selected turn, so no target-owned - // refs exist to restage. The edit refuses rather than silently dropping - // the files (#5109 review, #5274 review). - toastApi.info(copy.revisionUnavailableTitle, copy.revisionAttachmentsUnsupported); - return; - } - // The selected message's quotes and attachments restage into the surface's // plates (#5109): the plates make the carried context visible and // explicitly removable, and the commit re-keys them onto the branch child - // (prepareRevisionSend). The edit refuses while the user has own context + // (prepareRevisionSend). Attachments restage as retained pending items + // carrying their source-owned refs — prepare swaps those for the copy's + // target-owned refs, because the Host stages the artifacts into the new + // Session at copy time. The edit refuses while the user has own context // staged, so the plate ends up holding exactly the source context. const staged = stagedContext(); if (staged.quotes.length > 0) { toastApi.info(copy.revisionUnavailableTitle, copy.revisionDraftQuoteConflict); return; } + if (staged.attachments.length > 0) { + toastApi.info(copy.revisionUnavailableTitle, copy.revisionDraftAttachmentConflict); + return; + } if (userMessage.displayText !== undefined && userMessage.displayText !== userMessage.text) { toastApi.info(copy.revisionUnavailableTitle, copy.revisionTransformedTextUnsupported); return; @@ -505,6 +546,29 @@ export function createRevisionActions< // source key. staged.restoreQuotes(newSession.id, staged.quotes); staged.clearQuotes(startedDraft.sourceSessionId); + // Restaged attachments re-key onto the branch child with their refs + // swapped for the copy's target-owned ones (#5109): the copy response + // lists the edited turn's attachments in source order, matched back to + // the plate items by their source ref. An attachment whose rewrite is + // missing keeps its source ref — the Host's session-owned guard will + // fail that submit loudly rather than send a dangling reference. + const rewritten = newSession.stagedAttachments ?? []; + const refKey = (ref: AttachmentRef['ref']): string => JSON.stringify(ref); + staged.restoreAttachments( + newSession.id, + staged.attachments.map((item) => { + const source = item.source; + if (source.type !== 'retained') return item; + const index = startedDraft.originalAttachments.findIndex( + (candidate) => refKey(candidate.ref) === refKey(source.attachment.ref), + ); + const target = index >= 0 ? rewritten[index] : undefined; + return target + ? { ...item, source: { type: 'retained' as const, attachment: target } } + : item; + }), + ); + staged.clearAttachments(startedDraft.sourceSessionId); composerRef.current?.focus(); toastApi.info(copy.revisionReadyTitle, copy.revisionReadyDescription); return true; @@ -548,6 +612,7 @@ export function createRevisionActions< stagedContext(), [draft.sourceSessionId, draft.draftSessionId], draft.originalQuotes, + draft.originalAttachments, ); composerRef.current?.setDraft(draft.sourceSessionId, draft.previousComposerText); if (draft.draftSessionId !== draft.sourceSessionId) { diff --git a/packages/ui/src/use-composer-attachments.ts b/packages/ui/src/use-composer-attachments.ts index a32728d7fc2..49849b55c0b 100644 --- a/packages/ui/src/use-composer-attachments.ts +++ b/packages/ui/src/use-composer-attachments.ts @@ -549,6 +549,14 @@ export function useComposerAttachments(options: { for (const item of staged) lifecycle.stagedKeys.add(item.stagingKey); } + /** Removes and returns one owner key's pending attachments, so the revision + * lifecycle can re-key a plate (#5109). */ + function clearAttachmentsAt(ownerKey: string): readonly PendingAttachment[] { + const removed = selectPending(pendingStateRef.current.attachments, ownerKey); + updateAttachments(ownerKey, (map) => ({ ...map, [ownerKey]: [] })); + return removed; + } + /** Restore Main-owned recovery approvals and Host references without consuming either source. */ function restoreMessageContext(ownerKey: string, _hostId: string | undefined, input: { attachments: readonly AttachmentRef[]; @@ -666,6 +674,7 @@ export function useComposerAttachments(options: { clearSubmittedContext, clearSubmittedAttachments, clearAllAttachments, + clearAttachmentsAt, // Acquire before the send's first await; end in finally, after successful // submission clears only its captured staging keys. Failed sends keep drafts. retainAttachments: (attachments: readonly PendingAttachment[] | undefined) => From ecfbaf69e0b276465e8529b57220e630362a6a90 Mon Sep 17 00:00:00 2001 From: ggbdpq Date: Fri, 23 Oct 2026 05:04:09 +0800 Subject: [PATCH 2/8] chore(protocol): bump the compatibility epoch for staged revision attachments Older peers reject the additional stagedAttachments field in the strict copy-result decoder, so the wire change is observable: 227 -> 228. Generated-by: GLM-5.3-Flash (ZCode) --- packages/runtime-host/src/protocol/index.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/runtime-host/src/protocol/index.ts b/packages/runtime-host/src/protocol/index.ts index e25dcc76501..5ef0d79bf12 100644 --- a/packages/runtime-host/src/protocol/index.ts +++ b/packages/runtime-host/src/protocol/index.ts @@ -108,7 +108,10 @@ export const RUNTIME_HOST_PROTOCOL_VERSION = 0 as const; // Increment when the same protocol version no longer guarantees safe Client-Host // interoperability. Mismatches are rejected before domain commands are admitted. // Re-check against main before merging; sibling PRs may use the same next epoch. -export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 227 as const; +export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 228 as const; +// 228: Revision copy responses carry stagedAttachments — the edited turn's +// attachments restaged into the copy with target-owned refs (#5109). Older +// peers reject the additional field in the strict copy-result decoder. // 227: Goal projections carry eventWait and support goal.event-wait.cancel. // Older peers cannot decode the wait summary or cancel a specific wait. // 226: assistant.allocate-workspace allocates a Host-owned fallback for projectless assistants. From a32490f27f49915a25efd53929676cfd2229f875 Mon Sep 17 00:00:00 2001 From: ggbdpq Date: Fri, 23 Oct 2026 20:00:07 +0800 Subject: [PATCH 3/8] fix(ui): stage revision attachments through a verbatim pending restorer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The revision lifecycle's restorer contract hands the surface PendingAttachment[] items, but the desktop stagedContext bound the shared hook's ref-based restoreAttachments, which re-wraps each entry through retainedToPending: the wiring broke the desktop compile on the pending/ref type mismatch (TS2322), and at runtime every restaged item lost its display name and size and carried its real ref one level down, so the prepare swap and the submit could never match it. The lifecycle tests missed this because their fake restorer appends verbatim. Add restorePendingAttachments(ownerKey, items) to the shared hook — it appends to the owner bucket and registers the staging keys untouched — and bind it as the desktop stagedContext's restorer, covering the prepare re-key and the cancel restore alike. The lifecycle contract is unchanged; a hook-level test pins the verbatim staging the fake could not catch. Review thread: db=4354065450 (apache/maka#6246). Generated-by: GLM-5.3-Flash (ZCode) --- .../controller/use-composer-attachments.ts | 2 +- ...oser-restored-pending-attachments.test.tsx | 133 ++++++++++++++++++ packages/ui/src/use-composer-attachments.ts | 13 ++ 3 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 packages/ui/src/__tests__/composer-restored-pending-attachments.test.tsx diff --git a/apps/desktop/src/renderer/features/conversation/controller/use-composer-attachments.ts b/apps/desktop/src/renderer/features/conversation/controller/use-composer-attachments.ts index 7431734c64f..f9949f05385 100644 --- a/apps/desktop/src/renderer/features/conversation/controller/use-composer-attachments.ts +++ b/apps/desktop/src/renderer/features/conversation/controller/use-composer-attachments.ts @@ -55,7 +55,7 @@ export function useComposerAttachments(options: Omit; + +async function withAttachments(run: (hook: () => Hook) => Promise): Promise { + const original = { + document: globalThis.document, + window: globalThis.window, + IS_REACT_ACT_ENVIRONMENT: (globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; + }).IS_REACT_ACT_ENVIRONMENT, + }; + const { document, window } = parseHTML('
'); + Object.assign(globalThis, { document, window, IS_REACT_ACT_ENVIRONMENT: true }); + const container = document.querySelector('#root'); + assert.ok(container); + const root = createRoot(container); + let latest: Hook | undefined; + function Harness() { + latest = useComposerAttachments({ + copy: { + attachmentFailedTitle: 'failed', + tryAgain: 'try again', + imageAttachmentNotDirectTitle: 'image', + imageAttachmentNotDirectDescription: 'image context', + folderNotAttachable: 'folder', + folderNotAttachableUseReference: 'folder reference', + }, + formatError: (_error, fallback) => fallback, + draftKey: 'session-1', + toastApi: { error() {} }, + service: { + pickFiles: async () => ({ ok: false, reason: 'cancelled' }), + previewApproval: async () => ({ ok: false, reason: 'unused' }), + }, + }); + return null; + } + try { + await act(async () => { + root.render(); + }); + await run(() => { + assert.ok(latest); + return latest; + }); + } finally { + await act(async () => { + root.unmount(); + }); + Object.assign(globalThis, original); + } +} + +const sourceRef: AttachmentRef = { + kind: 'code', + name: 'source.txt', + mimeType: 'text/plain', + bytes: 14, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'a/src' }, +}; + +test('the revision restorer stages pending items verbatim, never re-wrapped (#6246 review)', async () => { + await withAttachments(async (hook) => { + // The desktop stagedContext binds this hook entry as the lifecycle's + // restorer — the revision lifecycle hands it PendingAttachment[] items it + // created from the source message. Binding the ref-based restoreAttachments + // here instead is what double-wrapped every restaged item (#6246 review). + const staged: RevisionStagedContext = { + quotes: [], + attachments: [], + restoreQuotes: () => {}, + clearQuotes: () => [], + restoreAttachments: hook().restorePendingAttachments, + clearAttachments: hook().clearAttachmentsAt, + }; + await act(async () => { + stageRevisionSourceContext(staged, 'session-1', { attachments: [sourceRef] }); + }); + const plate = hook().pendingAttachments; + assert.deepEqual(plate.map((item) => item.displayName), ['source.txt']); + assert.deepEqual(plate.map((item) => item.size), [14]); + assert.deepEqual( + plate[0]?.source.type === 'retained' ? plate[0].source.attachment : undefined, + sourceRef, + 'the pending item carries the source-owned ref, not a wrapped pending item', + ); + // The restored keys are live, so preview cleanup tracks the items. + assert.ok(hook().imageNoticeLifecycle.stagedKeys.has(plate[0]?.stagingKey ?? '')); + + // The cancel path re-restores survivors through the same entry: they come + // back exactly as they left, identity included. + let cleared: readonly PendingAttachment[] = []; + await act(async () => { + cleared = hook().clearAttachmentsAt('session-1'); + }); + assert.deepEqual(cleared, plate); + await act(async () => { + hook().restorePendingAttachments('session-1', cleared); + }); + assert.deepEqual(hook().pendingAttachments, [...cleared], 're-restored items keep their identity'); + }); +}); diff --git a/packages/ui/src/use-composer-attachments.ts b/packages/ui/src/use-composer-attachments.ts index 49849b55c0b..d1d5fd441a6 100644 --- a/packages/ui/src/use-composer-attachments.ts +++ b/packages/ui/src/use-composer-attachments.ts @@ -549,6 +549,18 @@ export function useComposerAttachments(options: { for (const item of staged) lifecycle.stagedKeys.add(item.stagingKey); } + /** Appends already-staged pending items verbatim — the revision lifecycle's + * restorer (#5109). The items go on the plate untouched, keeping the staging + * keys they were created with so the cancel path can still pick the edit's + * own entries out; running them through {@link restoreAttachments} would + * re-wrap each pending item as a source ref, losing its display name and + * size and stranding the real ref one level down (#6246 review). */ + function restorePendingAttachments(ownerKey: string, items: readonly PendingAttachment[]): void { + if (!lifecycle.mounted || items.length === 0) return; + updateAttachments(ownerKey, (map) => appendPending(map, ownerKey, items)); + for (const item of items) lifecycle.stagedKeys.add(item.stagingKey); + } + /** Removes and returns one owner key's pending attachments, so the revision * lifecycle can re-key a plate (#5109). */ function clearAttachmentsAt(ownerKey: string): readonly PendingAttachment[] { @@ -668,6 +680,7 @@ export function useComposerAttachments(options: { pickAttachments, attachFilePaths, restoreAttachments, + restorePendingAttachments, restoreMessageContext, restoreDirectories, removeAttachment, From ff3c49be9a55afa37369d54d0429f121691f85ad Mon Sep 17 00:00:00 2001 From: ggbdpq Date: Fri, 23 Oct 2026 20:00:07 +0800 Subject: [PATCH 4/8] fix(ui): track edit-owned staged attachments by staging key on cancel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clearRevisionStagedContext matched the edit's own attachments by JSON-stringifying whole AttachmentRefs, but the plate items nest the ref under source.attachment, so the shapes never matched and cancelling an edit left its carried attachments staged; after prepare the refs are target-owned anyway, so a source-shape match could never survive the rewrite either. Record the restaged items' staging keys on the draft (editOwnedStagingKeys) — the identity that survives prepare's ref rewrite — and drop by key on cancel. A carried attachment now drops while the user's own additions survive, before and after prepare. Review thread: db=4354065453 (apache/maka#6246). Generated-by: GLM-5.3-Flash (ZCode) --- .../__tests__/revision-staged-context.test.ts | 92 +++++++++++++++++++ packages/ui/src/revision-staged-context.ts | 58 ++++++------ 2 files changed, 124 insertions(+), 26 deletions(-) diff --git a/packages/ui/src/__tests__/revision-staged-context.test.ts b/packages/ui/src/__tests__/revision-staged-context.test.ts index 8720dcd5d80..7be1d483dba 100644 --- a/packages/ui/src/__tests__/revision-staged-context.test.ts +++ b/packages/ui/src/__tests__/revision-staged-context.test.ts @@ -367,6 +367,98 @@ describe('revision lifecycle (#5109)', () => { ); }); + it('cancelling drops the carried attachment but keeps the user addition (#6246 review)', async () => { + const sourceRef: AttachmentRef = { + kind: 'code', + name: 'source.txt', + mimeType: 'text/plain', + bytes: 14, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'a/src' }, + }; + const staged = emptyStagedLog(); + const h = createEnv({ + messages: [userMessage('turn-1', 'with attachment', { attachments: [sourceRef] })], + staged, + }); + const actions = createRevisionActions(h.env); + + actions.beginEditUserMessage('turn-1'); + // During the edit the user stages a file of their own beside the carried + // one; the cancel undoes the edit, not the user's own staging. + const userItem: PendingAttachment = { + stagingKey: 'user-added', + displayName: 'new.txt', + mimeType: 'text/plain', + kind: 'code', + size: 3, + source: { + type: 'retained', + attachment: { + ...sourceRef, + name: 'new.txt', + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'a/new' }, + }, + }, + }; + staged.attachments.push(userItem); + staged.attachmentBuckets.get('session-1')?.push(userItem); + await actions.cancelRevisionDraft(); + + assert.deepEqual( + staged.attachments.map((item) => item.displayName), + ['new.txt'], + 'the edit-owned source attachment is dropped; the user addition survives', + ); + }); + + it('cancelling a prepared edit still drops the carried attachment after its ref was rewritten (#6246 review)', async () => { + const sourceRef: AttachmentRef = { + kind: 'code', + name: 'source.txt', + mimeType: 'text/plain', + bytes: 14, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'a/src' }, + }; + const targetRef: AttachmentRef = { + ...sourceRef, + ref: { kind: 'session_file', sessionId: 'session-2', relativePath: 'a/tgt' }, + }; + const staged = emptyStagedLog(); + const h = createEnv({ + messages: [userMessage('turn-1', 'with attachment', { attachments: [sourceRef] })], + staged, + stagedAttachments: [targetRef], + }); + const actions = createRevisionActions(h.env); + + actions.beginEditUserMessage('turn-1'); + assert.equal(await actions.prepareRevisionSend('with attachment'), true); + const userItem: PendingAttachment = { + stagingKey: 'user-added', + displayName: 'new.txt', + mimeType: 'text/plain', + kind: 'code', + size: 3, + source: { + type: 'retained', + attachment: { + ...sourceRef, + name: 'new.txt', + ref: { kind: 'session_file', sessionId: 'session-2', relativePath: 'a/new' }, + }, + }, + }; + staged.attachments.push(userItem); + staged.attachmentBuckets.get('session-2')?.push(userItem); + await actions.cancelRevisionDraft(); + + assert.deepEqual( + staged.attachments.map((item) => item.displayName), + ['new.txt'], + 'the ref rewrite must not hide the edit-owned entry from the cancel', + ); + }); + it('keeps a quote added on the branch child when a prepared edit is cancelled', async () => { const staged = emptyStagedLog(); const h = createEnv({ diff --git a/packages/ui/src/revision-staged-context.ts b/packages/ui/src/revision-staged-context.ts index c763ca694b8..c873791acd8 100644 --- a/packages/ui/src/revision-staged-context.ts +++ b/packages/ui/src/revision-staged-context.ts @@ -64,6 +64,10 @@ export type TurnRevisionDraftBase = { previousComposerText: string; originalQuotes: readonly QuoteRef[]; originalAttachments: readonly AttachmentRef[]; + /** Staging keys of the entries the edit staged itself, so the cancel path + * can drop them by identity: their refs mutate across prepare's rewrite, + * but the staging key survives it (#6246 review). */ + editOwnedStagingKeys: readonly string[]; }; @@ -103,23 +107,25 @@ function quoteKey(quote: QuoteRef): string { /** * Snapshot the source message's context at edit start and restage its quotes * and attachments under the edit's owner key, so the gate can later tell the - * edit's own staging from the user's. + * edit's own staging from the user's. The restaged items go onto the plate + * verbatim (the surface's restorer appends them untouched), so their staging + * keys name the edit's own entries for the cancel path. */ export function stageRevisionSourceContext( staged: Pick, ownerKey: string, message: { quotes?: readonly QuoteRef[]; attachments?: readonly AttachmentRef[] }, -): RevisionStagedSource { +): RevisionStagedSource & { editOwnedStagingKeys: readonly string[] } { const sourceQuotes = [...(message.quotes ?? [])]; if (sourceQuotes.length > 0) staged.restoreQuotes(ownerKey, sourceQuotes); const sourceAttachments = [...(message.attachments ?? [])]; - if (sourceAttachments.length > 0) { - staged.restoreAttachments( - ownerKey, - sourceAttachments.map(retainedAttachmentToPending), - ); - } - return { originalQuotes: sourceQuotes, originalAttachments: sourceAttachments }; + const stagedItems = sourceAttachments.map(retainedAttachmentToPending); + if (stagedItems.length > 0) staged.restoreAttachments(ownerKey, stagedItems); + return { + originalQuotes: sourceQuotes, + originalAttachments: sourceAttachments, + editOwnedStagingKeys: stagedItems.map((item) => item.stagingKey), + }; } /** @@ -158,7 +164,7 @@ export function clearRevisionStagedContext( >, ownerKeys: readonly string[], editOwnedQuotes: readonly QuoteRef[], - editOwnedAttachments: readonly AttachmentRef[] = [], + editOwnedStagingKeys: readonly string[] = [], ): void { const keys = [...new Set(ownerKeys)]; const owned = new Map(); @@ -179,18 +185,17 @@ export function clearRevisionStagedContext( if (ownedCount <= 0) kept.push(quote); } if (keys.length > 0 && kept.length > 0) staged.restoreQuotes(keys[0], kept); - // Attachments: the edit's own entries (matched by their source ref) are - // dropped; anything else on the plates was staged by the user during the - // edit and survives on the first owner key. ponytail: count-based survival - // can mirror the quote logic if duplicated user additions matter. - const editOwnedRefs = new Set(editOwnedAttachments.map((ref) => JSON.stringify(ref))); + // Attachments: the edit's own entries are dropped by staging key — the + // identity that survives prepare's ref rewrite. Matching the source ref's + // JSON cannot work: the rewritten entry no longer carries the source shape, + // and the ref nested in a pending item is not the whole ref the snapshot + // holds (#6246 review). Anything else on the plates was staged by the user + // during the edit and survives on the first owner key. ponytail: count-based + // survival can mirror the quote logic if duplicated user additions matter. + const editOwnedKeys = new Set(editOwnedStagingKeys); const dropped: PendingAttachment[] = []; for (const ownerKey of keys) dropped.push(...staged.clearAttachments(ownerKey)); - const keptAttachments = dropped.filter( - (item) => - item.source.type !== 'retained' || - !editOwnedRefs.has(JSON.stringify(item.source.attachment.ref)), - ); + const keptAttachments = dropped.filter((item) => !editOwnedKeys.has(item.stagingKey)); if (keys.length > 0 && keptAttachments.length > 0) { staged.restoreAttachments(keys[0], keptAttachments); } @@ -373,11 +378,11 @@ export function createRevisionActions< revisionCopyKey(sessionId, turnId), turnId, ); - const { originalQuotes, originalAttachments } = stageRevisionSourceContext( - staged, - sessionId, - userMessage, - ); + const { + originalQuotes, + originalAttachments, + editOwnedStagingKeys, + } = stageRevisionSourceContext(staged, sessionId, userMessage); commitRevisionDraft({ sourceSessionId: sessionId, sourceTurnId: copyAttempt.sourceTurnId, @@ -387,6 +392,7 @@ export function createRevisionActions< previousComposerText: composerRef.current?.getText() ?? '', originalQuotes, originalAttachments, + editOwnedStagingKeys, }); composerRef.current?.setText(prompt); composerRef.current?.focus(); @@ -612,7 +618,7 @@ export function createRevisionActions< stagedContext(), [draft.sourceSessionId, draft.draftSessionId], draft.originalQuotes, - draft.originalAttachments, + draft.editOwnedStagingKeys, ); composerRef.current?.setDraft(draft.sourceSessionId, draft.previousComposerText); if (draft.draftSessionId !== draft.sourceSessionId) { From 5529fa931e0b33f710db110952667aac3f827ee4 Mon Sep 17 00:00:00 2001 From: ggbdpq Date: Fri, 23 Oct 2026 20:00:07 +0800 Subject: [PATCH 5/8] fix(host): stage uploaded revision attachments by artifact id too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The revision copy built stagedAttachments only from the copy's relativePaths map, but a user upload's ref names the upload's Artifact record id, which copyConversationArtifacts reports under artifactIds — so an uploaded attachment was copied into the new Session and then dropped off the committed response, leaving the client holding its fail-closed source ref and breaking edit-and-resend for uploads (#5109). Look the ref up in artifactIds first, matching the transcript rewriter's order, then relativePaths. The two-client UDS test now revises the upload turn itself and requires the rewritten, readable target ref on the committed response. Review thread: db=4354065457 (apache/maka#6246). Generated-by: GLM-5.3-Flash (ZCode) --- .../session-revision-two-client-uds.test.ts | 39 +++++++++++++++++++ .../server/session-revision-coordinator.ts | 9 ++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts b/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts index cdaa8e56083..d5ae7d708ee 100644 --- a/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts +++ b/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts @@ -552,6 +552,45 @@ async function verifyConcurrentRevisionAuthority( ); const sourceAfterRevision = await querySession(tui, sourceSessionId); + // A revision of the upload turn itself (#5109): the edited turn's own + // attachment is a user upload whose ref names the Artifact record id, which + // the copy reports under `artifactIds`, not `relativePaths` — the committed + // response must still stage the rewritten ref (#6246 review). + const uploadedTurnRevision = await desktop.request('session.revision.create', { + sourceSessionId, + targetSessionId: 'staged-upload-revision-target', + sourceTurnId: 'turn-1', + expectedSourceRevision: sourceAfterRevision.revision, + }); + assert.equal(uploadedTurnRevision.kind, 'committed'); + if (uploadedTurnRevision.kind !== 'committed') assert.fail('Revision must commit'); + assert.equal(uploadedTurnRevision.stagedAttachments?.length, 1); + const stagedUpload = uploadedTurnRevision.stagedAttachments?.[0]; + assert.equal(stagedUpload?.name, 'source.txt'); + assert.equal(stagedUpload?.ref.kind, 'session_file'); + if (stagedUpload?.ref.kind !== 'session_file') + assert.fail('Staged ref must stay session-backed'); + assert.equal(stagedUpload.ref.sessionId, 'staged-upload-revision-target'); + assert.notEqual(stagedUpload.ref.relativePath, 'source-artifact'); + assert.deepEqual( + await desktop.request('artifact.query', { + kind: 'read_text', + sessionId: 'staged-upload-revision-target', + artifactId: stagedUpload.ref.relativePath, + }), + { + kind: 'text', + sessionId: 'staged-upload-revision-target', + artifactId: stagedUpload.ref.relativePath, + preview: { ok: true, text: 'retained bytes' }, + }, + ); + assert.deepEqual( + await desktop.request('session.revision.abandon', { + targetSessionId: 'staged-upload-revision-target', + }), + { kind: 'abandoned', sessionId: 'staged-upload-revision-target' }, + ); const staleExpectedRevision = sourceAfterRevision.revision + 1; const stale = await tui.request('session.revision.create', { ...revisionInput, diff --git a/packages/runtime-host/src/server/session-revision-coordinator.ts b/packages/runtime-host/src/server/session-revision-coordinator.ts index 86f4c9504fd..81964d3f73e 100644 --- a/packages/runtime-host/src/server/session-revision-coordinator.ts +++ b/packages/runtime-host/src/server/session-revision-coordinator.ts @@ -624,7 +624,14 @@ export class HostSessionRevisionCoordinator { ) { return []; } - const relativePath = artifactCopy.relativePaths.get(attachment.ref.relativePath); + // A user upload's ref names the upload's Artifact record id, which the + // copy reports under `artifactIds`; only path-named Artifacts land in + // `relativePaths`. Look the id up first, matching the transcript + // rewriter's order, so an uploaded attachment is staged too (#6246 + // review) instead of being copied and then dropped off the response. + const sourceId = attachment.ref.relativePath; + const relativePath = + artifactCopy.artifactIds.get(sourceId) ?? artifactCopy.relativePaths.get(sourceId); if (!relativePath) return []; return [ { From 97107422f1021fa96eafdcd4903fe309275fd367 Mon Sep 17 00:00:00 2001 From: ggbdpq Date: Sat, 24 Oct 2026 06:58:34 +0800 Subject: [PATCH 6/8] test(desktop): align revision staged-context fixtures with attachment restaging (#6246) The staged revision attachments feature (#5109) restages a carrying message's attachments on edit instead of refusing it, and prepareRevisionSend now re-keys the composer plates through restoreAttachments/clearAttachments. The UI-layer suite was updated alongside the feature, but the desktop-layer fixtures still modeled the #5274-era contract: their fake stagedContext lacked the attachment methods (so prepare threw and returned false), and one test still asserted the old refusal guard. None of this surfaced in CI before, because every earlier head of this branch died on the protocol epoch guard long before the test stage. Fill in restoreAttachments/clearAttachments on the three desktop fixtures, and rewrite the refusal test to assert the restaging semantics already locked in by the UI suite. Generated-by: GLM-5.3-Flash (ZCode) --- .../app-shell-revision-actions.test.ts | 53 +++++++++++-------- .../app-shell-revision-resend.test.ts | 2 + .../use-composer-quotes-revision-send.test.ts | 2 + 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/apps/desktop/src/main/__tests__/app-shell-revision-actions.test.ts b/apps/desktop/src/main/__tests__/app-shell-revision-actions.test.ts index 1f7240f3b41..341c49bcc5d 100644 --- a/apps/desktop/src/main/__tests__/app-shell-revision-actions.test.ts +++ b/apps/desktop/src/main/__tests__/app-shell-revision-actions.test.ts @@ -93,6 +93,10 @@ function createActions(input: { messages: StoredMessage[]; failRefresh?: boolean staged.clearedKeys.push(ownerKey); return staged.quotes.splice(0, staged.quotes.length); }, + clearAttachments: (ownerKey: string) => { + staged.clearedKeys.push(ownerKey); + return []; + }, }), }, openSessionInChat: (sessionId: string) => { @@ -166,35 +170,40 @@ describe('app-shell revision actions with structured context (#5109)', () => { assert.equal(h.composerState.text, 'plain follow-up'); }); - it('refuses editing a message that carries attachments (#5274 review)', () => { - // Attachment ownership does not follow a revision copy — the copied - // transcript stops before the selected turn, so no target-owned refs - // exist client-side to restage. The edit refuses rather than silently - // dropping the files. + it('restages the attachments a message carries as retained pending items (#5109)', () => { + // The Host stages the edited turn's artifacts into the revision copy and + // hands back target-owned refs on the copy response, so the edit restages + // the attachment (source-owned until prepare swaps the ref) instead of + // refusing the edit outright. + const attachment = { + kind: 'image', + name: 'chart.png', + mimeType: 'image/png', + bytes: 10, + ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'a.png' }, + }; const h = createActions({ messages: [ - userMessage('turn-1', 'with image', { - attachments: [ - { - kind: 'image', - name: 'chart.png', - mimeType: 'image/png', - bytes: 10, - ref: { kind: 'session_file', sessionId: 'session-1', relativePath: 'a.png' }, - }, - ], - }), + userMessage('turn-1', 'with image', { attachments: [attachment] }), ], }); h.beginEditUserMessage('turn-1'); - assert.equal( - h.drafts.at(-1), - undefined, - 'a revision copy excludes the revised turn, so no target-owned attachment rewrite exists to restage', + const draft = h.drafts.at(-1) as { originalAttachments?: unknown[] } | undefined; + assert.ok(draft, 'a carrying message is editable now'); + assert.deepEqual(draft?.originalAttachments, [attachment]); + assert.deepEqual( + h.composerState.attachments.map((item) => (item as { displayName?: string }).displayName), + ['chart.png'], + 'the source attachment restages into the composer plates', + ); + assert.deepEqual( + (h.composerState.attachments[0] as { source?: unknown } | undefined)?.source, + { type: 'retained', attachment }, + 'the pending item carries the source-owned ref until prepare', ); - assert.equal(h.composerState.text, '', 'the composer stays untouched'); + assert.equal(h.composerState.text, 'with image'); }); it('stages a source message quotes into the composer', () => { @@ -329,6 +338,8 @@ describe('revision draft lifecycle over a prepared send', () => { attachments: [], restoreQuotes: (_ownerKey: string, _quotes: unknown[]) => {}, clearQuotes: (_ownerKey: string) => [], + restoreAttachments: (_ownerKey: string, _attachments: unknown[]) => {}, + clearAttachments: (_ownerKey: string) => [], }), }, openSessionInChat: (sessionId: string) => { diff --git a/apps/desktop/src/main/__tests__/app-shell-revision-resend.test.ts b/apps/desktop/src/main/__tests__/app-shell-revision-resend.test.ts index 8141a42ba15..87faae82061 100644 --- a/apps/desktop/src/main/__tests__/app-shell-revision-resend.test.ts +++ b/apps/desktop/src/main/__tests__/app-shell-revision-resend.test.ts @@ -222,6 +222,8 @@ async function mountRevisionWorld(): Promise { attachments: [], restoreQuotes: () => {}, clearQuotes: () => [], + restoreAttachments: () => {}, + clearAttachments: () => [], }), }, openSessionInChat: (sessionId: string) => { diff --git a/apps/desktop/src/main/__tests__/use-composer-quotes-revision-send.test.ts b/apps/desktop/src/main/__tests__/use-composer-quotes-revision-send.test.ts index 08b7fc4af9d..17027673f7b 100644 --- a/apps/desktop/src/main/__tests__/use-composer-quotes-revision-send.test.ts +++ b/apps/desktop/src/main/__tests__/use-composer-quotes-revision-send.test.ts @@ -99,6 +99,8 @@ test('the in-flight revision send still reads the re-keyed plate (#5274 review)' attachments: [], restoreQuotes: surface.restoreQuotes, clearQuotes: surface.clearQuotes, + restoreAttachments: () => {}, + clearAttachments: () => [], }), }, openSessionInChat: (sessionId: string) => { From e379bf0ada256df36dae9da64207f31d4011cca9 Mon Sep 17 00:00:00 2001 From: ggbdpq Date: Sat, 24 Oct 2026 12:04:30 +0800 Subject: [PATCH 7/8] fix(runtime): answer retried revision copies with the staged attachments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The committed-probe retry path returned a bare committed response, so a client that re-sends an interrupted revision.create never learns the stagedAttachments the first attempt staged (#6246 review P3). The retry now re-derives the edited turn's attachments and resolves their copies with `existingTarget: 'reuse_verified'` — idempotent, nothing is re-copied — and answers with the same target-owned refs. The staging helpers move to module scope so the retry path shares them with the primary copy. Generated-by: GLM-5.3-Flash (ZCode) --- .../session-revision-two-client-uds.test.ts | 19 +++ .../server/session-revision-coordinator.ts | 133 +++++++++++++----- 2 files changed, 115 insertions(+), 37 deletions(-) diff --git a/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts b/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts index d5ae7d708ee..a2336f58e7e 100644 --- a/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts +++ b/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts @@ -585,6 +585,25 @@ async function verifyConcurrentRevisionAuthority( preview: { ok: true, text: 'retained bytes' }, }, ); + // A retried request whose target already committed resolves through the + // committed-probe path: it must answer with the same staged attachments + // the first attempt returned, not a bare committed response (#6246 + // review). + const uploadedTurnRevisionRetry = await desktop.request('session.revision.create', { + sourceSessionId, + targetSessionId: 'staged-upload-revision-target', + sourceTurnId: 'turn-1', + expectedSourceRevision: sourceAfterRevision.revision, + }); + assert.equal(uploadedTurnRevisionRetry.kind, 'committed'); + if (uploadedTurnRevisionRetry.kind !== 'committed') assert.fail('Retry must stay committed'); + assert.equal(uploadedTurnRevisionRetry.stagedAttachments?.length, 1); + const retriedStaged = uploadedTurnRevisionRetry.stagedAttachments?.[0]; + assert.equal(retriedStaged?.name, 'source.txt'); + assert.equal(retriedStaged?.ref.kind, 'session_file'); + if (retriedStaged?.ref.kind !== 'session_file') + assert.fail('Retried staged ref must stay session-backed'); + assert.equal(retriedStaged.ref.sessionId, 'staged-upload-revision-target'); assert.deepEqual( await desktop.request('session.revision.abandon', { targetSessionId: 'staged-upload-revision-target', diff --git a/packages/runtime-host/src/server/session-revision-coordinator.ts b/packages/runtime-host/src/server/session-revision-coordinator.ts index 81964d3f73e..9eeb2ef2c19 100644 --- a/packages/runtime-host/src/server/session-revision-coordinator.ts +++ b/packages/runtime-host/src/server/session-revision-coordinator.ts @@ -43,6 +43,8 @@ import { } from '@maka/runtime/conversation-copy'; import { type SessionManager } from '@maka/runtime/session-manager'; import { RuntimeReadModelError } from '@maka/runtime/runtime-read-model'; +import type { AttachmentRef } from '@maka/core/events'; +import type { StoredMessage } from '@maka/core/session'; import { authenticateInteractiveArtifactStoreWriter, type InteractiveArtifactStoreWriter, @@ -430,18 +432,10 @@ export class HostSessionRevisionCoordinator { // the replacement submit. const editedTurnAttachments = kind === 'revision' && input.sourceTurnId !== undefined - ? source.messages.flatMap((message) => - message.turnId === input.sourceTurnId && message.type === 'user' - ? (message.attachments ?? []) - : [], - ) + ? editedTurnAttachmentsOf(source.messages, input.sourceTurnId) : []; const editedTurnArtifactIds = new Set( - editedTurnAttachments.flatMap((attachment) => - attachment.ref.kind === 'session_file' && attachment.ref.sessionId === input.sourceSessionId - ? [attachment.ref.relativePath] - : [], - ), + editedTurnArtifactIdsOf(editedTurnAttachments, input.sourceSessionId), ); const missingGraphChildSessionIds = linkedChildCopyAdmissionSessionIds({ sourceSessionId: input.sourceSessionId, @@ -617,33 +611,12 @@ export class HostSessionRevisionCoordinator { // Only refs that actually landed in the target are staged; a missed // rewrite stays off the list and the client keeps its fail-closed gate // for that attachment instead of submitting a dangling reference. - const stagedAttachments = editedTurnAttachments.flatMap((attachment) => { - if ( - attachment.ref.kind !== 'session_file' || - attachment.ref.sessionId !== input.sourceSessionId - ) { - return []; - } - // A user upload's ref names the upload's Artifact record id, which the - // copy reports under `artifactIds`; only path-named Artifacts land in - // `relativePaths`. Look the id up first, matching the transcript - // rewriter's order, so an uploaded attachment is staged too (#6246 - // review) instead of being copied and then dropped off the response. - const sourceId = attachment.ref.relativePath; - const relativePath = - artifactCopy.artifactIds.get(sourceId) ?? artifactCopy.relativePaths.get(sourceId); - if (!relativePath) return []; - return [ - { - ...attachment, - ref: { - kind: 'session_file' as const, - sessionId: input.targetSessionId, - relativePath, - }, - }, - ]; - }); + const stagedAttachments = stagedRefsFor( + editedTurnAttachments, + input.sourceSessionId, + input.targetSessionId, + artifactCopy, + ); return copySuccess({ kind: 'committed', session: projectSessionCatalogRecord( @@ -774,11 +747,40 @@ export class HostSessionRevisionCoordinator { } if (copy.state === 'committed') { try { + // A retried request must answer with the same staged attachments the + // first attempt returned (#5246/#5109): the artifacts are already in + // the target, so the maps come back idempotently under + // `reuse_verified` without re-copying anything. + let stagedAttachments: readonly AttachmentRef[] | undefined; + if (kind === 'revision' && input.sourceTurnId !== undefined) { + const source = await this.options.manager.readConversationCopySnapshot( + input.sourceSessionId, + ); + const edited = editedTurnAttachmentsOf(source.messages, input.sourceTurnId); + const includeIds = editedTurnArtifactIdsOf(edited, input.sourceSessionId); + if (edited.length > 0 && includeIds.length > 0) { + const retryCopy = await this.#artifacts.copyConversationArtifacts({ + sourceSessionId: input.sourceSessionId, + targetSessionId: input.targetSessionId, + turnIds: [], + includeArtifactIds: includeIds, + existingTarget: 'reuse_verified', + }); + const refs = stagedRefsFor( + edited, + input.sourceSessionId, + input.targetSessionId, + retryCopy, + ); + if (refs.length > 0) stagedAttachments = refs; + } + } return copySuccess({ kind: 'committed', session: projectSessionCatalogRecord( await this.#stores.sessionStore.readCatalogRecord(input.targetSessionId), ), + ...(stagedAttachments ? { stagedAttachments } : {}), }); } catch { return copyFailure( @@ -950,6 +952,63 @@ function persistedConversationCopyKind(kind: ConversationCopySemanticKind): Conv return kind === 'revision' ? 'revision' : 'branch'; } +/** The selected turn's own attachments: they sit outside the copied slice, so + * the transcript copy never carries them and a revision must stage them + * separately (#5109). */ +function editedTurnAttachmentsOf( + messages: readonly StoredMessage[], + sourceTurnId: string, +): readonly AttachmentRef[] { + return messages.flatMap((message) => + message.turnId === sourceTurnId && message.type === 'user' ? (message.attachments ?? []) : [], + ); +} + +function editedTurnArtifactIdsOf( + attachments: readonly AttachmentRef[], + sourceSessionId: string, +): string[] { + return attachments.flatMap((attachment) => + attachment.ref.kind === 'session_file' && attachment.ref.sessionId === sourceSessionId + ? [attachment.ref.relativePath] + : [], + ); +} + +/** + * The staged form of the edited turn's attachments: source-owned refs rewired + * to the target Session through the copy's result maps. A user upload's ref + * names the upload's Artifact record id, which the copy reports under + * `artifactIds`; only path-named Artifacts land in `relativePaths`. Look the + * id up first, matching the transcript rewriter's order, so an uploaded + * attachment is staged too (#6246 review) instead of being copied and then + * dropped off the response. Refs whose rewrite is missing stay off the list — + * the client keeps its fail-closed gate for those rather than submitting a + * dangling reference. + */ +function stagedRefsFor( + attachments: readonly AttachmentRef[], + sourceSessionId: string, + targetSessionId: string, + copy: Awaited>, +): AttachmentRef[] { + return attachments.flatMap((attachment) => { + if (attachment.ref.kind !== 'session_file' || attachment.ref.sessionId !== sourceSessionId) { + return []; + } + const relativePath = + copy.artifactIds.get(attachment.ref.relativePath) ?? + copy.relativePaths.get(attachment.ref.relativePath); + if (!relativePath) return []; + return [ + { + ...attachment, + ref: { kind: 'session_file' as const, sessionId: targetSessionId, relativePath }, + }, + ]; + }); +} + const LOGGED_COPY_FAILURE_DIAGNOSTICS = 5; /** From d99aaae89a6da67ebeb54a52ffe40f29128445eb Mon Sep 17 00:00:00 2001 From: ggbdpq Date: Sat, 24 Oct 2026 14:06:44 +0800 Subject: [PATCH 8/8] fix(runtime): make retried revision copies strictly read-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A retry of a committed revision.create answered through the artifact copy writer, whose `reuse_verified` re-creates a target record when it is absent — so a retry republished a staged attachment another client had deleted (#6246 review). The landed source-to-target id pairs now persist on the copy header at commit time, and the retry rebuilds its response with read-only `getInSession` lookups: a staged attachment that was deleted after the commit stays deleted and drops off the response, leaving the client's fail-closed gate to mark the edit no longer restageable. The staging helpers move to module scope shared by both paths. Generated-by: GLM-5.3-Flash (ZCode) --- packages/core/src/session.ts | 30 ++++- .../session-revision-two-client-uds.test.ts | 20 +++ .../server/session-revision-coordinator.ts | 115 ++++++++++++------ 3 files changed, 123 insertions(+), 42 deletions(-) diff --git a/packages/core/src/session.ts b/packages/core/src/session.ts index cec7620a20e..a08e568977b 100644 --- a/packages/core/src/session.ts +++ b/packages/core/src/session.ts @@ -190,6 +190,15 @@ export interface SessionConversationCopy { requestFingerprint: `sha256:${string}`; state: 'preparing' | 'committed'; intent?: 'side_conversation'; + /** Revision copies (#5109): where each staged attachment of the edited turn + * landed in the target Session — source Artifact id to target record id. + * Durable so a retried copy request can rebuild its response with read-only + * lookups instead of re-running artifact writes (which would republish + * artifacts another client has since deleted). */ + stagedArtifacts?: readonly { + readonly sourceId: string; + readonly targetId: string; + }[]; } export type SubagentSessionRuntimeSummary = Omit< @@ -557,7 +566,7 @@ const SUBAGENT_SESSION_SPAWN_IDENTITY_SHAPE = defineObjectShape()( ['kind', 'sourceSessionId', 'requestFingerprint', 'state'], - ['sourceTurnId', 'intent'], + ['sourceTurnId', 'intent', 'stagedArtifacts'], ); const SESSION_LINEAGE_ID_MAX_CHARS = 512; const SESSION_LINEAGE_CONTROL_CHARACTERS = /[\u0000-\u001f\u007f]/; @@ -655,7 +664,24 @@ export function isSessionConversationCopy(value: unknown): value is SessionConve : isSessionLineageId(value.sourceTurnId)) && typeof value.requestFingerprint === 'string' && /^sha256:[0-9a-f]{64}$/.test(value.requestFingerprint) && - (value.state === 'preparing' || value.state === 'committed') + (value.state === 'preparing' || value.state === 'committed') && + isStagedArtifactIdPairs(value.stagedArtifacts) + ); +} + +/** `stagedArtifacts` entries (#5109): both ids are Artifact record ids. */ +function isStagedArtifactIdPairs(value: unknown): boolean { + return ( + value === undefined || + (Array.isArray(value) && + value.every( + (entry) => + isRecord(entry) && + typeof entry.sourceId === 'string' && + entry.sourceId.length > 0 && + typeof entry.targetId === 'string' && + entry.targetId.length > 0, + )) ); } diff --git a/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts b/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts index a2336f58e7e..929248c7ab2 100644 --- a/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts +++ b/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts @@ -604,6 +604,26 @@ async function verifyConcurrentRevisionAuthority( if (retriedStaged?.ref.kind !== 'session_file') assert.fail('Retried staged ref must stay session-backed'); assert.equal(retriedStaged.ref.sessionId, 'staged-upload-revision-target'); + // A staged attachment deleted from the target after the commit stays + // deleted: the replay is read-only and must not republish it (#6246 + // review). + await desktop.request('artifact.delete', { + sessionId: 'staged-upload-revision-target', + artifactId: retriedStaged.ref.relativePath, + }); + const afterDelete = await desktop.request('session.revision.create', { + sourceSessionId, + targetSessionId: 'staged-upload-revision-target', + sourceTurnId: 'turn-1', + expectedSourceRevision: sourceAfterRevision.revision, + }); + assert.equal(afterDelete.kind, 'committed'); + if (afterDelete.kind !== 'committed') assert.fail('Replay must stay committed'); + assert.equal( + afterDelete.stagedAttachments, + undefined, + 'a deleted staged attachment drops off the replay instead of being republished', + ); assert.deepEqual( await desktop.request('session.revision.abandon', { targetSessionId: 'staged-upload-revision-target', diff --git a/packages/runtime-host/src/server/session-revision-coordinator.ts b/packages/runtime-host/src/server/session-revision-coordinator.ts index 9eeb2ef2c19..5f50ecd1cab 100644 --- a/packages/runtime-host/src/server/session-revision-coordinator.ts +++ b/packages/runtime-host/src/server/session-revision-coordinator.ts @@ -596,10 +596,23 @@ export class HostSessionRevisionCoordinator { }); // `cloneConversationRuntimeLedger` already wrote the copy's own spine, and // the copy reads back off that: nothing here writes a second transcript. + // Only refs that actually landed in the target are staged; a missed + // rewrite stays off the list and the client keeps its fail-closed gate + // for that attachment instead of submitting a dangling reference. The + // landed pairs persist on the header so a retried request rebuilds its + // response with read-only lookups instead of artifact writes (#6246 + // review: a retry must not republish an artifact another client deleted). + const { refs: stagedAttachments, pairs: stagedArtifacts } = stagedStaging( + editedTurnAttachments, + input.sourceSessionId, + input.targetSessionId, + artifactCopy, + ); await this.#stores.sessionStore.updateHeader(input.targetSessionId, { conversationCopy: { ...createInput.conversationCopy!, state: 'committed', + ...(stagedArtifacts.length > 0 ? { stagedArtifacts } : {}), }, isFlagged: sourceHeader.isFlagged, titleIsManual: kind === 'branch' ? false : sourceHeader.titleIsManual, @@ -608,15 +621,6 @@ export class HostSessionRevisionCoordinator { copiedMessages.some((message) => message.type === 'user'), }); await this.options.continuity.refreshCanonical(input.targetSessionId, lease); - // Only refs that actually landed in the target are staged; a missed - // rewrite stays off the list and the client keeps its fail-closed gate - // for that attachment instead of submitting a dangling reference. - const stagedAttachments = stagedRefsFor( - editedTurnAttachments, - input.sourceSessionId, - input.targetSessionId, - artifactCopy, - ); return copySuccess({ kind: 'committed', session: projectSessionCatalogRecord( @@ -748,32 +752,46 @@ export class HostSessionRevisionCoordinator { if (copy.state === 'committed') { try { // A retried request must answer with the same staged attachments the - // first attempt returned (#5246/#5109): the artifacts are already in - // the target, so the maps come back idempotently under - // `reuse_verified` without re-copying anything. + // first attempt returned (#5109) — but strictly read-only: the landed + // pairs were persisted on the header at commit time, so the rebuild + // looks the target records up instead of calling the copy writer, + // whose `reuse_verified` would re-create an artifact another client + // has since deleted (#6246 review). A deleted staged attachment stays + // deleted: it drops off the response, and the client's fail-closed + // gate treats the edit as no longer restageable. let stagedAttachments: readonly AttachmentRef[] | undefined; if (kind === 'revision' && input.sourceTurnId !== undefined) { const source = await this.options.manager.readConversationCopySnapshot( input.sourceSessionId, ); - const edited = editedTurnAttachmentsOf(source.messages, input.sourceTurnId); - const includeIds = editedTurnArtifactIdsOf(edited, input.sourceSessionId); - if (edited.length > 0 && includeIds.length > 0) { - const retryCopy = await this.#artifacts.copyConversationArtifacts({ - sourceSessionId: input.sourceSessionId, - targetSessionId: input.targetSessionId, - turnIds: [], - includeArtifactIds: includeIds, - existingTarget: 'reuse_verified', + const bySourceId = new Map( + editedTurnAttachmentsOf(source.messages, input.sourceTurnId) + .filter( + (attachment) => + attachment.ref.kind === 'session_file' && + attachment.ref.sessionId === input.sourceSessionId, + ) + .map((attachment) => [ + attachment.ref.kind === 'session_file' ? attachment.ref.relativePath : '', + attachment, + ]), + ); + const replayed: AttachmentRef[] = []; + for (const pair of copy.stagedArtifacts ?? []) { + const sourceAttachment = bySourceId.get(pair.sourceId); + if (!sourceAttachment) continue; + const entry = await this.#artifacts.getInSession(input.targetSessionId, pair.targetId); + if (!entry.record) continue; + replayed.push({ + ...sourceAttachment, + ref: { + kind: 'session_file' as const, + sessionId: input.targetSessionId, + relativePath: pair.targetId, + }, }); - const refs = stagedRefsFor( - edited, - input.sourceSessionId, - input.targetSessionId, - retryCopy, - ); - if (refs.length > 0) stagedAttachments = refs; } + if (replayed.length > 0) stagedAttachments = replayed; } return copySuccess({ kind: 'committed', @@ -986,27 +1004,44 @@ function editedTurnArtifactIdsOf( * the client keeps its fail-closed gate for those rather than submitting a * dangling reference. */ -function stagedRefsFor( +/** + * The staged form of the edited turn's attachments: source-owned refs rewired + * to the target Session through the copy's result maps, plus the landed + * source→target id pairs for the copy header. A user upload's ref names the + * upload's Artifact record id, which the copy reports under `artifactIds`; + * only path-named Artifacts land in `relativePaths`. Look the id up first, + * matching the transcript rewriter's order, so an uploaded attachment is + * staged too (#6246 review) instead of being copied and then dropped off the + * response. Refs whose rewrite is missing stay off the list — the client + * keeps its fail-closed gate for those rather than submitting a dangling + * reference. + */ +function stagedStaging( attachments: readonly AttachmentRef[], sourceSessionId: string, targetSessionId: string, copy: Awaited>, -): AttachmentRef[] { - return attachments.flatMap((attachment) => { +): { + refs: AttachmentRef[]; + pairs: { sourceId: string; targetId: string }[]; +} { + const refs: AttachmentRef[] = []; + const pairs: { sourceId: string; targetId: string }[] = []; + for (const attachment of attachments) { if (attachment.ref.kind !== 'session_file' || attachment.ref.sessionId !== sourceSessionId) { - return []; + continue; } const relativePath = copy.artifactIds.get(attachment.ref.relativePath) ?? copy.relativePaths.get(attachment.ref.relativePath); - if (!relativePath) return []; - return [ - { - ...attachment, - ref: { kind: 'session_file' as const, sessionId: targetSessionId, relativePath }, - }, - ]; - }); + if (!relativePath) continue; + refs.push({ + ...attachment, + ref: { kind: 'session_file' as const, sessionId: targetSessionId, relativePath }, + }); + pairs.push({ sourceId: attachment.ref.relativePath, targetId: relativePath }); + } + return { refs, pairs }; } const LOGGED_COPY_FAILURE_DIAGNOSTICS = 5;