From 3b86d21001b4c8e56b6c21f0bda75fc89bb85346 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 26 Sep 2026 19:45:43 -0700 Subject: [PATCH 01/43] docs(plan): Undo Close keeps a derived TLDR identity (#1347) Co-Authored-By: Claude Opus 5.5 --- docs/plans/2026-09-26-undo-close-derived-identity.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 docs/plans/2026-09-26-undo-close-derived-identity.md diff --git a/docs/plans/2026-09-26-undo-close-derived-identity.md b/docs/plans/2026-09-26-undo-close-derived-identity.md new file mode 100644 index 000000000..1a286d490 --- /dev/null +++ b/docs/plans/2026-09-26-undo-close-derived-identity.md @@ -0,0 +1,10 @@ +# Undo Close keeps a derived TLDR identity (#1347) + +## Problem +Undo Close respawns a closed agent with `tldrIdentity: meta.tldrIdentity`. An agent whose identity is DERIVED has no explicit field. That happens when reporting domains are on but main created the agent, or its metadata arrived after it acquired TLDR or Goal. The rule is `tldrIdentityForSession`: the identity is the session id. Undo Close passes `undefined` for such an agent, so `spawn` mints a fresh UUID: the restored agent gets a new TLDR/Goal identity, and Agent Analytics (#1342 keys by it) splits its time. Reload Agents already passes `tldrIdentityForSession(oldId, meta)`. + +## Decision +The Undo Close respawn uses `tldrIdentityForSession(closedSessionId, meta)`, the same rule as reload. `respawn` now takes the closed id; every caller has it (a single pane's entry, a project member). + +## Test +`undoCloseFailure.renderer.test.tsx`: a closed agent with a reporting domain and no explicit identity is respawned with its closed session id as `tldrIdentity`; an explicit identity is still passed through unchanged. Red on main. From fba9ec6c7542cd0bdf6f3fb986308d2fd615fe9e Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 26 Sep 2026 19:48:56 -0700 Subject: [PATCH 02/43] fix(undo-close): a restored agent keeps its derived TLDR identity Undo Close passed only the explicit tldrIdentity, so an agent whose identity was derived (reporting domains on, no explicit field) came back with a fresh one. It now uses tldrIdentityForSession, the rule Reload Agents uses. Co-Authored-By: Claude Opus 5.5 --- .../src/workspace/hook/actions/undoClose.ts | 14 ++++++++--- .../undoCloseFailure.renderer.test.tsx | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/workspace/hook/actions/undoClose.ts b/src/renderer/src/workspace/hook/actions/undoClose.ts index 71e66e0b3..6ed635b42 100644 --- a/src/renderer/src/workspace/hook/actions/undoClose.ts +++ b/src/renderer/src/workspace/hook/actions/undoClose.ts @@ -1,3 +1,4 @@ +import { tldrIdentityForSession } from '@renderer/features/tldr/identity' import { carriedRelationships } from '@renderer/workspace/idRemap' import { sessionDisplayTitle } from '@renderer/workspace/sessionDisplayTitle' import { sessionMcpOverrides } from '@renderer/workspace/mcpDomains' @@ -163,7 +164,7 @@ export function useUndoCloseAction( // when main refused because the cwd is gone (see RespawnOutcome). `spawned` // says whether a backend now exists that a bail-out must kill. const respawn = useCallback( - async (meta: SessionMeta): Promise => { + async (meta: SessionMeta, closedSessionId: SessionId): Promise => { const kind: SessionKind = meta.kind ?? DEFAULT_PROVIDER if (kind === 'extension-view') { return { sessionId: crypto.randomUUID() as SessionId, spawned: false } @@ -185,7 +186,12 @@ export function useUndoCloseAction( // undo-restored transcript silently lose tools. The effective list is // deliberately not restored — the restored session is a NEW provider // process, so it resolves those choices against current Settings. - tldrIdentity: meta.tldrIdentity, + // The same rule as Reload Agents (#1347): an identity can be + // DERIVED (reporting domains on, no explicit field), and then it + // is the closed session id. Passing only the explicit field gave + // such an agent a fresh identity: new TLDR/Goal claims, and its + // analytics split in two. + tldrIdentity: tldrIdentityForSession(closedSessionId, meta), builtInMcpOverrides: sessionMcpOverrides(meta), }) return { sessionId, spawned: true } @@ -217,7 +223,7 @@ export function useUndoCloseAction( if (projectId === undefined) return 'stale' if (!refs.stateRef.current.tabs.some(tab => tab.id === projectId)) return 'stale' - const respawned = await respawn(meta) + const respawned = await respawn(meta, entry.sessionId) if (respawned === 'folder-missing') { // WHY 'stale' and not 'retryable-failure' (#1264 R2-1): the entry // would be pushed back and re-fail forever — a worktree removed after @@ -330,7 +336,7 @@ export function useUndoCloseAction( // missing session could corrupt, so the gentler rule covers everyone. let foldersMissing = 0 for (const member of entry.sessions) { - const respawned = await respawn(member.meta) + const respawned = await respawn(member.meta, member.sessionId) if (respawned === 'folder-missing') foldersMissing++ if (typeof respawned === 'string') continue idMap.set(member.sessionId, respawned.sessionId) diff --git a/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx index fa00ce5f3..07743c0f4 100644 --- a/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx @@ -227,3 +227,28 @@ it('does not push a deleted-folder member back with its group when a sibling fai expect(harness.showToast).toHaveBeenLastCalledWith('Could not restore "ProviderDown": Session failed to start. Check provider setup and retry.', 8000) harness.unmount() }) + +// #1347: an agent whose TLDR/Goal identity is DERIVED (a reporting domain on, +// no explicit field) is identified by its session id (tldrIdentityForSession). +// Undo Close passed only the explicit field, so the restored agent got a fresh +// identity and Agent Analytics split its time; Reload Agents already used the +// derived rule. +it('restores an agent with its derived TLDR identity, and an explicit one unchanged', async () => { + const { state, refs, writer } = setup() + refs.undoStackRef.current.pop() + refs.undoStackRef.current.push({ + type: 'session', closedAt: Date.now(), sessionId: 'explicit-pane', + sessionMeta: { cwd: '/projects/agent-code', kind: 'claude', title: 'Explicit', projectId: 'tab-parent', joinedAt: 1, tldrIdentity: 'tldr-explicit', builtInMcpDomains: ['tldr'] }, + }) + refs.undoStackRef.current.push({ + type: 'session', closedAt: Date.now(), sessionId: 'derived-pane', + sessionMeta: { cwd: '/projects/agent-code', kind: 'claude', title: 'Derived', projectId: 'tab-parent', joinedAt: 1, builtInMcpDomains: ['tldr'] }, + }) + const spawn = vi.fn().mockResolvedValueOnce('restored-derived').mockResolvedValueOnce('restored-explicit') + const harness = mount(state, refs, writer, spawn) + await act(async () => { await harness.undo() }) + await act(async () => { await harness.undo() }) + expect(spawn.mock.calls.map(([, opts]) => (opts as { tldrIdentity?: string }).tldrIdentity)).toEqual(['derived-pane', 'tldr-explicit']) + harness.unmount() +}) + From 08515a18d085965bfb6d52f897f8630efba3fa05 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 26 Sep 2026 20:51:04 -0700 Subject: [PATCH 03/43] test(undo-close): pin the derived identity on the project-restore path Co-Authored-By: Claude Opus 5.5 --- .../actions/undoCloseFailure.renderer.test.tsx | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx index 07743c0f4..71e428b84 100644 --- a/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx @@ -252,3 +252,20 @@ it('restores an agent with its derived TLDR identity, and an explicit one unchan harness.unmount() }) +// #1356 review b: the project-restore call site passes each MEMBER's closed id; +// a fresh id there survived the single-pane case above. +it('restores each agent of a project with its own derived TLDR identity', async () => { + const { state, refs, writer } = setup() + refs.undoStackRef.current.pop() + const meta = { cwd: '/projects/agent-code', kind: 'claude' as const, projectId: 'closed-tab', joinedAt: 1, builtInMcpDomains: ['goal'] as never } + refs.undoStackRef.current.push({ + type: 'tab', closedAt: Date.now(), tab: { id: 'closed-tab', title: 'agent-code' }, tabIndex: 0, + sessions: [{ sessionId: 'derived-member', meta }, { sessionId: 'explicit-member', meta: { ...meta, tldrIdentity: 'tldr-explicit-member' } }], + }) + const spawn = vi.fn().mockResolvedValueOnce('restored-one').mockResolvedValueOnce('restored-two') + const harness = mount(state, refs, writer, spawn) + await act(async () => { await harness.undo() }) + expect(spawn.mock.calls.map(([, opts]) => (opts as { tldrIdentity?: string }).tldrIdentity)).toEqual(['derived-member', 'tldr-explicit-member']) + harness.unmount() +}) + From c5ef44892ffc9b3584504fbbcb681a515ab33349 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 26 Sep 2026 22:46:02 -0700 Subject: [PATCH 04/43] test(undo-close): pin distinct derived identities for two project members Co-Authored-By: Claude Opus 5.5 --- .../hook/actions/undoCloseFailure.renderer.test.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx index 71e428b84..d19830287 100644 --- a/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/undoCloseFailure.renderer.test.tsx @@ -260,12 +260,14 @@ it('restores each agent of a project with its own derived TLDR identity', async const meta = { cwd: '/projects/agent-code', kind: 'claude' as const, projectId: 'closed-tab', joinedAt: 1, builtInMcpDomains: ['goal'] as never } refs.undoStackRef.current.push({ type: 'tab', closedAt: Date.now(), tab: { id: 'closed-tab', title: 'agent-code' }, tabIndex: 0, - sessions: [{ sessionId: 'derived-member', meta }, { sessionId: 'explicit-member', meta: { ...meta, tldrIdentity: 'tldr-explicit-member' } }], + // Two DERIVED members (#1356 verification c): with only one, passing the + // first member's closed id for every member went unnoticed. + sessions: [{ sessionId: 'derived-member', meta }, { sessionId: 'second-derived-member', meta }, { sessionId: 'explicit-member', meta: { ...meta, tldrIdentity: 'tldr-explicit-member' } }], }) - const spawn = vi.fn().mockResolvedValueOnce('restored-one').mockResolvedValueOnce('restored-two') + const spawn = vi.fn().mockResolvedValueOnce('restored-one').mockResolvedValueOnce('restored-two').mockResolvedValueOnce('restored-three') const harness = mount(state, refs, writer, spawn) await act(async () => { await harness.undo() }) - expect(spawn.mock.calls.map(([, opts]) => (opts as { tldrIdentity?: string }).tldrIdentity)).toEqual(['derived-member', 'tldr-explicit-member']) + expect(spawn.mock.calls.map(([, opts]) => (opts as { tldrIdentity?: string }).tldrIdentity)).toEqual(['derived-member', 'second-derived-member', 'tldr-explicit-member']) harness.unmount() }) From 10457ebe6680f0be16839c71d8190e4b5a0b9912 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 26 Sep 2026 23:49:57 -0700 Subject: [PATCH 05/43] docs(plan): deliver the bootstrap prompt to a late-created orchestration child (#1370) Co-Authored-By: Claude Opus 5.5 --- ...2026-09-27-orchestration-late-bootstrap.md | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 docs/plans/2026-09-27-orchestration-late-bootstrap.md diff --git a/docs/plans/2026-09-27-orchestration-late-bootstrap.md b/docs/plans/2026-09-27-orchestration-late-bootstrap.md new file mode 100644 index 000000000..a4832622a --- /dev/null +++ b/docs/plans/2026-09-27-orchestration-late-bootstrap.md @@ -0,0 +1,25 @@ +# Deliver the bootstrap prompt to a late-created orchestration child (#1370) + +## Problem +`orchestration_create_agent` waits at most 30 s for the renderer's `create-agent` answer (`OrchestrationBridge.dispatchRendererRequest`). The renderer answers only after the child's provider has started, and a healthy but slow start can take longer: the bundled OpenCode took 16.7, 18.3 and 43.1 s under CPU contention (#1367 review a). + +When the deadline passes: +- the caller gets `OrchestrationOutcomeUnknownError` ("do not repeat"); +- the MCP handler returns without delivering the prompt; +- the late answer is adopted (`adoptLateResponse`) with `bootstrapPromptDelivered: false`. + +The parent is left with an idle child that never received its brief. It was told not to repeat the create, and nothing tells it the brief is missing. + +## Decision (the issue's option a) +**When a timed-out create is adopted late, deliver its bootstrap prompt then**, through exactly the path a punctual create uses. +- The tool handler hands the bridge a late-create continuation along with the request. The bridge keeps it on the abandoned pending entry and runs it from `adoptLateResponse`. +- The handler's bootstrap delivery is extracted into one function that both paths call: deliver → `armPromptWhenReady` when the composer is not ready → mark the bootstrap delivered. The punctual and late paths therefore cannot drift. +- The late path has no caller to answer, so its outcome (delivered / pending / failed) goes to the incident journal. The adoption incident now records whether a bootstrap was attempted. +- **The timeout reply says so:** when a prompt was given, the outcome-unknown message adds that a child created late will receive its prompt automatically and must not be sent it again. Otherwise the parent's natural move, re-sending the brief after `list_agents` shows the child, would deliver it twice. + +**Rejected: option b** (deriving the create deadline from the provider's start budget). MCP clients have their own tool-call timeouts, and a Codex parent's defaults are shorter than 120 s. A longer deadline would turn "unknown" into the client's own opaque timeout. + +## Tests +- `src/main/orchestration/timeoutOutcome.test.ts`: a create with a late-create continuation times out, then the renderer answers. The continuation runs once, with the adopted (enriched) agent. A FAILED late answer runs nothing. A create that answered in time never runs it. +- The handler level: the create tool with a prompt, a renderer that answers after the deadline, and `deliverPromptToAgent` observed receiving the bootstrap for the late child. The timeout reply carries the "do not send it again" wording. +- Fail-first: both are red on `origin/main`. From 2e4bd3841bbb07c63f3fa23e92ce107fc055343f Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 00:03:04 -0700 Subject: [PATCH 06/43] fix(orchestration): deliver the bootstrap prompt to a child created after the deadline (#1370) A healthy provider can take longer than the bridge's 30 s create deadline to start (the bundled OpenCode took 43.1 s under load). The caller got "outcome unknown - do not repeat", the tool handler returned without delivering the prompt, and the late answer was adopted as an idle child with no brief and nothing telling the parent. - The tool handler hands the bridge a late-create continuation; the bridge keeps it for the outstanding request and runs it from adoptLateResponse (never for an in-time or failed answer). A throw is recorded as a prompt_delivery_failed incident. - The handler's bootstrap delivery is one function shared by the punctual and late paths (deliver, armPromptWhenReady, mark delivered). - The outcome-unknown reply says a late child receives its prompt automatically and must not be sent it again. Fail-first: bridge and real-MCP-handler tests are red on origin/main. Mutations killed: adoption skipping the continuation, the handler not registering it. Co-Authored-By: Claude Opus 5.5 --- src/main/orchestration/OrchestrationBridge.ts | 59 +++++++++- src/main/orchestration/timeoutOutcome.test.ts | 65 +++++++++++ src/mcp/runtime/createBuiltInMcpServer.ts | 46 +++++--- .../orchestrationLateBootstrap.system.test.ts | 107 ++++++++++++++++++ 4 files changed, 256 insertions(+), 21 deletions(-) create mode 100644 src/mcp/runtime/orchestrationLateBootstrap.system.test.ts diff --git a/src/main/orchestration/OrchestrationBridge.ts b/src/main/orchestration/OrchestrationBridge.ts index 28b188b85..309a486d7 100644 --- a/src/main/orchestration/OrchestrationBridge.ts +++ b/src/main/orchestration/OrchestrationBridge.ts @@ -96,10 +96,13 @@ export class OrchestrationOutcomeUnknownError extends Error { readonly requestId: string, readonly requestType: OrchestrationRendererRequest['type'], readonly parentSessionId: string, + /** Appended to the message when the caller must also be told what happens next (#1370). */ + note?: string, ) { super( `The renderer did not answer ${requestType} in time. It may still complete, so the outcome is UNKNOWN: ` - + `do not repeat it. Call orchestration_list_agents for this parent to see what exists before acting.`, + + `do not repeat it. Call orchestration_list_agents for this parent to see what exists before acting.` + + (note ? ` ${note}` : ''), ) this.name = 'OrchestrationOutcomeUnknownError' } @@ -253,6 +256,18 @@ export class OrchestrationBridge { * every POST — a map owned by the server would never see the duplicate. */ private readonly createCallsInFlight = new Map>() + /** + * What to do with a child whose create answered AFTER the caller gave up, keyed by requestId + * (#1370). The tool handler registers its bootstrap delivery here; `adoptLateResponse` runs it. + * + * WHY: a healthy provider can take longer than the 30 s bridge deadline to start (OpenCode took + * 43 s under load). The caller then got "outcome unknown — do not repeat", the handler returned + * without delivering the prompt, and the late child was adopted idle, with no brief, and nothing + * told the parent. Running the same delivery the punctual path runs, at adoption time, keeps the + * promise the create made. An entry lives only while its request is outstanding: it is removed + * when the create answers in time, fails outright, or is adopted. + */ + private readonly lateCreates = new Map Promise>() private readonly closedAgents = new Map() private readonly parentSessionByChildSession = new Map() private readonly listAgentsCache = new Map>() @@ -276,6 +291,8 @@ export class OrchestrationBridge { runId?: string builtInMcpDomains?: BuiltInMcpDomain[] inheritParentContext?: boolean + /** Runs if this create times out and the renderer's answer is adopted late (#1370). */ + onLateCreate?: (agent: OrchestrationAgentRecord) => Promise }): Promise { // WHY validate before sending a renderer request: an unsupported launch // must fail before a child or ownership record can exist. The factory is @@ -285,12 +302,34 @@ export class OrchestrationBridge { if (params.providerRuntime === 'terminal' && !getMainProvider(params.kind).createTerminalSession) { throw new Error(`${getMainProvider(params.kind).name} does not support a terminal runtime`) } + // The continuation is main-side only: a function cannot cross into the renderer. + const { onLateCreate, ...requestParams } = params const attempt: OrchestrationRendererRequest = { requestId: randomUUID(), type: 'create-agent', - ...params, + ...requestParams, + } + if (onLateCreate) this.lateCreates.set(attempt.requestId, onLateCreate) + let response: OrchestrationRendererResponse + try { + response = await this.request(attempt) + } catch (error) { + if (!(error instanceof OrchestrationOutcomeUnknownError)) { + this.lateCreates.delete(attempt.requestId) + throw error + } + // Still outstanding: keep the continuation for a late answer, and tell the caller about it, + // or its natural next move — sending the brief once list_agents shows the child — would + // deliver it twice. + if (!onLateCreate) throw error + throw new OrchestrationOutcomeUnknownError( + error.requestId, + error.requestType, + error.parentSessionId, + 'If the child is created late, its bootstrap prompt is delivered to it automatically: do not send it again.', + ) } - const response = await this.request(attempt) + this.lateCreates.delete(attempt.requestId) if (!response.ok) throw new Error(response.message) if (response.type !== 'create-agent') { throw new Error(`Unexpected orchestration response: ${response.type}`) @@ -584,6 +623,8 @@ export class OrchestrationBridge { * status cache keeps a stale answer. */ private adoptLateResponse(response: OrchestrationRendererResponse): void { + const onLateCreate = this.lateCreates.get(response.requestId) + this.lateCreates.delete(response.requestId) if (!response.ok || response.type !== 'create-agent') return const parentSessionId = response.agent.orchestrationParentId this.promptDeliveries.set(response.agent.sessionId, { @@ -605,8 +646,20 @@ export class OrchestrationBridge { // running work nobody is waiting on. Worth an incident even though // recovery succeeded. bootstrapPromptDelivered: false, + // Whether the create's own bootstrap delivery now runs for it (#1370). + bootstrapFollows: onLateCreate !== undefined, }, }) + if (!onLateCreate) return + const agent = this.enrichAgent(response.agent) + void onLateCreate(agent).catch((error: unknown) => { + this.journal?.recordIncident({ + kind: 'orchestration.prompt_delivery_failed', + severity: 'error', + reason: 'create_agent_late_bootstrap', + context: { sessionId: agent.sessionId, message: error instanceof Error ? error.message : 'unknown error' }, + }) + }) } private async cachedListAgents(params: { diff --git a/src/main/orchestration/timeoutOutcome.test.ts b/src/main/orchestration/timeoutOutcome.test.ts index b7823c646..16b8c8192 100644 --- a/src/main/orchestration/timeoutOutcome.test.ts +++ b/src/main/orchestration/timeoutOutcome.test.ts @@ -177,3 +177,68 @@ describe('a late answer is reconciled, never dropped', () => { expect(incidents.map(incident => incident.kind)).not.toContain('orchestration.late_response_adopted') }) }) + +// #1370: a healthy provider can take longer than the 30 s deadline to start. The timed-out create +// told the caller "do not repeat", and the late child was adopted idle, without the brief the +// create promised. The create's own bootstrap delivery now runs when the late answer is adopted. +describe('a late-created child still gets its bootstrap prompt (#1370)', () => { + it('runs the create\'s late continuation once, with the adopted child, and says so up front', async () => { + const late = vi.fn(async (_agent: { sessionId: string }) => {}) + const create = bridge.createAgent({ parentSessionId: 'parent-1', kind: 'claude', onLateCreate: late }) + const settled = create.catch((error: unknown) => error) + const request = lastSent('create-agent') as { requestId: string; onLateCreate?: unknown } + // The continuation is main-side only; the renderer never sees it. + expect(request).not.toHaveProperty('onLateCreate') + await vi.advanceTimersByTimeAsync(30_000) + const error = await settled + expect(error).toBeInstanceOf(OrchestrationOutcomeUnknownError) + // Otherwise the parent's next move — sending the brief once list_agents shows the child — + // would deliver it twice. + expect(String(error)).toMatch(/delivered to it automatically: do not send it again/) + expect(late).not.toHaveBeenCalled() + + bridge.resolve({ requestId: request.requestId, ok: true, type: 'create-agent', agent: child('child-late') } as never) + await vi.advanceTimersByTimeAsync(0) + expect(late).toHaveBeenCalledTimes(1) + expect(late.mock.calls[0]![0]).toMatchObject({ sessionId: 'child-late' }) + const adopted = incidents.find(incident => incident.kind === 'orchestration.late_response_adopted') + expect(adopted?.context).toMatchObject({ bootstrapFollows: true }) + }) + + it('runs nothing for a FAILED late answer', async () => { + const late = vi.fn(async () => {}) + const create = bridge.createAgent({ parentSessionId: 'parent-1', kind: 'claude', onLateCreate: late }) + const settled = create.catch((error: unknown) => error) + const request = lastSent('create-agent') as { requestId: string } + await vi.advanceTimersByTimeAsync(30_000) + await settled + bridge.resolve({ requestId: request.requestId, ok: false, message: 'no window' } as never) + await vi.advanceTimersByTimeAsync(0) + expect(late).not.toHaveBeenCalled() + }) + + it('never runs it for a create that answered in time', async () => { + const late = vi.fn(async () => {}) + const create = bridge.createAgent({ parentSessionId: 'parent-1', kind: 'claude', onLateCreate: late }) + const request = lastSent('create-agent') as { requestId: string } + bridge.resolve({ requestId: request.requestId, ok: true, type: 'create-agent', agent: child('child-fast') } as never) + await expect(create).resolves.toMatchObject({ sessionId: 'child-fast' }) + await vi.advanceTimersByTimeAsync(30_000) + expect(late).not.toHaveBeenCalled() + }) + + it('reports a late delivery that throws as an incident instead of losing it', async () => { + const create = bridge.createAgent({ + parentSessionId: 'parent-1', kind: 'claude', onLateCreate: async () => { throw new Error('composer gone') }, + }) + const settled = create.catch((error: unknown) => error) + const request = lastSent('create-agent') as { requestId: string } + await vi.advanceTimersByTimeAsync(30_000) + await settled + bridge.resolve({ requestId: request.requestId, ok: true, type: 'create-agent', agent: child('child-late') } as never) + await vi.advanceTimersByTimeAsync(0) + const failed = incidents.find(incident => incident.kind === 'orchestration.prompt_delivery_failed') + expect(failed?.context).toMatchObject({ sessionId: 'child-late', message: 'composer gone' }) + }) +}) + diff --git a/src/mcp/runtime/createBuiltInMcpServer.ts b/src/mcp/runtime/createBuiltInMcpServer.ts index 9cd4fb5d8..55c256651 100644 --- a/src/mcp/runtime/createBuiltInMcpServer.ts +++ b/src/mcp/runtime/createBuiltInMcpServer.ts @@ -1017,25 +1017,14 @@ function orchestrationCreateAgentCallKey( return await bridge.createAgentCallOnce( orchestrationCreateAgentCallKey(scope.sessionId, args), async () => { - const agent = await bridge.createAgent({ - parentSessionId: scope.sessionId, - kind: args.kind as OrchestrationAgentKind, - ...(args.providerRuntime ? { providerRuntime: args.providerRuntime } : {}), - cwd: args.cwd, - title: args.title, - role: args.role, - runId: args.runId, - // WHY force clean children even if an older tool caller passes true: - // the inheritance implementation is intentionally disabled in this PR. - // Keeping the schema field avoids breaking stale provider tool caches, - // but honoring it would re-enable the broken clone/translate path. - inheritParentContext: false, - builtInMcpDomains: args.builtInMcpDomains as BuiltInMcpDomain[] | undefined, - }) - - if (args.prompt && args.prompt.trim().length > 0) { + // The bootstrap delivery, shared by a punctual create and a late-adopted one (#1370): a create + // that outlived the 30 s bridge deadline is adopted when the renderer finally answers, and + // gets exactly this delivery then, through `onLateCreate`. For the late path the returned + // tool result has no reader; every failure branch below also records an incident, which is + // where a late outcome is seen. + const deliverBootstrap = async (agent: OrchestrationAgentRecord, task: string) => { const prompt = buildOrchestrationBootstrapPrompt({ - task: args.prompt, + task, }) const delivery = await manager.deliverPromptToAgent(agent.sessionId, prompt) // A child that is not ready YET is not a failed child (#854). @@ -1182,6 +1171,27 @@ function orchestrationCreateAgentCallKey( }) } } + const agent = await bridge.createAgent({ + parentSessionId: scope.sessionId, + kind: args.kind as OrchestrationAgentKind, + ...(args.providerRuntime ? { providerRuntime: args.providerRuntime } : {}), + cwd: args.cwd, + title: args.title, + role: args.role, + runId: args.runId, + // WHY force clean children even if an older tool caller passes true: + // the inheritance implementation is intentionally disabled in this PR. + // Keeping the schema field avoids breaking stale provider tool caches, + // but honoring it would re-enable the broken clone/translate path. + inheritParentContext: false, + builtInMcpDomains: args.builtInMcpDomains as BuiltInMcpDomain[] | undefined, + ...(args.prompt && args.prompt.trim().length > 0 + ? { onLateCreate: async (late: OrchestrationAgentRecord) => { await deliverBootstrap(late, args.prompt!) } } + : {}), + }) + + const task = args.prompt && args.prompt.trim().length > 0 ? args.prompt : undefined + if (task) return await deliverBootstrap(agent, task) return toolText({ ok: true, diff --git a/src/mcp/runtime/orchestrationLateBootstrap.system.test.ts b/src/mcp/runtime/orchestrationLateBootstrap.system.test.ts new file mode 100644 index 000000000..c0c99c881 --- /dev/null +++ b/src/mcp/runtime/orchestrationLateBootstrap.system.test.ts @@ -0,0 +1,107 @@ +import { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +// --------------------------------------------------------------------------- +// #1370. A healthy provider can take longer than the bridge's 30 s create deadline to start (the +// bundled OpenCode took 43.1 s under CPU contention, #1367 review a). The caller got "outcome +// unknown — do not repeat", the tool handler returned without delivering the prompt, and when the +// renderer finally answered, the child was adopted idle: no brief, and nothing told the parent. +// +// This drives the REAL MCP server, tool handler and OrchestrationBridge over an in-memory +// transport. The true edges are faked: `sendToWindow` (the renderer, which here answers the create +// only when the test says so) and the session manager (no provider process). +// --------------------------------------------------------------------------- + +const renderer = { + requests: [] as Array>, + heldCreate: null as null | Record, +} + +vi.mock('@main/window/windowRegistry.js', () => ({ + windowForSession: () => 'test-window', + sendToWindow: (_windowId: string, _channel: string, request: Record) => { + renderer.requests.push(request) + if (request.type === 'create-agent') { + // The slow provider start: held until the test answers it, after the deadline. + renderer.heldCreate = request + return true + } + queueMicrotask(() => { + if (request.type === 'mark-bootstrap-prompt-delivered') { + bridge.resolve({ + requestId: request.requestId as string, ok: true, type: 'mark-bootstrap-prompt-delivered', + agent: agent(request.sessionId as string, true), + } as never) + return + } + bridge.resolve({ requestId: request.requestId as string, ok: true, type: request.type, agents: [] } as never) + }) + return true + }, +})) + +const { OrchestrationBridge } = await import('@main/orchestration/OrchestrationBridge.js') +const { createBuiltInMcpServer } = await import('@mcp/runtime/createBuiltInMcpServer.js') + +let bridge: InstanceType + +function agent(sessionId: string, bootstrapped = false) { + return { + sessionId, kind: 'claude', cwd: '/tmp/project', + orchestrationParentId: 'parent-1', orchestrationRootId: 'parent-1', + ...(bootstrapped ? { orchestrationBootstrapPromptDelivered: true } : {}), + } +} + +beforeEach(() => { + renderer.requests.length = 0 + renderer.heldCreate = null + bridge = new OrchestrationBridge() + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) +}) +afterEach(() => { vi.useRealTimers() }) + +describe('create_agent whose child starts after the 30 s deadline (#1370)', () => { + it('delivers the bootstrap prompt to the late child, and tells the caller not to send it again', async () => { + const deliverPromptToAgent = vi.fn(async () => ({ ok: true })) + const sessionManager = { + deliverPromptToAgent, + canWaitForPromptReadiness: vi.fn(() => true), + deliverPromptWhenReady: vi.fn(async () => ({ ok: true })), + getSessionKind: vi.fn(() => 'claude'), + } + const server = createBuiltInMcpServer( + { sessionId: 'parent-1', cwd: '/tmp/project', domains: ['orchestration'] }, + { orchestrationBridge: bridge as never, sessionManager: sessionManager as never }, + ) + const client = new Client({ name: 'late-bootstrap-test', version: '0.0.0' }) + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() + try { + await server.connect(serverTransport) + await client.connect(clientTransport) + const call = client.callTool({ name: 'orchestration_create_agent', arguments: { kind: 'claude', prompt: 'review the PR' } }) + await vi.waitFor(() => expect(renderer.heldCreate).not.toBeNull()) + await vi.advanceTimersByTimeAsync(30_000) + const reply = await call + const text = ((reply.content as Array<{ text: string }>)[0]!).text + expect(text).toMatch(/UNKNOWN/) + expect(text).toMatch(/delivered to it automatically: do not send it again/) + expect(deliverPromptToAgent).not.toHaveBeenCalled() + + // The provider finally starts and the renderer answers. + bridge.resolve({ + requestId: renderer.heldCreate!.requestId as string, ok: true, type: 'create-agent', agent: agent('child-late'), + } as never) + await vi.waitFor(() => expect(deliverPromptToAgent).toHaveBeenCalledTimes(1)) + const [sessionId, prompt] = deliverPromptToAgent.mock.calls[0] as unknown as [string, string] + expect(sessionId).toBe('child-late') + expect(prompt).toContain('review the PR') + // And it is recorded as the child's bootstrap, exactly as a punctual create would record it. + await vi.waitFor(() => expect(renderer.requests.map(request => request.type)).toContain('mark-bootstrap-prompt-delivered')) + } finally { + await client.close() + await server.close() + } + }) +}) From 7f95c5cc2b48b34348d5ae993f5a5553d921d393 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 00:21:55 -0700 Subject: [PATCH 07/43] fix(merge): restore main's codex-headless pointer The hand-resolved main merges (1937b5c7, ff32bce5) staged the worktree's stale codex-headless checkout (6334d5ac) over main's d42cc1da, so CI read fixtures that only exist in the newer package (composer-0157). No change to this PR's own diff. Co-Authored-By: Claude Opus 5.5 --- packages/codex-headless | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/codex-headless b/packages/codex-headless index 6334d5ac0..d42cc1da5 160000 --- a/packages/codex-headless +++ b/packages/codex-headless @@ -1 +1 @@ -Subproject commit 6334d5ac06435720e9b4f2a85df4102cecdb54fc +Subproject commit d42cc1da589138c0cbd1d757abe715b98a022936 From e558a14411edd7a60c255cd74ac6b8beadf1807f Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 00:32:07 -0700 Subject: [PATCH 08/43] docs(provider-switch): plan carrying the native projection report (#927) Co-Authored-By: Claude Opus 5.5 --- ...-27-provider-switch-projection-fidelity.md | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 docs/plans/2026-09-27-provider-switch-projection-fidelity.md diff --git a/docs/plans/2026-09-27-provider-switch-projection-fidelity.md b/docs/plans/2026-09-27-provider-switch-projection-fidelity.md new file mode 100644 index 000000000..27a0103c1 --- /dev/null +++ b/docs/plans/2026-09-27-provider-switch-projection-fidelity.md @@ -0,0 +1,55 @@ +# Carry the native projection report through switch, duplicate and rewind (#927) + +Short plan: bug with a known root cause. Part of #918's B06; see `docs/superpowers/plans/2026-09-12-session-operation-ownership.md` §6.2. + +## Outcome +`switchProvider`, `duplicateSession` and `rewindSession` each call a real native-resume projector. Each writes `projection.values` and throws `projection.report` away, so nobody can see what the projector preserved, dropped, demoted, repaired or synthesized. A switch returns only `shrinkSummary`, which describes context REDUCTION (the fit ladder) and is null whenever nothing had to be shrunk, even when the projection itself lost content. + +After this change, all three results carry a bounded, curated `projectionFidelity` summary of the real report, through IPC to the renderer. A switch or duplicate that loses material content says so in its existing pane toast, even when the context reduction reports no shrink. + +## Evidence (verified 2026-09-27 against the recorded Stage 0 sequences; do not re-derive) +All 13 `packages/agent-transcript-parser/fixtures/evidence/observed-sequences/*` cases were projected through the REAL projectors (Claude 2.1.0 / Codex 0.157.1 profiles): +- **Every Codex case loses something in both directions,** including Codex → Codex, which is what a duplicate or rewind does. + - The codes seen: `opaque.dropped` 1–30, and to Claude also `message.developer.dropped`, `reasoning.foreign-dropped` and `tool-call.input-object-repaired`. +- **Claude → Codex:** `reasoning.encrypted-content-demoted` (19–485), `tool-result.error-status-demoted`, `compaction.foreign-summary-demoted` and `content.image.dropped`. +- **Claude → Claude:** mostly lossless; `claude-sequence-oversized-turns` drops 110 opaque records and repairs 4 tool results. +- **Framing** (`session-meta.synthesized`, `turn-framing.synthesized`) appears on every Codex target. It is identity and framing, not user-visible loss. +- `ProjectionChange.message` is projector prose; `code` is a closed vocabulary (`native-resume..`). `evidence` carries source claims. + +## Design (contract) +- **`src/shared/types/projectionFidelity.ts`:** + - `NativeProjectionFidelity = { profile: 'native-resume', sourceProvider, targetProvider, providerProfileId, providerEvidence: { sourceCommit?, cliVersion?, observedAt? }, counts: Record, codes: Array<{ kind, code, count, sourceLines: number[] }>, sourceLinesOmitted: number }`. + - One `codes` row per distinct (kind, code). `sourceLines` is capped at 20 per row, and the rest is COUNTED in `sourceLinesOmitted`, never silently dropped. + - **Excluded on purpose:** `message` and `evidence`. The summary crosses IPC and reaches the UI, where only curated text may appear (q22/q39); the code identifies each change without projector prose or source content. +- **`src/main/providerSwitch/projectionFidelity.ts`:** `summarizeProjectionReport(result: NativeResumeProjectionResult): NativeProjectionFidelity`. It is pure and is called right after each `projectNativeResume`, before `write`, so the summary exists even if publication fails later. (Receipts are B07; nothing here pretends to persist one.) +- **Results:** `SwitchProviderResult` (the `switched` arm), `DuplicateSessionResult` and `RewindSessionResult` gain `projectionFidelity: NativeProjectionFidelity`. The preload types use the shared type. +- **`materialProjectionLoss(f): string | null`**, one line for toasts, or null. + - **UNCONFIRMED default:** material means `dropped` changes other than `native-resume.opaque.dropped`, plus every `demoted` change. + - Opaque records, repairs and synthesized framing are not disclosed in the toast; they remain inspectable in the result. + - **Format, UNCONFIRMED wording:** `history: 12 dropped, 19 demoted`. +- **Renderer:** + - The switch pane toast (`workspace/hook/actions/provider.ts`) appends the line and uses the longer lossy duration when it is present. + - The bulk switch summary (`bulkProviderSwitch.ts`) adds it to its notes. + - The duplicate command's toast (`sessionCommands.ts`) appends it too. + - Rewind carries the field for inspection. Its own toast lives in `pane.ts`, which open PR #1369 (another worker) edits, so it is not touched here; see Out of scope. + +## Tests +- **Real reports, real projectors:** `switchProvider`, `duplicateSession` and `rewindSession` tests use a recorded sequence through the REAL transcript engine projector. + - Only disk writes and lookups stay mocked, because a projection mock returning only `values` cannot prove this contract (B06 exit gate). + - Each asserts the result's `projectionFidelity` equals `summarizeProjectionReport` of that real projection, with non-zero loss. + - The switch case has a null `shrinkSummary`, so loss is visible without context reduction. + - All red on main: the field is absent. +- **`summarizeProjectionReport` on a real report:** grouping, the 20-line cap with the omitted count, and no `message`/`evidence` fields in the output. +- **`materialProjectionLoss`** on recorded counts: Codex → Claude and Claude → Codex are material; `codex-sequence-compaction` → Codex (opaque only) is null. +- **Renderer:** + - the switch toast includes the line for a material fidelity result; + - the bulk notes include it; + - the duplicate toast includes it. + +## Verification boundary +Main-process tests drive the real entry points with real projectors; the app is not launched. Whether the toast wording reads well is the owner's call (UNCONFIRMED). + +## Out of scope +- **B07 receipts / operation records:** no receipt store exists yet. The summary is on the result, which is what the client can inspect today. +- **The rewind toast in `pane.ts`:** open PR #1369 edits that file. The rewind result carries the field for a follow-up. +- **Draft-restoration reports** (#929) and literal-markup provenance (#930). From 68c433941968385f74c71bfdee58e8e35c6d49f5 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 01:17:17 -0700 Subject: [PATCH 09/43] fix(provider-switch): carry the native projection report through switch, duplicate and rewind (#927) All three operations ran a real native-resume projector and threw its report away, so a projection that dropped or demoted content was invisible whenever the context reduction reported no shrink. Each result now carries a bounded, curated NativeProjectionFidelity summary (counts, per-code counts, capped source lines with an exact omitted count; no projector prose or evidence), through IPC to the renderer. The switch toast, the bulk summary and the duplicate command name material loss (UNCONFIRMED default: non-opaque drops and demotions). Co-Authored-By: Claude Opus 5.5 --- .../providerSwitch/duplicateSession.test.ts | 9 +- src/main/providerSwitch/duplicateSession.ts | 8 +- .../projectionFidelity.system.test.ts | 151 ++++++++++++++++++ src/main/providerSwitch/projectionFidelity.ts | 50 ++++++ src/main/providerSwitch/rewindSession.test.ts | 3 +- src/main/providerSwitch/rewindSession.ts | 6 + .../providerSwitch/switchProvider.test.ts | 2 + src/main/providerSwitch/switchProvider.ts | 10 ++ .../testing/losslessProjection.ts | 27 ++++ src/preload/api/provider.ts | 8 + .../tldr/sessionContinuity.renderer.test.tsx | 2 +- .../commands/sessionCommands.renderer.test.ts | 28 ++++ .../workspace/commands/sessionCommands.ts | 14 +- .../control/lifecycle.renderer.test.tsx | 2 +- .../bulkProviderSwitch.renderer.test.ts | 14 ++ .../hook/actions/bulkProviderSwitch.ts | 7 + .../provider.projectionLoss.renderer.test.tsx | 68 ++++++++ .../src/workspace/hook/actions/provider.ts | 12 +- .../providerSwitchCore.renderer.test.ts | 2 + .../hook/actions/providerSwitchCore.ts | 7 +- .../testing/recordedProjectionFidelity.ts | 45 ++++++ src/shared/types/projectionFidelity.ts | 83 ++++++++++ 22 files changed, 546 insertions(+), 12 deletions(-) create mode 100644 src/main/providerSwitch/projectionFidelity.system.test.ts create mode 100644 src/main/providerSwitch/projectionFidelity.ts create mode 100644 src/main/providerSwitch/testing/losslessProjection.ts create mode 100644 src/renderer/src/workspace/hook/actions/provider.projectionLoss.renderer.test.tsx create mode 100644 src/renderer/src/workspace/hook/actions/testing/recordedProjectionFidelity.ts create mode 100644 src/shared/types/projectionFidelity.ts diff --git a/src/main/providerSwitch/duplicateSession.test.ts b/src/main/providerSwitch/duplicateSession.test.ts index 82ae59d1f..008744b24 100644 --- a/src/main/providerSwitch/duplicateSession.test.ts +++ b/src/main/providerSwitch/duplicateSession.test.ts @@ -28,6 +28,8 @@ vi.mock('@main/providerSwitch/transcriptEngine.js', () => ({ import { duplicateSession } from './duplicateSession.js' import { projectGrokNativeResume } from 'agent-transcript-parser' +import { losslessProjection } from './testing/losslessProjection.js' +import { summarizeProjectionReport } from './projectionFidelity.js' const conversation = { schemaVersion: 1 as const, @@ -46,7 +48,7 @@ describe('duplicateSession neutral integration', () => { beforeEach(() => { vi.clearAllMocks() mocks.read.mockResolvedValue(conversation) - mocks.project.mockResolvedValue({ values: [{ sessionId: 'new-session' }] }) + mocks.project.mockResolvedValue(losslessProjection([{ sessionId: 'new-session' }])) mocks.sessionId.mockReturnValue('new-session') mocks.write.mockResolvedValue('/target/new-session.jsonl') }) @@ -68,11 +70,12 @@ describe('duplicateSession neutral integration', () => { targetSessionId: '00000000-0000-4000-8000-000000000098', }), ) - expect(mocks.write).toHaveBeenCalledWith('/target', { values: [{ sessionId: 'new-session' }] }) + expect(mocks.write).toHaveBeenCalledWith('/target', losslessProjection([{ sessionId: 'new-session' }])) expect(result).toEqual({ provider: 'claude', newProviderSessionId: 'new-session', newFilePath: '/target/new-session.jsonl', + projectionFidelity: summarizeProjectionReport(losslessProjection([{ sessionId: 'new-session' }])), }) }) @@ -128,7 +131,7 @@ describe('duplicate keeps the source conversation\'s own model', () => { // Call history from the suites above would otherwise be inspected here. for (const mock of Object.values(mocks)) mock.mockReset() mocks.read.mockResolvedValue({ entries: [{ kind: 'message', role: 'user' }], sourceSessionIds: ['ses_source'] }) - mocks.project.mockResolvedValue({ values: [{}] }) + mocks.project.mockResolvedValue(losslessProjection([{}])) mocks.write.mockResolvedValue('/fixture/new.json') mocks.sessionId.mockReturnValue('ses_new') }) diff --git a/src/main/providerSwitch/duplicateSession.ts b/src/main/providerSwitch/duplicateSession.ts index 295442c21..0fb2e9df9 100644 --- a/src/main/providerSwitch/duplicateSession.ts +++ b/src/main/providerSwitch/duplicateSession.ts @@ -1,8 +1,10 @@ import { randomUUID } from 'node:crypto' import type { AgentProviderKind } from '@shared/types/providerKind.js' +import type { NativeProjectionFidelity } from '@shared/types/projectionFidelity.js' import { getHostTranscriptAdapter } from '@main/providerSwitch/transcriptEngine.js' +import { summarizeProjectionReport } from '@main/providerSwitch/projectionFidelity.js' export type DuplicateSessionRequest = { provider: AgentProviderKind @@ -16,6 +18,9 @@ export type DuplicateSessionResult = { provider: AgentProviderKind newProviderSessionId: string newFilePath: string + /** What the projector did to the copy (#927). A same-provider duplicate + * is not automatically lossless: Codex -> Codex drops opaque records. */ + projectionFidelity: NativeProjectionFidelity } export async function duplicateSession( @@ -64,7 +69,8 @@ export async function duplicateSession( now: new Date().toISOString(), ...(sourceProfile ? { targetProfile: sourceProfile } : {}), }) + const projectionFidelity = summarizeProjectionReport(projection) const newProviderSessionId = adapter.sessionId(projection) const newFilePath = await adapter.write(targetCwd, projection) - return { provider: request.provider, newProviderSessionId, newFilePath } + return { provider: request.provider, newProviderSessionId, newFilePath, projectionFidelity } } diff --git a/src/main/providerSwitch/projectionFidelity.system.test.ts b/src/main/providerSwitch/projectionFidelity.system.test.ts new file mode 100644 index 000000000..4563d618a --- /dev/null +++ b/src/main/providerSwitch/projectionFidelity.system.test.ts @@ -0,0 +1,151 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + claudeNativeResumeProjector, + codexNativeResumeProjector, + estimateConversationCharacters, +} from 'agent-transcript-parser' +import type { ConversationDocument } from 'agent-transcript-parser' + +// #927: switch, duplicate and rewind must hand back what the REAL +// native-resume projector reported. The #918 B06 exit gate says a mock +// returning only `values` cannot establish this contract. So here the +// projector is the parser's own, run over recorded Stage 0 sequences, and only +// the disk edges (read, write, session id, target profile) are stubbed. + +const state = vi.hoisted(() => ({ + conversation: null as ConversationDocument | null, + written: [] as unknown[], +})) + +vi.mock('node:crypto', () => ({ randomUUID: () => '00000000-0000-4000-8000-000000000927' })) + +vi.mock('@main/providerSwitch/transcriptEngine.js', () => ({ + getHostTranscriptAdapter(provider: string) { + if (provider !== 'claude' && provider !== 'codex') throw new Error(`unexpected provider ${provider}`) + return { + provider, + read: async () => state.conversation, + targetProfile: async () => ({ + model: provider === 'claude' ? 'claude-sonnet-5' : 'gpt-5', + modelProvider: 'openai', + // Derived from the fixture, never a real-world budget (the fixtures + // are value-redacted; see testing/fixtureConversations.ts). + budgetCharacters: estimateConversationCharacters(state.conversation!) * 10, + }), + projectNativeResume: async (conversation: ConversationDocument, context: { cwd: string; targetSessionId: string; now: string }) => ( + provider === 'claude' + ? claudeNativeResumeProjector.projectNativeResume(conversation, { ...context, version: '2.1.0', model: 'claude-sonnet-5' }) + : codexNativeResumeProjector.projectNativeResume(conversation, { ...context, cliVersion: '0.157.1', modelProvider: 'openai', model: 'gpt-5' }) + ), + sessionId: () => 'new-session', + write: async (_cwd: string, publication: unknown) => { + state.written.push(publication) + return '/recorded/new-session.jsonl' + }, + draft: (content: unknown) => ({ promptText: JSON.stringify(content).slice(0, 20), promptMode: 'prompt', promptImages: [], promptAttachments: [] }), + } + }, +})) + +import { materialProjectionLoss } from '@shared/types/projectionFidelity.js' +import { duplicateSession } from './duplicateSession.js' +import { summarizeProjectionReport } from './projectionFidelity.js' +import { rewindSession } from './rewindSession.js' +import { switchProvider } from './switchProvider.js' +import { loadFixtureConversation } from './testing/fixtureConversations.js' + +beforeEach(() => { + state.written = [] +}) + +// The fidelity a caller receives must be the summary of the projection that +// was actually WRITTEN, not of some other run. +function writtenFidelity() { + expect(state.written).toHaveLength(1) + return summarizeProjectionReport(state.written[0] as Parameters[0]) +} + +describe('projection fidelity through each real caller (#927)', () => { + // Recorded: claude-sequence-oversized -> Codex has no compaction, so the + // context reduction reports nothing (`shrinkSummary` null, strategy + // native), while the projector demotes 19 entries (encrypted reasoning, + // tool-result error status). Before #927 this switch toasted as lossless. + it('switch returns the real report even when context reduction reports nothing', async () => { + state.conversation = await loadFixtureConversation('claude-sequence-oversized', 'claude') + const result = await switchProvider({ sourceKind: 'claude', targetKind: 'codex', sourceProviderSessionId: 'source', cwd: '/recorded' }) + expect(result.kind).toBe('switched') + if (result.kind !== 'switched') return + expect(result.strategy).toBe('native') + expect(result.shrinkSummary).toBeNull() + expect(result.projectionFidelity).toEqual(writtenFidelity()) + expect(result.projectionFidelity.codes.map(row => row.code)).toEqual(expect.arrayContaining([ + 'native-resume.reasoning.encrypted-content-demoted', + 'native-resume.tool-result.error-status-demoted', + ])) + expect(materialProjectionLoss(result.projectionFidelity)).toBe('history: 19 demoted') + }) + + // Recorded: codex-sequence-compacted-once -> Claude drops developer messages + // and foreign reasoning: material drops. + it('switch names material drops in its loss line', async () => { + state.conversation = await loadFixtureConversation('codex-sequence-compacted-once', 'codex') + const result = await switchProvider({ sourceKind: 'codex', targetKind: 'claude', sourceProviderSessionId: 'source', cwd: '/recorded' }) + if (result.kind !== 'switched') throw new Error('expected a switch') + expect(result.projectionFidelity).toEqual(writtenFidelity()) + expect(result.projectionFidelity.codes.map(row => row.code)).toEqual(expect.arrayContaining([ + 'native-resume.message.developer.dropped', + 'native-resume.reasoning.foreign-dropped', + ])) + expect(materialProjectionLoss(result.projectionFidelity)).toMatch(/^history: \d+ dropped$/) + }) + + // A same-provider duplicate is not automatically lossless: Codex -> Codex + // drops the rollout's opaque records. That loss is reported, but it is not + // MATERIAL, so the toast stays quiet (UNCONFIRMED default). + it('duplicate returns the report of a same-provider copy', async () => { + state.conversation = await loadFixtureConversation('codex-sequence-compaction', 'codex') + const result = await duplicateSession({ provider: 'codex', sourceProviderSessionId: 'source', cwd: '/recorded' }) + expect(result.projectionFidelity).toEqual(writtenFidelity()) + expect(result.projectionFidelity.codes).toEqual(expect.arrayContaining([ + expect.objectContaining({ kind: 'dropped', code: 'native-resume.opaque.dropped' }), + ])) + expect(materialProjectionLoss(result.projectionFidelity)).toBeNull() + }) + + it('rewind returns the report of the rewound copy', async () => { + const conversation = await loadFixtureConversation('codex-sequence-compacted-once', 'codex') + state.conversation = conversation + const prompts = conversation.entries.filter(entry => entry.kind === 'message' && entry.role === 'user') + const anchor = prompts[prompts.length - 1]! + const result = await rewindSession({ + provider: 'codex', + sourceProviderSessionId: 'source', + cwd: '/recorded', + anchor: { provider: 'codex', line: anchor.source.line!, sessionId: conversation.sourceSessionIds[0]! }, + }) + expect(result.projectionFidelity).toEqual(writtenFidelity()) + expect(result.projectionFidelity.counts.preserved).toBeGreaterThan(0) + }) +}) + +describe('summarizeProjectionReport on a real report', () => { + // Recorded: claude-sequence-oversized-turns -> Codex, 485 demoted encrypted + // reasoning blocks: enough changes of one code to exercise the line cap. + it('groups by code, caps listed lines with an exact omitted count, and carries no prose or evidence', async () => { + const conversation = await loadFixtureConversation('claude-sequence-oversized-turns', 'claude') + const projection = codexNativeResumeProjector.projectNativeResume(conversation, { + cwd: '/recorded', targetSessionId: '00000000-0000-4000-8000-000000000927', now: '2026-09-27T00:00:00.000Z', + cliVersion: '0.157.1', modelProvider: 'openai', model: 'gpt-5', + }) + const fidelity = summarizeProjectionReport(projection) + expect(fidelity.codes.reduce((sum, row) => sum + row.count, 0)).toBe(projection.report.changes.length) + expect(fidelity.counts).toEqual(projection.report.counts) + for (const row of fidelity.codes) expect(row.sourceLines.length).toBeLessThanOrEqual(20) + const listed = fidelity.codes.reduce((sum, row) => sum + row.sourceLines.length, 0) + const withLines = projection.report.changes.filter(change => change.sourceLine !== null).length + expect(listed + fidelity.sourceLinesOmitted).toBe(withLines) + expect(fidelity.sourceLinesOmitted).toBeGreaterThan(0) + expect(JSON.stringify(fidelity)).not.toMatch(/"message"|"evidence"/) + expect(materialProjectionLoss(fidelity)).toMatch(/demoted/) + }) +}) diff --git a/src/main/providerSwitch/projectionFidelity.ts b/src/main/providerSwitch/projectionFidelity.ts new file mode 100644 index 000000000..48edd0e64 --- /dev/null +++ b/src/main/providerSwitch/projectionFidelity.ts @@ -0,0 +1,50 @@ +import type { NativeResumeProjectionResult } from 'agent-transcript-parser' + +import { + PROJECTION_FIDELITY_MAX_LINES, + type NativeProjectionFidelity, + type ProjectionFidelityCode, +} from '@shared/types/projectionFidelity.js' + +/** + * The bounded, curated summary of a real native-resume projection (#927). + * + * Called right after `projectNativeResume` and before `write`, so every + * caller holds the summary even when publication fails afterwards. Pure: it + * reads the report and copies what it keeps, so a later reuse of the + * projector's objects cannot change a summary already handed out (#918 §6.2). + * See `@shared/types/projectionFidelity` for why messages and evidence are + * left out. + */ +export function summarizeProjectionReport(projection: NativeResumeProjectionResult): NativeProjectionFidelity { + const { report, providerProfile } = projection + const rows = new Map() + let sourceLinesOmitted = 0 + for (const change of report.changes) { + const key = `${change.kind}\u0000${change.code}` + let row = rows.get(key) + if (!row) { + row = { kind: change.kind, code: change.code, count: 0, sourceLines: [] } + rows.set(key, row) + } + row.count += 1 + if (change.sourceLine === null) continue + if (row.sourceLines.length < PROJECTION_FIDELITY_MAX_LINES) row.sourceLines.push(change.sourceLine) + else sourceLinesOmitted += 1 + } + const evidence = providerProfile.evidence + return { + profile: 'native-resume', + sourceProvider: report.sourceProvider, + targetProvider: report.targetProvider, + providerProfileId: providerProfile.id, + providerEvidence: { + ...(evidence.sourceCommit ? { sourceCommit: evidence.sourceCommit } : {}), + ...(evidence.cliVersion ? { cliVersion: evidence.cliVersion } : {}), + ...(evidence.observedAt ? { observedAt: evidence.observedAt } : {}), + }, + counts: { ...report.counts }, + codes: [...rows.values()], + sourceLinesOmitted, + } +} diff --git a/src/main/providerSwitch/rewindSession.test.ts b/src/main/providerSwitch/rewindSession.test.ts index 1369357ce..f0b1d7807 100644 --- a/src/main/providerSwitch/rewindSession.test.ts +++ b/src/main/providerSwitch/rewindSession.test.ts @@ -30,6 +30,7 @@ vi.mock('@main/providerSwitch/transcriptEngine.js', () => ({ import { listRewindPrompts, rewindSession } from './rewindSession.js' import { projectGrokNativeResume } from 'agent-transcript-parser' +import { losslessProjection } from './testing/losslessProjection.js' const sourceConversation = { schemaVersion: 1 as const, @@ -70,7 +71,7 @@ describe('rewindSession neutral integration', () => { promptImages: [], promptAttachments: [], }) - mocks.project.mockResolvedValue({ values: [{ type: 'session_meta' }] }) + mocks.project.mockResolvedValue(losslessProjection([{ type: 'session_meta' }])) mocks.sessionId.mockReturnValue('new-session') mocks.write.mockResolvedValue('/target/rollout.jsonl') }) diff --git a/src/main/providerSwitch/rewindSession.ts b/src/main/providerSwitch/rewindSession.ts index be70c07ed..aa8ac5999 100644 --- a/src/main/providerSwitch/rewindSession.ts +++ b/src/main/providerSwitch/rewindSession.ts @@ -2,6 +2,7 @@ import { randomUUID } from 'node:crypto' import type { RewindAttachment } from '@main/providerSwitch/rewindAttachments.js' import type { AgentProviderKind } from '@shared/types/providerKind.js' +import type { NativeProjectionFidelity } from '@shared/types/projectionFidelity.js' import type { ListRewindPromptsRequest, RewindPrompt, @@ -10,6 +11,7 @@ import type { import { rewindConversation } from 'agent-transcript-parser' import { getHostTranscriptAdapter } from '@main/providerSwitch/transcriptEngine.js' +import { summarizeProjectionReport } from '@main/providerSwitch/projectionFidelity.js' export type RewindSessionRequest = { provider: AgentProviderKind @@ -49,6 +51,8 @@ export type RewindSessionResult = { /** Every attachment the rewound prompt had, bytes excluded — see above. */ promptAttachments: RewindSessionAttachment[] promptTimestamp: string | null + /** What the projector did to the rewound copy (#927). */ + projectionFidelity: NativeProjectionFidelity } /** @@ -126,6 +130,7 @@ export async function rewindSession( now: new Date().toISOString(), ...(sourceProfile ? { targetProfile: sourceProfile } : {}), }) + const projectionFidelity = summarizeProjectionReport(projection) const newProviderSessionId = adapter.sessionId(projection) const draft = adapter.draft(rewind.draft) @@ -152,5 +157,6 @@ export async function rewindSession( entry.source.line === rewind.anchor.line ))?.timestamp ?? null : null, + projectionFidelity, } } diff --git a/src/main/providerSwitch/switchProvider.test.ts b/src/main/providerSwitch/switchProvider.test.ts index 697c8bd1e..9fbd9c7ae 100644 --- a/src/main/providerSwitch/switchProvider.test.ts +++ b/src/main/providerSwitch/switchProvider.test.ts @@ -47,6 +47,7 @@ vi.mock('@main/providerSwitch/transcriptEngine.js', () => ({ import { describeShrink, switchProvider } from './switchProvider.js' import type { ShrinkReport } from 'agent-transcript-parser' import { loadFixtureConversation } from './testing/fixtureConversations.js' +import { summarizeProjectionReport } from './projectionFidelity.js' const conversation = { schemaVersion: 1 as const, @@ -141,6 +142,7 @@ describe('switchProvider neutral hub integration', () => { truncatedBeforeSwitch: false, strategy: 'native', shrinkSummary: null, + projectionFidelity: summarizeProjectionReport(projection), }) }) diff --git a/src/main/providerSwitch/switchProvider.ts b/src/main/providerSwitch/switchProvider.ts index b79dc3e8b..9025211ef 100644 --- a/src/main/providerSwitch/switchProvider.ts +++ b/src/main/providerSwitch/switchProvider.ts @@ -3,6 +3,7 @@ import { randomUUID } from 'node:crypto' import type { AgentProviderKind } from '@shared/types/providerKind.js' +import type { NativeProjectionFidelity } from '@shared/types/projectionFidelity.js' import { compactionAvailability, conversationAfterLatestPortableCompaction, @@ -16,6 +17,7 @@ import type { } from 'agent-transcript-parser' import { getHostTranscriptAdapter } from '@main/providerSwitch/transcriptEngine.js' +import { summarizeProjectionReport } from '@main/providerSwitch/projectionFidelity.js' export type SwitchProviderRequest = { sourceKind: AgentProviderKind @@ -108,6 +110,10 @@ export type SwitchProviderResult = * the host is the only layer that can put the loss in front of a user. */ shrinkSummary: string | null + /** What the native-resume projector preserved, dropped, demoted, + * repaired or synthesized (#927). Independent of `shrinkSummary`: a + * switch that needed no shrinking can still be a lossy projection. */ + projectionFidelity: NativeProjectionFidelity } | { kind: 'source-empty' @@ -364,6 +370,9 @@ export async function switchProvider( now: new Date().toISOString(), targetProfile, }) + // Summarized BEFORE the write, so the evidence exists even if publication + // fails (#918 §6.2); the report is otherwise discarded with `projection`. + const projectionFidelity = summarizeProjectionReport(projection) const targetProviderSessionId = target.sessionId(projection) const targetFilePath = await target.write(targetCwd, projection) @@ -376,6 +385,7 @@ export async function switchProvider( truncatedBeforeSwitch, strategy, shrinkSummary, + projectionFidelity, } } diff --git a/src/main/providerSwitch/testing/losslessProjection.ts b/src/main/providerSwitch/testing/losslessProjection.ts new file mode 100644 index 000000000..62bc33e0a --- /dev/null +++ b/src/main/providerSwitch/testing/losslessProjection.ts @@ -0,0 +1,27 @@ +import type { NativeResumeProjectionResult } from 'agent-transcript-parser' + +// A native-resume projection with an EMPTY report, for host tests whose +// subject is not fidelity (identity, cwd, profile, write order). +// +// WHY a helper and not `{ values }`: since #927 every operation summarizes +// `projection.report`, and a projection without one is a shape no real +// projector returns. Tests that DO assert fidelity must not use this: they run +// the real projector over a recorded sequence (projectionFidelity.system.test). +export function losslessProjection( + values: Record[], + provider = 'codex', +): NativeResumeProjectionResult { + return { + profile: 'native-resume', + targetProvider: provider, + providerProfile: { id: 'test', provider, evidence: {} }, + values, + report: { + profile: 'native-resume', + sourceProvider: provider, + targetProvider: provider, + changes: [], + counts: { preserved: 0, dropped: 0, demoted: 0, synthesized: 0, repaired: 0, retargeted: 0, opaque: 0 }, + }, + } +} diff --git a/src/preload/api/provider.ts b/src/preload/api/provider.ts index 71af48ba1..5dba1ae39 100644 --- a/src/preload/api/provider.ts +++ b/src/preload/api/provider.ts @@ -1,4 +1,5 @@ import type { AgentProviderKind } from '@shared/types/providerKind.js' +import type { NativeProjectionFidelity } from '@shared/types/projectionFidelity.js' import type { ListRewindPromptsRequest, RewindPrompt, @@ -67,6 +68,9 @@ export const providerApi = { /** One human-readable line describing what `shrunk` cost, else null. The * renderer shows it per pane and counts strategies in a batch summary. */ shrinkSummary: string | null + /** The native-resume projector's report, summarized (#927). Independent + * of `shrinkSummary`, which only covers context reduction. */ + projectionFidelity: NativeProjectionFidelity } | { /** A provider id can identify a pre-created but still blank session. */ kind: 'source-empty' @@ -128,6 +132,8 @@ export const providerApi = { provider: AgentProviderKind newProviderSessionId: string newFilePath: string + /** What the projector did to the copy (#927). */ + projectionFidelity: NativeProjectionFidelity }> => ipcRenderer.invoke('session:duplicate', params), /** @@ -198,5 +204,7 @@ export const providerApi = { name: string | null }> promptTimestamp: string | null + /** What the projector did to the rewound copy (#927). */ + projectionFidelity: NativeProjectionFidelity }> => ipcRenderer.invoke('session:rewind-to-prompt', params), } diff --git a/src/renderer/src/features/tldr/sessionContinuity.renderer.test.tsx b/src/renderer/src/features/tldr/sessionContinuity.renderer.test.tsx index 237b2a884..7e2693b89 100644 --- a/src/renderer/src/features/tldr/sessionContinuity.renderer.test.tsx +++ b/src/renderer/src/features/tldr/sessionContinuity.renderer.test.tsx @@ -36,7 +36,7 @@ describe('TLDR identity through real session actions', () => { const saved = { text: 'The original work is complete. PR #123 is merged.', revision: 1, updatedAt: '2026-09-11T00:00:00.000Z' } window.api = { ...originalApi, spawnSession, killOwnedSession: vi.fn(async () => true), - rewindToPrompt: vi.fn(async () => ({ provider: 'codex' as const, newProviderSessionId: 'native-rewound', newFilePath: '/recorded/rewound.jsonl', promptText: 'Earlier prompt', promptTimestamp: null, promptMode: 'prompt' as const, promptImages: [], promptAttachments: [] })), + rewindToPrompt: vi.fn(async () => ({ provider: 'codex' as const, newProviderSessionId: 'native-rewound', newFilePath: '/recorded/rewound.jsonl', promptText: 'Earlier prompt', promptTimestamp: null, promptMode: 'prompt' as const, promptImages: [], promptAttachments: [], projectionFidelity: { profile: 'native-resume' as const, sourceProvider: 'codex', targetProvider: 'codex', providerProfileId: 'test', providerEvidence: {}, counts: { preserved: 0, dropped: 0, demoted: 0, synthesized: 0, repaired: 0, retargeted: 0, opaque: 0 }, codes: [], sourceLinesOmitted: 0 } })), stripCodexCyberPolicy: vi.fn(async () => ({ provider: 'codex' as const, newProviderSessionId: 'native-rewound', newFilePath: '/recorded/rewound.jsonl' })), readTldrs: vi.fn(async (ids: string[]): Promise> => ids.includes('summary-source') ? { 'summary-source': saved } : {}), onTldrChanged: () => () => {}, diff --git a/src/renderer/src/features/workspace/commands/sessionCommands.renderer.test.ts b/src/renderer/src/features/workspace/commands/sessionCommands.renderer.test.ts index a7ea8ecc7..6dc2f4f3f 100644 --- a/src/renderer/src/features/workspace/commands/sessionCommands.renderer.test.ts +++ b/src/renderer/src/features/workspace/commands/sessionCommands.renderer.test.ts @@ -4,6 +4,7 @@ import type { CommandContext } from '@renderer/features/command-palette/types' import type { Workspace } from '@renderer/workspace/workspaceStore' import { sessionCommands } from '@renderer/features/workspace/commands/sessionCommands' import { oneLaneStage } from '@renderer/workspace/testing/stageFixtures' +import { DEMOTING_SWITCH_FIDELITY, OPAQUE_ONLY_DUPLICATE_FIDELITY } from '@renderer/workspace/hook/actions/testing/recordedProjectionFidelity' const originalApiDescriptor = Object.getOwnPropertyDescriptor(window, 'api') @@ -76,6 +77,33 @@ describe('Duplicate Agent command', () => { expect(closePalette).toHaveBeenCalledOnce() }) + // #927: a duplicate is re-projected, not byte-copied, and can lose content. + // Material loss is named on the source pane; the recorded opaque-only Codex + // copy stays silent because the placed copy is its own evidence. + it.each([ + ['names material projection loss on the source pane', DEMOTING_SWITCH_FIDELITY, 'Duplicated · history: 19 demoted'], + ['stays silent when only opaque records were dropped', OPAQUE_ONLY_DUPLICATE_FIDELITY, null], + ] as const)('%s', async (_label, projectionFidelity, toast) => { + const duplicateSession = vi.fn().mockResolvedValue({ newProviderSessionId: 'provider-clone', projectionFidelity }) + Object.defineProperty(window, 'api', { configurable: true, value: { duplicateSession } }) + const showPaneToast = vi.fn() + const workspace = { + state: { + activeTabId: 'tab-klay', + stage: oneLaneStage('source'), pinnedSessionIds: [], + sessions: { source: { cwd: '/projects/klay', kind: 'codex', providerSessionId: 'provider-source', projectId: 'tab-klay', joinedAt: 0 } }, + tabs: [{ id: 'tab-klay' }], + }, + splitFocused: vi.fn().mockResolvedValue(undefined), + showPaneToast, + } as unknown as Workspace + const context = { workspace, ui: { closePalette: vi.fn() }, flags: {} } as unknown as CommandContext + const command = sessionCommands.find(candidate => candidate.id === 'duplicate-agent')! + await command.run(context) + if (toast) expect(showPaneToast).toHaveBeenCalledWith('source', toast, 8000) + else expect(showPaneToast).not.toHaveBeenCalled() + }) + it('never hands a clone the root-management grant', async () => { const duplicateSession = vi.fn().mockResolvedValue({ newProviderSessionId: 'provider-clone' }) Object.defineProperty(window, 'api', { configurable: true, value: { duplicateSession } }) diff --git a/src/renderer/src/features/workspace/commands/sessionCommands.ts b/src/renderer/src/features/workspace/commands/sessionCommands.ts index 905804847..fa72a28f3 100644 --- a/src/renderer/src/features/workspace/commands/sessionCommands.ts +++ b/src/renderer/src/features/workspace/commands/sessionCommands.ts @@ -27,6 +27,7 @@ import { ROOT_MANAGEMENT_DOMAIN, rootManagementReloadLabels, } from '@renderer/features/workspace/lib/rootManagement' +import { materialProjectionLoss } from '@shared/types/projectionFidelity' function targetSupportsBuiltInMcpDomain( workspace: CommandContext['workspace'], @@ -819,12 +820,17 @@ export const sessionCommands: CommandDef[] = [ if (!getProviderFeatures(kind).transcriptDuplicate) return if (!isAgentProviderKind(kind) || !meta?.providerSessionId) return try { - const { newProviderSessionId } = await window.api.duplicateSession({ + const { newProviderSessionId, projectionFidelity } = await window.api.duplicateSession({ provider: kind, sourceProviderSessionId: meta.providerSessionId, cwd: meta.cwd, }) ui.closePalette() + // #927: a duplicate is not automatically lossless. The projector + // rebuilds the transcript, and a Codex -> Codex copy already drops + // opaque records. Only MATERIAL loss is named (see + // materialProjectionLoss); the full summary is on the result. + const duplicateLoss = materialProjectionLoss(projectionFidelity) // Open the clone as a SIBLING pane (vertical split) of the // source. Using `workspace.newTab` would push the clone into // a new tab and hide the source behind a tab switch — not @@ -872,9 +878,13 @@ export const sessionCommands: CommandDef[] = [ // Unplaced means nothing on screen changed, so say where it went. // (A targeted pane toast also shows globally while the source is // off screen — see targetedCommandContext.) - workspace.showPaneToast(sessionId, 'Duplicated — the copy is marked new in the Sessions list', 4000) + workspace.showPaneToast(sessionId, `Duplicated — the copy is marked new in the Sessions list${duplicateLoss ? ` · ${duplicateLoss}` : ''}`, duplicateLoss ? 8000 : 4000) } else { await workspace.splitFocused(kind, continuation) + // The placed copy is its own evidence, so a clean duplicate stays + // silent here. A lossy one is not a faithful copy, and the source + // pane says so (#927). + if (duplicateLoss) workspace.showPaneToast(sessionId, `Duplicated · ${duplicateLoss}`, 8000) } } catch (err) { // Surface the failure as a pane toast, not just console.warn. Native diff --git a/src/renderer/src/workspace/control/lifecycle.renderer.test.tsx b/src/renderer/src/workspace/control/lifecycle.renderer.test.tsx index d1893215b..d34ff90c0 100644 --- a/src/renderer/src/workspace/control/lifecycle.renderer.test.tsx +++ b/src/renderer/src/workspace/control/lifecycle.renderer.test.tsx @@ -66,7 +66,7 @@ it('reports a domain refusal instead of treating a resolved void transaction as it('keeps draft edits made during native rewind recoverable by undo', async () => { const { invoke, revision, replaceSession, report, refs } = setup() - window.api.rewindToPrompt = vi.fn().mockResolvedValue({ provider: 'codex', newProviderSessionId: 'rewound-native', newFilePath: '/recorded/rewound.jsonl', promptText: 'Historical prompt', promptImages: [], promptAttachments: [], promptMode: 'prompt', promptTimestamp: null }) + window.api.rewindToPrompt = vi.fn().mockResolvedValue({ provider: 'codex', newProviderSessionId: 'rewound-native', newFilePath: '/recorded/rewound.jsonl', promptText: 'Historical prompt', promptImages: [], promptAttachments: [], promptMode: 'prompt', promptTimestamp: null, projectionFidelity: { profile: 'native-resume' as const, sourceProvider: 'codex', targetProvider: 'codex', providerProfileId: 'test', providerEvidence: {}, counts: { preserved: 0, dropped: 0, demoted: 0, synthesized: 0, repaired: 0, retargeted: 0, opaque: 0 }, codes: [], sourceLinesOmitted: 0 } }) // The actual replacement contract is independently exercised in // sessionReplacementHandoff: this boundary returns its latest carried draft, // including edits made after the original lifecycle inspection. diff --git a/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.renderer.test.ts b/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.renderer.test.ts index cb6090dcb..886fff306 100644 --- a/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.renderer.test.ts +++ b/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.renderer.test.ts @@ -6,6 +6,7 @@ import type { WorkspaceRefs } from '@renderer/workspace/hook/refs' import type { SessionActions } from '@renderer/workspace/hook/actions/session' import type { ProviderSwitchBatch } from '@renderer/workspace/types' import { useBulkProviderSwitchActions } from '@renderer/workspace/hook/actions/bulkProviderSwitch' +import { DEMOTING_SWITCH_FIDELITY } from '@renderer/workspace/hook/actions/testing/recordedProjectionFidelity' const { switchAgentProvider } = vi.hoisted(() => ({ switchAgentProvider: vi.fn() })) // WHY the constant is restated here instead of pulled through `importOriginal`: @@ -260,6 +261,19 @@ describe('bulk switch reporting', () => { expect(toastDurations[0]).toBe(10_000) }) + // #927: a return whose fit shrank nothing can still be a lossy projection. + // The recorded Claude -> Codex summary demotes 19 entries with a null + // shrinkSummary; before #927 the batch toasted it as clean. + it('surfaces projection loss even when nothing was shrunk', async () => { + switchAgentProvider.mockResolvedValue({ status: 'switched', shrinkSummary: null, projectionFidelity: DEMOTING_SWITCH_FIDELITY }) + const { result, toasts, toastDurations } = harness(batchOf('a')) + + await result.current.returnLastProviderSwitchBatch() + + expect(toasts[0]).toContain('history: 19 demoted') + expect(toastDurations[0]).toBe(10_000) + }) + it('keeps the default duration when nothing was lost', async () => { switchAgentProvider.mockResolvedValue({ status: 'switched', shrinkSummary: null }) const { result, toastDurations } = harness(batchOf('a')) diff --git a/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.ts b/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.ts index a57997e54..c9cb50f59 100644 --- a/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.ts +++ b/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.ts @@ -10,6 +10,7 @@ import type { SessionActions } from '@renderer/workspace/hook/actions/session' import { LOSSY_SWITCH_TOAST_MS, switchAgentProvider } from '@renderer/workspace/hook/actions/providerSwitchCore' import type { SwitchStrategy } from '@renderer/workspace/hook/actions/providerSwitchCore' import { pluralAgents } from '@renderer/features/workspace/lib/sessionDisplay' +import { materialProjectionLoss } from '@shared/types/projectionFidelity' // Bulk provider switch + remembered-batch return. // @@ -211,6 +212,10 @@ export function useBulkProviderSwitchActions( } else if (meta && originalKind) { counts[result.strategy] += 1 if (result.shrinkSummary) notes.add(result.shrinkSummary) + // #927: projection loss is reported beside the shrink note, not + // folded into the strategy; see the single-pane toast in provider.ts. + const loss = materialProjectionLoss(result.projectionFidelity) + if (loss) notes.add(loss) switched.push({ sessionId: result.newSessionId, cwd: meta.cwd, @@ -320,6 +325,8 @@ export function useBulkProviderSwitchActions( if (result.status === 'switched') { returned += 1 if (result.shrinkSummary) notes.add(result.shrinkSummary) + const loss = materialProjectionLoss(result.projectionFidelity) + if (loss) notes.add(loss) } else if (result.status === 'failed') { failed += 1 notes.add(result.message) diff --git a/src/renderer/src/workspace/hook/actions/provider.projectionLoss.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/provider.projectionLoss.renderer.test.tsx new file mode 100644 index 000000000..0f8d8201b --- /dev/null +++ b/src/renderer/src/workspace/hook/actions/provider.projectionLoss.renderer.test.tsx @@ -0,0 +1,68 @@ +import { act, cleanup, renderHook } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import { useAppStore } from '@renderer/app-state/store' +import { emptyRuntime } from '@renderer/session-runtime/state' +import { makeRefs, sessionActionsWithSpawn } from '@renderer/workspace/hook/actions/testing/paneActionsHarness' +import { DEMOTING_SWITCH_FIDELITY } from '@renderer/workspace/hook/actions/testing/recordedProjectionFidelity' + +// #927: the single-pane switch toast names the projector's material loss. +// The transaction itself is mocked: this pins how the pane TELLS the user, +// using a real summary copied from a recorded projection (see +// recordedProjectionFidelity). The projection itself is pinned in main's +// projectionFidelity.system.test. +const { switchAgentProvider } = vi.hoisted(() => ({ switchAgentProvider: vi.fn() })) +vi.mock('@renderer/workspace/hook/actions/providerSwitchCore', async importOriginal => ({ + ...await importOriginal(), + switchAgentProvider, +})) + +import { useProviderActions } from '@renderer/workspace/hook/actions/provider' +import { LOSSY_SWITCH_TOAST_MS } from '@renderer/workspace/hook/actions/providerSwitchCore' + +const original = useAppStore.getState() +afterEach(() => { + cleanup() + useAppStore.setState(original, true) + switchAgentProvider.mockReset() +}) + +function setup() { + useAppStore.setState({ + workspaceState: { + ...original.workspaceState, + activeTabId: 'project', + tabs: [{ id: 'project', title: 'Project' }], + sessions: { source: { kind: 'claude', cwd: '/source', providerSessionId: 'native-source', projectId: 'project', joinedAt: 0 } }, + }, + workspaceRuntimes: { source: emptyRuntime() }, + }) + const refs = makeRefs(useAppStore.getState().workspaceState) + refs.latestRuntimesRef.current = useAppStore.getState().workspaceRuntimes + const showPaneToast = vi.fn() + const mounted = renderHook(() => useProviderActions( + refs, + useAppStore.getState().setWorkspaceRuntimes, + showPaneToast, + sessionActionsWithSpawn(vi.fn(), {} as never), + )) + return { mounted, showPaneToast } +} + +it('names projection loss on a native (unshrunk) switch, for long enough to read', async () => { + switchAgentProvider.mockResolvedValue({ + status: 'switched', newSessionId: 'target', targetKind: 'codex', strategy: 'native', shrinkSummary: null, + projectionFidelity: DEMOTING_SWITCH_FIDELITY, + }) + const { mounted, showPaneToast } = setup() + await act(async () => { await mounted.result.current.switchSessionProvider('source', 'codex') }) + expect(showPaneToast).toHaveBeenCalledWith('target', expect.stringMatching(/^Switched to .+ · history: 19 demoted$/), LOSSY_SWITCH_TOAST_MS) +}) + +it('keeps a lossless switch quiet', async () => { + switchAgentProvider.mockResolvedValue({ + status: 'switched', newSessionId: 'target', targetKind: 'codex', strategy: 'native', shrinkSummary: null, projectionFidelity: null, + }) + const { mounted, showPaneToast } = setup() + await act(async () => { await mounted.result.current.switchSessionProvider('source', 'codex') }) + expect(showPaneToast).toHaveBeenCalledWith('target', expect.not.stringContaining('history'), undefined) +}) diff --git a/src/renderer/src/workspace/hook/actions/provider.ts b/src/renderer/src/workspace/hook/actions/provider.ts index 5293fee53..759814bb4 100644 --- a/src/renderer/src/workspace/hook/actions/provider.ts +++ b/src/renderer/src/workspace/hook/actions/provider.ts @@ -1,4 +1,5 @@ import { describeRewindAttachmentLoss } from '@renderer/workspace/hook/actions/rewindAttachmentLoss' +import { materialProjectionLoss } from '@shared/types/projectionFidelity' import { sessionMcpOverrides } from '@renderer/workspace/mcpDomains' import { getRendererProviderCapabilities } from '@providers/registry.renderer.capabilities' import { tldrIdentityForSession } from '@renderer/features/tldr/identity' @@ -96,9 +97,16 @@ export function useProviderActions( // lossless case should not add noise to every switch; the two lossy // strategies name themselves, and `shrunk` carries the one line saying // what the ladder had to remove. - const note = result.strategy === 'native' + // #927: the projector's own loss rides the same toast. It is independent + // of the strategy: a `native` fit (nothing shrunk) can still drop or + // demote content the target cannot carry (Codex -> Claude drops foreign + // reasoning and developer messages on every recorded sequence), so + // before this a lossy switch could toast as a clean one. + const loss = materialProjectionLoss(result.projectionFidelity) + const note = (result.strategy === 'native' ? '' - : ` · ${result.strategy}${result.shrinkSummary ? ` (${result.shrinkSummary})` : ''}` + : ` · ${result.strategy}${result.shrinkSummary ? ` (${result.shrinkSummary})` : ''}`) + + (loss ? ` · ${loss}` : '') showPaneToast( result.newSessionId, `Switched to ${providerChoiceLabel(result.targetKind, targetProviderRuntime)}${note}`, diff --git a/src/renderer/src/workspace/hook/actions/providerSwitchCore.renderer.test.ts b/src/renderer/src/workspace/hook/actions/providerSwitchCore.renderer.test.ts index 0bf95fef9..ae68ad868 100644 --- a/src/renderer/src/workspace/hook/actions/providerSwitchCore.renderer.test.ts +++ b/src/renderer/src/workspace/hook/actions/providerSwitchCore.renderer.test.ts @@ -197,6 +197,8 @@ describe('switchAgentProvider', () => { // translated at all (#821). strategy: 'native', shrinkSummary: null, + // Nothing was projected, so there is no report to carry (#927). + projectionFidelity: null, }) expect(switchProvider).toHaveBeenCalledWith(expect.objectContaining({ diff --git a/src/renderer/src/workspace/hook/actions/providerSwitchCore.ts b/src/renderer/src/workspace/hook/actions/providerSwitchCore.ts index c52a83248..58101fad4 100644 --- a/src/renderer/src/workspace/hook/actions/providerSwitchCore.ts +++ b/src/renderer/src/workspace/hook/actions/providerSwitchCore.ts @@ -13,6 +13,7 @@ import { enabledProviderSwitchChoices, } from '@renderer/workspace/providerChoices' import { enabledAgentProviderKindsSnapshot } from '@renderer/features/providers/store' +import type { NativeProjectionFidelity } from '@shared/types/projectionFidelity' // Single-agent provider switch — the shared core. // @@ -48,6 +49,9 @@ export type SwitchAgentProviderResult = strategy: SwitchStrategy /** One human-readable line describing what `shrunk` cost, else null. */ shrinkSummary: string | null + /** The projector's report, summarized (#927); null when nothing was + * projected (an empty source is replaced without a transcript). */ + projectionFidelity: NativeProjectionFidelity | null } | { status: 'skipped'; reason: string } | { status: 'failed'; message: string } @@ -221,7 +225,7 @@ export async function switchAgentProvider(params: { // `native` is the honest strategy for an empty source: no transcript was // translated, so nothing could be lost. Reporting null instead would make // the batch tally under-count agents that switched perfectly well. - return { status: 'switched', newSessionId, targetKind, strategy: 'native', shrinkSummary: null } + return { status: 'switched', newSessionId, targetKind, strategy: 'native', shrinkSummary: null, projectionFidelity: null } } const sourceRuntime = refs.latestRuntimesRef.current[sessionId] @@ -446,6 +450,7 @@ export async function switchAgentProvider(params: { targetKind: result.targetKind, strategy: result.strategy, shrinkSummary: result.shrinkSummary, + projectionFidelity: result.projectionFidelity, } } catch (err) { const message = diff --git a/src/renderer/src/workspace/hook/actions/testing/recordedProjectionFidelity.ts b/src/renderer/src/workspace/hook/actions/testing/recordedProjectionFidelity.ts new file mode 100644 index 000000000..36505c0f2 --- /dev/null +++ b/src/renderer/src/workspace/hook/actions/testing/recordedProjectionFidelity.ts @@ -0,0 +1,45 @@ +import type { NativeProjectionFidelity } from '@shared/types/projectionFidelity' + +// Renderer tests cannot run the parser's projectors, so these are the REAL +// summaries, copied from `summarizeProjectionReport` over the recorded Stage 0 +// sequences (projected to Codex 0.157.1 as projectionFidelity.system.test.ts +// does) (#927). Only `sourceLines` is trimmed to two per row. Counts, codes, +// profile and evidence are exactly as produced. + +/** claude-sequence-oversized -> Codex: nothing shrunk, 19 entries demoted. */ +export const DEMOTING_SWITCH_FIDELITY: NativeProjectionFidelity = { + profile: 'native-resume', + sourceProvider: 'claude', + targetProvider: 'codex', + providerProfileId: 'codex-rollout-source-8035cb03', + providerEvidence: { sourceCommit: '8035cb03f1a5061d0342cb8fa3a10a18068ca683' }, + counts: { preserved: 67, dropped: 0, demoted: 19, synthesized: 2, repaired: 0, retargeted: 0, opaque: 0 }, + codes: [ + { kind: 'synthesized', code: 'native-resume.session-meta.synthesized', count: 1, sourceLines: [] }, + { kind: 'synthesized', code: 'native-resume.turn-framing.synthesized', count: 1, sourceLines: [] }, + { kind: 'preserved', code: 'native-resume.message.preserved', count: 6, sourceLines: [0, 14] }, + { kind: 'preserved', code: 'native-resume.reasoning.preserved', count: 9, sourceLines: [1, 8] }, + { kind: 'demoted', code: 'native-resume.reasoning.encrypted-content-demoted', count: 9, sourceLines: [1, 8] }, + { kind: 'preserved', code: 'native-resume.tool-call.preserved', count: 26, sourceLines: [2, 4] }, + { kind: 'preserved', code: 'native-resume.tool-result.preserved', count: 26, sourceLines: [3, 6] }, + { kind: 'demoted', code: 'native-resume.tool-result.error-status-demoted', count: 10, sourceLines: [3, 6] }, + ], + sourceLinesOmitted: 12, +} + +/** codex-sequence-compaction -> Codex (a duplicate): only an opaque record + * dropped, which is not material. */ +export const OPAQUE_ONLY_DUPLICATE_FIDELITY: NativeProjectionFidelity = { + profile: 'native-resume', + sourceProvider: 'codex', + targetProvider: 'codex', + providerProfileId: 'codex-rollout-source-8035cb03', + providerEvidence: { sourceCommit: '8035cb03f1a5061d0342cb8fa3a10a18068ca683' }, + counts: { preserved: 1, dropped: 1, demoted: 0, synthesized: 1, repaired: 0, retargeted: 0, opaque: 0 }, + codes: [ + { kind: 'synthesized', code: 'native-resume.session-meta.synthesized', count: 1, sourceLines: [] }, + { kind: 'dropped', code: 'native-resume.opaque.dropped', count: 1, sourceLines: [0] }, + { kind: 'preserved', code: 'native-resume.compaction.preserved', count: 1, sourceLines: [1] }, + ], + sourceLinesOmitted: 0, +} diff --git a/src/shared/types/projectionFidelity.ts b/src/shared/types/projectionFidelity.ts new file mode 100644 index 000000000..94185967a --- /dev/null +++ b/src/shared/types/projectionFidelity.ts @@ -0,0 +1,83 @@ +// What a provider switch, duplicate or rewind actually carried over (#927). +// +// WHY this exists: all three operations run the conversation through a REAL +// native-resume projector (agent-transcript-parser), which reports every +// entry it preserved, dropped, demoted, repaired or synthesized. Before #927 +// the host wrote `projection.values` and threw `projection.report` away, so +// the only account a user or a caller ever saw was the switch's +// `shrinkSummary`. That describes context REDUCTION (the fit ladder) and is +// null whenever nothing had to be shrunk, even when the projection itself +// dropped content. Every recorded Stage 0 sequence loses something in at +// least one direction; a Codex -> Codex duplicate drops opaque records too. +// +// WHY a curated summary and not the parser's report as-is: this crosses IPC +// and reaches pane toasts. `ProjectionChange.message` is projector prose, and +// `evidence` carries source claims. Neither belongs in user-visible text +// (q22/q39: curated, bounded). The `code` is a closed vocabulary +// (`native-resume..`) that identifies each change without +// either. Source line numbers are kept (they locate the change) but bounded, +// with the rest COUNTED, never silently dropped (#918 §6.2: shortening a +// summary must not delete the fidelity record). + +export type ProjectionChangeKind = + | 'preserved' + | 'dropped' + | 'demoted' + | 'synthesized' + | 'repaired' + | 'retargeted' + | 'opaque' + +export type ProjectionFidelityCode = { + kind: ProjectionChangeKind + /** The projector's closed change code, e.g. `native-resume.opaque.dropped`. */ + code: string + count: number + /** Source line of each change, first `PROJECTION_FIDELITY_MAX_LINES` only. */ + sourceLines: number[] +} + +export type NativeProjectionFidelity = { + profile: 'native-resume' + sourceProvider: string + targetProvider: string + /** The projector profile the guarantee is stated against. */ + providerProfileId: string + providerEvidence: { sourceCommit?: string; cliVersion?: string; observedAt?: string } + counts: Record + /** One row per distinct (kind, code), in first-seen order. */ + codes: ProjectionFidelityCode[] + /** Source lines beyond the per-code cap, counted rather than listed. */ + sourceLinesOmitted: number +} + +export const PROJECTION_FIDELITY_MAX_LINES = 20 + +// Loss a user should be told about in the operation's toast. +// +// UNCONFIRMED product default (#927 plan): material = `dropped` changes other +// than opaque records, plus every `demoted` change. +// - Opaque records are the provider's non-conversation rows (telemetry, +// environment and turn bookkeeping), dropped by every Codex projection +// (1-30 per recorded case). Naming them on every switch would train users to +// ignore the line. +// - Repairs fix shape (an input object, content blocks) without losing +// content. +// - Synthesized framing is the target's own session and turn identity. +// All of those stay in the result for inspection; only the toast skips them. +const QUIET_DROP_CODES = new Set(['native-resume.opaque.dropped']) + +export function materialProjectionLoss(fidelity: NativeProjectionFidelity | null | undefined): string | null { + if (!fidelity) return null + let dropped = 0 + let demoted = 0 + for (const row of fidelity.codes) { + if (row.kind === 'dropped' && !QUIET_DROP_CODES.has(row.code)) dropped += row.count + if (row.kind === 'demoted') demoted += row.count + } + const parts: string[] = [] + if (dropped > 0) parts.push(`${dropped} dropped`) + if (demoted > 0) parts.push(`${demoted} demoted`) + // UNCONFIRMED wording: "history: 12 dropped, 19 demoted". + return parts.length > 0 ? `history: ${parts.join(', ')}` : null +} From e3963b8fd71f485dcb10eed4a488a5d7ee0428af Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 01:38:49 -0700 Subject: [PATCH 10/43] fix(provider-switch): relay fidelity through control and rewind, loss first in toasts, narrow the failure claim (#1384 round 1, steering q90) Co-Authored-By: Claude Opus 5.5 --- ...-27-provider-switch-projection-fidelity.md | 22 +++++++++++ .../projectionFidelity.system.test.ts | 33 ++++++++++++++++ src/main/providerSwitch/projectionFidelity.ts | 10 ++++- src/main/providerSwitch/switchProvider.ts | 5 ++- .../commands/sessionCommands.renderer.test.ts | 22 +++++++++++ .../control/lifecycle.renderer.test.tsx | 14 +++++++ .../src/workspace/control/lifecycle.ts | 24 ++++++++---- .../bulkProviderSwitch.renderer.test.ts | 17 +++++++++ .../provider.projectionLoss.renderer.test.tsx | 34 ++++++++++++++++- .../src/workspace/hook/actions/provider.ts | 38 ++++++++++++++----- .../providerSwitchCore.renderer.test.ts | 6 ++- src/shared/types/projectionFidelity.ts | 7 +++- 12 files changed, 207 insertions(+), 25 deletions(-) diff --git a/docs/plans/2026-09-27-provider-switch-projection-fidelity.md b/docs/plans/2026-09-27-provider-switch-projection-fidelity.md index 27a0103c1..309f6272e 100644 --- a/docs/plans/2026-09-27-provider-switch-projection-fidelity.md +++ b/docs/plans/2026-09-27-provider-switch-projection-fidelity.md @@ -53,3 +53,25 @@ Main-process tests drive the real entry points with real projectors; the app is - **B07 receipts / operation records:** no receipt store exists yet. The summary is on the result, which is what the client can inspect today. - **The rewind toast in `pane.ts`:** open PR #1369 edits that file. The rewind result carries the field for a follow-up. - **Draft-restoration reports** (#929) and literal-markup provenance (#930). + +## Review round 1 (a and c: FIX-BEFORE-MERGE; b: FIX, test gaps) and steering q90 +- **a (major): a publication that rejects after the effect loses the summary.** It may have linked the native file. + - **Ruling:** narrow the claim instead of carrying the summary on the failure. Electron's IPC keeps only an error's message, so a summary on a thrown error never reaches the renderer. A new failure arm on all three results is #918 B07's receipt work. + - The comments now say the summary is returned only on success. + - A recorded-sequence test pins it: a write that rejects after receiving the projection rejects the operation with that error and returns no success-shaped result. +- **c (major): the control entry points discarded the summary.** + - `agents.switchProvider` and `agents.rewind` (through the `result` helper) and `agents.duplicate` now return `projectionFidelity`, and their descriptions name it. Duplicate is no longer called "exact". + - The rewind action returns it and names material loss in its toast. + - Correction: the rewind toast lives in `provider.ts`, not in `pane.ts` as this plan first said, so it is in scope. +- **a (major): toast order.** Material loss now comes before the shrink prose, because PaneToast clamps to three lines. +- **Pins, each killing its surviving mutation:** + - the populated main → renderer relay (`providerSwitchCore`); + - the forward bulk note; + - the unplaced (row-menu) duplicate toast; + - the control relay, through the real rewind action; + - the rewind toast; + - exact source lines per code, the profile id, and `counts` copied rather than aliased. +- **b:** the false comment "every recorded sequence loses something" is corrected (two Claude sequences are lossless). +- **Still for the owner:** + - the material rule (b notes that `reasoning.encrypted-content-demoted` and `tool-result.error-status-demoted` keep their content, so every Claude → Codex switch toasts a demotion line); + - the wording. diff --git a/src/main/providerSwitch/projectionFidelity.system.test.ts b/src/main/providerSwitch/projectionFidelity.system.test.ts index 4563d618a..d09c6246f 100644 --- a/src/main/providerSwitch/projectionFidelity.system.test.ts +++ b/src/main/providerSwitch/projectionFidelity.system.test.ts @@ -15,6 +15,7 @@ import type { ConversationDocument } from 'agent-transcript-parser' const state = vi.hoisted(() => ({ conversation: null as ConversationDocument | null, written: [] as unknown[], + failWrite: null as Error | null, })) vi.mock('node:crypto', () => ({ randomUUID: () => '00000000-0000-4000-8000-000000000927' })) @@ -40,6 +41,7 @@ vi.mock('@main/providerSwitch/transcriptEngine.js', () => ({ sessionId: () => 'new-session', write: async (_cwd: string, publication: unknown) => { state.written.push(publication) + if (state.failWrite) throw state.failWrite return '/recorded/new-session.jsonl' }, draft: (content: unknown) => ({ promptText: JSON.stringify(content).slice(0, 20), promptMode: 'prompt', promptImages: [], promptAttachments: [] }), @@ -56,6 +58,7 @@ import { loadFixtureConversation } from './testing/fixtureConversations.js' beforeEach(() => { state.written = [] + state.failWrite = null }) // The fidelity a caller receives must be the summary of the projection that @@ -128,6 +131,24 @@ describe('projection fidelity through each real caller (#927)', () => { }) }) +// #1384 review a / steering q90: the narrowed contract. The summary is +// returned only on success. A publication that rejects AFTER the projection +// (possibly after linking the native file) rejects the whole operation with +// the write's error and carries no summary; accounting for that ambiguous +// outcome is #918 B07's receipt. Pinned so a later change cannot quietly +// return a success-shaped result for a failed publication. +describe('a rejected publication', () => { + it('rejects the operation with the write error and returns no summary', async () => { + state.conversation = await loadFixtureConversation('claude-sequence-oversized', 'claude') + const failing = new Error('publication failed after link') + state.failWrite = failing + await expect(switchProvider({ sourceKind: 'claude', targetKind: 'codex', sourceProviderSessionId: 'source', cwd: '/recorded' })).rejects.toBe(failing) + await expect(duplicateSession({ provider: 'codex', sourceProviderSessionId: 'source', cwd: '/recorded' })).rejects.toBe(failing) + // The write really received the projection: the effect may exist. + expect(state.written.length).toBe(2) + }) +}) + describe('summarizeProjectionReport on a real report', () => { // Recorded: claude-sequence-oversized-turns -> Codex, 485 demoted encrypted // reasoning blocks: enough changes of one code to exercise the line cap. @@ -146,6 +167,18 @@ describe('summarizeProjectionReport on a real report', () => { expect(listed + fidelity.sourceLinesOmitted).toBe(withLines) expect(fidelity.sourceLinesOmitted).toBeGreaterThan(0) expect(JSON.stringify(fidelity)).not.toMatch(/"message"|"evidence"/) + // #1384 review a/b: the evidence itself, not only its shape. Each row's + // lines are the real source addresses of that (kind, code), in order; the + // profile is the projector's own; `counts` is a copy, not the report's + // object (a later reuse of the report must not rewrite a summary). + for (const row of fidelity.codes) { + const lines = projection.report.changes + .filter(change => change.kind === row.kind && change.code === row.code && change.sourceLine !== null) + .map(change => change.sourceLine) + expect(row.sourceLines).toEqual(lines.slice(0, 20)) + } + expect(fidelity.providerProfileId).toBe(projection.providerProfile.id) + expect(fidelity.counts).not.toBe(projection.report.counts) expect(materialProjectionLoss(fidelity)).toMatch(/demoted/) }) }) diff --git a/src/main/providerSwitch/projectionFidelity.ts b/src/main/providerSwitch/projectionFidelity.ts index 48edd0e64..934e4232e 100644 --- a/src/main/providerSwitch/projectionFidelity.ts +++ b/src/main/providerSwitch/projectionFidelity.ts @@ -9,8 +9,14 @@ import { /** * The bounded, curated summary of a real native-resume projection (#927). * - * Called right after `projectNativeResume` and before `write`, so every - * caller holds the summary even when publication fails afterwards. Pure: it + * Called right after `projectNativeResume` and before `write`. It is returned + * only on SUCCESS: if the write or publication rejects, the operation + * rejects with that error and the summary is not carried (Electron's IPC + * keeps only an error's message). A rejected publication can still have + * linked the native file (see `publishNativeTranscript`), so that outcome is + * ambiguous, and accounting for it is #918 B07's receipt, not this summary. + * (#1384 review a showed the first version of this comment claimed more.) + * Pure: it * reads the report and copies what it keeps, so a later reuse of the * projector's objects cannot change a summary already handed out (#918 §6.2). * See `@shared/types/projectionFidelity` for why messages and evidence are diff --git a/src/main/providerSwitch/switchProvider.ts b/src/main/providerSwitch/switchProvider.ts index 9025211ef..545544a15 100644 --- a/src/main/providerSwitch/switchProvider.ts +++ b/src/main/providerSwitch/switchProvider.ts @@ -370,8 +370,9 @@ export async function switchProvider( now: new Date().toISOString(), targetProfile, }) - // Summarized BEFORE the write, so the evidence exists even if publication - // fails (#918 §6.2); the report is otherwise discarded with `projection`. + // Summarized before the write, from the projection that is written. It is + // returned only on success; a rejected publication is an ambiguous outcome + // for #918 B07's receipts (see summarizeProjectionReport). const projectionFidelity = summarizeProjectionReport(projection) const targetProviderSessionId = target.sessionId(projection) const targetFilePath = await target.write(targetCwd, projection) diff --git a/src/renderer/src/features/workspace/commands/sessionCommands.renderer.test.ts b/src/renderer/src/features/workspace/commands/sessionCommands.renderer.test.ts index 6dc2f4f3f..54609cc95 100644 --- a/src/renderer/src/features/workspace/commands/sessionCommands.renderer.test.ts +++ b/src/renderer/src/features/workspace/commands/sessionCommands.renderer.test.ts @@ -104,6 +104,28 @@ describe('Duplicate Agent command', () => { else expect(showPaneToast).not.toHaveBeenCalled() }) + // #1384 review b/c: the Sessions-row menu's duplicate (explicit target) is + // left unplaced and says so in its own toast; the loss rides that toast. + it('names material projection loss in the unplaced (row-menu) duplicate toast', async () => { + const duplicateSession = vi.fn().mockResolvedValue({ newProviderSessionId: 'provider-clone', projectionFidelity: DEMOTING_SWITCH_FIDELITY }) + Object.defineProperty(window, 'api', { configurable: true, value: { duplicateSession } }) + const showPaneToast = vi.fn() + const workspace = { + state: { + activeTabId: 'tab-klay', + stage: oneLaneStage('source'), pinnedSessionIds: [], + sessions: { source: { cwd: '/projects/klay', kind: 'codex', providerSessionId: 'provider-source', projectId: 'tab-klay', joinedAt: 0 } }, + tabs: [{ id: 'tab-klay' }], + }, + createDetachedDispatchAgent: vi.fn().mockResolvedValue('clone-pane'), + splitFocused: vi.fn(), + showPaneToast, + } as unknown as Workspace + const context = { workspace, target: 'source', ui: { closePalette: vi.fn() }, flags: {} } as unknown as CommandContext + await sessionCommands.find(candidate => candidate.id === 'duplicate-agent')!.run(context) + expect(showPaneToast).toHaveBeenCalledWith('source', 'Duplicated — the copy is marked new in the Sessions list · history: 19 demoted', 8000) + }) + it('never hands a clone the root-management grant', async () => { const duplicateSession = vi.fn().mockResolvedValue({ newProviderSessionId: 'provider-clone' }) Object.defineProperty(window, 'api', { configurable: true, value: { duplicateSession } }) diff --git a/src/renderer/src/workspace/control/lifecycle.renderer.test.tsx b/src/renderer/src/workspace/control/lifecycle.renderer.test.tsx index d34ff90c0..9a50d3f12 100644 --- a/src/renderer/src/workspace/control/lifecycle.renderer.test.tsx +++ b/src/renderer/src/workspace/control/lifecycle.renderer.test.tsx @@ -6,6 +6,7 @@ import { useProviderActions } from '@renderer/workspace/hook/actions/provider' import { makeRefs, sessionActionsWithSpawn } from '@renderer/workspace/hook/actions/testing/paneActionsHarness' import type { Workspace } from '@renderer/workspace/hook' import { lifecycleControlCapabilities } from './lifecycle' +import { DEMOTING_SWITCH_FIDELITY } from '@renderer/workspace/hook/actions/testing/recordedProjectionFidelity' const original = useAppStore.getState() const originalApi = window.api @@ -80,6 +81,19 @@ it('keeps draft edits made during native rewind recoverable by undo', async () = expect(useAppStore.getState().workspaceRuntimes.replacement).toMatchObject({ draftInput: 'Historical prompt', pendingRewindUndo: { previousDraftInput: 'Edited during replacement' } }) }) +// #1384 review c / steering q90: a control caller that rewinds an agent reads +// the projection report from operations.read. Before, the lifecycle result +// helper kept only the IDs, and the rewind action dropped the report before +// anything could inspect it. +it('relays the rewound copy\'s projection fidelity to the control result', async () => { + const { invoke, revision, report } = setup() + window.api.rewindToPrompt = vi.fn().mockResolvedValue({ provider: 'codex', newProviderSessionId: 'rewound-native', newFilePath: '/recorded/rewound.jsonl', promptText: 'Historical prompt', promptImages: [], promptAttachments: [], promptMode: 'prompt', promptTimestamp: null, projectionFidelity: DEMOTING_SWITCH_FIDELITY }) + await invoke('agents.rewind', { sessionId: 'source', revision: await revision(), address: { provider: 'codex', sessionId: 'native-source', line: 1 } }) + await vi.waitFor(() => expect(report).toHaveBeenCalledWith(expect.objectContaining({ capabilityId: 'operations.finish', input: { + callId: 'original-call', result: { ok: true, value: { sourceSessionId: 'source', newSessionId: 'replacement', status: 'completed', projectionFidelity: DEMOTING_SWITCH_FIDELITY } }, + } }))) +}) + it('refuses to rewind a native terminal agent (Pi), whose TUI has no composer for the rewound prompt', async () => { const { invoke, revision, replaceSession } = setup() // Restored kind-only, as the catalog and switches create Pi panes. diff --git a/src/renderer/src/workspace/control/lifecycle.ts b/src/renderer/src/workspace/control/lifecycle.ts index 2fce452fc..9a1a93ada 100644 --- a/src/renderer/src/workspace/control/lifecycle.ts +++ b/src/renderer/src/workspace/control/lifecycle.ts @@ -11,6 +11,7 @@ import { AGENT_PROVIDER_KINDS, effectiveProviderRuntime, isAgentProviderKind, pr import { getProviderFeatures } from '@providers/shared/featureCapabilities' import { resolveTabSessions } from '@renderer/workspace/queries' import { startControlTask } from './startTask' +import type { NativeProjectionFidelity } from '@shared/types/projectionFidelity' const target = z.object({ sessionId: z.string().min(1).describe('Exact Agent Code sessionId from agents.search; not the native transcript ID.') }).strict() const revision = z.string().describe('Revision from agents.lifecycleRead. Refresh it after any lifecycle or draft change.') @@ -48,10 +49,19 @@ export function lifecycleControlCapabilities(getWorkspace: () => Workspace) { if (current.revision !== input.revision) throw new ControlError('stale_cursor', 'Agent lifecycle or draft changed; read agents.lifecycleRead again') return current } - const result = (value: { status: string; reason?: string; message?: string; newSessionId?: string }, sourceSessionId: string) => { + // `projectionFidelity` is relayed when the operation re-projected a + // transcript (switch, rewind; #927). Before, this helper kept only the IDs, + // so a control caller that switched or rewound an agent had no way to see + // what the projector dropped or demoted; the UI toast was the only record. + const result = (value: { status: string; reason?: string; message?: string; newSessionId?: string; projectionFidelity?: NativeProjectionFidelity | null }, sourceSessionId: string) => { if (value.status === 'skipped') throw new ControlError('unavailable', value.reason ?? 'Operation declined') if (value.status === 'failed' || !value.newSessionId) throw new ControlError('failed', value.message ?? 'Replacement was not observed', 'unknown') - return { sourceSessionId, newSessionId: value.newSessionId, status: value.status } + return { + sourceSessionId, + newSessionId: value.newSessionId, + status: value.status, + ...(value.projectionFidelity ? { projectionFidelity: value.projectionFidelity } : {}), + } } return [ defineCapability({ id: 'agents.resume', title: 'Resume a native session in a project', execution: 'window', effect: 'mutation', completion: 'accepted', target: { kind: 'project', field: 'tabId' }, @@ -73,8 +83,8 @@ export function lifecycleControlCapabilities(getWorkspace: () => Workspace) { }) }, }), - defineCapability({ id: 'agents.duplicate', title: 'Branch an exact agent conversation', execution: 'window', effect: 'mutation', completion: 'accepted', target: { kind: 'session', field: 'sessionId' }, - description: 'Copy an idle native conversation to a new native identity and create an agent in the chosen project. Preserves provider/runtime and enabled built-in domain names; leaves the source and its draft intact. Requires a fresh lifecycle revision and an explicit target project/anchor in the same window. Use operations.read for both new IDs, then agents.show to put the copy in a lane. It fills the captured focused lane only when that lane is empty (selectCreated:false never places it). A failed placement can leave a native transcript copy; do not blindly retry unknown outcomes.', + defineCapability({ id: 'agents.duplicate', title: 'Branch an agent conversation', execution: 'window', effect: 'mutation', completion: 'accepted', target: { kind: 'session', field: 'sessionId' }, + description: 'Copy an idle native conversation to a new native identity and create an agent in the chosen project. Preserves provider/runtime and enabled built-in domain names; leaves the source and its draft intact. Requires a fresh lifecycle revision and an explicit target project/anchor in the same window. Use operations.read for both new IDs and the projectionFidelity of the copy (what re-projecting the transcript preserved, dropped or demoted; a copy is not byte-exact), then agents.show to put the copy in a lane. It fills the captured focused lane only when that lane is empty (selectCreated:false never places it). A failed placement can leave a native transcript copy; do not blindly retry unknown outcomes.', input: target.extend({ revision, tabId: z.string(), anchorSessionId: z.string(), selectCreated: z.boolean().default(true).describe('False preserves the active tab and all lane selections.') }), output: accepted, handler: (input, context) => { const check = () => { @@ -96,7 +106,7 @@ export function lifecycleControlCapabilities(getWorkspace: () => Workspace) { const newSessionId = await getWorkspace().createDetachedSession({ kind: value.provider, providerRuntime: meta.providerRuntime }, { tabId: input.tabId, anchorSessionId: input.anchorSessionId }, { cwd: value.cwd, resumeSessionId: clone.newProviderSessionId, builtInMcpOverrides: clonedMcpOverrides(meta) }, { selectCreated: input.selectCreated }) if (!newSessionId) throw new ControlError('failed', `Native copy ${clone.newProviderSessionId} exists but no placement was committed`, 'unknown') - return { sourceSessionId: input.sessionId, newSessionId, nativeSessionId: clone.newProviderSessionId } + return { sourceSessionId: input.sessionId, newSessionId, nativeSessionId: clone.newProviderSessionId, projectionFidelity: clone.projectionFidelity } }) }, }), @@ -107,7 +117,7 @@ export function lifecycleControlCapabilities(getWorkspace: () => Workspace) { handler: ({ sessionId }) => inspect(sessionId), }), defineCapability({ id: 'agents.switchProvider', title: 'Switch an exact agent provider', execution: 'window', effect: 'mutation', completion: 'accepted', target: { kind: 'session', field: 'sessionId' }, - description: 'Move an observed agent to one of agents.lifecycleRead switchChoices through the normal translation, capacity/compaction and replacement transaction. May open a confirmation or take minutes. Returns a task callId; use operations.read for the new Agent Code session ID or failure. Draft and supported internal MCP-domain continuity follow the ordinary UI operation. Never assume the source ID remains valid.', + description: 'Move an observed agent to one of agents.lifecycleRead switchChoices through the normal translation, capacity/compaction and replacement transaction. May open a confirmation or take minutes. Returns a task callId; use operations.read for the new Agent Code session ID, the projectionFidelity summary (what the projector preserved, dropped or demoted), or failure. Draft and supported internal MCP-domain continuity follow the ordinary UI operation. Never assume the source ID remains valid.', input: target.extend({ revision, provider: z.enum(AGENT_PROVIDER_KINDS), runtime: z.enum(['terminal']).optional().describe('Supply only when the chosen switchChoices entry declares this runtime; omit for structured rendering.') }), output: accepted, handler: (input, context) => { const current = guard(input) @@ -125,7 +135,7 @@ export function lifecycleControlCapabilities(getWorkspace: () => Workspace) { }, }), defineCapability({ id: 'agents.rewind', title: 'Rewind an exact agent to a native prompt', execution: 'window', effect: 'mutation', completion: 'accepted', target: { kind: 'session', field: 'sessionId' }, - description: 'Create a new native transcript ending before an exact prompt address from nativeHistory.prompts, and replace this idle agent in place. The original transcript remains intact. The selected historical prompt becomes the new unsent draft, replacing the current draft; undoRewind can restore the prior conversation/draft until the next submission. First read agents.lifecycleRead. Use operations.read for the final newSessionId; acceptance alone is not completion.', + description: 'Create a new native transcript ending before an exact prompt address from nativeHistory.prompts, and replace this idle agent in place. The original transcript remains intact. The selected historical prompt becomes the new unsent draft, replacing the current draft; undoRewind can restore the prior conversation/draft until the next submission. First read agents.lifecycleRead. Use operations.read for the final newSessionId and the projectionFidelity of the rewound copy; acceptance alone is not completion.', input: target.extend({ revision, address: address.describe('Exact address from nativeHistory.prompts for this native session; never infer line numbers from rendered feed rows.') }), output: accepted, handler: (input, context) => { const check = () => { const value = guard(input); if (!value.nativeSessionId || value.processActive) throw new ControlError('unavailable', 'Rewind requires an idle resumable agent') diff --git a/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.renderer.test.ts b/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.renderer.test.ts index 886fff306..27327fa70 100644 --- a/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.renderer.test.ts +++ b/src/renderer/src/workspace/hook/actions/bulkProviderSwitch.renderer.test.ts @@ -287,6 +287,23 @@ describe('bulk switch reporting', () => { // #1271: a batch holds the app (the modal locks input) for up to five minutes // per agent. A stop requested during the batch ends it after the agent in // flight, and the summary says what was not attempted. +// #1384 review b/c: the FORWARD batch has no per-pane toast, so its summary +// note is that flow's only disclosure of projection loss. +describe('switchAgentsToProvider projection loss', () => { + it('names the loss a native (unshrunk) switch reported', async () => { + const { result, state, toasts, toastDurations } = harness(null) + ;(state.sessions as Record).a = { cwd: '/recorded', kind: 'claude', title: 'a' } + switchAgentProvider.mockResolvedValue({ + status: 'switched', strategy: 'native', shrinkSummary: null, newSessionId: 'a-new', projectionFidelity: DEMOTING_SWITCH_FIDELITY, + }) + await result.current.switchAgentsToProvider(['a'] as never, 'codex', { + allowSourceTurns: false, compactOnArrival: false, sourceCompactionConfirmed: false, + }) + expect(toasts.at(-1)).toContain('history: 19 demoted') + expect(toastDurations.at(-1)).toBe(10_000) + }) +}) + describe('switchAgentsToProvider stop', () => { it('stops before the next agent and reports the rest as not attempted', async () => { const { result, state, toasts } = harness(null) diff --git a/src/renderer/src/workspace/hook/actions/provider.projectionLoss.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/provider.projectionLoss.renderer.test.tsx index 0f8d8201b..262efac90 100644 --- a/src/renderer/src/workspace/hook/actions/provider.projectionLoss.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/provider.projectionLoss.renderer.test.tsx @@ -20,9 +20,11 @@ import { useProviderActions } from '@renderer/workspace/hook/actions/provider' import { LOSSY_SWITCH_TOAST_MS } from '@renderer/workspace/hook/actions/providerSwitchCore' const original = useAppStore.getState() +const originalApi = window.api afterEach(() => { cleanup() useAppStore.setState(original, true) + window.api = originalApi switchAgentProvider.mockReset() }) @@ -43,7 +45,7 @@ function setup() { refs, useAppStore.getState().setWorkspaceRuntimes, showPaneToast, - sessionActionsWithSpawn(vi.fn(), {} as never), + sessionActionsWithSpawn(vi.fn(), { replaceSession: vi.fn().mockResolvedValue('replacement') } as never), )) return { mounted, showPaneToast } } @@ -66,3 +68,33 @@ it('keeps a lossless switch quiet', async () => { await act(async () => { await mounted.result.current.switchSessionProvider('source', 'codex') }) expect(showPaneToast).toHaveBeenCalledWith('target', expect.not.stringContaining('history'), undefined) }) + +// #1384 review a / steering q90: PaneToast clamps to three lines. A recorded +// oversized switch's shrink summary is a long sentence, so the loss goes +// BEFORE it rather than trailing out of sight. +it('puts projection loss before a long shrink summary', async () => { + switchAgentProvider.mockResolvedValue({ + status: 'switched', newSessionId: 'target', targetKind: 'codex', strategy: 'shrunk', + shrinkSummary: '1 attachment omitted, 130 oldest entries dropped (17k → 13k chars)', + projectionFidelity: DEMOTING_SWITCH_FIDELITY, + }) + const { mounted, showPaneToast } = setup() + await act(async () => { await mounted.result.current.switchSessionProvider('source', 'codex') }) + const message = showPaneToast.mock.calls.at(-1)![1] as string + expect(message.indexOf('history: 19 demoted')).toBeGreaterThan(0) + expect(message.indexOf('history: 19 demoted')).toBeLessThan(message.indexOf('shrunk')) +}) + +// #1384 review c: a rewind re-projects the kept prefix, and its toast names +// material loss first, like the switch toast. +it('names projection loss in the rewind toast', async () => { + const { mounted, showPaneToast } = setup() + window.api = { ...window.api, rewindToPrompt: vi.fn().mockResolvedValue({ + provider: 'claude', newProviderSessionId: 'rewound-native', newFilePath: '/recorded/rewound.jsonl', promptText: 'Earlier', promptImages: [], + promptAttachments: [], promptMode: 'prompt', promptTimestamp: null, projectionFidelity: DEMOTING_SWITCH_FIDELITY, + }) } as typeof window.api + await act(async () => { + await mounted.result.current.rewindSessionToPrompt('source', { provider: 'claude', sessionId: 'native-source', line: 1, uuid: 'u' } as never) + }) + expect(showPaneToast).toHaveBeenCalledWith('replacement', 'Rewound to prompt · history: 19 demoted - Undo Rewind available until next submit', 6000) +}) diff --git a/src/renderer/src/workspace/hook/actions/provider.ts b/src/renderer/src/workspace/hook/actions/provider.ts index 759814bb4..0490b8592 100644 --- a/src/renderer/src/workspace/hook/actions/provider.ts +++ b/src/renderer/src/workspace/hook/actions/provider.ts @@ -1,5 +1,5 @@ import { describeRewindAttachmentLoss } from '@renderer/workspace/hook/actions/rewindAttachmentLoss' -import { materialProjectionLoss } from '@shared/types/projectionFidelity' +import { materialProjectionLoss, type NativeProjectionFidelity } from '@shared/types/projectionFidelity' import { sessionMcpOverrides } from '@renderer/workspace/mcpDomains' import { getRendererProviderCapabilities } from '@providers/registry.renderer.capabilities' import { tldrIdentityForSession } from '@renderer/features/tldr/identity' @@ -39,7 +39,14 @@ import { providerChoiceLabel } from '@renderer/workspace/providerChoices' // Promise cannot distinguish a declined operation from a replacement; // the transport must not infer success from a toast or a changed session census. export type AgentLifecycleResult = - | { status: 'completed'; sourceSessionId: SessionId; newSessionId: SessionId } + | { + status: 'completed' + sourceSessionId: SessionId + newSessionId: SessionId + /** Present for a rewind, which re-projects the transcript (#927). The + * control plane relays it so an external caller can inspect it. */ + projectionFidelity?: NativeProjectionFidelity + } | { status: 'skipped'; reason: string } | { status: 'failed'; message: string } @@ -102,11 +109,17 @@ export function useProviderActions( // demote content the target cannot carry (Codex -> Claude drops foreign // reasoning and developer messages on every recorded sequence), so // before this a lossy switch could toast as a clean one. + // + // The loss goes FIRST (#1384 review a, steering q90): PaneToast clamps + // to three lines, and a shrink summary can be a long sentence ("1 + // attachment omitted, 130 oldest entries dropped (17k -> 13k chars)"), + // which pushed a trailing "history: 132 demoted" out of sight on a + // recorded oversized switch. const loss = materialProjectionLoss(result.projectionFidelity) - const note = (result.strategy === 'native' - ? '' - : ` · ${result.strategy}${result.shrinkSummary ? ` (${result.shrinkSummary})` : ''}`) - + (loss ? ` · ${loss}` : '') + const note = (loss ? ` · ${loss}` : '') + + (result.strategy === 'native' + ? '' + : ` · ${result.strategy}${result.shrinkSummary ? ` (${result.shrinkSummary})` : ''}`) showPaneToast( result.newSessionId, `Switched to ${providerChoiceLabel(result.targetKind, targetProviderRuntime)}${note}`, @@ -316,11 +329,16 @@ export function useProviderActions( const attachmentLoss = describeRewindAttachmentLoss(result.promptAttachments, { composerCarriesImages: getRendererProviderCapabilities(kind).supportsImageAttachments, }) + // #927: a rewind re-projects the kept prefix and can lose content too. + // Named first, like the switch toast, so the three-line clamp cannot + // hide it behind the attachment sentence. + const projectionLoss = materialProjectionLoss(result.projectionFidelity) + const rewound = projectionLoss ? `Rewound to prompt · ${projectionLoss}` : 'Rewound to prompt' showPaneToast( newSessionId, attachmentLoss - ? `Rewound to prompt, but ${attachmentLoss} Undo Rewind available until next submit` - : 'Rewound to prompt - Undo Rewind available until next submit', + ? `${rewound}, but ${attachmentLoss} Undo Rewind available until next submit` + : `${rewound} - Undo Rewind available until next submit`, // WHY a longer toast for the loss case (#1100 review): the default is // 2000 ms and `PaneToast` clamps to three lines, which for a sentence // naming two files and two reasons is a message nobody finishes @@ -328,9 +346,9 @@ export function useProviderActions( // are 5-6 s (a copied command, a saved path), and this is the same // kind of thing. The ordinary success toast keeps the default, // because "it worked" needs no reading time. - attachmentLoss ? 6000 : undefined, + attachmentLoss || projectionLoss ? 6000 : undefined, ) - return { status: 'completed', sourceSessionId, newSessionId } + return { status: 'completed', sourceSessionId, newSessionId, projectionFidelity: result.projectionFidelity } } catch (err) { const message = err instanceof Error && err.message.length > 0 diff --git a/src/renderer/src/workspace/hook/actions/providerSwitchCore.renderer.test.ts b/src/renderer/src/workspace/hook/actions/providerSwitchCore.renderer.test.ts index ae68ad868..53e0d15da 100644 --- a/src/renderer/src/workspace/hook/actions/providerSwitchCore.renderer.test.ts +++ b/src/renderer/src/workspace/hook/actions/providerSwitchCore.renderer.test.ts @@ -4,6 +4,7 @@ import type { WorkspaceSetRuntimes } from '@renderer/workspace/hook/context' import type { WorkspaceRefs } from '@renderer/workspace/hook/refs' import type { SessionActions } from '@renderer/workspace/hook/actions/session' import { switchAgentProvider } from '@renderer/workspace/hook/actions/providerSwitchCore' +import { DEMOTING_SWITCH_FIDELITY } from '@renderer/workspace/hook/actions/testing/recordedProjectionFidelity' const originalApiDescriptor = Object.getOwnPropertyDescriptor(window, 'api') @@ -367,6 +368,7 @@ describe('switchAgentProvider', () => { truncatedBeforeSwitch: false, strategy: 'raw', shrinkSummary: null, + projectionFidelity: DEMOTING_SWITCH_FIDELITY, }) const replaceSession = vi.fn().mockResolvedValue('target-pane') const compactAfterSwitch = vi.fn().mockResolvedValue({ ok: false, message: 'Claude did not accept /compact: composer unavailable' }) @@ -424,7 +426,9 @@ describe('switchAgentProvider', () => { }) // The switch itself succeeded and says nothing about the follow-up. - expect(result).toMatchObject({ status: 'switched', newSessionId: 'target-pane' }) + // #1384 review b/c: the populated report main returned reaches every + // renderer consumer through this relay (an `undefined` relay survived). + expect(result).toMatchObject({ status: 'switched', newSessionId: 'target-pane', projectionFidelity: DEMOTING_SWITCH_FIDELITY }) // Addressed to the NEW pane, with the target transcript the switch wrote. expect(compactAfterSwitch).toHaveBeenCalledWith({ sessionId: 'target-pane', diff --git a/src/shared/types/projectionFidelity.ts b/src/shared/types/projectionFidelity.ts index 94185967a..e9ce20f4f 100644 --- a/src/shared/types/projectionFidelity.ts +++ b/src/shared/types/projectionFidelity.ts @@ -7,8 +7,11 @@ // the only account a user or a caller ever saw was the switch's // `shrinkSummary`. That describes context REDUCTION (the fit ladder) and is // null whenever nothing had to be shrunk, even when the projection itself -// dropped content. Every recorded Stage 0 sequence loses something in at -// least one direction; a Codex -> Codex duplicate drops opaque records too. +// dropped content. Every recorded Codex sequence loses something in both +// directions, a Codex -> Codex duplicate included (opaque records). Two +// recorded Claude sequences (prompts, tool-cycle) lose nothing either way, so +// "lossless" is a real outcome too (#1384 review b corrected the first +// version of this sentence). // // WHY a curated summary and not the parser's report as-is: this crosses IPC // and reaches pane toasts. `ProjectionChange.message` is projector prose, and From 876926830272f4d88f7312f5ae3840909e8dd802 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 07:21:59 -0700 Subject: [PATCH 11/43] docs(conversations): plan saying an unreadable conversation file (#1306) Co-Authored-By: Claude Opus 5.5 --- ...6-09-27-conversation-prompts-unreadable.md | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 docs/plans/2026-09-27-conversation-prompts-unreadable.md diff --git a/docs/plans/2026-09-27-conversation-prompts-unreadable.md b/docs/plans/2026-09-27-conversation-prompts-unreadable.md new file mode 100644 index 000000000..877763cef --- /dev/null +++ b/docs/plans/2026-09-27-conversation-prompts-unreadable.md @@ -0,0 +1,38 @@ +# View Prompts says an unreadable conversation, not "no prompts" (#1306) + +Short plan: a bug with a known root cause (#1306, class C3, P3). Fixes #1306. + +## Outcome +View Prompts on a conversation whose transcript or store is there but unreadable (damaged, or permission-broken) says "Couldn't read this conversation's prompts." instead of "no prompts". The issue asked to check every source: all five had the pattern. + +## Root cause (verified in source, origin/main) +- **Pi** (`pi.ts`): `catch { return [] }` around resolution and branch reading. The package's resolution also skips a file whose header it cannot read, so an unreadable session reads as "no such session". +- **Grok** (`grok.ts`): `catch { return [] }` around the snapshot load. +- **Claude and Codex** read through `extractPromptsFromFile` (`promptFolder.ts`), which turned every stat and read failure into an empty answer. +- **OpenCode** (`opencode.ts`): a store that could not be opened answered `[]`. +- `ViewPromptsModal` showed the raw IPC message for a rejection (q22). + +## Design (contract) +- **`ConversationPromptsUnreadable`** (`sources/types.ts`): a typed error with a fixed message and the cause. +- **The rule for every source:** no file (yet) answers `[]`, because a fresh session has no transcript and View Prompts then shows the live feed. A file or store that is there but unreadable throws. + - `promptFolder` returns empty only for ENOENT/ENOTDIR, and rethrows anything else. + - Pi checks whether a file named for the session exists but cannot be opened. + - OpenCode throws only when `opencode.db` exists. +- **The service's `prompts`** logs the cause in main and rethrows. Search already catches per conversation, so an unreadable one degrades to label-only. +- **`ViewPromptsModal`** shows the fixed sentence. + +## Tests (real files: the recorded conversation corpus, made unreadable with chmod) +- **`promptsUnreadable.system.test.ts`:** Claude, Codex, OpenCode (a non-database store) and Grok each throw the typed error, and a missing file is still `[]`. +- **`pi.system.test.ts`:** the same for Pi. +- **`service.system.test.ts`:** View Prompts rejects, and search still finds another conversation. +- **`ViewPromptsModal.renderer.test.tsx`:** fixed words, no IPC text. +- All red on main. + +## Out of scope +- Discovery's own handling of unreadable files (it already counts them as `unreadable`). + +## Steering q116: finding the file is part of the rule +- **The discovery step still mapped every failure to "not here".** That covered Claude's direct `stat` and the projects walk, Codex's rollout walk, and Pi's session listing, and it ran before the typed read handling, so a transient EACCES/EIO on a known conversation still read as "no prompts". Each now treats only ENOENT/ENOTDIR as absence and raises `ConversationPromptsUnreadable` for anything else. +- **The Codex and Pi packages swallow their own `readdir` errors.** They answer "none", so each source probes its root directories first: Codex's `sessions`, and Pi's sessions root and per-cwd directory. The packages are unchanged. +- **Tests (fail then recover, through `prompts()`):** an unlistable Claude project directory, then a successful read; an unlistable Codex sessions tree, then "no such thread" (a missing tree stays `[]`); an unlistable Pi session directory, then `[]`. All three are red on the previous head. +- **History:** the branch is rebuilt so its first commit holds only this plan (worker-common line 33). From 5cee073ef931ed6bcdd30b62fd0dd6db1afb2398 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 07:22:00 -0700 Subject: [PATCH 12/43] fix(conversations): an unreadable conversation file is said, not 'no prompts' (#1306) Every source (Claude, Codex, OpenCode, Pi, Grok) raises a typed ConversationPromptsUnreadable when a transcript or store is there but cannot be read or listed; only genuine absence answers []. promptFolder rethrows non-ENOENT errors; View Prompts says it in fixed words; search degrades that one row. Steering q116: discovery (stat/readdir walks) follows the same rule. Co-Authored-By: Claude Opus 5.5 --- .../conversations/prompts/promptFolder.ts | 18 ++- src/main/conversations/service.system.test.ts | 29 +++- src/main/conversations/service.ts | 7 +- src/main/conversations/sources/claude.ts | 41 +++-- src/main/conversations/sources/codex.ts | 23 ++- src/main/conversations/sources/grok.ts | 8 +- src/main/conversations/sources/opencode.ts | 14 +- .../conversations/sources/pi.system.test.ts | 20 ++- src/main/conversations/sources/pi.ts | 51 +++++- .../sources/promptsUnreadable.system.test.ts | 151 ++++++++++++++++++ src/main/conversations/sources/types.ts | 24 +++ .../ui/ViewPromptsModal.renderer.test.tsx | 16 ++ .../workspace/ui/ViewPromptsModal.tsx | 9 +- 13 files changed, 374 insertions(+), 37 deletions(-) create mode 100644 src/main/conversations/sources/promptsUnreadable.system.test.ts diff --git a/src/main/conversations/prompts/promptFolder.ts b/src/main/conversations/prompts/promptFolder.ts index 20e75ba59..41ed01edc 100644 --- a/src/main/conversations/prompts/promptFolder.ts +++ b/src/main/conversations/prompts/promptFolder.ts @@ -202,9 +202,14 @@ async function extractPromptsUnlocked( const st = await stat(file) size = st.size mtime = st.mtime.getTime() - } catch { + } catch (error) { span.end({ result: 'stat-failed' }) - return { prompts: [], cwd: '' } + // WHY only a MISSING file is "no prompts" (#1306): a file that is there + // but cannot be read is not an empty conversation. The error propagates + // to the source, which says it (View Prompts) or skips the row (search). + const code = (error as NodeJS.ErrnoException | undefined)?.code + if (code === 'ENOENT' || code === 'ENOTDIR') return { prompts: [], cwd: '' } + throw error } if ( entry && @@ -258,9 +263,14 @@ async function extractPromptsUnlocked( entry.cwd = await readHeadCwd(file, size) entry.headCwdChecked = true } - } catch { + } catch (error) { span.end({ result: 'read-failed' }) - return { prompts: [], cwd: '' } + // A file that vanished between stat and read is missing; anything else + // (a permission change, an I/O error) is unreadable and propagates + // (#1306). The cache entry is not updated on this path. + const code = (error as NodeJS.ErrnoException | undefined)?.code + if (code === 'ENOENT') return { prompts: [], cwd: '' } + throw error } entry.mtime = mtime entry.size = size diff --git a/src/main/conversations/service.system.test.ts b/src/main/conversations/service.system.test.ts index 31750b88c..9381f946a 100644 --- a/src/main/conversations/service.system.test.ts +++ b/src/main/conversations/service.system.test.ts @@ -1,11 +1,14 @@ +import { appendFile, chmod } from 'node:fs/promises' import { join } from 'node:path' -import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' import { ClaudeHistoryIndex } from './sources/claudeHistory.js' import { ClaudeConversationSource } from './sources/claude.js' import { CodexConversationSource } from './sources/codex.js' import { OpencodeConversationSource } from './sources/opencode.js' import { ConversationService } from './service.js' +import { ConversationPromptsUnreadable } from './sources/types.js' +import { sanitizePath } from '@shared/runtime/projectDir.js' import { corpusWorktreesPorcelain, installConversationCorpus, type InstalledCorpus } from '../../../testing/support/conversations/installCorpus.js' // One corpus install per file: the install copies 450 files and costs more @@ -60,6 +63,30 @@ describe('ConversationService', () => { expect(children.every(c => c.parentNativeId === codexParent!.nativeId)).toBe(true) }) + // #1306: an unreadable conversation file now throws from its source. View + // Prompts rejects with the typed error; search degrades that one row to + // label-only and still lists the rest. + it('rejects View Prompts for an unreadable file, and search still lists', async () => { + const s = service() + const all = await s.list({ cwd: '/fixture/repo', scope: 'repository', includeChildren: true, limit: 5000 }) + const claudeRows = all.rows.filter(r => r.provider === 'claude' && (r.promptCount ?? 0) > 1 && r.cwd) + const row = claudeRows[0]! + const other = claudeRows[1]! + const needle = (await s.prompts({ provider: 'claude', nativeId: other.nativeId, cwd: other.cwd! }))[0]!.text.slice(0, 10) + const file = join(corpus.claudeConfigDir, 'projects', sanitizePath(row.cwd!), `${row.nativeId}.jsonl`) + await appendFile(file, '\n') + await chmod(file, 0o000) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + await expect(s.prompts({ provider: 'claude', nativeId: row.nativeId, cwd: row.cwd! })).rejects.toBeInstanceOf(ConversationPromptsUnreadable) + const hits = await s.list({ cwd: '/fixture/repo', scope: 'repository', query: needle, includeChildren: true, limit: 5000 }) + expect(hits.rows.some(r => r.nativeId === other.nativeId)).toBe(true) + } finally { + await chmod(file, 0o600) + warn.mockRestore() + } + }) + it('serves a second listing from cache without re-discovering within the freshness window', async () => { const s = service() const first = await s.list({ cwd: '/fixture/repo', scope: 'repository', limit: 20 }) diff --git a/src/main/conversations/service.ts b/src/main/conversations/service.ts index 374a096a4..f582a17bc 100644 --- a/src/main/conversations/service.ts +++ b/src/main/conversations/service.ts @@ -174,7 +174,12 @@ export class ConversationService { async prompts(request: ConversationPromptsRequest): Promise { const source = this.source(request.provider) if (!source) return [] - const raw = await source.prompts(request.nativeId, request.cwd, { need: 'all' }) + const raw = await source.prompts(request.nativeId, request.cwd, { need: 'all' }).catch((error: unknown) => { + // #1306: the cause (a path, an OS code) stays in the main log; the + // renderer gets the typed error and says it in fixed words. + console.warn(`[conversations] ${request.provider} prompts unreadable:`, (error as { cause?: unknown }).cause ?? error) + throw error + }) // The folder reports wrappers verbatim; the prompt list shows what the // user typed, so unwrap here and drop injected messages. return raw.flatMap(p => { diff --git a/src/main/conversations/sources/claude.ts b/src/main/conversations/sources/claude.ts index e48be53d2..7b0fa19b1 100644 --- a/src/main/conversations/sources/claude.ts +++ b/src/main/conversations/sources/claude.ts @@ -8,7 +8,7 @@ import { streamJsonl } from '@shared/runtime/streamJsonl.js' import { performanceService } from '@main/performance/PerformanceService.js' import { extractPromptsFromFile } from '@main/conversations/prompts/promptFolder.js' import type { ClaudeHistoryIndex } from './claudeHistory.js' -import type { ConversationSource, SourceConversation, SourceScope, PromptReadOptions } from './types.js' +import { ConversationPromptsUnreadable, isMissingFileError, type ConversationSource, type SourceConversation, type SourceScope, type PromptReadOptions } from './types.js' // Claude Code stores one directory per cwd under ~/.claude/projects, named by // sanitizePath(cwd), and one `.jsonl` per session inside it. There is @@ -343,29 +343,44 @@ export class ClaudeConversationSource implements ConversationSource { async prompts(nativeId: string, cwd: string, options: PromptReadOptions = {}): Promise { const direct = join(this.deps.projectsDir, sanitizePath(cwd), `${nativeId}.jsonl`) let file: string | null = null + // WHY only ENOENT/ENOTDIR mean "not here" (#1306, steering q116): every + // other stat or readdir failure (EACCES, EIO) is UNKNOWN, and answering + // [] for it is the "no prompts" this issue is about. + const notHere = (error: unknown): boolean => { + if (isMissingFileError(error)) return true + throw new ConversationPromptsUnreadable('claude', error) + } try { await stat(direct) file = direct - } catch { + } catch (error) { + notHere(error) // A conversation listed from a worktree dir is asked for with its own // cwd, so the direct path is the common case; the walk is the rare one. + let dirs: string[] = [] try { - for (const dir of await readdir(this.deps.projectsDir)) { - const candidate = join(this.deps.projectsDir, dir, `${nativeId}.jsonl`) - try { - await stat(candidate) - file = candidate - break - } catch { - // keep looking - } + dirs = await readdir(this.deps.projectsDir) + } catch (listError) { + notHere(listError) + } + for (const dir of dirs) { + const candidate = join(this.deps.projectsDir, dir, `${nativeId}.jsonl`) + try { + await stat(candidate) + file = candidate + break + } catch (candidateError) { + notHere(candidateError) } - } catch { - file = null } } if (!file) return [] + // #1306: a found file that cannot be read is said, typed, not raw. const { prompts } = await extractPromptsFromFile('claude', nativeId, file, options.need ?? 'all', { maxBytes: options.maxBytes }) + .catch((error: unknown) => { + if (isMissingFileError(error)) return { prompts: [] } + throw new ConversationPromptsUnreadable('claude', error) + }) return prompts.map(p => ({ text: p.text, timestamp: p.ts })) } } diff --git a/src/main/conversations/sources/codex.ts b/src/main/conversations/sources/codex.ts index 5a25536ca..ff81b4607 100644 --- a/src/main/conversations/sources/codex.ts +++ b/src/main/conversations/sources/codex.ts @@ -9,7 +9,7 @@ import { performanceService } from '@main/performance/PerformanceService.js' import { extractPromptsFromFile } from '@main/conversations/prompts/promptFolder.js' import { findCodexRolloutPathByThreadId } from 'codex-headless' import { newestCodexStateDb, openReadOnlySqlite } from './sqlite.js' -import type { ConversationSource, SourceConversation, SourceScope, PromptReadOptions } from './types.js' +import { ConversationPromptsUnreadable, isMissingFileError, type ConversationSource, type SourceConversation, type SourceScope, type PromptReadOptions } from './types.js' // Codex keeps its own index at ~/.codex/state_N.sqlite (`threads`, // `thread_spawn_edges`), maintained by the CLI and backfilled from rollouts. @@ -316,14 +316,31 @@ export class CodexConversationSource implements ConversationSource { } } if (!file) { + const sessionsDir = join(this.deps.codexHome, 'sessions') + // The package's walk swallows its own readdir errors (it answers null), + // so an unreadable sessions tree would still read as "no prompts". Probe + // the root here: absent is "not here", anything else is unknown. try { - file = await findCodexRolloutPathByThreadId(join(this.deps.codexHome, 'sessions'), nativeId) - } catch { + await readdir(sessionsDir) + } catch (error) { + if (!isMissingFileError(error)) throw new ConversationPromptsUnreadable('codex', error) + } + try { + file = await findCodexRolloutPathByThreadId(sessionsDir, nativeId) + } catch (error) { + // Only absence is "not here" (#1306, steering q116): a rollout walk + // that fails for any other reason is unknown, never "no prompts". + if (!isMissingFileError(error)) throw new ConversationPromptsUnreadable('codex', error) file = null } } if (!file) return [] + // #1306: a found rollout that cannot be read is said, typed, not raw. const { prompts } = await extractPromptsFromFile('codex', nativeId, file, options.need ?? 'all', { maxBytes: options.maxBytes }) + .catch((error: unknown) => { + if (isMissingFileError(error)) return { prompts: [] } + throw new ConversationPromptsUnreadable('codex', error) + }) return prompts.map(p => ({ text: p.text, timestamp: p.ts })) } } diff --git a/src/main/conversations/sources/grok.ts b/src/main/conversations/sources/grok.ts index b279c205c..9fed911b1 100644 --- a/src/main/conversations/sources/grok.ts +++ b/src/main/conversations/sources/grok.ts @@ -7,7 +7,7 @@ import { loadGrokSnapshotAt } from '@main/providerSwitch/grokTranscript.js' import type { ConversationPrompt } from '@shared/conversations/types.js' import { performanceService } from '@main/performance/PerformanceService.js' -import type { ConversationSource, SourceConversation, SourceScope } from './types.js' +import { ConversationPromptsUnreadable, isMissingFileError, type ConversationSource, type SourceConversation, type SourceScope } from './types.js' // Grok conversation discovery: native's own sessions index. Every session // directory under the Grok home carries a summary.json (id, cwd, titles, @@ -55,8 +55,10 @@ export class GrokConversationSource implements ConversationSource { let snapshot: Awaited> try { snapshot = await loadGrokSnapshotAt(resolveGrokTranscriptPath(cwd, nativeId, this.deps.grokHome)) - } catch { - return [] + } catch (error) { + // #1306: only a missing file is "no prompts"; a damaged one is said. + if (isMissingFileError(error)) return [] + throw new ConversationPromptsUnreadable('grok', error) } const texts: string[] = [] for (const entry of snapshot.conversation.entries) { diff --git a/src/main/conversations/sources/opencode.ts b/src/main/conversations/sources/opencode.ts index 89b3a391f..95f236011 100644 --- a/src/main/conversations/sources/opencode.ts +++ b/src/main/conversations/sources/opencode.ts @@ -1,3 +1,4 @@ +import { existsSync } from 'node:fs' import { homedir } from 'node:os' import { join } from 'node:path' @@ -5,7 +6,7 @@ import type { ConversationPrompt } from '@shared/conversations/types.js' import { parseJsonRecord } from '@shared/lib/asRecord.js' import { performanceService } from '@main/performance/PerformanceService.js' import { openReadOnlySqlite } from './sqlite.js' -import type { ConversationSource, SourceConversation, SourceScope } from './types.js' +import { ConversationPromptsUnreadable, type ConversationSource, type SourceConversation, type SourceScope } from './types.js' // OpenCode keeps every session in ~/.local/share/opencode/opencode.db: // `session` (id, parent_id, directory, title, time_created, time_updated, @@ -133,8 +134,15 @@ export class OpencodeConversationSource implements ConversationSource { // discovery reads gives every user text part in one statement. Rewind keeps // the export because it needs the export's message positions as addresses. async prompts(nativeId: string, _cwd: string): Promise { - const opened = openReadOnlySqlite(join(this.deps.dataDir, 'opencode.db'), OPENCODE_COLUMNS) - if (!opened.ok) return [] + const dbPath = join(this.deps.dataDir, 'opencode.db') + const opened = openReadOnlySqlite(dbPath, OPENCODE_COLUMNS) + if (!opened.ok) { + // #1306: no database at all is "no prompts"; one that is there but + // cannot be opened or read is said (the reason names the path, so it + // stays on the error's cause, never in the message). + if (!existsSync(dbPath)) return [] + throw new ConversationPromptsUnreadable('opencode', new Error(opened.reason)) + } try { const rows = opened.db.prepare( `select p.data as part, m.data as message, m.time_created as created from part p join message m on m.id = p.message_id diff --git a/src/main/conversations/sources/pi.system.test.ts b/src/main/conversations/sources/pi.system.test.ts index 31fa93757..a44b0928a 100644 --- a/src/main/conversations/sources/pi.system.test.ts +++ b/src/main/conversations/sources/pi.system.test.ts @@ -1,4 +1,4 @@ -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { basename, join } from 'node:path' @@ -8,6 +8,7 @@ import { encodeCwdForSessionDir } from 'pi-terminal-headless' import { resolveFamily } from '../family.js' import { PiConversationSource } from './pi.js' +import { ConversationPromptsUnreadable } from './types.js' // The Pi conversation source over Stage 0 recordings of the real pi 0.87.1, // laid out the way pi lays them out: one directory per cwd under @@ -77,6 +78,23 @@ describe('Pi conversation source', () => { expect(await source.prompts('no-such-session', repo)).toEqual([]) }) + // #1306: a session file that is THERE but cannot be read used to answer + // [] like a missing one, so View Prompts said "no prompts". A missing file + // still answers [] (a new session has none yet); an unreadable one throws. + it('says an unreadable session file instead of answering no prompts', async () => { + const { home, place } = sandbox() + const repo = join(home, 'repo') + const tree = place(Object.values(loadLiveFixture('tree').files)[0]!, repo) + const source = new PiConversationSource({ env: {}, homeDirectory: home }) + chmodSync(tree.file, 0o000) + try { + await expect(source.prompts(tree.id, repo)).rejects.toBeInstanceOf(ConversationPromptsUnreadable) + } finally { + chmodSync(tree.file, 0o600) + } + expect(await source.prompts('no-such-session', repo)).toEqual([]) + }) + it('a /name the user gave the session is its custom title', async () => { const { home, place } = sandbox() const repo = join(home, 'repo') diff --git a/src/main/conversations/sources/pi.ts b/src/main/conversations/sources/pi.ts index 1cfc91444..6b864b606 100644 --- a/src/main/conversations/sources/pi.ts +++ b/src/main/conversations/sources/pi.ts @@ -1,11 +1,11 @@ -import { open, stat } from 'node:fs/promises' +import { open, readdir, stat } from 'node:fs/promises' import { basename } from 'node:path' -import { listAllPiSessionFiles, readPiBranch, resolvePiSessionFile, sessionIdFromFileName } from 'pi-terminal-headless' +import { listAllPiSessionFiles, readPiBranch, resolvePiSessionDir, resolvePiSessionFile, resolvePiSessionsRoot, sessionIdFromFileName } from 'pi-terminal-headless' import type { ConversationPrompt } from '@shared/conversations/types.js' import { performanceService } from '@main/performance/PerformanceService.js' -import type { ConversationSource, SourceConversation, SourceScope } from './types.js' +import { ConversationPromptsUnreadable, isMissingFileError, type ConversationSource, type SourceConversation, type SourceScope } from './types.js' // Pi conversation discovery, straight from Pi's session files — the same // files pi's own /resume picker lists. There is no separate index: each @@ -124,8 +124,42 @@ export class PiConversationSource implements ConversationSource { /** Every user prompt on the session's active branch, newest first. */ async prompts(nativeId: string, cwd: string): Promise { try { - const file = await resolvePiSessionFile({ env: this.deps.env ?? process.env, cwd, sessionId: nativeId, ...(this.deps.homeDirectory ? { homeDirectory: this.deps.homeDirectory } : {}) }) - if (!file) return [] + const env = { env: this.deps.env ?? process.env, ...(this.deps.homeDirectory ? { homeDirectory: this.deps.homeDirectory } : {}) } + // The package's listings swallow their own readdir errors (they answer + // "none"), so probe the two directories here (#1306, steering q116): + // absent is "not here", anything else is unknown, never "no prompts". + for (const dir of [(await resolvePiSessionsRoot(env)).root, await resolvePiSessionDir({ ...env, cwd })]) { + try { + await readdir(dir) + } catch (error) { + if (!isMissingFileError(error)) throw new ConversationPromptsUnreadable('pi', error) + } + } + const file = await resolvePiSessionFile({ ...env, cwd, sessionId: nativeId }) + if (!file) { + // WHY look again (#1306): resolution mirrors Pi, which skips a file + // whose header it cannot read, so an UNREADABLE session reads as "no + // such session". A file named for this session that cannot even be + // opened is that case, and is said; a readable one whose header + // simply does not match this cwd is genuinely not this session. + // A listing that fails for anything but absence is unknown, never + // "no prompts" (steering q116). + const named = (await listAllPiSessionFiles(env).catch((error: unknown) => { + if (isMissingFileError(error)) return [] as string[] + throw new ConversationPromptsUnreadable('pi', error) + })) + .find(candidate => sessionIdFromFileName(basename(candidate)) === nativeId) + if (named) { + let handle + try { + handle = await open(named, 'r') + } catch (error) { + throw new ConversationPromptsUnreadable('pi', error) + } + await handle.close() + } + return [] + } const { rows } = await readPiBranch(file) const prompts: ConversationPrompt[] = [] for (const row of rows) { @@ -139,8 +173,11 @@ export class PiConversationSource implements ConversationSource { if (text.trim()) prompts.push({ text, timestamp: typeof message.timestamp === 'number' ? message.timestamp : null }) } return prompts.reverse() - } catch { - return [] + } catch (error) { + // #1306: only a missing file is "no prompts"; a damaged one is said. + if (error instanceof ConversationPromptsUnreadable) throw error + if (isMissingFileError(error)) return [] + throw new ConversationPromptsUnreadable('pi', error) } } diff --git a/src/main/conversations/sources/promptsUnreadable.system.test.ts b/src/main/conversations/sources/promptsUnreadable.system.test.ts new file mode 100644 index 000000000..4d4715bd0 --- /dev/null +++ b/src/main/conversations/sources/promptsUnreadable.system.test.ts @@ -0,0 +1,151 @@ +import { appendFile, chmod, mkdir, writeFile } from 'node:fs/promises' +import { join } from 'node:path' + +import { existsSync } from 'node:fs' + +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' +import { resolveGrokTranscriptPath } from 'grok-code-headless' +import { encodeCwdForSessionDir } from 'pi-terminal-headless' + +import { sanitizePath } from '@shared/runtime/projectDir.js' +import { resolveFamily } from '../family.js' +import { ClaudeHistoryIndex } from './claudeHistory.js' +import { ClaudeConversationSource } from './claude.js' +import { CodexConversationSource } from './codex.js' +import { PiConversationSource } from './pi.js' +import { GrokConversationSource } from './grok.js' +import { OpencodeConversationSource } from './opencode.js' +import { ConversationPromptsUnreadable } from './types.js' +import { corpusWorktreesPorcelain, installConversationCorpus } from '../../../../testing/support/conversations/installCorpus.js' + +// #1306: a conversation whose transcript or store is THERE but unreadable +// answered [] from every source, so View Prompts said "no prompts" for a +// damaged file. Each source now throws ConversationPromptsUnreadable for it, +// and a missing file still answers [] (a new session has none yet). The Pi +// source is pinned in pi.system.test.ts. +// +// Real files: the recorded conversation corpus, made unreadable with chmod +// (or, for OpenCode's store, replaced by bytes that are not a database). + +const cleanups: Array<() => Promise> = [] +let corpus: Awaited> +let listWorktrees: () => Promise> +beforeAll(async () => { + corpus = await installConversationCorpus() + cleanups.push(corpus.cleanup) + const porcelain = await corpusWorktreesPorcelain() + const worktrees = porcelain.split('\n').filter(l => l.startsWith('worktree ')).map(l => ({ path: l.slice('worktree '.length) })) + listWorktrees = async () => worktrees +}) +afterAll(async () => { for (const c of cleanups.splice(0)) await c() }) + +// The prompt folder caches by size and mtime, and a cached answer for an +// unchanged file is correct. The file changes first (as a live transcript +// does), so the read after it is a real read of an unreadable file. +async function unreadable(file: string, run: () => Promise): Promise { + await appendFile(file, '\n') + await chmod(file, 0o000) + try { + return await run() + } finally { + await chmod(file, 0o600) + } +} + +describe('a conversation file that is there but unreadable', () => { + it('Claude says so; a missing one is still no prompts', async () => { + const history = new ClaudeHistoryIndex(join(corpus.claudeConfigDir, 'history.jsonl')) + await history.refresh() + const source = new ClaudeConversationSource({ projectsDir: join(corpus.claudeConfigDir, 'projects'), history }) + const family = await resolveFamily('/fixture/repo', 'cwd', { listWorktrees }) + const row = (await source.discover({ scope: 'cwd', family })).find(r => (r.promptCount ?? 0) > 0 && r.cwd)! + const file = join(corpus.claudeConfigDir, 'projects', sanitizePath(row.cwd!), `${row.nativeId}.jsonl`) + await expect(unreadable(file, () => source.prompts(row.nativeId, row.cwd!))).rejects.toBeInstanceOf(ConversationPromptsUnreadable) + expect(await source.prompts('00000000-0000-0000-0000-000000000000', row.cwd!)).toEqual([]) + }) + + it('Codex says so for a known rollout', async () => { + const source = new CodexConversationSource({ codexHome: corpus.codexHome }) + const family = await resolveFamily('/fixture/repo', 'repository', { listWorktrees }) + const rows = await source.discover({ scope: 'repository', family }) + const paths = (source as unknown as { rolloutPaths: Map }).rolloutPaths + // A rollout the corpus actually holds (some recorded paths point elsewhere). + const row = rows.find(r => paths.has(r.nativeId) && existsSync(paths.get(r.nativeId)!))! + const file = paths.get(row.nativeId)! + expect(file).toBeTruthy() + await expect(unreadable(file, () => source.prompts(row.nativeId, row.cwd ?? ''))).rejects.toBeInstanceOf(ConversationPromptsUnreadable) + }) + + it('OpenCode says so when its store is not a database; no store at all is no prompts', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + try { + const damaged = join(corpus.opencodeDataDir, '..', 'opencode-damaged') + await mkdir(damaged, { recursive: true }) + await writeFile(join(damaged, 'opencode.db'), 'this is not an sqlite database') + await expect(new OpencodeConversationSource({ dataDir: damaged }).prompts('ses_x', '/fixture/repo')).rejects.toBeInstanceOf(ConversationPromptsUnreadable) + expect(await new OpencodeConversationSource({ dataDir: join(damaged, 'absent') }).prompts('ses_x', '/fixture/repo')).toEqual([]) + } finally { + warn.mockRestore() + } + }) + + it('Grok says so; a missing one is still no prompts', async () => { + const grokHome = join(corpus.opencodeDataDir, '..', 'grok-home') + // Grok resolves the cwd's real path, so it must exist. + const cwd = join(corpus.opencodeDataDir, '..', 'grok-project') + await mkdir(cwd, { recursive: true }) + const id = '11111111-1111-4111-8111-111111111111' + const file = resolveGrokTranscriptPath(cwd, id, grokHome) + await mkdir(join(file, '..'), { recursive: true }) + // A session the loader accepts: its summary names it, format version 1. + await writeFile(join(file, '..', 'summary.json'), JSON.stringify({ info: { id, cwd }, chat_format_version: 1 })) + await writeFile(file, '') + const source = new GrokConversationSource({ grokHome }) + await expect(unreadable(file, () => source.prompts(id, cwd))).rejects.toBeInstanceOf(ConversationPromptsUnreadable) + expect(await source.prompts('22222222-2222-4222-8222-222222222222', cwd)).toEqual([]) + }) +}) + +// Steering q116: the step BEFORE the read (finding the file) also mapped every +// failure to "not here". A stat or readdir that fails for anything but absence +// is unknown and is said; after the directory recovers, the same call reads. +describe('finding a conversation file that cannot be listed', () => { + async function unlisted(dir: string, run: () => Promise): Promise { + await chmod(dir, 0o000) + try { + return await run() + } finally { + await chmod(dir, 0o700) + } + } + + it('Claude: an unlistable project directory is unreadable, then reads once it recovers', async () => { + const history = new ClaudeHistoryIndex(join(corpus.claudeConfigDir, 'history.jsonl')) + await history.refresh() + const source = new ClaudeConversationSource({ projectsDir: join(corpus.claudeConfigDir, 'projects'), history }) + const family = await resolveFamily('/fixture/repo', 'cwd', { listWorktrees }) + const row = (await source.discover({ scope: 'cwd', family })).find(r => (r.promptCount ?? 0) > 1 && r.cwd)! + const dir = join(corpus.claudeConfigDir, 'projects', sanitizePath(row.cwd!)) + await expect(unlisted(dir, () => source.prompts(row.nativeId, row.cwd!))).rejects.toBeInstanceOf(ConversationPromptsUnreadable) + expect((await source.prompts(row.nativeId, row.cwd!)).length).toBeGreaterThan(0) + }) + + it('Codex: an unlistable sessions tree is unreadable; a missing one is no prompts', async () => { + const codexHome = join(corpus.opencodeDataDir, '..', 'codex-unlistable') + await mkdir(join(codexHome, 'sessions'), { recursive: true }) + const source = new CodexConversationSource({ codexHome }) + await expect(unlisted(join(codexHome, 'sessions'), () => source.prompts('019-thread', ''))).rejects.toBeInstanceOf(ConversationPromptsUnreadable) + expect(await source.prompts('019-thread', '')).toEqual([]) + expect(await new CodexConversationSource({ codexHome: join(codexHome, 'absent') }).prompts('019-thread', '')).toEqual([]) + }) + + it('Pi: an unlistable session directory is unreadable, then answers once it recovers', async () => { + const home = join(corpus.opencodeDataDir, '..', 'pi-home') + const cwd = join(home, 'project') + const dir = join(home, '.pi', 'agent', 'sessions', encodeCwdForSessionDir(cwd)) + await mkdir(dir, { recursive: true }) + const source = new PiConversationSource({ env: {}, homeDirectory: home }) + await expect(unlisted(dir, () => source.prompts('session-1', cwd))).rejects.toBeInstanceOf(ConversationPromptsUnreadable) + expect(await source.prompts('session-1', cwd)).toEqual([]) + }) +}) diff --git a/src/main/conversations/sources/types.ts b/src/main/conversations/sources/types.ts index 54f811a25..b24ef0563 100644 --- a/src/main/conversations/sources/types.ts +++ b/src/main/conversations/sources/types.ts @@ -48,6 +48,30 @@ export type PromptReadOptions = { maxBytes?: number } +/** + * A conversation whose transcript or store EXISTS but cannot be read (#1306). + * Sources used to answer `[]` for it, so View Prompts said "no prompts" for a + * damaged file. The rule every source follows: + * - no file (yet): `[]`. A freshly started session has no transcript, and + * View Prompts then shows the live feed's prompts; that is not a failure; + * - a file or store that is there but unreadable: throw this. + * Search catches it per conversation (label-only search for that row); View + * Prompts surfaces it. The cause stays on the error for the main-side log; the + * message is fixed, because it crosses IPC to the UI (q22). + */ +export class ConversationPromptsUnreadable extends Error { + constructor(readonly provider: string, readonly cause: unknown) { + super('The conversation file could not be read') + this.name = 'ConversationPromptsUnreadable' + } +} + +/** True for the "no such file" family: a missing transcript is not a failure. */ +export function isMissingFileError(error: unknown): boolean { + const code = (error as NodeJS.ErrnoException | undefined)?.code + return code === 'ENOENT' || code === 'ENOTDIR' +} + export interface ConversationSource { readonly provider: AgentProviderKind discover(scope: SourceScope): Promise diff --git a/src/renderer/src/features/workspace/ui/ViewPromptsModal.renderer.test.tsx b/src/renderer/src/features/workspace/ui/ViewPromptsModal.renderer.test.tsx index f64396a2d..195dd4d65 100644 --- a/src/renderer/src/features/workspace/ui/ViewPromptsModal.renderer.test.tsx +++ b/src/renderer/src/features/workspace/ui/ViewPromptsModal.renderer.test.tsx @@ -37,6 +37,22 @@ describe('ViewPromptsModal', () => { expect(listConversationPrompts).toHaveBeenCalledWith({ provider: 'claude', nativeId: 'native-1', cwd: '/repo/project' }) }) + // #1306: main now rejects when the conversation file is there but + // unreadable (it used to answer "no prompts"). The modal says so in fixed + // words, never the IPC text (q22). + it('says an unreadable conversation in fixed words, not the IPC text', async () => { + Object.defineProperty(window, 'api', { configurable: true, value: { + listConversationPrompts: vi.fn(async () => { throw new Error("Error invoking remote method 'conversations:prompts': ConversationPromptsUnreadable: The conversation file could not be read") }), + } }) + const workspace = { + state: { sessions: { s: { kind: 'pi', cwd: '/repo/project', providerSessionId: 'native-1' } } }, + getRuntime: () => ({ entries: [], hasOlderHistory: false, loadingOlderHistory: false }), + } as unknown as Workspace + render() + expect(await screen.findByText("Couldn't read this conversation's prompts.")).toBeInTheDocument() + expect(document.body.textContent).not.toContain('Error invoking remote method') + }) + it('focuses a scroller that is a Tab stop, and closes from one Close ⎋ button', async () => { // Plan S8: the read-only viewer's scroller was tabIndex -1, so after // tabbing to the footer Shift+Tab could never get back to scrolling; the diff --git a/src/renderer/src/features/workspace/ui/ViewPromptsModal.tsx b/src/renderer/src/features/workspace/ui/ViewPromptsModal.tsx index 5461d701b..9e5250221 100644 --- a/src/renderer/src/features/workspace/ui/ViewPromptsModal.tsx +++ b/src/renderer/src/features/workspace/ui/ViewPromptsModal.tsx @@ -34,6 +34,9 @@ type Props = { onClose: () => void } + +export const PROMPTS_UNREADABLE = "Couldn't read this conversation's prompts." + export function ViewPromptsModal({ open, sessionId, @@ -67,7 +70,11 @@ export function ViewPromptsModal({ .catch((error: unknown) => { if (cancelled) return setFromDisk(null) - setLoadError(error instanceof Error && error.message.length > 0 ? error.message : 'Could not read prompts.') + // Fixed words (q22, #1306): the rejection is IPC text, and main's + // cause can name a path. Main now rejects when the conversation file + // is there but unreadable, instead of answering "no prompts". + void error + setLoadError(PROMPTS_UNREADABLE) }) .finally(() => { if (!cancelled) setLoading(false) From 4345b4fb7e7cfc79d3feda19a91b0ca30667aa76 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 08:20:34 -0700 Subject: [PATCH 13/43] docs(cli-updates): plan an update that cannot start saying so (#1425) Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-cli-update-could-not-start.md | 81 +++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 docs/plans/2026-09-27-cli-update-could-not-start.md diff --git a/docs/plans/2026-09-27-cli-update-could-not-start.md b/docs/plans/2026-09-27-cli-update-could-not-start.md new file mode 100644 index 000000000..69bb4a547 --- /dev/null +++ b/docs/plans/2026-09-27-cli-update-could-not-start.md @@ -0,0 +1,81 @@ +# An update that cannot start says so and stays dismissable (#1425) + +Size: short plan. The root cause is known and the change is bounded +(found by #1423's review b). + +## Outcome + +When **Update Now** or an automatic update cannot start, the CLI update +banner stops showing an undismissable "Updating…" row forever. It shows a +dismissable failure in fixed words, with **Update now** to retry. A click +whose IPC call rejects is also said instead of dropped. + +## Evidence (verified 2026-09-27, do not re-derive) + +- `CliUpdateOrchestrator.runUpdate` publishes `updating`, then awaits + `openLog(cli)`, which `mkdir`s `CLI_UPDATE_LOG_DIR` outside any catch. + An EACCES or ENOSPC there rejects, and the snapshot stays `updating`. + `describeState` marks `updating` as `undismissable`. +- Every later await in `runUpdate` already swallows or returns a result: + - `appendLog` and `appendDiagnostics` catch; + - `readInstalledVersion` returns `{ ok }`; + - the command runs inside a try. + + So `openLog` is the one throw that strands the state. +- `CliUpdateBanner` fires `cliUpdatesUpdateNow` with `void` in both the + `notify` and the user-`deferred` rows. The IPC handler's rejection + (`updateOnce` throwing) is dropped, and nothing is said. +- #1423 already gave banner actions a result contract: `onClick` may + resolve `false`, and the row then shows `failureText`, keyed by + `resultKey`. +- `failed.logPath` is read only by the `cli-updates:open-log` handler and + the banner's `resultKey`. + +## Change + +- `@shared/types/cliUpdate`: + - `failed.logPath: string | null` (null = there is no log, because the + update never started); + - `CliUpdateFailureReason` adds `'could-not-start'`. +- `runUpdate`: `openLog` runs in a try. On rejection it warns the cause + in main's log (never the renderer; q22) and publishes + `failed { reason: 'could-not-start', logPath: null }`, then returns + without running the command. + - Ruling: do not run the update without a log. A state dir we cannot + write to (ENOSPC, EACCES) makes a package install likely to fail too, + and a failure with no log would leave the user nothing to look at. + Cost if wrong: one retry after the user fixes the disk. +- `cli-updates:open-log`: a `failed` state with `logPath === null` opens + nothing and answers `false`. The banner shows no View Log for it anyway. +- Banner: + - `could-not-start` reads: "Couldn't start the