From 92d0ef9e1dc3c5f83090b7bab5b9207204fea7de Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 26 Sep 2026 17:32:07 -0700 Subject: [PATCH 01/19] docs(plan): bound a live Claude session's proxy-events.jsonl (#1273 residual) Refs #1273 Co-Authored-By: Claude Opus 5.5 --- .../2026-09-26-proxy-events-retention.md | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 docs/plans/2026-09-26-proxy-events-retention.md diff --git a/docs/plans/2026-09-26-proxy-events-retention.md b/docs/plans/2026-09-26-proxy-events-retention.md new file mode 100644 index 000000000..553b66a0e --- /dev/null +++ b/docs/plans/2026-09-26-proxy-events-retention.md @@ -0,0 +1,68 @@ +# Bound a live Claude session's proxy-events.jsonl (#1273 residual) + +## Problem + +#1284 / claude-code-headless#62 stopped writing request bodies past 256 MiB per events file. What the +issue still has open: + +1. The file is still unbounded. Responses, stream chunks and body-less request records keep + appending — about 7 % of the old byte rate, ~170 MB per 2.4 GB of old-rate traffic — for as long + as the session lives. +2. A live file is never reclaimed: `debugRetention.ts` skips any run written in the last 10 minutes + (`ACTIVE_GRACE_MS`), and a session that stays open for days keeps its file "live" all that time. + +## Evidence + +- Composition of a real post-#62 events file (owner's machine, run + `…/resume-fa48baff…/2026-09-25T17-34-30-657Z`): `request` lines 98.1 % (bodies until the budget), + `response-chunk` 1.7 %, `response` 0.2 %. Past the budget, chunks/responses/body-less requests are + what keeps growing. +- The file is not just a log: it is the **transport** from mitmdump to the app. `ProxyServer` + polls it every 200 ms from a byte offset and emits each complete line as a live event; the + adapter builds the transcript view from the `response-chunk` lines. So "stop writing chunks past a + budget" would break the live view, and truncating in place would drop or duplicate events. + +## Decisions (defaults) + +- **Rotate in the addon, keep one previous generation.** When the events file reaches + `PROXY_EVENTS_ROTATE_BYTES` (default 512 MiB), the addon renames it to `proxy-events.1.jsonl` + (atomically replacing the older generation) and the next write starts a fresh + `proxy-events.jsonl`. A live run therefore holds at most ~2 × 512 MiB (+ the 16 MiB latest-body + sidecar), instead of growing for the life of the session. Rotation failure is non-fatal: the addon + keeps appending (forensics must never disturb the proxy). + - WHY 512 MiB and one generation: the body budget (256 MiB) still applies per file, so each + generation keeps ~100 turns of bodies plus a long stretch of body-less traffic; the debug bundle + only ever ships the last 5 MiB. Deleting generation 2 is the point of the change. + - WHY rotate on the writer side: mitmdump is the single writer, runs single-threaded, and writes + each line with open-append-close, so after `os.replace` no write can land in the old inode. + Rotating from the app would race the writer. +- **The poller follows rotations (tail -F semantics).** It records the events file's inode. When the + path's inode changes, it first drains the rotated generation (`proxy-events.1.jsonl`) from the + saved offset to its end, then restarts at offset 0 on the new file. Result: every event is emitted + exactly once, in order, across a rotation. The existing "file shrank ⇒ restart from 0" fallback + stays for a recreated file whose inode we never saw. + - The poll logic moves out of `ProxyServer` into a small `EventsFileTail` so it can be driven + directly in tests (no mitmdump, no timers). +- **Debug bundle reads across the rotation.** `proxyEventsReader` (app) prepends the tail of + `proxy-events.1.jsonl` when the current file is smaller than the 5 MiB bundle cap, so a bug report + made right after a rotation still carries recent history. +- **Retention unchanged.** With rotation, a live run is bounded, so the 10-minute grace no longer + lets one session fill the disk; old oversized files from before this change age out normally. + +## Tests + +- Package, real addon + real tailer: drive the real `mitmAddon.py` (`request` / `response` / + chunk hooks, a small `PROXY_EVENTS_ROTATE_BYTES`) through several rotations while an + `EventsFileTail` polls between writes; assert every event is emitted exactly once and in order, + only one previous generation exists, and the live file stays under the threshold + one line. + Fail-first: without rotation the file exceeds the bound; without the poller's rotation handling, + events written between the last poll and the rename are lost (asserted by name). +- Poll ordering: rename observed while the new file is still absent, and a rotation that happens + while the old generation ended mid-poll — both orders pinned. +- App: `proxyEventsReader` bundle test with a small current file + a previous generation. + +## Delivery + +claude-code-headless PR (addon + tailer), three reviews, then the agent-code PR bumping the pointer +(lockfile resync for the `file:` dep) with the `proxyEventsReader` change and this plan. +`Fixes #1273` goes on the agent-code PR only if both residual items are covered. From 0905ba8387c2063073e7b7a269d77529a7e56797 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 26 Sep 2026 17:45:21 -0700 Subject: [PATCH 02/19] fix(debug): read the proxy wire log across a rotation in debug bundles The Claude proxy addon now rotates proxy-events.jsonl into proxy-events.1.jsonl at 512 MiB. Right after a rotation the live file holds a few lines, so a bundle would miss the recent traffic a report is about. When the live file is under the 5 MiB cap, the rest is filled from the end of the previous generation, cut at a line boundary. Refs #1273 Co-Authored-By: Claude Opus 5.5 --- src/main/storage/proxyEventsReader.test.ts | 34 +++++++++ src/main/storage/proxyEventsReader.ts | 84 ++++++++++++++-------- 2 files changed, 88 insertions(+), 30 deletions(-) diff --git a/src/main/storage/proxyEventsReader.test.ts b/src/main/storage/proxyEventsReader.test.ts index ee7e95a4e..d74c94cbf 100644 --- a/src/main/storage/proxyEventsReader.test.ts +++ b/src/main/storage/proxyEventsReader.test.ts @@ -36,3 +36,37 @@ it('is unchanged for a run that never passed its budget', async () => { const section = await readProxyEventsForBundle({ cwd, sessionKey: 'session-b' }) expect(section.proxyEvents).toBe(`${request}\n`) }) + +// #1273 residual: the Claude addon rotates proxy-events.jsonl into +// proxy-events.1.jsonl at 512 MiB. A bundle made just after a rotation must +// still carry the recent traffic, which now sits at the end of the previous +// generation. +it('fills the bundle from the previous generation when the live file is small', async () => { + const older = [1, 2, 3].map(n => JSON.stringify({ kind: 'response-chunk', seq: n })) + const live = JSON.stringify({ kind: 'response-chunk', seq: 4 }) + const cwd = await runDir('session-rotated', { + 'proxy-events.1.jsonl': `${older.join('\n')}\n`, + 'proxy-events.jsonl': `${live}\n`, + }) + const section = await readProxyEventsForBundle({ cwd, sessionKey: 'session-rotated' }) + expect(section.proxyEvents?.trim().split('\n')).toEqual([...older, live]) +}) + +it('keeps the bundle within its cap across a rotation, newest traffic last', async () => { + const cap = 5 * 1024 * 1024 + const filler = JSON.stringify({ kind: 'response-chunk', pad: 'x'.repeat(1000) }) + const lines = Array.from({ length: Math.ceil(cap / filler.length) + 50 }, (_, n) => filler.replace('"pad"', `"seq":${n},"pad"`)) + const live = JSON.stringify({ kind: 'response-end', seq: 'live' }) + const cwd = await runDir('session-rotated-big', { + 'proxy-events.1.jsonl': `${lines.join('\n')}\n`, + 'proxy-events.jsonl': `${live}\n`, + }) + const section = await readProxyEventsForBundle({ cwd, sessionKey: 'session-rotated-big' }) + const out = section.proxyEvents!.trim().split('\n') + expect(JSON.parse(out[0]!)).toMatchObject({ kind: 'truncated' }) + expect(out.at(-1)).toBe(live) + expect(out.at(-2)).toBe(lines.at(-1)) + // Every kept line is whole, and the payload (minus the header) fits the cap. + for (const line of out) expect(() => JSON.parse(line)).not.toThrow() + expect(out.slice(1).join('\n').length).toBeLessThanOrEqual(cap) +}) diff --git a/src/main/storage/proxyEventsReader.ts b/src/main/storage/proxyEventsReader.ts index cae9cd78b..d7d838f0e 100644 --- a/src/main/storage/proxyEventsReader.ts +++ b/src/main/storage/proxyEventsReader.ts @@ -121,8 +121,7 @@ export async function readProxyEventsForBundle(opts: { const latest = await findLatestRun(projectDir, selection.segments) if (!latest) return empty - const eventsPath = join(latest.runDir, 'proxy-events.jsonl') - const tail = await readEventsTail(eventsPath, latest.size) + const tail = await readEventsTail(latest.runDir, latest.size) const newestBody = await readLatestRequestBody(latest.runDir) const proxyEvents = tail === null ? newestBody : newestBody ? `${tail.replace(/\n?$/, '\n')}${newestBody}` : tail const sessionMeta = await readSessionMeta(join(latest.runDir, 'session-meta.json')) @@ -221,40 +220,65 @@ async function findLatestRun( } -async function readEventsTail(path: string, size: number): Promise { +// The previous generation of a rotated Claude events file (agent-code #1273). +// claude-code-headless's mitm addon renames proxy-events.jsonl to this name at +// 512 MiB and starts a fresh file; the name must match its `_rotated_path()`. +// Codex proxy runs never rotate, so for them this file simply does not exist. +const ROTATED_EVENTS_FILE = 'proxy-events.1.jsonl' + +/** + * The last PROXY_EVENTS_BUNDLE_MAX_BYTES of the run's wire log, read across a + * rotation as if the previous generation and the live file were one file. + * + * WHY across the rotation: right after a rotation the live file holds a few + * lines, so a bundle made then would carry almost none of the recent traffic + * a bug report is about. When the live file is under the cap, the remainder is + * filled from the end of the previous generation. + */ +async function readEventsTail(runDir: string, size: number): Promise { try { - if (size <= PROXY_EVENTS_BUNDLE_MAX_BYTES) { - return await readFile(path, 'utf-8') - } - // Read the trailing PROXY_EVENTS_BUNDLE_MAX_BYTES bytes. We can't - // use readFile with a position arg (no slice option in fs.promises - // readFile), so open + read directly. - const { open } = await import('node:fs/promises') - const handle = await open(path, 'r') - try { - const start = size - PROXY_EVENTS_BUNDLE_MAX_BYTES - const buf = Buffer.alloc(PROXY_EVENTS_BUNDLE_MAX_BYTES) - await handle.read(buf, 0, PROXY_EVENTS_BUNDLE_MAX_BYTES, start) - // Drop the first partial line so consumers always see a clean - // JSONL boundary at byte 0 of the captured content. - const content = buf.toString('utf-8') - const firstNewline = content.indexOf('\n') - const trimmed = firstNewline >= 0 ? content.slice(firstNewline + 1) : content - const droppedBytes = size - trimmed.length - const header = JSON.stringify({ - kind: 'truncated', - reason: `proxy-events.jsonl exceeded ${PROXY_EVENTS_BUNDLE_MAX_BYTES} bytes; only the trailing portion is included in this bundle. Full log on disk: ${path}`, - dropped_bytes: droppedBytes, - }) - return `${header}\n${trimmed}` - } finally { - await handle.close() - } + const livePath = join(runDir, 'proxy-events.jsonl') + const live = await readTail(livePath, size, PROXY_EVENTS_BUNDLE_MAX_BYTES) + const room = PROXY_EVENTS_BUNDLE_MAX_BYTES - live.text.length + const rotatedPath = join(runDir, ROTATED_EVENTS_FILE) + const rotatedSize = room > 0 && live.droppedBytes === 0 + ? await stat(rotatedPath).then(st => st.size, () => null) + : null + const older = rotatedSize ? await readTail(rotatedPath, rotatedSize, room) : null + const text = `${older?.text ?? ''}${live.text}` + const droppedBytes = live.droppedBytes + (older?.droppedBytes ?? 0) + if (droppedBytes === 0) return text + const header = JSON.stringify({ + kind: 'truncated', + reason: `proxy-events.jsonl exceeded ${PROXY_EVENTS_BUNDLE_MAX_BYTES} bytes; only the trailing portion is included in this bundle. Full log on disk: ${older ? `${rotatedPath} + ${livePath}` : livePath}`, + dropped_bytes: droppedBytes, + }) + return `${header}\n${text}` } catch { return null } } +/** The last `maxBytes` of a JSONL file, cut at a line boundary. */ +async function readTail(path: string, size: number, maxBytes: number): Promise<{ text: string; droppedBytes: number }> { + if (size <= maxBytes) return { text: await readFile(path, 'utf-8'), droppedBytes: 0 } + // readFile has no position/slice option, so open + read the tail directly. + const { open } = await import('node:fs/promises') + const handle = await open(path, 'r') + try { + const buf = Buffer.alloc(maxBytes) + await handle.read(buf, 0, maxBytes, size - maxBytes) + // Drop the first partial line so consumers always see a clean JSONL + // boundary at byte 0 of the captured content. + const content = buf.toString('utf-8') + const firstNewline = content.indexOf('\n') + const text = firstNewline >= 0 ? content.slice(firstNewline + 1) : content + return { text, droppedBytes: size - text.length } + } finally { + await handle.close() + } +} + /** * The newest request body the Claude proxy addon kept after its events file From 0a462fad516937752790ee039292bea6a977ea54 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 26 Sep 2026 17:58:59 -0700 Subject: [PATCH 03/19] revert(debug): drop the #1273 bundle reader change; W1 owns proxyEventsReader (#1332) Steering q54: W1's #1332 owns one provider-neutral rotation-safe reader. This branch will merge main after #1332 lands and keep only the Claude-specific wiring. Refs #1273 Co-Authored-By: Claude Opus 5.5 --- src/main/storage/proxyEventsReader.test.ts | 34 --------- src/main/storage/proxyEventsReader.ts | 84 ++++++++-------------- 2 files changed, 30 insertions(+), 88 deletions(-) diff --git a/src/main/storage/proxyEventsReader.test.ts b/src/main/storage/proxyEventsReader.test.ts index d74c94cbf..ee7e95a4e 100644 --- a/src/main/storage/proxyEventsReader.test.ts +++ b/src/main/storage/proxyEventsReader.test.ts @@ -36,37 +36,3 @@ it('is unchanged for a run that never passed its budget', async () => { const section = await readProxyEventsForBundle({ cwd, sessionKey: 'session-b' }) expect(section.proxyEvents).toBe(`${request}\n`) }) - -// #1273 residual: the Claude addon rotates proxy-events.jsonl into -// proxy-events.1.jsonl at 512 MiB. A bundle made just after a rotation must -// still carry the recent traffic, which now sits at the end of the previous -// generation. -it('fills the bundle from the previous generation when the live file is small', async () => { - const older = [1, 2, 3].map(n => JSON.stringify({ kind: 'response-chunk', seq: n })) - const live = JSON.stringify({ kind: 'response-chunk', seq: 4 }) - const cwd = await runDir('session-rotated', { - 'proxy-events.1.jsonl': `${older.join('\n')}\n`, - 'proxy-events.jsonl': `${live}\n`, - }) - const section = await readProxyEventsForBundle({ cwd, sessionKey: 'session-rotated' }) - expect(section.proxyEvents?.trim().split('\n')).toEqual([...older, live]) -}) - -it('keeps the bundle within its cap across a rotation, newest traffic last', async () => { - const cap = 5 * 1024 * 1024 - const filler = JSON.stringify({ kind: 'response-chunk', pad: 'x'.repeat(1000) }) - const lines = Array.from({ length: Math.ceil(cap / filler.length) + 50 }, (_, n) => filler.replace('"pad"', `"seq":${n},"pad"`)) - const live = JSON.stringify({ kind: 'response-end', seq: 'live' }) - const cwd = await runDir('session-rotated-big', { - 'proxy-events.1.jsonl': `${lines.join('\n')}\n`, - 'proxy-events.jsonl': `${live}\n`, - }) - const section = await readProxyEventsForBundle({ cwd, sessionKey: 'session-rotated-big' }) - const out = section.proxyEvents!.trim().split('\n') - expect(JSON.parse(out[0]!)).toMatchObject({ kind: 'truncated' }) - expect(out.at(-1)).toBe(live) - expect(out.at(-2)).toBe(lines.at(-1)) - // Every kept line is whole, and the payload (minus the header) fits the cap. - for (const line of out) expect(() => JSON.parse(line)).not.toThrow() - expect(out.slice(1).join('\n').length).toBeLessThanOrEqual(cap) -}) diff --git a/src/main/storage/proxyEventsReader.ts b/src/main/storage/proxyEventsReader.ts index d7d838f0e..cae9cd78b 100644 --- a/src/main/storage/proxyEventsReader.ts +++ b/src/main/storage/proxyEventsReader.ts @@ -121,7 +121,8 @@ export async function readProxyEventsForBundle(opts: { const latest = await findLatestRun(projectDir, selection.segments) if (!latest) return empty - const tail = await readEventsTail(latest.runDir, latest.size) + const eventsPath = join(latest.runDir, 'proxy-events.jsonl') + const tail = await readEventsTail(eventsPath, latest.size) const newestBody = await readLatestRequestBody(latest.runDir) const proxyEvents = tail === null ? newestBody : newestBody ? `${tail.replace(/\n?$/, '\n')}${newestBody}` : tail const sessionMeta = await readSessionMeta(join(latest.runDir, 'session-meta.json')) @@ -220,65 +221,40 @@ async function findLatestRun( } -// The previous generation of a rotated Claude events file (agent-code #1273). -// claude-code-headless's mitm addon renames proxy-events.jsonl to this name at -// 512 MiB and starts a fresh file; the name must match its `_rotated_path()`. -// Codex proxy runs never rotate, so for them this file simply does not exist. -const ROTATED_EVENTS_FILE = 'proxy-events.1.jsonl' - -/** - * The last PROXY_EVENTS_BUNDLE_MAX_BYTES of the run's wire log, read across a - * rotation as if the previous generation and the live file were one file. - * - * WHY across the rotation: right after a rotation the live file holds a few - * lines, so a bundle made then would carry almost none of the recent traffic - * a bug report is about. When the live file is under the cap, the remainder is - * filled from the end of the previous generation. - */ -async function readEventsTail(runDir: string, size: number): Promise { +async function readEventsTail(path: string, size: number): Promise { try { - const livePath = join(runDir, 'proxy-events.jsonl') - const live = await readTail(livePath, size, PROXY_EVENTS_BUNDLE_MAX_BYTES) - const room = PROXY_EVENTS_BUNDLE_MAX_BYTES - live.text.length - const rotatedPath = join(runDir, ROTATED_EVENTS_FILE) - const rotatedSize = room > 0 && live.droppedBytes === 0 - ? await stat(rotatedPath).then(st => st.size, () => null) - : null - const older = rotatedSize ? await readTail(rotatedPath, rotatedSize, room) : null - const text = `${older?.text ?? ''}${live.text}` - const droppedBytes = live.droppedBytes + (older?.droppedBytes ?? 0) - if (droppedBytes === 0) return text - const header = JSON.stringify({ - kind: 'truncated', - reason: `proxy-events.jsonl exceeded ${PROXY_EVENTS_BUNDLE_MAX_BYTES} bytes; only the trailing portion is included in this bundle. Full log on disk: ${older ? `${rotatedPath} + ${livePath}` : livePath}`, - dropped_bytes: droppedBytes, - }) - return `${header}\n${text}` + if (size <= PROXY_EVENTS_BUNDLE_MAX_BYTES) { + return await readFile(path, 'utf-8') + } + // Read the trailing PROXY_EVENTS_BUNDLE_MAX_BYTES bytes. We can't + // use readFile with a position arg (no slice option in fs.promises + // readFile), so open + read directly. + const { open } = await import('node:fs/promises') + const handle = await open(path, 'r') + try { + const start = size - PROXY_EVENTS_BUNDLE_MAX_BYTES + const buf = Buffer.alloc(PROXY_EVENTS_BUNDLE_MAX_BYTES) + await handle.read(buf, 0, PROXY_EVENTS_BUNDLE_MAX_BYTES, start) + // Drop the first partial line so consumers always see a clean + // JSONL boundary at byte 0 of the captured content. + const content = buf.toString('utf-8') + const firstNewline = content.indexOf('\n') + const trimmed = firstNewline >= 0 ? content.slice(firstNewline + 1) : content + const droppedBytes = size - trimmed.length + const header = JSON.stringify({ + kind: 'truncated', + reason: `proxy-events.jsonl exceeded ${PROXY_EVENTS_BUNDLE_MAX_BYTES} bytes; only the trailing portion is included in this bundle. Full log on disk: ${path}`, + dropped_bytes: droppedBytes, + }) + return `${header}\n${trimmed}` + } finally { + await handle.close() + } } catch { return null } } -/** The last `maxBytes` of a JSONL file, cut at a line boundary. */ -async function readTail(path: string, size: number, maxBytes: number): Promise<{ text: string; droppedBytes: number }> { - if (size <= maxBytes) return { text: await readFile(path, 'utf-8'), droppedBytes: 0 } - // readFile has no position/slice option, so open + read the tail directly. - const { open } = await import('node:fs/promises') - const handle = await open(path, 'r') - try { - const buf = Buffer.alloc(maxBytes) - await handle.read(buf, 0, maxBytes, size - maxBytes) - // Drop the first partial line so consumers always see a clean JSONL - // boundary at byte 0 of the captured content. - const content = buf.toString('utf-8') - const firstNewline = content.indexOf('\n') - const text = firstNewline >= 0 ? content.slice(firstNewline + 1) : content - return { text, droppedBytes: size - text.length } - } finally { - await handle.close() - } -} - /** * The newest request body the Claude proxy addon kept after its events file From cee3a18883398eddb0991759e807e43b834b7adf Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sat, 26 Sep 2026 18:30:17 -0700 Subject: [PATCH 04/19] docs(plan): #1273 plan follows the q53 tail redesign and q54 reader ownership Refs #1273 Co-Authored-By: Claude Opus 5.5 --- .../2026-09-26-proxy-events-retention.md | 46 +++++++++++++------ 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/docs/plans/2026-09-26-proxy-events-retention.md b/docs/plans/2026-09-26-proxy-events-retention.md index 553b66a0e..462e43621 100644 --- a/docs/plans/2026-09-26-proxy-events-retention.md +++ b/docs/plans/2026-09-26-proxy-events-retention.md @@ -36,22 +36,40 @@ issue still has open: - WHY rotate on the writer side: mitmdump is the single writer, runs single-threaded, and writes each line with open-append-close, so after `os.replace` no write can land in the old inode. Rotating from the app would race the writer. -- **The poller follows rotations (tail -F semantics).** It records the events file's inode. When the - path's inode changes, it first drains the rotated generation (`proxy-events.1.jsonl`) from the - saved offset to its end, then restarts at offset 0 on the new file. Result: every event is emitted - exactly once, in order, across a rotation. The existing "file shrank ⇒ restart from 0" fallback - stays for a recreated file whose inode we never saw. - - The poll logic moves out of `ProxyServer` into a small `EventsFileTail` so it can be driven - directly in tests (no mitmdump, no timers). -- **Debug bundle reads across the rotation.** `proxyEventsReader` (app) prepends the tail of - `proxy-events.1.jsonl` when the current file is smaller than the 5 MiB bundle cap, so a bug report - made right after a rotation still carries recent history. +- **The tail holds the generation it reads open** (revised after review of claude-code-headless#64, + steering q53). + - The first version stat()ed the path and later open()ed it by name. A rotation between the two made + it read the new file with the old offset; reviewers reproduced lost and duplicated events. + - `EventsFileTail` now keeps a `FileHandle` on the current generation. Its size comes from `fstat` on + that handle, and rotation is detected when the *path's* inode changes. + - On rotation, the tail finishes the held generation, **opens the live file first**, and then reads + whole any unseen generation at `.1` before the live one. Anything between the two held handles can + only be at `.1`. + - Pinned by a test that rotates before every path-level await point. +- **Gap policy: a bounded, reported gap, not an acknowledgement protocol** (q53 asked us to choose). + - The addon bumps `proxy-events.rotations` before each rename. + - When the poller stalls through more rotations than it can hold or drain (about 1 GiB of traffic at + the default), the generations deleted unread are counted as `lostGenerations`. `ProxyServer` + surfaces them as a `transport-gap` event plus one warning. The claim is "exactly once, or an + explicit gap". + - An ack protocol would need a second writer in the app. A stalled or dead app would then let the + proxy's disk grow without bound again, which is this issue. +- **Addon hardening** (same review): + - `_write` never raises out of a mitmproxy hook, since the stream tap carries the user's live + response; + - a crashed partial line is terminated at startup, so the next event is not glued to it; + - the live file is recreated in its own step. +- **Debug bundle reader: owned by W1 in #1332** (steering q54). This branch's own `readEventsTail` + change was reverted (`0a462fad`). #1332 makes one provider-neutral, rotation-safe reader: one + handle, `bytesRead` honoured, filling from `.1`. This branch merges main after #1332 and keeps only + Claude-specific wiring, if any is still needed. - **Retention unchanged.** With rotation, a live run is bounded, so the 10-minute grace no longer lets one session fill the disk; old oversized files from before this change age out normally. ## Tests -- Package, real addon + real tailer: drive the real `mitmAddon.py` (`request` / `response` / +- Package, real addon + real tailer (see the PR body for the final list, incl. the per-await-point + rotation cases and the ProxyServer wiring test): drive the real `mitmAddon.py` (`request` / `response` / chunk hooks, a small `PROXY_EVENTS_ROTATE_BYTES`) through several rotations while an `EventsFileTail` polls between writes; assert every event is emitted exactly once and in order, only one previous generation exists, and the live file stays under the threshold + one line. @@ -59,10 +77,10 @@ issue still has open: events written between the last poll and the rename are lost (asserted by name). - Poll ordering: rename observed while the new file is still absent, and a rotation that happens while the old generation ended mid-poll — both orders pinned. -- App: `proxyEventsReader` bundle test with a small current file + a previous generation. +- App: none on this branch; the bundle reader's tests live in W1's #1332 (q54). ## Delivery -claude-code-headless PR (addon + tailer), three reviews, then the agent-code PR bumping the pointer -(lockfile resync for the `file:` dep) with the `proxyEventsReader` change and this plan. +claude-code-headless#64 (addon + tailer), three reviews plus verification, then the agent-code PR +bumping the pointer (lockfile resync for the `file:` dep) with this plan, after #1332 merges. `Fixes #1273` goes on the agent-code PR only if both residual items are covered. From 8996d9d3aa56ab5a216fd17f84ae8d9b1bc979de Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 00:13:38 -0700 Subject: [PATCH 05/19] fix(claude): bump claude-code-headless to the rotating proxy-events log (#1273) claude-code-headless#64 (merged, f52fc82): the addon rotates proxy-events.jsonl at 512 MiB keeping one previous generation, each generation stamped with a header; the app's tail holds a FileHandle on the generation it reads and reports generations deleted unread as a `transport-gap`. A live Claude run is now bounded instead of growing for the life of the session. No lockfile resync: the app consumes the package from source and its runtime deps are root deps; the bump changes only the package's devDependencies. The plan records the header design, the accepted crash limitation, and that #1332 (W1) owns the rotation-safe bundle reader. Co-Authored-By: Claude Opus 5.5 --- .../2026-09-26-proxy-events-retention.md | 21 ++++++++++--------- packages/claude-code-headless | 2 +- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/docs/plans/2026-09-26-proxy-events-retention.md b/docs/plans/2026-09-26-proxy-events-retention.md index 462e43621..2fc19ad2b 100644 --- a/docs/plans/2026-09-26-proxy-events-retention.md +++ b/docs/plans/2026-09-26-proxy-events-retention.md @@ -47,13 +47,13 @@ issue still has open: only be at `.1`. - Pinned by a test that rotates before every path-level await point. - **Gap policy: a bounded, reported gap, not an acknowledgement protocol** (q53 asked us to choose). - - The addon bumps `proxy-events.rotations` before each rename. - - When the poller stalls through more rotations than it can hold or drain (about 1 GiB of traffic at - the default), the generations deleted unread are counted as `lostGenerations`. `ProxyServer` - surfaces them as a `transport-gap` event plus one warning. The claim is "exactly once, or an - explicit gap". - - An ack protocol would need a second writer in the app. A stalled or dead app would then let the - proxy's disk grow without bound again, which is this issue. + - Each generation carries its number in a header line, `{"kind":"generation","generation":n}`, created atomically with the live file. The tail strips it. + - This replaced a first design with a `proxy-events.rotations` counter file, which a reader could pair with the wrong generation (round 2 of #64). + - When the poller stalls through more rotations than it can hold or drain (about 1 GiB of traffic at the default), the generations deleted unread are counted as `lostGenerations`. `ProxyServer` surfaces them as a `transport-gap` event plus one warning. + - An ack protocol would need a second writer in the app. A stalled or dead app would then let the proxy's disk grow without bound again, which is this issue. +- **Known limitation** (accepted by the manager under the final-pass cap, stated in claude-code-headless#64): + - A process crash between renaming the live file to `.1` and publishing the next header is repaired when the addon restarts (`2218918`). + - If the addon is never restarted, the rotated generation's unread events are not delivered and **not** reported as a gap. - **Addon hardening** (same review): - `_write` never raises out of a mitmproxy hook, since the stream tap carries the user's live response; @@ -81,6 +81,7 @@ issue still has open: ## Delivery -claude-code-headless#64 (addon + tailer), three reviews plus verification, then the agent-code PR -bumping the pointer (lockfile resync for the `file:` dep) with this plan, after #1332 merges. -`Fixes #1273` goes on the agent-code PR only if both residual items are covered. +claude-code-headless#64 (addon + tailer): three reviews, verification, and a final round 3, MERGED (`f52fc82`). This agent-code PR only bumps the pointer. #1332 (W1's rotation-safe bundle reader) is already on main. +- **No lockfile resync is needed.** The app consumes claude-code-headless from source (tsconfig and Vite aliases), not as a `file:` dependency, and its runtime dependencies (`chokidar`, `@xterm/headless`) are already root dependencies. The bump changes only the package's own devDependencies; `npm install --package-lock-only` leaves `package-lock.json` unchanged. +- **The generation header** is one more JSON line with an unknown `kind` to the app's only direct reader (the bundle reader, which ships raw bytes). The addon recreates `proxy-events.jsonl` immediately, so `debugRetention`'s run detection is unaffected. +- **Both residual items are covered:** the live file is bounded by rotation, and a live run's disk use is bounded. So this PR carries `Fixes #1273`. diff --git a/packages/claude-code-headless b/packages/claude-code-headless index 64fd0eaf1..f52fc82d9 160000 --- a/packages/claude-code-headless +++ b/packages/claude-code-headless @@ -1 +1 @@ -Subproject commit 64fd0eaf18a0a6cef58b9048e3768843b3e0e65b +Subproject commit f52fc82d9d185314992cd845dafce42a723b2248 From 0de5424512ce4b4b5b84a9aae0dd84979de815f9 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 00:59:52 -0700 Subject: [PATCH 06/19] fix(claude): surface proxy transport gaps; count rotated-only runs in retention (review) Review of #1376 (a, b, c): - ClaudeSession listened to the proxy's `event` channel only, so the `transport-gap` claude-code-headless#64 emits for generations deleted unread never left the package. attachProxyServer subscribes both and re-emits `proxy-transport-gap`; SessionManager records a claude.proxy_transport_gap incident and re-emits it with the session id. The normal event wiring is now pinned (deleting it passed every test before). - collectProxyRunDirs counts a run holding only proxy-events.1.jsonl. - sslkeylog.log is a separate unbounded file (and TLS secrets on disk): filed as #1380; the PR narrows to Refs #1273. Mutations killed: session ignoring transport-gap, session ignoring event, manager ignoring the gap, retention ignoring `.1`. Co-Authored-By: Claude Opus 5.5 --- .../2026-09-26-proxy-events-retention.md | 6 +- src/main/incident/journalTypes.ts | 1 + src/main/sessionManager.proxyGap.test.ts | 89 +++++++++++++++++++ src/main/sessionManager.ts | 21 +++++ src/main/storage/debugRetention.test.ts | 19 +++- src/main/storage/debugRetention.ts | 15 +++- .../runtime/claudeSession.suspension.test.ts | 45 ++++++++++ src/providers/claude/runtime/claudeSession.ts | 50 ++++++++--- 8 files changed, 231 insertions(+), 15 deletions(-) create mode 100644 src/main/sessionManager.proxyGap.test.ts diff --git a/docs/plans/2026-09-26-proxy-events-retention.md b/docs/plans/2026-09-26-proxy-events-retention.md index 2fc19ad2b..e9058e5a9 100644 --- a/docs/plans/2026-09-26-proxy-events-retention.md +++ b/docs/plans/2026-09-26-proxy-events-retention.md @@ -84,4 +84,8 @@ issue still has open: claude-code-headless#64 (addon + tailer): three reviews, verification, and a final round 3, MERGED (`f52fc82`). This agent-code PR only bumps the pointer. #1332 (W1's rotation-safe bundle reader) is already on main. - **No lockfile resync is needed.** The app consumes claude-code-headless from source (tsconfig and Vite aliases), not as a `file:` dependency, and its runtime dependencies (`chokidar`, `@xterm/headless`) are already root dependencies. The bump changes only the package's own devDependencies; `npm install --package-lock-only` leaves `package-lock.json` unchanged. - **The generation header** is one more JSON line with an unknown `kind` to the app's only direct reader (the bundle reader, which ships raw bytes). The addon recreates `proxy-events.jsonl` immediately, so `debugRetention`'s run detection is unaffected. -- **Both residual items are covered:** the live file is bounded by rotation, and a live run's disk use is bounded. So this PR carries `Fixes #1273`. +- **Review of #1376 (a, b, c):** + - **The app dropped the gap signal.** `ClaudeSession` subscribed to the proxy's `event` channel only, so `transport-gap` never left the package. It now subscribes to both (`attachProxyServer`/`detachProxyServer`) and re-emits `proxy-transport-gap`. `SessionManager` records a `claude.proxy_transport_gap` incident and re-emits it with the session id. The normal `event` wiring is now pinned too; deleting it used to pass every test. + - **A run holding only `proxy-events.1.jsonl` was invisible to retention.** `collectProxyRunDirs` now counts it. + - **`sslkeylog.log` also grows without bound** in every live run directory, and holds TLS secrets. This is outside this bump, so it is filed as #1380. The PR says `Refs #1273`, not `Fixes`: the events file is bounded, but a live run directory is not fully bounded until #1380 lands. + - **The latest-body sidecar** is ≤ 16 MiB raw, about 21.3 MiB on disk after base64 encoding. During the atomic replace, twice that is briefly possible. diff --git a/src/main/incident/journalTypes.ts b/src/main/incident/journalTypes.ts index 3c1dd640c..4dedb304f 100644 --- a/src/main/incident/journalTypes.ts +++ b/src/main/incident/journalTypes.ts @@ -120,6 +120,7 @@ export type AppRunIncidentKind = // the child was adopted — but it ran work nobody was waiting on, so the // timeout budget or the renderer's responsiveness is worth looking at (#926). | 'orchestration.late_response_adopted' + | 'claude.proxy_transport_gap' | 'orchestration.prompt_delivery_failed' | 'mcp.host_start_failed' // Remote mobile companion (src/main/remote/) — declared here because this diff --git a/src/main/sessionManager.proxyGap.test.ts b/src/main/sessionManager.proxyGap.test.ts new file mode 100644 index 000000000..c4b12ddf5 --- /dev/null +++ b/src/main/sessionManager.proxyGap.test.ts @@ -0,0 +1,89 @@ +import { EventEmitter } from 'node:events' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { createSession, createTerminalSession } = vi.hoisted(() => ({ + createSession: vi.fn(), + createTerminalSession: vi.fn(), +})) + +vi.mock('@main/workspaceDirectory.js', () => ({ + // These suites spawn into synthetic paths ('/tmp/project', '/recorded/worktree') + // that intentionally do not exist on disk. The real spawn-path guard stats the + // cwd, so it is stubbed here; workspaceDirectory.test.ts covers the guard + // itself, and the missing-folder case below overrides this mock to prove the + // manager surfaces it. + MissingWorkspaceDirectoryError: class MissingWorkspaceDirectoryError extends Error { + constructor(readonly cwd: string) { + super(`Workspace folder is missing: ${cwd}`) + this.name = 'MissingWorkspaceDirectoryError' + } + }, + assertWorkspaceDirectoryExists: vi.fn(async () => {}), +})) + +vi.mock('@providers/registry.main.js', () => ({ + getMainProvider: () => ({ + name: 'Claude', + createSession, + createTerminalSession, + }), +})) + +vi.mock('@main/setup/toolchain.js', () => ({ + getToolPath: () => '/usr/bin/true', +})) + +vi.mock('@main/performance/PerformanceService.js', () => ({ + performanceService: { + mark: vi.fn(), + record: vi.fn(), + error: vi.fn(), + metric: vi.fn(), + span: () => ({ end: vi.fn(), fail: vi.fn() }), + }, +})) + +vi.mock('@main/storage/feedDebugLog.js', () => ({ + forgetFeedDebugSession: vi.fn(), +})) + +class FakeAgentSession extends EventEmitter { + async start(): Promise { + this.emit('started', { projectDir: '/tmp/project' }) + } + + async stop(): Promise {} + + write(): void {} + + resize(): void {} +} + +// Review of #1376 (a, b, c): claude-code-headless#64 reports proxy generations deleted before the +// app read them as `transport-gap`; ClaudeSession re-emits it as `proxy-transport-gap`. This pins +// the manager's half: an always-on incident for the session, and the event re-emitted with its id. +describe('a Claude proxy transport gap', () => { + beforeEach(() => { + createSession.mockReset() + }) + + it('is recorded as an incident and re-emitted with the session id', async () => { + const { SessionManager } = await import('./sessionManager') + const session = new FakeAgentSession() + createSession.mockImplementationOnce(() => session) + const incidents: Array<{ kind: string; context?: Record }> = [] + const journal = { recordIncident: (incident: { kind: string; context?: Record }) => { incidents.push(incident) }, record: vi.fn(), recordError: vi.fn() } + const manager = new SessionManager(null, null, journal as never) + const gaps: unknown[] = [] + manager.on('proxy-transport-gap', gap => { gaps.push(gap) }) + + await manager.recover({ sessionId: 's1', kind: 'claude', cwd: '/tmp/project' }) + session.emit('proxy-transport-gap', { lostGenerations: 3 }) + + expect(gaps).toEqual([{ sessionId: 's1', lostGenerations: 3 }]) + expect(incidents).toContainEqual(expect.objectContaining({ + kind: 'claude.proxy_transport_gap', + context: { sessionId: 's1', lostGenerations: 3 }, + })) + }) +}) diff --git a/src/main/sessionManager.ts b/src/main/sessionManager.ts index 3d463350c..1b0050b1b 100644 --- a/src/main/sessionManager.ts +++ b/src/main/sessionManager.ts @@ -195,6 +195,7 @@ type ManagerEvents = { observation?: AgentTranscriptObservationMetadata }] 'jsonl-error': [{ sessionId: string; error: Error }] + 'proxy-transport-gap': [{ sessionId: string; lostGenerations: number }] /** Durable-history generation boundary (grok). Never completion or idle; * consumers apply renderer/session-runtime/historyBoundary.ts decisions. */ 'history-boundary': [{ sessionId: string; type: 'reset' | 'caught-up'; generation: number; snapshotByteLength: number; byteOffset?: number; complete?: boolean; file: string }] @@ -569,6 +570,11 @@ export type ResolveConditionResult = failedAtStep?: string } +/** The one Claude-only event SessionManager subscribes to (ClaudeSessionEvents declares it). */ +type ProxyGapSource = { + on(event: 'proxy-transport-gap', listener: (gap: { lostGenerations: number }) => void): unknown +} + export class SessionManager extends EventEmitter { private readonly monitorResponses = new ResponseTracker(mainOperations) private readonly sessions = new Map() @@ -3316,6 +3322,21 @@ export class SessionManager extends EventEmitter { } this.emit('jsonl-error', { sessionId, error }) }) + // claude-code-headless#64 / review of #1376: proxy events deleted before the app read them. + // Recorded as an always-on incident (debug bundles carry the journal) and re-emitted, so a + // hole in the live Claude feed is never silent. + // Claude-only (its ClaudeSessionEvents declares it; other providers have no proxy tail), so it + // is subscribed through that type rather than widening every provider's event map. + if (kind === 'claude') (session as unknown as ProxyGapSource).on('proxy-transport-gap', (gap: { lostGenerations: number }) => { + if (!ownsEntry()) return + this.journal?.recordIncident({ + kind: 'claude.proxy_transport_gap', + severity: 'warn', + reason: 'events_deleted_unread', + context: { sessionId, lostGenerations: gap.lostGenerations }, + }) + this.emit('proxy-transport-gap', { sessionId, lostGenerations: gap.lostGenerations }) + }) session.on('transcript-diagnostic', (diagnostic: unknown) => { if (!ownsEntry()) return if (kind === 'codex' && this.observeCodexTranscriptDiagnostic( diff --git a/src/main/storage/debugRetention.test.ts b/src/main/storage/debugRetention.test.ts index e4c0683f7..9b79a5f8c 100644 --- a/src/main/storage/debugRetention.test.ts +++ b/src/main/storage/debugRetention.test.ts @@ -4,7 +4,7 @@ import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { collectSessionRecordingDirs, runPrunePasses } from './debugRetention.js' +import { collectProxyRunDirs, collectSessionRecordingDirs, runPrunePasses } from './debugRetention.js' import type { DebugStorageArtifact, DebugStorageBucket, @@ -226,3 +226,20 @@ describe('runPrunePasses', () => { expect(result).toEqual({ removed: 0, bytesFreed: 0, remainingBytes: 500 }) }) }) + +// Review of #1376 (a): claude-code-headless#64 rotates proxy-events.jsonl to proxy-events.1.jsonl +// before creating the next live file. A run left holding only `.1` (the creation failed, or the +// addon died in between) was invisible to every retention pass. +describe('collectProxyRunDirs', () => { + it('counts a run that holds only the rotated generation', async () => { + const live = join(root, 'project', 'session', 'run-live') + const rotated = join(root, 'project', 'session', 'run-rotated') + mkdirSync(live, { recursive: true }) + mkdirSync(rotated, { recursive: true }) + writeFileSync(join(live, 'proxy-events.jsonl'), '{}\n') + writeFileSync(join(rotated, 'proxy-events.1.jsonl'), '{}\n') + const runs = (await collectProxyRunDirs(root)).map(artifact => artifact.path).sort() + expect(runs).toEqual([live, rotated]) + }) +}) + diff --git a/src/main/storage/debugRetention.ts b/src/main/storage/debugRetention.ts index 5532ae0f1..a16608f6e 100644 --- a/src/main/storage/debugRetention.ts +++ b/src/main/storage/debugRetention.ts @@ -585,7 +585,18 @@ async function loadManualLegacyBundlePaths(): Promise> { return manual } -async function collectProxyRunDirs(root: string): Promise { +/** + * A proxy run is a directory holding the live events file OR only its rotated generation. + * + * WHY `.1` too (review of #1376, a): claude-code-headless#64 renames the live file to + * `proxy-events.1.jsonl` before creating the next one. If that creation fails, or the addon dies in + * between, the run holds only `.1`; matching the live name alone made such a run invisible to + * TTL, cap and budget alike, so stranded runs could accumulate. The bundle reader already treats + * `.1` as a run (proxyEventsReader.ts). + */ +const PROXY_RUN_MARKERS = new Set(['proxy-events.jsonl', 'proxy-events.1.jsonl']) + +export async function collectProxyRunDirs(root: string): Promise { const out: Artifact[] = [] async function walk(dir: string, depth: number): Promise { let entries @@ -594,7 +605,7 @@ async function collectProxyRunDirs(root: string): Promise { } catch { return } - if (entries.some(entry => entry.isFile() && entry.name === 'proxy-events.jsonl')) { + if (entries.some(entry => entry.isFile() && PROXY_RUN_MARKERS.has(entry.name))) { const artifact = await collectDirArtifact(dir, 'proxy') if (artifact) out.push(artifact) return diff --git a/src/providers/claude/runtime/claudeSession.suspension.test.ts b/src/providers/claude/runtime/claudeSession.suspension.test.ts index 70b692614..a8b191d26 100644 --- a/src/providers/claude/runtime/claudeSession.suspension.test.ts +++ b/src/providers/claude/runtime/claudeSession.suspension.test.ts @@ -1,3 +1,5 @@ +import { EventEmitter } from 'node:events' + import { describe, expect, it, vi } from 'vitest' import { ClaudeSession } from './claudeSession.js' @@ -51,3 +53,46 @@ describe('ClaudeSession.readComposer', () => { expect(session.readComposer()).toEqual({ screen: '❯ typed', attributes }) }) }) + +// Review of #1376 (a, b, c): the app subscribed to the proxy's `event` channel only, so the +// `transport-gap` claude-code-headless#64 reports (generations rotated away unread) never left the +// package — and deleting even the `event` subscription passed every Claude runtime test. +describe('ClaudeSession proxy wiring', () => { + function wired() { + const session = new ClaudeSession() + const proxy = new EventEmitter() + const handleProxyTransportEvent = vi.fn() + const internals = session as unknown as { + proxyServer: unknown + headless: unknown + attachProxyServer(): void + detachProxyServer(): void + } + internals.proxyServer = proxy + internals.headless = { handleProxyTransportEvent } + internals.attachProxyServer() + return { session, proxy, handleProxyTransportEvent, detach: () => internals.detachProxyServer() } + } + + it('forwards every proxy event to the adapter', () => { + const { proxy, handleProxyTransportEvent } = wired() + const chunk = { kind: 'response-chunk', flow_id: 'f1', chunk_b64: 'eA==' } + proxy.emit('event', chunk) + expect(handleProxyTransportEvent).toHaveBeenCalledWith(chunk) + }) + + it('surfaces a transport gap as a session event', () => { + const { session, proxy } = wired() + const gaps: unknown[] = [] + session.on('proxy-transport-gap', gap => { gaps.push(gap) }) + proxy.emit('transport-gap', { lostGenerations: 2 }) + expect(gaps).toEqual([{ lostGenerations: 2 }]) + }) + + it('detaches both channels', () => { + const { proxy, detach } = wired() + detach() + expect(proxy.listenerCount('event') + proxy.listenerCount('transport-gap')).toBe(0) + }) +}) + diff --git a/src/providers/claude/runtime/claudeSession.ts b/src/providers/claude/runtime/claudeSession.ts index 361e97aa8..eb6760db3 100644 --- a/src/providers/claude/runtime/claudeSession.ts +++ b/src/providers/claude/runtime/claudeSession.ts @@ -97,6 +97,7 @@ export type ClaudeSessionEvents = { // Declared for the provider-neutral AgentSession contract. Claude currently // emits no transcript-discovery diagnostics, so this event never fires. 'transcript-diagnostic': [unknown] + 'proxy-transport-gap': [{ lostGenerations: number }] // Optional status: the spinner verb ("Cogitating…", "Cascading…", // …) so the renderer can label its activity indicator with what CC // is actually doing rather than a generic "thinking…" placeholder. @@ -168,6 +169,7 @@ export class ClaudeSession extends EventEmitter { // proxy shutdown path drop the emitter isn't enough — the closure // captures `this.headless` and delays GC of the session object. private proxyEventHandler: ((ev: unknown) => void) | null = null + private proxyGapHandler: ((gap: { lostGenerations: number }) => void) | null = null private exited = false /** Gate for the committed `tool_result` bridge. False until the * JSONL tailer's initial replay has quiesced (250 ms without a new @@ -512,16 +514,7 @@ export class ClaudeSession extends EventEmitter { // the EventEmitter keeps a reference to the closure (which closes // over `this.headless`) even after teardown, preventing GC of // the session object until the proxy process itself is collected. - if (this.proxyServer) { - this.proxyEventHandler = ev => { - this.headless?.handleProxyTransportEvent( - ev as Parameters< - NonNullable['handleProxyTransportEvent'] - >[0], - ) - } - this.proxyServer.on('event', this.proxyEventHandler) - } + this.attachProxyServer() this.pty.onData((data: string) => this.emit('pty-data', data)) @@ -1165,12 +1158,47 @@ export class ClaudeSession extends EventEmitter { // a leak-relevance (a live mitmdump keeps its port bound), and two // hand-maintained copies of it is exactly how the rollback path missed // it in the first place (#495 A9). - private async teardownProxy(): Promise { + /** + * Subscribe to the proxy's two channels. Only when useProxy was set (`proxyServer` is otherwise + * null), and AFTER headless exists, so the adapter is there before any chunk arrives. + * + * WHY `transport-gap` too (review of #1376, a/b/c): claude-code-headless#64 rotates the events file + * and, when the poller stalls through rotations, reports generations deleted unread as + * `transport-gap`. The app listened to `event` only, so a whole span of chunks could vanish with + * nothing but a main-process console line — the "every event exactly once, or an explicit gap" + * contract stopped at the package boundary. The gap is re-emitted as `proxy-transport-gap`, which + * SessionManager records as an always-on incident for this session. + */ + private attachProxyServer(): void { + if (!this.proxyServer) return + this.proxyEventHandler = ev => { + this.headless?.handleProxyTransportEvent( + ev as Parameters< + NonNullable['handleProxyTransportEvent'] + >[0], + ) + } + this.proxyGapHandler = gap => { this.emit('proxy-transport-gap', gap) } + this.proxyServer.on('event', this.proxyEventHandler) + this.proxyServer.on('transport-gap', this.proxyGapHandler) + } + + /** Detach both proxy channels; see attachProxyServer. */ + private detachProxyServer(): void { if (!this.proxyServer) return if (this.proxyEventHandler) { this.proxyServer.off('event', this.proxyEventHandler) this.proxyEventHandler = null } + if (this.proxyGapHandler) { + this.proxyServer.off('transport-gap', this.proxyGapHandler) + this.proxyGapHandler = null + } + } + + private async teardownProxy(): Promise { + if (!this.proxyServer) return + this.detachProxyServer() try { // WHY a deadline around the package's stop(): ProxyServer.stop() // awaits `child.once('exit')` after SIGTERM/SIGKILL, and that promise From 33c046cfbbb9ebb7fdd6cfd2f9fbe8fdb1527f3d Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 01:02:49 -0700 Subject: [PATCH 07/19] docs(plan): the proxy gap is diagnostic-only in the app; user-visible marker is #1381 (q87) Co-Authored-By: Claude Opus 5.5 --- docs/plans/2026-09-26-proxy-events-retention.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/plans/2026-09-26-proxy-events-retention.md b/docs/plans/2026-09-26-proxy-events-retention.md index e9058e5a9..3a89fba93 100644 --- a/docs/plans/2026-09-26-proxy-events-retention.md +++ b/docs/plans/2026-09-26-proxy-events-retention.md @@ -85,7 +85,7 @@ claude-code-headless#64 (addon + tailer): three reviews, verification, and a fin - **No lockfile resync is needed.** The app consumes claude-code-headless from source (tsconfig and Vite aliases), not as a `file:` dependency, and its runtime dependencies (`chokidar`, `@xterm/headless`) are already root dependencies. The bump changes only the package's own devDependencies; `npm install --package-lock-only` leaves `package-lock.json` unchanged. - **The generation header** is one more JSON line with an unknown `kind` to the app's only direct reader (the bundle reader, which ships raw bytes). The addon recreates `proxy-events.jsonl` immediately, so `debugRetention`'s run detection is unaffected. - **Review of #1376 (a, b, c):** - - **The app dropped the gap signal.** `ClaudeSession` subscribed to the proxy's `event` channel only, so `transport-gap` never left the package. It now subscribes to both (`attachProxyServer`/`detachProxyServer`) and re-emits `proxy-transport-gap`. `SessionManager` records a `claude.proxy_transport_gap` incident and re-emits it with the session id. The normal `event` wiring is now pinned too; deleting it used to pass every test. + - **The app dropped the gap signal.** `ClaudeSession` subscribed to the proxy's `event` channel only, so `transport-gap` never left the package. It now subscribes to both (`attachProxyServer`/`detachProxyServer`) and re-emits `proxy-transport-gap`. `SessionManager` records a `claude.proxy_transport_gap` incident and re-emits it with the session id. This is diagnostic only (steering q87): no missing-span marker is rendered yet, so the user-visible gap is #1381. The normal `event` wiring is now pinned too; deleting it used to pass every test. - **A run holding only `proxy-events.1.jsonl` was invisible to retention.** `collectProxyRunDirs` now counts it. - **`sslkeylog.log` also grows without bound** in every live run directory, and holds TLS secrets. This is outside this bump, so it is filed as #1380. The PR says `Refs #1273`, not `Fixes`: the events file is bounded, but a live run directory is not fully bounded until #1380 lands. - **The latest-body sidecar** is ≤ 16 MiB raw, about 21.3 MiB on disk after base64 encoding. During the atomic replace, twice that is briefly possible. From 49ee59858fe763c7c85f5f949d4187f899da82e8 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 01:23:58 -0700 Subject: [PATCH 08/19] test(claude): a replaced session's late proxy gap is not recorded against its successor (focused review) Focused review of #1376 (c): the ownership guard on the gap subscription was unpinned (deleting it passed). Also corrects the plan: generation 0 has no header, and PROXY_EVENTS_ROTATE_BYTES=0 disables the bound. Co-Authored-By: Claude Opus 5.5 --- .../2026-09-26-proxy-events-retention.md | 3 ++- src/main/sessionManager.proxyGap.test.ts | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/docs/plans/2026-09-26-proxy-events-retention.md b/docs/plans/2026-09-26-proxy-events-retention.md index 3a89fba93..cabcdd65c 100644 --- a/docs/plans/2026-09-26-proxy-events-retention.md +++ b/docs/plans/2026-09-26-proxy-events-retention.md @@ -47,7 +47,8 @@ issue still has open: only be at `.1`. - Pinned by a test that rotates before every path-level await point. - **Gap policy: a bounded, reported gap, not an acknowledgement protocol** (q53 asked us to choose). - - Each generation carries its number in a header line, `{"kind":"generation","generation":n}`, created atomically with the live file. The tail strips it. + - Each generation created by a rotation carries its number in a header line, `{"kind":"generation","generation":n}`, created atomically with the live file. Generation 0 (the run's first file) has none; the tail reads a missing header as generation 0 and strips the line when present. + - The bound holds under the default `PROXY_EVENTS_ROTATE_BYTES`; setting it to 0 (the forensic override) disables rotation. - This replaced a first design with a `proxy-events.rotations` counter file, which a reader could pair with the wrong generation (round 2 of #64). - When the poller stalls through more rotations than it can hold or drain (about 1 GiB of traffic at the default), the generations deleted unread are counted as `lostGenerations`. `ProxyServer` surfaces them as a `transport-gap` event plus one warning. - An ack protocol would need a second writer in the app. A stalled or dead app would then let the proxy's disk grow without bound again, which is this issue. diff --git a/src/main/sessionManager.proxyGap.test.ts b/src/main/sessionManager.proxyGap.test.ts index c4b12ddf5..8b5731686 100644 --- a/src/main/sessionManager.proxyGap.test.ts +++ b/src/main/sessionManager.proxyGap.test.ts @@ -86,4 +86,27 @@ describe('a Claude proxy transport gap', () => { context: { sessionId: 's1', lostGenerations: 3 }, })) }) + + // Focused review of #1376 (c): a replaced session's late gap must not be recorded against the + // session id its successor now owns. + it('ignores a gap from a session that has been replaced', async () => { + const { SessionManager } = await import('./sessionManager') + const first = new FakeAgentSession() + const second = new FakeAgentSession() + createSession.mockImplementationOnce(() => first).mockImplementationOnce(() => second) + const incidents: Array<{ kind: string }> = [] + const journal = { recordIncident: (incident: { kind: string }) => { incidents.push(incident) }, record: vi.fn(), recordError: vi.fn() } + const manager = new SessionManager(null, null, journal as never) + const gaps: unknown[] = [] + manager.on('proxy-transport-gap', gap => { gaps.push(gap) }) + + await manager.recover({ sessionId: 's1', kind: 'claude', cwd: '/tmp/project' }) + first.emit('exit', { exitCode: 0 }) + await manager.recover({ sessionId: 's1', kind: 'claude', cwd: '/tmp/project' }) + first.emit('proxy-transport-gap', { lostGenerations: 1 }) + + expect(gaps).toEqual([]) + expect(incidents.filter(incident => incident.kind === 'claude.proxy_transport_gap')).toEqual([]) + }) }) + From 8a23b49ddbc748c2f3ceb0ae63ec8cd22c765ecc Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 03:53:12 -0700 Subject: [PATCH 09/19] test(extensions): count only runtime egress in the runtime harness (#1187) The flaky ['/'] hit was the host Agent Code app's LanePortWatcher probing the harness's listener with GET / (Node fetch) because the suite ran inside an agent lane; under load it landed before the egress assertion. The ZodError is the harness's own forgery probe, and 'No handler registered' is normal disposal noise. Only requests for the probed path count as egress now. Reproduced 2/10 under load before, 0/10 after; a guard-removal mutant still fails. Co-Authored-By: Claude Opus 5.5 --- ...-09-27-extension-runtime-harness-egress.md | 14 +++++++++++ src/main/extensions/testing/runtimeHarness.ts | 23 ++++++++++++++++--- 2 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 docs/plans/2026-09-27-extension-runtime-harness-egress.md diff --git a/docs/plans/2026-09-27-extension-runtime-harness-egress.md b/docs/plans/2026-09-27-extension-runtime-harness-egress.md new file mode 100644 index 000000000..9f9355852 --- /dev/null +++ b/docs/plans/2026-09-27-extension-runtime-harness-egress.md @@ -0,0 +1,14 @@ +# Extension runtime harness: count only runtime egress (#1187) + +## Evidence +- **Reproduced locally under CPU load:** 2 of 10 runs of `node scripts/check-extension-frames.mjs --runtime-only` failed with exactly the issue's output. +- **Temporary request logging on the harness's egress server** showed one `GET /` in EVERY run, failing or passing, about 1.6 s after startup. Its headers were `user-agent: node`, `accept-language: *` and `sec-fetch-mode: cors`, which is Node's undici `fetch`, not the extension renderer. +- **Source:** the running Agent Code app's browser-pocket `LanePortWatcher` (`src/main/browserPocket/lanePortsIo.ts:45`, `probe(port)`) walks each agent lane's process tree, finds listening sockets with `lsof`, and probes them with `GET /`. The test ran inside an agent lane, so the harness's server is in that tree. Normally the probe lands after the egress assertion; under load it lands before. +- **The other two log lines are not the bug.** The `ZodError ... "extensionId"` is the harness's deliberate forgery probe (`runtimeHarness.ts`, the `sandbox` command expects `spoofDenied: true`). `No handler registered for 'extensions:runtime-api'` also appears twice in every passing run during disposal. + +## Change +Every renderer egress probe (fetch, window.open, navigation) targets `/private-state`. The harness now counts only requests for that path as egress. Anything else is recorded as foreign, ignored, and named in the assertion message. + +## Verification +- 0 of 10 runs fail under the same load. +- A mutant that removes `will-navigate`, `will-frame-navigate` and the `webRequest` block still fails the egress assertion (foreign list empty), so the filter did not blind the check. diff --git a/src/main/extensions/testing/runtimeHarness.ts b/src/main/extensions/testing/runtimeHarness.ts index 35c6be7bb..3504fc02e 100644 --- a/src/main/extensions/testing/runtimeHarness.ts +++ b/src/main/extensions/testing/runtimeHarness.ts @@ -44,12 +44,29 @@ async function until(predicate: () => Promise, label: string): Promise< void (async () => { await app.whenReady() + // Only requests for EGRESS_PATH are runtime egress (#1187). Every renderer + // probe below (fetch, window.open, navigation) targets exactly that path. + // Anything else on this port comes from outside the extension runtime: when + // the suite runs inside an Agent Code agent lane, the host app's browser- + // pocket LanePortWatcher finds this listener in the lane's process tree and + // probes it with one `GET /` (src/main/browserPocket/lanePortsIo.ts, Node + // fetch, `user-agent: node`). Under load that probe landed before the + // assertion below and failed the run with `['/']`, which the issue read as + // a runtime-API race. Recorded with request headers: every local run got that + // one `GET /` about 1.6 s after startup; CI has no watching app. + const EGRESS_PATH = '/private-state' const hits: string[] = [] - const server = createServer((request, response) => { hits.push(request.url ?? ''); response.end('unexpected egress') }) + const foreignHits: string[] = [] + const server = createServer((request, response) => { + const url = request.url ?? '' + if (url.startsWith(EGRESS_PATH)) hits.push(url) + else foreignHits.push(`${request.method} ${url} ${request.headers['user-agent'] ?? ''}`) + response.end('unexpected egress') + }) await new Promise((resolve, reject) => { server.once('error', reject); server.listen(0, '127.0.0.1', resolve) }) const address = server.address() assert.ok(address && typeof address !== 'string') - const endpoint = `http://127.0.0.1:${address.port}/private-state` + const endpoint = `http://127.0.0.1:${address.port}${EGRESS_PATH}` const statuses: RuntimeStatus[] = [] const lifecycle: string[] = [] const projectRoot = join(root!, 'project') @@ -220,7 +237,7 @@ void (async () => { await command('navigate') await new Promise(resolve => setTimeout(resolve, 100)) assert.equal(object(await command('snapshot')).activations, 1, 'renderer navigation must be refused') - assert.deepEqual(hits, []) + assert.deepEqual(hits, [], `runtime egress reached the server (foreign, ignored: ${JSON.stringify(foreignHits)})`) console.log('PASS runtime: command errors, host/preload isolation, forged namespaces and direct network/navigation denial') await views.attach(66, 'update-view', 'engine', revision, 'engine.main') From dca6366e74e5716678dd66e56f5b43470f030175 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 04:16:25 -0700 Subject: [PATCH 10/19] test(extensions): excuse only the watcher's exact GET /, count every other request, add a positive control (#1406 review b) Co-Authored-By: Claude Opus 5.5 --- src/main/extensions/testing/runtimeHarness.ts | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/main/extensions/testing/runtimeHarness.ts b/src/main/extensions/testing/runtimeHarness.ts index 3504fc02e..b19b83102 100644 --- a/src/main/extensions/testing/runtimeHarness.ts +++ b/src/main/extensions/testing/runtimeHarness.ts @@ -44,29 +44,39 @@ async function until(predicate: () => Promise, label: string): Promise< void (async () => { await app.whenReady() - // Only requests for EGRESS_PATH are runtime egress (#1187). Every renderer - // probe below (fetch, window.open, navigation) targets exactly that path. - // Anything else on this port comes from outside the extension runtime: when - // the suite runs inside an Agent Code agent lane, the host app's browser- - // pocket LanePortWatcher finds this listener in the lane's process tree and - // probes it with one `GET /` (src/main/browserPocket/lanePortsIo.ts, Node - // fetch, `user-agent: node`). Under load that probe landed before the - // assertion below and failed the run with `['/']`, which the issue read as - // a runtime-API race. Recorded with request headers: every local run got that - // one `GET /` about 1.6 s after startup; CI has no watching app. + // Every request that reaches this server is runtime egress EXCEPT one exact + // shape: `GET /` (#1187). When the suite runs inside an Agent Code agent + // lane, the host app's browser-pocket LanePortWatcher finds this listener in + // the lane's process tree and probes it once with `GET /` + // (src/main/browserPocket/lanePortsIo.ts, Node fetch). Under load that probe + // landed before the assertion below and failed the run with `['/']`, which + // the issue read as a runtime-API race; request logging showed it in every + // local run, about 1.6 s after startup, and CI has no watching app. + // + // WHY ignore exactly `GET /` and not "anything but the probed path" (review + // b): a regression that leaked to any other path (`/leak`, an encoded or + // query variant) must still fail. Only the watcher's shape is excused, and + // the positive control below proves a non-probe path is counted. Residual: a + // runtime escape that requests exactly `GET /` would be excused too; the + // renderer probes all target EGRESS_PATH, so that would need a new probe. const EGRESS_PATH = '/private-state' const hits: string[] = [] - const foreignHits: string[] = [] + const ignoredWatcherProbes: string[] = [] const server = createServer((request, response) => { const url = request.url ?? '' - if (url.startsWith(EGRESS_PATH)) hits.push(url) - else foreignHits.push(`${request.method} ${url} ${request.headers['user-agent'] ?? ''}`) + if (request.method === 'GET' && url === '/') ignoredWatcherProbes.push(`${request.headers['user-agent'] ?? ''}`) + else hits.push(url) response.end('unexpected egress') }) await new Promise((resolve, reject) => { server.once('error', reject); server.listen(0, '127.0.0.1', resolve) }) const address = server.address() assert.ok(address && typeof address !== 'string') const endpoint = `http://127.0.0.1:${address.port}${EGRESS_PATH}` + // Positive control: a non-probe path IS counted (review b's surviving + // mutation replaced the classifier and the journey stayed green). + await fetch(`http://127.0.0.1:${address.port}/egress-control`).then(response => response.text()) + assert.deepEqual(hits, ['/egress-control'], 'the egress server must count a request to any non-probe path') + hits.length = 0 const statuses: RuntimeStatus[] = [] const lifecycle: string[] = [] const projectRoot = join(root!, 'project') @@ -237,7 +247,7 @@ void (async () => { await command('navigate') await new Promise(resolve => setTimeout(resolve, 100)) assert.equal(object(await command('snapshot')).activations, 1, 'renderer navigation must be refused') - assert.deepEqual(hits, [], `runtime egress reached the server (foreign, ignored: ${JSON.stringify(foreignHits)})`) + assert.deepEqual(hits, [], `runtime egress reached the server (ignored watcher probes: ${ignoredWatcherProbes.length})`) console.log('PASS runtime: command errors, host/preload isolation, forged namespaces and direct network/navigation denial') await views.attach(66, 'update-view', 'engine', revision, 'engine.main') From f04837d2b408c7f8cc33e02f140f8debf075f060 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 04:28:55 -0700 Subject: [PATCH 11/19] docs(plans): reconcile #1376 plan with the review outcome; drop EOF blank lines Reviewer C (round 2): Decisions/Tests/Delivery still stated the pre-review scope (a fully bounded run, no app tests, pointer-only). They now match the review appendix: the events file is bounded, the run directory is not (#1380), and the app-side wiring and its tests are listed. Co-Authored-By: Claude Opus 5.5 --- .../2026-09-26-proxy-events-retention.md | 21 +++++++++++++------ src/main/sessionManager.proxyGap.test.ts | 1 - src/main/storage/debugRetention.test.ts | 1 - .../runtime/claudeSession.suspension.test.ts | 1 - 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/docs/plans/2026-09-26-proxy-events-retention.md b/docs/plans/2026-09-26-proxy-events-retention.md index cabcdd65c..aeec6fcc1 100644 --- a/docs/plans/2026-09-26-proxy-events-retention.md +++ b/docs/plans/2026-09-26-proxy-events-retention.md @@ -27,8 +27,10 @@ issue still has open: - **Rotate in the addon, keep one previous generation.** When the events file reaches `PROXY_EVENTS_ROTATE_BYTES` (default 512 MiB), the addon renames it to `proxy-events.1.jsonl` (atomically replacing the older generation) and the next write starts a fresh - `proxy-events.jsonl`. A live run therefore holds at most ~2 × 512 MiB (+ the 16 MiB latest-body - sidecar), instead of growing for the life of the session. Rotation failure is non-fatal: the addon + `proxy-events.jsonl`. The **events file** therefore holds at most ~2 × 512 MiB instead of growing + for the life of the session. The run directory is NOT fully bounded by this change: the latest-body + sidecar is ~21.3 MiB on disk (16 MiB raw, base64; briefly twice that during its atomic replace), and + `sslkeylog.log` still grows without bound (#1380). See the review outcome under Delivery. Rotation failure is non-fatal: the addon keeps appending (forensics must never disturb the proxy). - WHY 512 MiB and one generation: the body budget (256 MiB) still applies per file, so each generation keeps ~100 turns of bodies plus a long stretch of body-less traffic; the debug bundle @@ -64,8 +66,10 @@ issue still has open: change was reverted (`0a462fad`). #1332 makes one provider-neutral, rotation-safe reader: one handle, `bytesRead` honoured, filling from `.1`. This branch merges main after #1332 and keeps only Claude-specific wiring, if any is still needed. -- **Retention unchanged.** With rotation, a live run is bounded, so the 10-minute grace no longer - lets one session fill the disk; old oversized files from before this change age out normally. +- **Retention: one change.** Run detection now also counts a directory holding only + `proxy-events.1.jsonl` (found in review). With rotation the events file is bounded, so the 10-minute + grace no longer lets one session's events fill the disk (`sslkeylog.log` is #1380); old oversized + files from before this change age out normally. ## Tests @@ -78,11 +82,16 @@ issue still has open: events written between the last poll and the rename are lost (asserted by name). - Poll ordering: rename observed while the new file is still absent, and a rotation that happens while the old generation ended mid-poll — both orders pinned. -- App: none on this branch; the bundle reader's tests live in W1's #1332 (q54). +- App (added after the #1376 review): `claudeSession.suspension.test.ts` pins that both the `event` + and `transport-gap` channels are forwarded and detached; `sessionManager.proxyGap.test.ts` pins the + `claude.proxy_transport_gap` incident, its re-emit, and that a replaced session's late gap is + ignored; `debugRetention.test.ts` pins that a run holding only `proxy-events.1.jsonl` is counted. + The bundle reader's own tests live in W1's #1332 (q54). ## Delivery -claude-code-headless#64 (addon + tailer): three reviews, verification, and a final round 3, MERGED (`f52fc82`). This agent-code PR only bumps the pointer. #1332 (W1's rotation-safe bundle reader) is already on main. +claude-code-headless#64 (addon + tailer): three reviews, verification, and a final round 3, MERGED (`f52fc82`). This agent-code PR bumps the pointer and, after its review, adds the app-side +wiring below (gap forwarding and incident, `.1`-only retention); it `Refs #1273` rather than fixing it. #1332 (W1's rotation-safe bundle reader) is already on main. - **No lockfile resync is needed.** The app consumes claude-code-headless from source (tsconfig and Vite aliases), not as a `file:` dependency, and its runtime dependencies (`chokidar`, `@xterm/headless`) are already root dependencies. The bump changes only the package's own devDependencies; `npm install --package-lock-only` leaves `package-lock.json` unchanged. - **The generation header** is one more JSON line with an unknown `kind` to the app's only direct reader (the bundle reader, which ships raw bytes). The addon recreates `proxy-events.jsonl` immediately, so `debugRetention`'s run detection is unaffected. - **Review of #1376 (a, b, c):** diff --git a/src/main/sessionManager.proxyGap.test.ts b/src/main/sessionManager.proxyGap.test.ts index 8b5731686..2be1d10fd 100644 --- a/src/main/sessionManager.proxyGap.test.ts +++ b/src/main/sessionManager.proxyGap.test.ts @@ -109,4 +109,3 @@ describe('a Claude proxy transport gap', () => { expect(incidents.filter(incident => incident.kind === 'claude.proxy_transport_gap')).toEqual([]) }) }) - diff --git a/src/main/storage/debugRetention.test.ts b/src/main/storage/debugRetention.test.ts index 9b79a5f8c..600c599af 100644 --- a/src/main/storage/debugRetention.test.ts +++ b/src/main/storage/debugRetention.test.ts @@ -242,4 +242,3 @@ describe('collectProxyRunDirs', () => { expect(runs).toEqual([live, rotated]) }) }) - diff --git a/src/providers/claude/runtime/claudeSession.suspension.test.ts b/src/providers/claude/runtime/claudeSession.suspension.test.ts index a8b191d26..46d42f896 100644 --- a/src/providers/claude/runtime/claudeSession.suspension.test.ts +++ b/src/providers/claude/runtime/claudeSession.suspension.test.ts @@ -95,4 +95,3 @@ describe('ClaudeSession proxy wiring', () => { expect(proxy.listenerCount('event') + proxy.listenerCount('transport-gap')).toBe(0) }) }) - From cfe7b1f26d516c8534175d37fe1c4248e7437061 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 04:29:58 -0700 Subject: [PATCH 12/19] docs(history): plan reporting a failed older-history load (#1250 row 12) Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-load-older-history-failure.md | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 docs/plans/2026-09-27-load-older-history-failure.md diff --git a/docs/plans/2026-09-27-load-older-history-failure.md b/docs/plans/2026-09-27-load-older-history-failure.md new file mode 100644 index 000000000..1c0799b50 --- /dev/null +++ b/docs/plans/2026-09-27-load-older-history-failure.md @@ -0,0 +1,30 @@ +# A failed "load older history" is said, not just un-spun (#1250 row 12) + +Short plan: a bug with a known root cause. The row comes from `temp/quality-loop/hunt-c3.md` (row 12, P3). #1250 is a batch issue, so this PR is `Refs #1250`. + +## Outcome +Scrolling to the top of an agent's feed pages in older history. When that page fails to load (an IPC error, or an unreadable transcript), the user is told, and learns how to retry. Today the catch in `useHistoryActions.loadOlderHistory` only clears `loadingOlderHistory`. The feed looks as if it simply has nothing older, although `hasOlderHistory` is still true. + +## Root cause (verified in source, origin/main) +- `src/renderer/src/workspace/hook/actions/history.ts`: on catch, it records a perf failure, warns to the console, and patches `loadingOlderHistory: false`. It returns `void` whatever happened, so no caller can tell a failure from a load. +- `TileLeaf.loadOlderHistory` awaits it and discards the result. `Feed` retries only on a NEW scroll event within 160 px of the top. + +## Design (contract) +- **`loadOlderHistory(sessionId): Promise<'loaded' | 'skipped' | 'failed'>`** (`OlderHistoryLoadResult`, exported from `history.ts`). + - `failed` only from the catch; `skipped` from every early return; `loaded` after the merge. +- **`TileLeaf`** shows a pane toast on `failed`: "Couldn't load older messages. Scroll up again to retry." + - Fixed text (q22): the error can carry a transcript path. + - Coalesced to one per 5 s per pane. While the scroller stays near the top, every scroll tick retries, and each failure must not stack another toast. + - It is a PANE toast because the failure belongs to this feed. +- `AgentFeed` and `Feed` keep `onLoadOlderHistory: () => Promise`. The phone mounts the same `AgentFeed` and is untouched. + +## Tests +- **`history.renderer.test.tsx`:** a rejecting `loadOlderHistory` IPC gives `failed` and leaves `hasOlderHistory` true (a retry stays possible). A successful page gives `loaded`; a missing marker gives `skipped`. Red on main: the result is `undefined`. +- **New `TileLeaf.olderHistory.renderer.test.tsx`:** with Feed probed for its `onLoadOlderHistory` prop and `workspace.loadOlderHistory` answering `failed`, the pane toast shows the fixed text once across two quick failures, and not at all on `loaded`. Red on main: no toast. + +## Verification boundary +Renderer tests drive the real hook and the real TileLeaf. The app is not launched. + +## Out of scope +- #1250's other rows. +- A persistent inline "retry" affordance in the feed header (a design call; the toast plus the existing scroll retry is enough to end the silence). From e9c9c9807d9686ef4307dd67cba081d886f019d2 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 04:49:27 -0700 Subject: [PATCH 13/19] fix(history): tell the user when an older-history page fails to load (#1250 row 12) loadOlderHistory now answers loaded/skipped/failed; TileLeaf shows a fixed, coalesced pane toast on failed. hasOlderHistory stays set so the next scroll to the top retries. Co-Authored-By: Claude Opus 5.5 --- .../hook/actions/history.renderer.test.tsx | 52 ++++++++++++ .../src/workspace/hook/actions/history.ts | 23 ++++-- .../TileLeaf.olderHistory.renderer.test.tsx | 82 +++++++++++++++++++ .../src/workspace/tile-tree/TileLeaf.tsx | 19 ++++- 4 files changed, 168 insertions(+), 8 deletions(-) create mode 100644 src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx diff --git a/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx index f1958a8fa..4bf0e43bc 100644 --- a/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx @@ -126,3 +126,55 @@ describe('older history and the ingest watermark (#915)', () => { }) }) }) + +// #1250 row 12: a failed page used to clear the spinner and nothing else, and +// returned nothing, so no caller could tell the user. The hook now answers +// what happened, and a failure leaves `hasOlderHistory` set so the next +// scroll to the top retries. +describe('what an older-history request reports', () => { + function harness(runtime: Partial) { + let runtimes: Record = { session: { ...emptyRuntime(), ...runtime } } + const refs = { + stateRef: ref({ sessions: { session: { kind: 'claude', cwd: '/tmp/project', providerSessionId: 'provider-session' } } }), + latestRuntimesRef: ref(runtimes), + seenUuidsRef: ref({}), + } as unknown as WorkspaceRefs + const setRuntimes: WorkspaceSetRuntimes = next => { + runtimes = typeof next === 'function' ? next(runtimes) : next + refs.latestRuntimesRef.current = runtimes + } + const updateRuntime = (id: string, patch: Partial) => { + setRuntimes(prev => ({ ...prev, [id]: { ...prev[id]!, ...patch } })) + } + const { result } = renderHook(() => useHistoryActions(setRuntimes, refs, updateRuntime, ipcSessionFeed)) + return { load: () => result.current.loadOlderHistory('session'), runtime: () => runtimes.session! } + } + + it('answers failed when the page cannot be read, and leaves a retry possible', async () => { + vi.spyOn(console, 'warn').mockImplementation(() => {}) + Object.defineProperty(window, 'api', { configurable: true, value: { + loadOlderHistory: vi.fn(async () => { throw new Error("ENOENT: no such file or directory, open '/Users/someone/.claude/projects/x.jsonl'") }), + gitWorktrees: vi.fn(async () => ({ ok: true, worktrees: [] })), + } }) + const { load, runtime } = harness({ hasOlderHistory: true, historyOldestMarker: 'anchor' }) + let answer: unknown + await act(async () => { answer = await load() }) + expect(answer).toBe('failed') + expect(runtime()).toMatchObject({ hasOlderHistory: true, loadingOlderHistory: false }) + vi.restoreAllMocks() + }) + + it('answers loaded for a page, and skipped when nothing was asked', async () => { + Object.defineProperty(window, 'api', { configurable: true, value: { + loadOlderHistory: vi.fn(async () => ({ entries: [], hasMore: false })), + gitWorktrees: vi.fn(async () => ({ ok: true, worktrees: [] })), + } }) + const loaded = harness({ hasOlderHistory: true, historyOldestMarker: 'anchor' }) + let answer: unknown + await act(async () => { answer = await loaded.load() }) + expect(answer).toBe('loaded') + const noMarker = harness({ hasOlderHistory: true, historyOldestMarker: null }) + await act(async () => { answer = await noMarker.load() }) + expect(answer).toBe('skipped') + }) +}) diff --git a/src/renderer/src/workspace/hook/actions/history.ts b/src/renderer/src/workspace/hook/actions/history.ts index c49cfb353..d41f1d92c 100644 --- a/src/renderer/src/workspace/hook/actions/history.ts +++ b/src/renderer/src/workspace/hook/actions/history.ts @@ -35,6 +35,12 @@ import type { SessionFeed } from '@shared/sessionFeed/SessionFeed' // markers so paged response_items get the same ownership metadata as // entries that arrived live. +/** What one older-history request did. `failed` means the page could not be + * read (the IPC rejected, or the transcript could not be parsed); `skipped` + * means nothing was asked (no provider session, nothing older, a load + * already running). Only `failed` is worth telling the user about. */ +export type OlderHistoryLoadResult = 'loaded' | 'skipped' | 'failed' + export function useHistoryActions( setRuntimes: WorkspaceSetRuntimes, refs: WorkspaceRefs, @@ -45,17 +51,17 @@ export function useHistoryActions( // and its one caller already holds the feed from context. feed: Pick, ): { - loadOlderHistory: (sessionId: SessionId) => Promise + loadOlderHistory: (sessionId: SessionId) => Promise } { const loadOlderHistory = useCallback( - async (sessionId: SessionId) => { + async (sessionId: SessionId): Promise => { const span = perf.span('workspace.history.loadOlder', { sessionId }) const currentState = refs.stateRef.current const meta = currentState.sessions[sessionId] const runtime = refs.latestRuntimesRef.current[sessionId] ?? emptyRuntime() if (!meta) { span.end({ skipped: 'missing-meta' }) - return + return 'skipped' } const kind = meta.kind ?? DEFAULT_PROVIDER @@ -65,19 +71,19 @@ export function useHistoryActions( // does page it gets the same answer. if (!isAgentProviderKind(kind) || !meta.providerSessionId) { span.end({ skipped: 'unsupported-or-missing-provider-session', kind }) - return + return 'skipped' } if (!runtime.hasOlderHistory || runtime.loadingOlderHistory) { span.end({ skipped: runtime.loadingOlderHistory ? 'already-loading' : 'no-older-history', kind, }) - return + return 'skipped' } if (!runtime.historyOldestMarker) { updateRuntime(sessionId, { hasOlderHistory: false, loadingOlderHistory: false }) span.end({ skipped: 'missing-marker', kind }) - return + return 'skipped' } updateRuntime(sessionId, { loadingOlderHistory: true }) @@ -246,10 +252,15 @@ export function useHistoryActions( prepended: prepend.length, hasMore: chunk.hasMore, }) + return 'loaded' } catch (err) { span.fail(err, { kind }) console.warn('[history] load older failed', err) + // `hasOlderHistory` is left as it was, so the next scroll to the top + // retries. The caller is TOLD (#1250 row 12): this used to be the + // whole of it, and the feed then looked as if it had nothing older. updateRuntime(sessionId, { loadingOlderHistory: false }) + return 'failed' } }, [refs.latestRuntimesRef, refs.seenUuidsRef, refs.stateRef, setRuntimes, updateRuntime, feed], diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx new file mode 100644 index 000000000..b4be6def9 --- /dev/null +++ b/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx @@ -0,0 +1,82 @@ +import { act, cleanup, render } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' + +import { useAppStore } from '@renderer/app-state/store' +import { emptyRuntime } from '@renderer/session-runtime/state' +import type { Workspace } from '@renderer/workspace/hook' +import type { OlderHistoryLoadResult } from '@renderer/workspace/hook/actions/history' +import { AgentTerminalOwnerVisibilityProvider } from '@renderer/workspace/terminal/AgentTerminalOwnership' +import { OLDER_HISTORY_FAILED, TileLeaf } from './TileLeaf' + +// #1250 row 12: a failed older-history page cleared its spinner and said +// nothing, so the feed looked as if it had nothing older. TileLeaf is where +// the answer from the hook meets the pane, so the wire is probed here: Feed is +// replaced by a probe that hands back the REAL `onLoadOlderHistory` prop +// TileLeaf passes down (through the real AgentFeed), and calling it is exactly +// what a scroll to the top does. + +let loadOlder: (() => Promise) | undefined +vi.mock('@renderer/features/feed/ui/Feed', () => ({ + Feed: ({ onLoadOlderHistory }: { onLoadOlderHistory?: () => Promise }) => { + loadOlder = onLoadOlderHistory + return
+ }, +})) +vi.mock('@renderer/features/sessionFeed/SessionFeedContext', () => ({ useSessionFeed: () => ({}) })) +vi.mock('@renderer/features/feed/ledger/useLedgerFeedItems', () => ({ useLedgerFeedItems: () => ({ items: [] }) })) +vi.mock('@renderer/features/usage-limit/useUsageLimitActions', () => ({ useUsageLimitActions: () => ({}) })) +vi.mock('@renderer/features/workflows/model/useSessionWorkflowViews', () => ({ + useSessionWorkflowViews: () => ({ references: [], allReferences: [], selectedReference: null }), +})) +vi.mock('./TileLeaf/useComposerKeybinds', () => ({ useComposerKeybinds: () => ({ onKeyDown: vi.fn(), slashMode: false, submitCurrentDraft: vi.fn() }) })) +vi.mock('./TileLeaf/useComposerDictation', () => ({ useComposerDictation: () => ({ handleShortcut: () => false }) })) +vi.mock('./TileLeaf/PaneHeader', () => ({ PaneHeader: () => null })) +vi.mock('./TileLeaf/QueueStrip', () => ({ QueueStrip: () => null })) +vi.mock('./TileLeaf/PaneToast', () => ({ PaneToast: () => null })) +vi.mock('./TileLeaf/ComposerInput', () => ({ ComposerInput: () => null })) +vi.mock('./TileLeaf/ComposerActions', () => ({ ComposerActions: () => null })) +vi.mock('@providers/shared/renderer/conditions/ProviderConditionOutlet', () => ({ ProviderConditionOutlet: () => null })) + +const original = useAppStore.getState() +beforeEach(() => { loadOlder = undefined }) +afterEach(() => { cleanup(); vi.useRealTimers(); useAppStore.setState(original, true) }) + +function mount(answers: OlderHistoryLoadResult[]) { + const paneToasts: string[] = [] + const workspace = { + state: { sessions: { agent: { kind: 'claude', cwd: '/trial' } } }, + acknowledgeSession: vi.fn(), + setDraftInput: vi.fn(), + loadOlderHistory: vi.fn(async () => answers.shift() ?? 'skipped'), + showPaneToast: (_sessionId: string, message: string) => { paneToasts.push(message) }, + } as unknown as Workspace + render( + + + , + ) + // A prop TileLeaf never passed would make every assertion below vacuous. + expect(loadOlder).toBeTypeOf('function') + return { paneToasts, workspace } +} + +it('says a failed page in fixed words, once per burst of retries', async () => { + vi.useFakeTimers({ toFake: ['Date'] }) + const { paneToasts } = mount(['failed', 'failed', 'failed']) + await act(async () => { await loadOlder!() }) + expect(paneToasts).toEqual([OLDER_HISTORY_FAILED]) + // Every scroll tick near the top retries; a burst is one toast. + await act(async () => { await loadOlder!() }) + expect(paneToasts).toHaveLength(1) + // After the coalescing window, a new failure is said again. + vi.setSystemTime(Date.now() + 5_001) + await act(async () => { await loadOlder!() }) + expect(paneToasts).toEqual([OLDER_HISTORY_FAILED, OLDER_HISTORY_FAILED]) +}) + +it('says nothing for a page that loaded or a request that was skipped', async () => { + const { paneToasts } = mount(['loaded', 'skipped']) + await act(async () => { await loadOlder!() }) + await act(async () => { await loadOlder!() }) + expect(paneToasts).toEqual([]) +}) diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx index 598b85ffb..9fdbd66b2 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx @@ -116,6 +116,10 @@ type Props = { showWorktreeBadges?: boolean } + +export const OLDER_HISTORY_FAILED = "Couldn't load older messages. Scroll up again to retry." +const OLDER_HISTORY_TOAST_COALESCE_MS = 5_000 + export function TileLeaf({ sessionId, runtime, @@ -508,9 +512,20 @@ export function TileLeaf({ } }, [acknowledgeSession, feed, runtime.conditions, sessionId, showToast]) + // WHY a pane toast on a failed page (#1250 row 12): the loader used to + // clear its spinner and nothing else, so the feed looked as if it had + // nothing older. Fixed words (q22): the error can name a transcript path. + // Coalesced, because while the scroller sits near the top every scroll + // tick retries, and each failure must not stack another toast. + const lastOlderHistoryToastAtRef = useRef(0) const loadOlderHistory = useCallback(async () => { - await workspace.loadOlderHistory(sessionId) - }, [sessionId, workspace.loadOlderHistory]) + const result = await workspace.loadOlderHistory(sessionId) + if (result !== 'failed') return + const now = Date.now() + if (now - lastOlderHistoryToastAtRef.current < OLDER_HISTORY_TOAST_COALESCE_MS) return + lastOlderHistoryToastAtRef.current = now + workspace.showPaneToast(sessionId, OLDER_HISTORY_FAILED) + }, [sessionId, workspace.loadOlderHistory, workspace.showPaneToast]) const appendRenderDebug = useCallback((entry: Parameters[1]) => { workspace.appendFeedDebug(sessionId, entry) From e905104e4c3c0a9e0409d8c8c4e5636500e36e57 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 05:10:17 -0700 Subject: [PATCH 14/19] fix(history): key the older-history toast cooldown by session (q106) Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-load-older-history-failure.md | 4 +++ src/main/sessions/historyLoader.test.ts | 16 +++++++++++ src/main/sessions/historyLoader.ts | 20 ++++++------- .../TileLeaf.olderHistory.renderer.test.tsx | 28 +++++++++++++++---- .../src/workspace/tile-tree/TileLeaf.tsx | 12 ++++++-- 5 files changed, 60 insertions(+), 20 deletions(-) diff --git a/docs/plans/2026-09-27-load-older-history-failure.md b/docs/plans/2026-09-27-load-older-history-failure.md index 1c0799b50..e5b1b9bb9 100644 --- a/docs/plans/2026-09-27-load-older-history-failure.md +++ b/docs/plans/2026-09-27-load-older-history-failure.md @@ -28,3 +28,7 @@ Renderer tests drive the real hook and the real TileLeaf. The app is not launche ## Out of scope - #1250's other rows. - A persistent inline "retry" affordance in the feed header (a design call; the toast plus the existing scroll retry is enough to end the silence). + +## Steering q106 +- The dispatch layout re-renders the SAME TileLeaf with another agent's `sessionId` when a lane switches. One timestamp per mounted leaf therefore let agent A's toast silence agent B's first failure for 5 s. +- The cooldown is now keyed by `sessionId`. Test: same-leaf rerender, A fails and toasts, the lane switches to B, B fails and toasts, and B's repeat still coalesces. It fails with the per-leaf timestamp. diff --git a/src/main/sessions/historyLoader.test.ts b/src/main/sessions/historyLoader.test.ts index 66f147ac3..881bd07fa 100644 --- a/src/main/sessions/historyLoader.test.ts +++ b/src/main/sessions/historyLoader.test.ts @@ -292,6 +292,22 @@ describe('readInitialTranscriptTail', () => { }) describe('readOlderTranscriptWindow', () => { + // #1413 review a: the older page of a transcript that became unreadable + // after its first page loaded came back as an empty page with + // `hasMore: false`. The renderer then dropped "older history exists" and + // said nothing. It now rejects, so the page is reported as failed and can + // be retried; a genuinely empty file stays an honest empty page. + it('rejects an older page of a transcript that became unreadable, but not of an empty one', async () => { + const file = writeClaude('vanishing.jsonl', 30) + const first = await loadInitialHistoryChunkFromFile(file, 12) + expect(first.hasMore).toBe(true) + rmSync(file) + await expect(loadOlderHistoryChunkFromFile(file, { kind: 'claude', beforeMarker: 'u-18', limit: 12 })).rejects.toThrow(/ENOENT/) + const empty = join(root, 'empty-older.jsonl') + writeFileSync(empty, '') + expect(await loadOlderHistoryChunkFromFile(empty, { kind: 'claude', beforeMarker: 'u-18', limit: 12 })).toEqual({ entries: [], hasMore: false }) + }) + it('pages older history identically to the forward marker scan at every depth, with and without an offset', async () => { const file = writeClaude('pages.jsonl', 300) const bytes = readFileSync(file) diff --git a/src/main/sessions/historyLoader.ts b/src/main/sessions/historyLoader.ts index 458af40ed..8b314647e 100644 --- a/src/main/sessions/historyLoader.ts +++ b/src/main/sessions/historyLoader.ts @@ -550,7 +550,14 @@ async function readOlderTranscriptWindow( entries: Record[] offsets: number[] }> { - const size = await stat(filePath).then(s => s.size).catch(() => 0) + // WHY an unreadable transcript THROWS here (#1413 review a): this reader + // serves only older-history paging, where the renderer already holds a + // page from this file. A failed stat/open/read used to come back as an + // empty page with `hasMore: false`, so the feed dropped "older history + // exists" for good and the user was told nothing. A rejection reaches + // the renderer as a failed page, which is said and can be retried. An + // EMPTY file (stat succeeded, size 0) is still an honest empty page. + const size = (await stat(filePath)).size const empty = { bytes: size, tailBytes: 0, @@ -571,13 +578,7 @@ async function readOlderTranscriptWindow( : extractCodexHistoryMarker const limit = Math.max(0, params.limit) - let handle: FileHandle - try { - handle = await open(filePath, 'r') - } catch (error) { - if (params.beforeRecordHash) throw error - return empty - } + const handle: FileHandle = await open(filePath, 'r') const stats = parseStats() try { let parseErrors = 0 @@ -642,9 +643,6 @@ async function readOlderTranscriptWindow( return false }) return finishWindow(size, tailBytes, parseErrors, parsed, found, found ? 'marker' : 'tail', kept, limit) - } catch (error) { - if (params.beforeRecordHash) throw error - return empty } finally { observeParse(stats) await handle.close().catch(() => {}) diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx index b4be6def9..2166a3f91 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx @@ -43,21 +43,23 @@ afterEach(() => { cleanup(); vi.useRealTimers(); useAppStore.setState(original, function mount(answers: OlderHistoryLoadResult[]) { const paneToasts: string[] = [] + const toastSessions: string[] = [] const workspace = { - state: { sessions: { agent: { kind: 'claude', cwd: '/trial' } } }, + state: { sessions: { agent: { kind: 'claude', cwd: '/trial' }, other: { kind: 'claude', cwd: '/trial' } } }, acknowledgeSession: vi.fn(), setDraftInput: vi.fn(), loadOlderHistory: vi.fn(async () => answers.shift() ?? 'skipped'), - showPaneToast: (_sessionId: string, message: string) => { paneToasts.push(message) }, + showPaneToast: (sessionId: string, message: string) => { paneToasts.push(message); toastSessions.push(sessionId) }, } as unknown as Workspace - render( + const leaf = (sessionId: string) => ( - - , + + ) + const view = render(leaf('agent')) // A prop TileLeaf never passed would make every assertion below vacuous. expect(loadOlder).toBeTypeOf('function') - return { paneToasts, workspace } + return { paneToasts, toastSessions, workspace, switchTo: (sessionId: string) => view.rerender(leaf(sessionId)) } } it('says a failed page in fixed words, once per burst of retries', async () => { @@ -80,3 +82,17 @@ it('says nothing for a page that loaded or a request that was skipped', async () await act(async () => { await loadOlder!() }) expect(paneToasts).toEqual([]) }) + +// Steering q106: the dispatch layout re-renders the SAME leaf with another +// agent when a lane switches. Agent A's toast must not silence agent B's +// first failure; repeated failures of one agent still coalesce. +it('does not let one agent\'s toast silence another agent in the same leaf', async () => { + vi.useFakeTimers({ toFake: ['Date'] }) + const { paneToasts, toastSessions, switchTo } = mount(['failed', 'failed', 'failed']) + await act(async () => { await loadOlder!() }) + switchTo('other') + await act(async () => { await loadOlder!() }) + expect(toastSessions).toEqual(['agent', 'other']) + await act(async () => { await loadOlder!() }) + expect(paneToasts).toEqual([OLDER_HISTORY_FAILED, OLDER_HISTORY_FAILED]) +}) diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx index 9fdbd66b2..10f3d027c 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx @@ -517,13 +517,19 @@ export function TileLeaf({ // nothing older. Fixed words (q22): the error can name a transcript path. // Coalesced, because while the scroller sits near the top every scroll // tick retries, and each failure must not stack another toast. - const lastOlderHistoryToastAtRef = useRef(0) + // + // Keyed by session, not per mounted leaf (steering q106): the dispatch + // layout re-renders the SAME TileLeaf with another agent's sessionId when + // a lane switches, so one timestamp let agent A's toast silence agent B's + // first failure for the rest of the window. + const lastOlderHistoryToastAtRef = useRef(new Map()) const loadOlderHistory = useCallback(async () => { const result = await workspace.loadOlderHistory(sessionId) if (result !== 'failed') return const now = Date.now() - if (now - lastOlderHistoryToastAtRef.current < OLDER_HISTORY_TOAST_COALESCE_MS) return - lastOlderHistoryToastAtRef.current = now + const last = lastOlderHistoryToastAtRef.current.get(sessionId) ?? 0 + if (now - last < OLDER_HISTORY_TOAST_COALESCE_MS) return + lastOlderHistoryToastAtRef.current.set(sessionId, now) workspace.showPaneToast(sessionId, OLDER_HISTORY_FAILED) }, [sessionId, workspace.loadOlderHistory, workspace.showPaneToast]) From f72f8670589125e896e2422964bf01c2ae30ea2e Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 05:16:05 -0700 Subject: [PATCH 15/19] fix(feed): an upward wheel at the top retries older history (#1413 review a, b) Also records in the plan that e905104e carried the loader fix (an unreadable transcript now rejects an older page instead of paging it as empty). Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-load-older-history-failure.md | 10 +++ .../ui/Feed.olderHistory.renderer.test.tsx | 32 +++++++++ src/renderer/src/features/feed/ui/Feed.tsx | 67 ++++++++++++------- 3 files changed, 83 insertions(+), 26 deletions(-) create mode 100644 src/renderer/src/features/feed/ui/Feed.olderHistory.renderer.test.tsx diff --git a/docs/plans/2026-09-27-load-older-history-failure.md b/docs/plans/2026-09-27-load-older-history-failure.md index e5b1b9bb9..74bce0140 100644 --- a/docs/plans/2026-09-27-load-older-history-failure.md +++ b/docs/plans/2026-09-27-load-older-history-failure.md @@ -32,3 +32,13 @@ Renderer tests drive the real hook and the real TileLeaf. The app is not launche ## Steering q106 - The dispatch layout re-renders the SAME TileLeaf with another agent's `sessionId` when a lane switches. One timestamp per mounted leaf therefore let agent A's toast silence agent B's first failure for 5 s. - The cooldown is now keyed by `sessionId`. Test: same-leaf rerender, A fails and toasts, the lane switches to B, B fails and toasts, and B's repeat still coalesces. It fails with the per-leaf timestamp. + +## Review round 1 (a, b: FIX-BEFORE-MERGE) +- **a (Major): an unreadable transcript was paged as an empty page with `hasMore: false`.** The main-side `readOlderTranscriptWindow` swallowed stat/open/read failures, so the renderer dropped "older history exists" and returned `loaded`. + - **Ruling:** this reader serves only older paging, so a failure now rejects. The page is reported `failed` and stays retryable. An empty file (stat succeeded, size 0) is still an honest empty page, and the initial-chunk reader is unchanged. + - Test in `historyLoader.test.ts`: the first page loads, the file is removed, the older page rejects with ENOENT, and an empty file stays empty. It fails on the old loader. + - This landed in `e905104e`, whose message names only q106. +- **a, b (Major): "Scroll up again to retry" was impossible at the top.** At `scrollTop` 0 an upward gesture fires no scroll event, and Feed triggered only on scroll. An upward wheel at the top now makes the same request. + - Test: the real Feed, two upward wheels at 0, two requests (red on the old Feed). A downward wheel makes none. It also kills a's survivor (`loadingOlderRef` left true). +- **b (Major): the cooldown was shared across agents in one lane.** This is q106, fixed. +- **b (Minor): the raw error still reaches `console.warn` and the perf span.** Declined: these are developer diagnostics, not user-visible text (q22 governs what the user sees). The perf journal already records file paths on this path by design (`finishOlderChunk`). diff --git a/src/renderer/src/features/feed/ui/Feed.olderHistory.renderer.test.tsx b/src/renderer/src/features/feed/ui/Feed.olderHistory.renderer.test.tsx new file mode 100644 index 000000000..52b14fdbf --- /dev/null +++ b/src/renderer/src/features/feed/ui/Feed.olderHistory.renderer.test.tsx @@ -0,0 +1,32 @@ +import { act, fireEvent, render, screen } from '@testing-library/react' +import { describe, expect, it, vi } from 'vitest' + +import { Feed } from '@renderer/features/feed/ui/Feed' + +// #1413 review a and b: after a failed older-history page, the pane says +// "Scroll up again to retry". The real Feed used to request a page only on a +// `scroll` event, and at scrollTop 0 an upward wheel moves nothing, so no +// event fired: the retry the toast promised could not happen at the top. + +describe('Feed older-history trigger', () => { + it('retries a failed page on an upward wheel at the very top', async () => { + const onLoadOlderHistory = vi.fn(async () => {}) + render() + const scroller = screen.getByRole('region', { name: 'Conversation' }) + expect(scroller.scrollTop).toBe(0) + // First request (as a scroll to the top makes it) fails in the hook. + await act(async () => { fireEvent.wheel(scroller, { deltaY: -40 }) }) + expect(onLoadOlderHistory).toHaveBeenCalledTimes(1) + // The user does what the toast says: another upward gesture, still at 0. + await act(async () => { fireEvent.wheel(scroller, { deltaY: -40 }) }) + expect(onLoadOlderHistory).toHaveBeenCalledTimes(2) + }) + + it('does not load on a downward wheel', async () => { + const onLoadOlderHistory = vi.fn(async () => {}) + render() + const scroller = screen.getByRole('region', { name: 'Conversation' }) + await act(async () => { fireEvent.wheel(scroller, { deltaY: 40 }) }) + expect(onLoadOlderHistory).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/features/feed/ui/Feed.tsx b/src/renderer/src/features/feed/ui/Feed.tsx index 384a98360..daed549a7 100644 --- a/src/renderer/src/features/feed/ui/Feed.tsx +++ b/src/renderer/src/features/feed/ui/Feed.tsx @@ -476,6 +476,33 @@ function FeedImpl({ useEffect(() => { const el = scrollerRef.current if (!el) return + const loadOlderNearTop = () => { + if ( + el.scrollTop < 160 && + hasOlderHistory && + !loadingOlderHistory && + !loadingOlderRef.current && + !tailMode && + onLoadOlderHistory + ) { + loadingOlderRef.current = true + const beforeHeight = el.scrollHeight + const beforeTop = el.scrollTop + void onLoadOlderHistory() + .then(() => { + requestAnimationFrame(() => { + const next = scrollerRef.current + if (!next) return + const delta = next.scrollHeight - beforeHeight + next.scrollTop = beforeTop + Math.max(0, delta) + lastScrollTopRef.current = next.scrollTop + }) + }) + .finally(() => { + loadingOlderRef.current = false + }) + } + } const onScroll = () => { if (tailMode) { el.scrollTop = el.scrollHeight @@ -520,34 +547,22 @@ function FeedImpl({ onScrollInfo({ fraction }) } - if ( - el.scrollTop < 160 && - hasOlderHistory && - !loadingOlderHistory && - !loadingOlderRef.current && - !tailMode && - onLoadOlderHistory - ) { - loadingOlderRef.current = true - const beforeHeight = el.scrollHeight - const beforeTop = el.scrollTop - void onLoadOlderHistory() - .then(() => { - requestAnimationFrame(() => { - const next = scrollerRef.current - if (!next) return - const delta = next.scrollHeight - beforeHeight - next.scrollTop = beforeTop + Math.max(0, delta) - lastScrollTopRef.current = next.scrollTop - }) - }) - .finally(() => { - loadingOlderRef.current = false - }) - } + loadOlderNearTop() } el.addEventListener('scroll', onScroll, { passive: true }) - return () => el.removeEventListener('scroll', onScroll) + // WHY a wheel trigger too (#1413 review a, b): the loader used to fire only + // on a `scroll` event. At scrollTop 0 an upward wheel or trackpad gesture + // moves nothing, so no scroll event fires, and after a failed page the + // pane's "Scroll up again to retry" could not be done without first + // scrolling down. An upward wheel AT the top is the same request. + const onWheel = (event: WheelEvent) => { + if (event.deltaY < 0 && el.scrollTop <= 0) loadOlderNearTop() + } + el.addEventListener('wheel', onWheel, { passive: true }) + return () => { + el.removeEventListener('scroll', onScroll) + el.removeEventListener('wheel', onWheel) + } }, [ sessionId, onScrollInfo, From 55527cd5d08228173a30d2539052b963950126be Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 05:16:29 -0700 Subject: [PATCH 16/19] test(extensions): the egress control also probes the real egress path; sync the plan (#1406 review c) Co-Authored-By: Claude Opus 5.5 --- ...026-09-27-extension-runtime-harness-egress.md | 16 +++++++++++++--- src/main/extensions/testing/runtimeHarness.ts | 6 +++++- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/docs/plans/2026-09-27-extension-runtime-harness-egress.md b/docs/plans/2026-09-27-extension-runtime-harness-egress.md index 9f9355852..71e1cfc20 100644 --- a/docs/plans/2026-09-27-extension-runtime-harness-egress.md +++ b/docs/plans/2026-09-27-extension-runtime-harness-egress.md @@ -7,8 +7,18 @@ - **The other two log lines are not the bug.** The `ZodError ... "extensionId"` is the harness's deliberate forgery probe (`runtimeHarness.ts`, the `sandbox` command expects `spoofDenied: true`). `No handler registered for 'extensions:runtime-api'` also appears twice in every passing run during disposal. ## Change -Every renderer egress probe (fetch, window.open, navigation) targets `/private-state`. The harness now counts only requests for that path as egress. Anything else is recorded as foreign, ignored, and named in the assertion message. +Every request that reaches the harness's egress server counts as runtime egress EXCEPT the watcher's exact shape, `GET /`, which is recorded and ignored. + +**Superseded first version:** it counted only `/private-state` and ignored every other path. Review b showed that this would miss a leak to any other path (`/leak`, encoded or query variants). + +**Positive control:** right after binding, main fetches both `/private-state` and `/egress-control`, and both must be counted before the journey starts (review c). ## Verification -- 0 of 10 runs fail under the same load. -- A mutant that removes `will-navigate`, `will-frame-navigate` and the `webRequest` block still fails the egress assertion (foreign list empty), so the filter did not blind the check. +- **Under load:** 0 of 10 runs fail with the fix; 2 of 10 failed before it. +- **Mutations, each failing the run:** + - an "ignore everything" classifier fails the control; + - excusing `/private-state` fails the control; + - a main-side `GET /leak` after the control fails the egress assertion; + - removing `will-navigate`, `will-frame-navigate` and the `webRequest` block fails the egress assertion. + +**Residual:** a runtime escape that requests exactly `GET /` is excused. The renderer probes all target `/private-state`. diff --git a/src/main/extensions/testing/runtimeHarness.ts b/src/main/extensions/testing/runtimeHarness.ts index b19b83102..50532ed88 100644 --- a/src/main/extensions/testing/runtimeHarness.ts +++ b/src/main/extensions/testing/runtimeHarness.ts @@ -74,8 +74,12 @@ void (async () => { const endpoint = `http://127.0.0.1:${address.port}${EGRESS_PATH}` // Positive control: a non-probe path IS counted (review b's surviving // mutation replaced the classifier and the journey stayed green). + // Both the path every renderer probe uses and an arbitrary one (review c: + // a regression that excused EGRESS_PATH itself kept an /egress-control-only + // control green while blinding the real check). + await fetch(endpoint).then(response => response.text()) await fetch(`http://127.0.0.1:${address.port}/egress-control`).then(response => response.text()) - assert.deepEqual(hits, ['/egress-control'], 'the egress server must count a request to any non-probe path') + assert.deepEqual(hits, [EGRESS_PATH, '/egress-control'], 'the egress server must count the probed path and any other non-watcher path') hits.length = 0 const statuses: RuntimeStatus[] = [] const lifecycle: string[] = [] From e349c814ce78a83431d92a5bcb9a61e7b12cabd4 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 05:23:58 -0700 Subject: [PATCH 17/19] test(history): pin skip classification, window size, literal text and target pane (#1413 review c) Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-load-older-history-failure.md | 6 ++++ .../hook/actions/history.renderer.test.tsx | 30 ++++++++++++++++++- .../TileLeaf.olderHistory.renderer.test.tsx | 16 ++++++++-- 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/docs/plans/2026-09-27-load-older-history-failure.md b/docs/plans/2026-09-27-load-older-history-failure.md index 74bce0140..59a93b91f 100644 --- a/docs/plans/2026-09-27-load-older-history-failure.md +++ b/docs/plans/2026-09-27-load-older-history-failure.md @@ -42,3 +42,9 @@ Renderer tests drive the real hook and the real TileLeaf. The app is not launche - Test: the real Feed, two upward wheels at 0, two requests (red on the old Feed). A downward wheel makes none. It also kills a's survivor (`loadingOlderRef` left true). - **b (Major): the cooldown was shared across agents in one lane.** This is q106, fixed. - **b (Minor): the raw error still reaches `console.warn` and the perf span.** Declined: these are developer diagnostics, not user-visible text (q22 governs what the user sees). The perf journal already records file paths on this path by design (`finishOlderChunk`). +- **c (MERGE-READY; four test gaps, each pinned):** + - every early return answers `skipped`; + - a retry 1 s into the window stays suppressed; + - the toast text is asserted literally; + - the toast goes to this pane. + c's mutations M7 (no-older → failed) and T3 (5 s → 500 ms) now fail. diff --git a/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx index 4bf0e43bc..61ed5db2a 100644 --- a/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/history.renderer.test.tsx @@ -147,7 +147,7 @@ describe('what an older-history request reports', () => { setRuntimes(prev => ({ ...prev, [id]: { ...prev[id]!, ...patch } })) } const { result } = renderHook(() => useHistoryActions(setRuntimes, refs, updateRuntime, ipcSessionFeed)) - return { load: () => result.current.loadOlderHistory('session'), runtime: () => runtimes.session! } + return { load: () => result.current.loadOlderHistory('session'), runtime: () => runtimes.session!, refs } } it('answers failed when the page cannot be read, and leaves a retry possible', async () => { @@ -164,6 +164,34 @@ describe('what an older-history request reports', () => { vi.restoreAllMocks() }) + // #1413 review c: every early return is `skipped`, never `failed`. A + // `failed` here would toast "Couldn't load older messages" on every scroll + // tick of a feed that simply has nothing older. + it.each([ + ['no older history', { hasOlderHistory: false, historyOldestMarker: 'anchor' }], + ['a load already running', { hasOlderHistory: true, loadingOlderHistory: true, historyOldestMarker: 'anchor' }], + ] as const)('answers skipped for %s', async (_name, runtime) => { + const loadOlderHistory = vi.fn() + Object.defineProperty(window, 'api', { configurable: true, value: { loadOlderHistory, gitWorktrees: vi.fn() } }) + const { load } = harness(runtime) + let answer: unknown + await act(async () => { answer = await load() }) + expect(answer).toBe('skipped') + expect(loadOlderHistory).not.toHaveBeenCalled() + }) + + it('answers skipped for a session it cannot page (no meta, no provider session)', async () => { + Object.defineProperty(window, 'api', { configurable: true, value: { loadOlderHistory: vi.fn(), gitWorktrees: vi.fn() } }) + const { load, refs } = harness({ hasOlderHistory: true, historyOldestMarker: 'anchor' }) + let answer: unknown + ;(refs.stateRef.current as { sessions: Record }).sessions = { session: { kind: 'claude', cwd: '/tmp/project' } } + await act(async () => { answer = await load() }) + expect(answer).toBe('skipped') + ;(refs.stateRef.current as { sessions: Record }).sessions = {} + await act(async () => { answer = await load() }) + expect(answer).toBe('skipped') + }) + it('answers loaded for a page, and skipped when nothing was asked', async () => { Object.defineProperty(window, 'api', { configurable: true, value: { loadOlderHistory: vi.fn(async () => ({ entries: [], hasMore: false })), diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx index 2166a3f91..d74287a97 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf.olderHistory.renderer.test.tsx @@ -64,14 +64,24 @@ function mount(answers: OlderHistoryLoadResult[]) { it('says a failed page in fixed words, once per burst of retries', async () => { vi.useFakeTimers({ toFake: ['Date'] }) - const { paneToasts } = mount(['failed', 'failed', 'failed']) + const { paneToasts, toastSessions } = mount(['failed', 'failed', 'failed', 'failed']) await act(async () => { await loadOlder!() }) - expect(paneToasts).toEqual([OLDER_HISTORY_FAILED]) + // The literal words (#1413 review c): comparing against the exported + // constant would let a reworded or interpolated message pass. And on THIS + // pane. + expect(paneToasts).toEqual(["Couldn't load older messages. Scroll up again to retry."]) + expect(toastSessions).toEqual(['agent']) + expect(OLDER_HISTORY_FAILED).toBe(paneToasts[0]) // Every scroll tick near the top retries; a burst is one toast. await act(async () => { await loadOlder!() }) expect(paneToasts).toHaveLength(1) + // Still inside the window a second later (review c: a window shrunk to + // 500 ms would toast here). + vi.setSystemTime(Date.now() + 1_000) + await act(async () => { await loadOlder!() }) + expect(paneToasts).toHaveLength(1) // After the coalescing window, a new failure is said again. - vi.setSystemTime(Date.now() + 5_001) + vi.setSystemTime(Date.now() + 4_001) await act(async () => { await loadOlder!() }) expect(paneToasts).toEqual([OLDER_HISTORY_FAILED, OLDER_HISTORY_FAILED]) }) From 5ef56d7752bed1940b3756d3293cef0ad8f49312 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 05:25:26 -0700 Subject: [PATCH 18/19] fix(feed): a downward touch drag at the top also retries older history (#1413 verification b) Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-load-older-history-failure.md | 1 + .../ui/Feed.olderHistory.renderer.test.tsx | 17 +++++++++++++++++ src/renderer/src/features/feed/ui/Feed.tsx | 19 +++++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/docs/plans/2026-09-27-load-older-history-failure.md b/docs/plans/2026-09-27-load-older-history-failure.md index 59a93b91f..8c9c6373d 100644 --- a/docs/plans/2026-09-27-load-older-history-failure.md +++ b/docs/plans/2026-09-27-load-older-history-failure.md @@ -48,3 +48,4 @@ Renderer tests drive the real hook and the real TileLeaf. The app is not launche - the toast text is asserted literally; - the toast goes to this pane. c's mutations M7 (no-older → failed) and T3 (5 s → 500 ms) now fail. +- **Verification b (Major): the touch form of the gesture was still missing.** A downward finger drag at the top now makes the same request. Test with the real Feed: an upward drag makes none, a downward one makes one. It fails on the previous Feed. diff --git a/src/renderer/src/features/feed/ui/Feed.olderHistory.renderer.test.tsx b/src/renderer/src/features/feed/ui/Feed.olderHistory.renderer.test.tsx index 52b14fdbf..543174d36 100644 --- a/src/renderer/src/features/feed/ui/Feed.olderHistory.renderer.test.tsx +++ b/src/renderer/src/features/feed/ui/Feed.olderHistory.renderer.test.tsx @@ -22,6 +22,23 @@ describe('Feed older-history trigger', () => { expect(onLoadOlderHistory).toHaveBeenCalledTimes(2) }) + // Verification b: the touch form of the same gesture. + it('retries on a downward finger drag at the very top, and not on an upward one', async () => { + const onLoadOlderHistory = vi.fn(async () => {}) + render() + const scroller = screen.getByRole('region', { name: 'Conversation' }) + await act(async () => { + fireEvent.touchStart(scroller, { touches: [{ clientY: 100 }] }) + fireEvent.touchMove(scroller, { touches: [{ clientY: 60 }] }) + }) + expect(onLoadOlderHistory).not.toHaveBeenCalled() + await act(async () => { + fireEvent.touchStart(scroller, { touches: [{ clientY: 100 }] }) + fireEvent.touchMove(scroller, { touches: [{ clientY: 140 }] }) + }) + expect(onLoadOlderHistory).toHaveBeenCalledTimes(1) + }) + it('does not load on a downward wheel', async () => { const onLoadOlderHistory = vi.fn(async () => {}) render() diff --git a/src/renderer/src/features/feed/ui/Feed.tsx b/src/renderer/src/features/feed/ui/Feed.tsx index daed549a7..b03e92977 100644 --- a/src/renderer/src/features/feed/ui/Feed.tsx +++ b/src/renderer/src/features/feed/ui/Feed.tsx @@ -559,9 +559,28 @@ function FeedImpl({ if (event.deltaY < 0 && el.scrollTop <= 0) loadOlderNearTop() } el.addEventListener('wheel', onWheel, { passive: true }) + // The touch form of the same gesture (#1413 verification b): a finger + // dragging DOWN at the top asks for older content and, at the boundary, + // fires no scroll event either. + let touchStartY: number | null = null + const onTouchStart = (event: TouchEvent) => { + touchStartY = event.touches[0]?.clientY ?? null + } + const onTouchMove = (event: TouchEvent) => { + const y = event.touches[0]?.clientY + if (touchStartY === null || y === undefined) return + if (y > touchStartY && el.scrollTop <= 0) { + touchStartY = null + loadOlderNearTop() + } + } + el.addEventListener('touchstart', onTouchStart, { passive: true }) + el.addEventListener('touchmove', onTouchMove, { passive: true }) return () => { el.removeEventListener('scroll', onScroll) el.removeEventListener('wheel', onWheel) + el.removeEventListener('touchstart', onTouchStart) + el.removeEventListener('touchmove', onTouchMove) } }, [ sessionId, From fd5c2ef97463f6529f850fa9419358c917304b45 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 05:28:03 -0700 Subject: [PATCH 19/19] fix(history): an older page with an unresolvable transcript rejects (#1413 verification a) Co-Authored-By: Claude Opus 5.5 --- docs/plans/2026-09-27-load-older-history-failure.md | 1 + .../sessions/historyLoader.providerOwned.test.ts | 12 ++++++++++++ src/main/sessions/historyLoader.ts | 11 +++++++++-- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/docs/plans/2026-09-27-load-older-history-failure.md b/docs/plans/2026-09-27-load-older-history-failure.md index 8c9c6373d..9803a0a96 100644 --- a/docs/plans/2026-09-27-load-older-history-failure.md +++ b/docs/plans/2026-09-27-load-older-history-failure.md @@ -49,3 +49,4 @@ Renderer tests drive the real hook and the real TileLeaf. The app is not launche - the toast goes to this pane. c's mutations M7 (no-older → failed) and T3 (5 s → 500 ms) now fail. - **Verification b (Major): the touch form of the gesture was still missing.** A downward finger drag at the top now makes the same request. Test with the real Feed: an upward drag makes none, a downward one makes one. It fails on the previous Feed. +- **Verification a (Major): a Codex rollout the resolver could no longer find still made an older page an empty `hasMore: false`.** `loadOlderHistoryChunk` returned that before reaching the now-strict reader. An older page with an unresolvable transcript now rejects. The perf span fails, and the phone's `RemoteServer` already turns a throw into a structured `ok: false`. Test in `historyLoader.providerOwned.test.ts` (Codex, resolver returns null), red on the previous loader. diff --git a/src/main/sessions/historyLoader.providerOwned.test.ts b/src/main/sessions/historyLoader.providerOwned.test.ts index 145304af2..c9c416816 100644 --- a/src/main/sessions/historyLoader.providerOwned.test.ts +++ b/src/main/sessions/historyLoader.providerOwned.test.ts @@ -57,6 +57,18 @@ describe('historyLoader with a provider-owned history source', () => { expect(span.end).not.toHaveBeenCalled() }) + // #1413 verification a: a Codex rollout the resolver can no longer find + // (removed, or unreadable) made an OLDER page an empty `hasMore: false`, + // so the pane dropped "older history exists" with nothing said. An older + // page is only asked for after one loaded; it now rejects, which the + // renderer reports as a failed, retryable page. + it('rejects an older page whose transcript can no longer be resolved', async () => { + registry.resolveTranscriptPath.mockResolvedValue(null) + await expect(loadOlderHistoryChunk({ kind: 'codex', cwd: '/w', providerSessionId: 'thread-1', beforeMarker: 'm', limit: 5 })) + .rejects.toThrow('could not be found') + expect(span.fail).toHaveBeenCalledOnce() + }) + it('keeps file-backed providers on the JSONL path', async () => { registry.resolveTranscriptPath.mockResolvedValue(null) // The assertion that matters is the ROUTING: a provider without its own diff --git a/src/main/sessions/historyLoader.ts b/src/main/sessions/historyLoader.ts index 8b314647e..a05a1f777 100644 --- a/src/main/sessions/historyLoader.ts +++ b/src/main/sessions/historyLoader.ts @@ -706,10 +706,17 @@ export async function loadOlderHistoryChunk( // + catch scaffolding in both). const filePath = await resolveHistoryTranscriptPath(params) if (!filePath) { + // WHY a rejection and not an empty page (#1413 verification a): an older + // page is only ever asked for after a page of this transcript loaded, so + // "no file now" means it vanished or cannot be located, not that there is + // nothing older. An empty `hasMore: false` made the renderer drop "older + // history exists" for good, with nothing said; a rejection is reported as + // a failed page and stays retryable. + const error = new Error('The transcript for this session could not be found') performanceService .span('historyLoader.loadOlderChunk', { kind: params.kind, limit: params.limit }) - .end({ result: 'missing-file' }) - return { entries: [], hasMore: false } + .fail(error, { result: 'missing-file' }) + throw error } return loadOlderHistoryChunkFromFile(filePath, params) }