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) => { diff --git a/apps/desktop/src/main/runtime-host-client.ts b/apps/desktop/src/main/runtime-host-client.ts index 5570fe5dbc5..ceb63afdf0c 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 b84c690be26..fe7b424aa8c 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 @@ -874,7 +874,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..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,6 +55,8 @@ export function useComposerAttachments(options: Omit()( ['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-protocol.test.ts b/packages/runtime-host/src/__tests__/session-revision-protocol.test.ts index 89da12fc80c..a599db216e9 100644 --- a/packages/runtime-host/src/__tests__/session-revision-protocol.test.ts +++ b/packages/runtime-host/src/__tests__/session-revision-protocol.test.ts @@ -212,6 +212,74 @@ describe('Session revision protocol', () => { ); }); + 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/__tests__/session-revision-two-client-uds.test.ts b/packages/runtime-host/src/__tests__/session-revision-two-client-uds.test.ts index cdaa8e56083..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 @@ -552,6 +552,84 @@ 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' }, + }, + ); + // 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'); + // 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', + }), + { 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/protocol/index.ts b/packages/runtime-host/src/protocol/index.ts index c3caac84e94..8a982247d5b 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 = 229 as const; +export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 230 as const; +// 230: 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. // 229: Session check-in proposes cards for unfinished work across Sessions. // 228: Executor catalog queries support per-agent filtering and process-free inventory. // Older peers reject the executorId and listOnly fields in the strict query decoder. 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..5f50ecd1cab 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, @@ -424,6 +426,17 @@ 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 + ? editedTurnAttachmentsOf(source.messages, input.sourceTurnId) + : []; + const editedTurnArtifactIds = new Set( + editedTurnArtifactIdsOf(editedTurnAttachments, input.sourceSessionId), + ); const missingGraphChildSessionIds = linkedChildCopyAdmissionSessionIds({ sourceSessionId: input.sourceSessionId, sessionHeaders, @@ -541,8 +554,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 ? { @@ -581,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, @@ -598,6 +626,7 @@ export class HostSessionRevisionCoordinator { session: projectSessionCatalogRecord( await this.#stores.sessionStore.readCatalogRecord(input.targetSessionId), ), + ...(stagedAttachments.length > 0 ? { stagedAttachments } : {}), }); } catch (error) { console.error( @@ -722,11 +751,54 @@ export class HostSessionRevisionCoordinator { } if (copy.state === 'committed') { try { + // A retried request must answer with the same staged attachments the + // 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 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, + }, + }); + } + if (replayed.length > 0) stagedAttachments = replayed; + } return copySuccess({ kind: 'committed', session: projectSessionCatalogRecord( await this.#stores.sessionStore.readCatalogRecord(input.targetSessionId), ), + ...(stagedAttachments ? { stagedAttachments } : {}), }); } catch { return copyFailure( @@ -898,6 +970,80 @@ 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. + */ +/** + * 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>, +): { + 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) { + continue; + } + const relativePath = + copy.artifactIds.get(attachment.ref.relativePath) ?? + copy.relativePaths.get(attachment.ref.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; /** diff --git a/packages/ui/src/__tests__/composer-restored-pending-attachments.test.tsx b/packages/ui/src/__tests__/composer-restored-pending-attachments.test.tsx new file mode 100644 index 00000000000..5d6d63a067f --- /dev/null +++ b/packages/ui/src/__tests__/composer-restored-pending-attachments.test.tsx @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { act } from 'react'; +import { createRoot } from 'react-dom/client'; +import { parseHTML } from 'linkedom'; +import type { AttachmentRef } from '@maka/core/events'; +import type { PendingAttachment } from '../composer-attachments.js'; +import { + stageRevisionSourceContext, + type RevisionStagedContext, +} from '../revision-staged-context.js'; +import { useComposerAttachments } from '../use-composer-attachments.js'; + +type Hook = ReturnType; + +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/__tests__/revision-staged-context.test.ts b/packages/ui/src/__tests__/revision-staged-context.test.ts index 7780f358b49..7be1d483dba 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 () => { @@ -337,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({ @@ -402,7 +524,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 +552,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 +577,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 +589,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..c873791acd8 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. */ @@ -61,9 +64,27 @@ 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[]; }; +/** 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 +106,26 @@ 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. 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, + staged: Pick, ownerKey: string, - message: { quotes?: readonly QuoteRef[] }, -): RevisionStagedSource { + message: { quotes?: readonly QuoteRef[]; attachments?: readonly AttachmentRef[] }, +): RevisionStagedSource & { editOwnedStagingKeys: readonly string[] } { const sourceQuotes = [...(message.quotes ?? [])]; if (sourceQuotes.length > 0) staged.restoreQuotes(ownerKey, sourceQuotes); - return { originalQuotes: sourceQuotes, originalAttachments: [] }; + const sourceAttachments = [...(message.attachments ?? [])]; + const stagedItems = sourceAttachments.map(retainedAttachmentToPending); + if (stagedItems.length > 0) staged.restoreAttachments(ownerKey, stagedItems); + return { + originalQuotes: sourceQuotes, + originalAttachments: sourceAttachments, + editOwnedStagingKeys: stagedItems.map((item) => item.stagingKey), + }; } /** @@ -128,9 +158,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[], + editOwnedStagingKeys: readonly string[] = [], ): void { const keys = [...new Set(ownerKeys)]; const owned = new Map(); @@ -151,6 +185,20 @@ 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 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) => !editOwnedKeys.has(item.stagingKey)); + 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 +267,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 +351,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; @@ -332,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, @@ -346,6 +392,7 @@ export function createRevisionActions< previousComposerText: composerRef.current?.getText() ?? '', originalQuotes, originalAttachments, + editOwnedStagingKeys, }); composerRef.current?.setText(prompt); composerRef.current?.focus(); @@ -505,6 +552,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 +618,7 @@ export function createRevisionActions< stagedContext(), [draft.sourceSessionId, draft.draftSessionId], draft.originalQuotes, + draft.editOwnedStagingKeys, ); 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..d1d5fd441a6 100644 --- a/packages/ui/src/use-composer-attachments.ts +++ b/packages/ui/src/use-composer-attachments.ts @@ -549,6 +549,26 @@ 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[] { + 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[]; @@ -660,12 +680,14 @@ export function useComposerAttachments(options: { pickAttachments, attachFilePaths, restoreAttachments, + restorePendingAttachments, restoreMessageContext, restoreDirectories, removeAttachment, 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) =>