From 577630fec4e96a65986788b39f128543a898ee5c Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 03:19:31 -0700 Subject: [PATCH 1/2] fix(debug): a fresh session's bundle falls back to its own shell- proxy run (#1336) Co-Authored-By: Claude Opus 5.5 --- .../debug/saveDebugBundle.renderer.test.ts | 49 +++++++++++++++++++ .../src/features/debug/saveDebugBundle.ts | 27 +++++++--- 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts b/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts index 5d6124223..a6c9f1ff0 100644 --- a/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts +++ b/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts @@ -44,3 +44,52 @@ it('saves the screen and tail history main recorded into the bundle', async () = const state = JSON.parse(saved.find(file => file.name.endsWith('state-snapshot.json'))!.content) as { recentScreen: string } expect(state.recentScreen).toBe('main recent') }) + +// #1336 (codex-headless#70 review a, P1): a fresh session's proxy run lives +// under `shell-`, chosen at process start, but once its first turn +// reveals the providerSessionId the bundle asked only for `resume-`. The +// reader answered `match: 'none'` and the bundle had no proxy section. The +// stub answers exactly as readProxyEventsForBundle does for a segment that +// does not exist (match 'none', nulls) and for one that does. +async function bundleWithProxy(providerSessionId: string | null, existingSegment: string) { + const saved: Array<{ name: string; content: string }> = [] + const asked: string[] = [] + const originalApi = window.api + window.api = { + ...originalApi, + flushPerformance: async () => {}, + getPerformanceSnapshot: async () => null, + getScreenDebug: async () => ({ screen: null, samples: [] }), + readProxyEvents: async ({ sessionKey }: { sessionKey: string }) => { + asked.push(sessionKey) + return sessionKey === existingSegment + ? { proxyEvents: '{"kind":"request"}\n{"kind":"request-body-latest"}\n', runDir: `/proxy/p/${sessionKey}/run1`, sessionMeta: null, match: 'exact', requestedSessionKey: sessionKey, matchedSessionSegment: sessionKey } + : { proxyEvents: null, runDir: null, sessionMeta: null, match: 'none', requestedSessionKey: sessionKey, matchedSessionSegment: null } + }, + saveDebugBundle: async (input: { files: Array<{ name: string; content: string }> }) => { + saved.push(...input.files) + return { bundlePath: '/tmp/bundle' } + }, + } as never + try { + await assembleAndSaveDebugBundle({ sessionId: 'pane-1', runtime: emptyRuntime(), kind: 'codex', cwd: '/repo', providerSessionId }) + } finally { + window.api = originalApi + } + const proxyFile = saved.find(file => file.name.includes('proxy') && file.content.includes('request-body-latest')) + const manifest = JSON.parse(saved.find(file => file.name.endsWith('manifest.json'))!.content) as Record + return { asked, proxyFile, manifest } +} + +it('finds a fresh session\'s proxy run after its first turn revealed the provider id', async () => { + const { asked, proxyFile, manifest } = await bundleWithProxy('thread-1', 'shell-pane-1') + expect(asked).toEqual(['resume-thread-1', 'shell-pane-1']) + expect(proxyFile).toBeDefined() + expect(JSON.stringify(manifest)).toContain('shell-pane-1') +}) + +it('prefers the resumed run when the process was launched to resume', async () => { + const { asked, proxyFile } = await bundleWithProxy('thread-1', 'resume-thread-1') + expect(asked).toEqual(['resume-thread-1']) + expect(proxyFile).toBeDefined() +}) diff --git a/src/renderer/src/features/debug/saveDebugBundle.ts b/src/renderer/src/features/debug/saveDebugBundle.ts index 21b69fd79..531680ec5 100644 --- a/src/renderer/src/features/debug/saveDebugBundle.ts +++ b/src/renderer/src/features/debug/saveDebugBundle.ts @@ -504,12 +504,27 @@ export async function assembleAndSaveDebugBundle(params: { // every minute-level autosave was one of the multipliers behind the 108 GB // debug-bundles directory; autosave should preserve orientation, not create // a second archive of already-persisted wire logs. - const proxySection = cwd && includeProxyPayload - ? await window.api.readProxyEvents({ - cwd, - sessionKey: proxySessionKey, - }).catch(() => null) - : null + // + // WHY a second, exact key (#1336, codex-headless#70 review a): the proxy + // writers choose the run's session segment ONCE, at process start: + // `resume-` only when the process was launched to resume a known + // conversation, else `shell-` (Codex: codexSession.ts + // allocateProxyEventsFile; Claude's createProxyServer does the same). A + // FRESH session learns its providerSessionId from its first turn, after the + // run dir already exists under `shell-`, and nothing renames it. + // So every manual bundle of a fresh session that had taken a turn asked + // only for `resume-`, got `match: 'none'`, and carried no proxy + // section at all (no events tail, no latest request body). Both keys name + // THIS pane's own runs, so the fallback keeps the reader's exact-provenance + // rule: never another session's run. `resume-` is tried first because a + // resumed process writes there, and its run is the current one. + const readProxy = (sessionKey: string) => window.api.readProxyEvents({ cwd: cwd!, sessionKey }).catch(() => null) + const shellSessionKey = `shell-${sessionId}` + let proxySection = cwd && includeProxyPayload ? await readProxy(proxySessionKey) : null + if (cwd && includeProxyPayload && proxySessionKey !== shellSessionKey && (!proxySection || proxySection.match === 'none')) { + const fresh = await readProxy(shellSessionKey) + if (fresh && fresh.match !== 'none') proxySection = fresh + } const files: BundleFile[] = [ { From edac97417f940bcbe87bec0e56461f80026128f7 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 03:46:55 -0700 Subject: [PATCH 2/2] fix(debug): correct the provenance claim; pin the fallback's miss and guard branches (review a) Co-Authored-By: Claude Opus 5.5 --- .../debug/saveDebugBundle.renderer.test.ts | 22 ++++++++++++++++++- .../src/features/debug/saveDebugBundle.ts | 14 ++++++++---- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts b/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts index a6c9f1ff0..2048e663b 100644 --- a/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts +++ b/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts @@ -85,7 +85,27 @@ it('finds a fresh session\'s proxy run after its first turn revealed the provide const { asked, proxyFile, manifest } = await bundleWithProxy('thread-1', 'shell-pane-1') expect(asked).toEqual(['resume-thread-1', 'shell-pane-1']) expect(proxyFile).toBeDefined() - expect(JSON.stringify(manifest)).toContain('shell-pane-1') + // The manifest describes the run that was actually read (#1399 review a). + expect(JSON.stringify(manifest)).toContain('"matchedSessionSegment":"shell-pane-1"') + expect(JSON.stringify(manifest)).toContain('"requestedSessionKey":"shell-pane-1"') +}) + +it('asks once, and bundles nothing, when neither key has a run', async () => { + // A `match: 'none'` fallback answer must not replace the first miss with + // an empty section (#1399 review a: this branch was untested). + const { asked, proxyFile, manifest } = await bundleWithProxy('thread-1', 'no-such-segment') + expect(asked).toEqual(['resume-thread-1', 'shell-pane-1']) + expect(proxyFile).toBeUndefined() + expect(JSON.stringify(manifest)).toContain('"requestedSessionKey":"resume-thread-1"') +}) + +it('does not repeat the read for a fresh session with no provider id yet', async () => { + const { asked, proxyFile } = await bundleWithProxy(null, 'shell-pane-1') + expect(asked).toEqual(['shell-pane-1']) + expect(proxyFile).toBeDefined() + // Also when that one key misses: the fallback key is the same key. + const missed = await bundleWithProxy(null, 'no-such-segment') + expect(missed.asked).toEqual(['shell-pane-1']) }) it('prefers the resumed run when the process was launched to resume', async () => { diff --git a/src/renderer/src/features/debug/saveDebugBundle.ts b/src/renderer/src/features/debug/saveDebugBundle.ts index 531680ec5..cdb087cdf 100644 --- a/src/renderer/src/features/debug/saveDebugBundle.ts +++ b/src/renderer/src/features/debug/saveDebugBundle.ts @@ -514,10 +514,16 @@ export async function assembleAndSaveDebugBundle(params: { // run dir already exists under `shell-`, and nothing renames it. // So every manual bundle of a fresh session that had taken a turn asked // only for `resume-`, got `match: 'none'`, and carried no proxy - // section at all (no events tail, no latest request body). Both keys name - // THIS pane's own runs, so the fallback keeps the reader's exact-provenance - // rule: never another session's run. `resume-` is tried first because a - // resumed process writes there, and its run is the current one. + // section at all (no events tail, no latest request body). + // + // The fallback key is pane-keyed, so it can only ever find THIS pane's own + // run. The `resume-` key is NOT (#1399 review a): it names a conversation, + // and another pane that resumed the same conversation writes there too. The + // reader's "exact" is a segment-name match, so a pane whose conversation was + // later resumed elsewhere can still bundle that other pane's run. That risk + // predates this fallback (main asked only for `resume-`); the proper fix is + // recording the launch-time key per pane, #1405. `resume-` stays first so a + // resumed pane (whose own run lives there) keeps what it had on main. const readProxy = (sessionKey: string) => window.api.readProxyEvents({ cwd: cwd!, sessionKey }).catch(() => null) const shellSessionKey = `shell-${sessionId}` let proxySection = cwd && includeProxyPayload ? await readProxy(proxySessionKey) : null