diff --git a/docs/plans/2026-09-25-screen-ipc-interest.md b/docs/plans/2026-09-25-screen-ipc-interest.md new file mode 100644 index 000000000..84290d600 --- /dev/null +++ b/docs/plans/2026-09-25-screen-ipc-interest.md @@ -0,0 +1,42 @@ +# Screen IPC by interest (#762) + +## Problem +`session:screen` is the largest IPC stream by far: +- **Share of traffic:** 93% of recorded IPC bytes in recording g00000190 (16,728 frames of about 8.8 KB over 123 minutes). The committed fixture `testing/fixtures/worktree-live-attribution/codex-live-channel-gap.json` shows the same shape: 2,358 of about 12,600 recorded events. +- **Rate:** 6–10 frames a second for each busy session. Every frame is structured-cloned, dispatched, and committed into the renderer's runtime map. + +## Evidence that nothing live needs it +All consumers were mapped (file:line in PR #1236's description): +- **Main never needs the renderer's copy.** Prompt delivery, paste confirmation and readiness read the screen in main through `session.snapshotScreen()`. The raw snapshot cache (`SessionManager.lastScreenSnapshot`) and the spinner gate are untouched. +- **The composer picker is owned by conditions.** It comes from the `claude.slash-picker` condition via `applyConditionSnapshot`. The screen handler's picker write was a second writer (split authority). +- **Dead code:** `latestScreenRef` (the Enter baseline) and the renderer `claudePaste` helpers have no callers. +- **ReaderView** has no screen fallback since #855. +- **What remains is debug-only:** DebugPanel, three dev modules, debug bundles (including the 60 s autosave), and the screen-tail trace. + +## Change +1. **Main, forwarder gate.** `session:screen` is forwarded only for a session that some renderer holds a lease on (`sessions/screenInterest.ts`). Every frame still feeds a bounded screen-tail history in main (`shared/debug/screenTail.ts`), because debug bundles must not depend on a panel having been open. +2. **Lease contract:** + - `session:screen-lease` acquires a lease and seeds the current screen down the ordinary `session:screen` path, the same seed `session:recover` sends. An idle backend therefore still shows a correct panel. + - `session:screen-release` releases a lease. + - Leases are owned by the calling webContents and the document that took them (a per-load id minted in preload). The preload announces its document on every load, leasing or not; a new document (a reload), or destruction, drops all of that owner's earlier leases, because a dying renderer never runs its cleanup. Review amendment: this replaced dropping on `did-start-navigation`, which Chromium fires before the throttle where this app blocks every `will-navigate`, so a blocked link click used to drop live leases. A release the owner does not hold is ignored. +3. **Debug bundles** read `session:get-screen-debug`: main's latest raw snapshot plus the tail history. +4. **Renderer:** + - The screen handler applies only the screen strings. + - DebugPanel and the dev modules that read the screen hold a lease while open (`useScreenLease`). + - Dead code is deleted: `latestScreenRef`, `claudePaste.ts`, and `ComposerSubmitIo.getScreen`. + +## Unchanged, deliberately +- Main's raw screen processing: the headless snapshot, condition parsers, prompt gate, the `lastScreenSnapshot` cache, and `ScreenFrameGate`. +- The phone feed (it taps main directly). +- Recover and resync seeds. + +## Tests (fail-first where the behaviour changes) +- **Forwarder:** the recorded Claude 2.1.278 screen frame is not forwarded without a lease, is forwarded with one, and is recorded in the tail history either way. The first assertion is red on origin/main. +- **Leases:** the IPC seed on acquire; dropping when a new document leases and on destroy, but never on a navigation event; counting across owners. +- **Picker:** a screen frame carrying a picker no longer sets `runtime.picker`. +- **Mutations:** removing the gate, recording only leased frames, dropping on sub-frames, removing the seed, and restoring the picker write each fail a test. + +## Risks +- **A missed release** costs bytes until that renderer reloads, never correctness. +- **Debug surfaces** show nothing new until their lease is acquired; the acquire seed covers the first frame. +- **The DevDebug copy payload** reads the renderer copy, which can be stale unless a leasing module is open. This is accepted for a copy-to-clipboard debug aid. diff --git a/packages/opencode-terminal-headless b/packages/opencode-terminal-headless index 6f343e7c8..75536312c 160000 --- a/packages/opencode-terminal-headless +++ b/packages/opencode-terminal-headless @@ -1 +1 @@ -Subproject commit 6f343e7c8d43d77a3f230b84f61b6cfc2cc9a64e +Subproject commit 75536312cb74920f4f9ea6bbb2e4df05b0c4a431 diff --git a/src/main/ipc/session.test.ts b/src/main/ipc/session.test.ts index 22b69fe6b..9d55e219b 100644 --- a/src/main/ipc/session.test.ts +++ b/src/main/ipc/session.test.ts @@ -12,7 +12,9 @@ vi.mock('electron', () => ({ }, }, ipcRenderer: { - invoke: (channel: string, ...args: unknown[]) => harness.handlers.get(channel)!({}, ...args), + // Asynchronous like the real one: the preload's load-time document + // announcement runs at import, before any handler is registered. + invoke: async (channel: string, ...args: unknown[]) => harness.handlers.get(channel)?.({}, ...args), }, })) @@ -149,3 +151,83 @@ describe('session input transcript observations', () => { expect(append).toHaveBeenCalledTimes(3) }) }) + +describe('screen leases (#762)', () => { + it('seeds the current screen on acquire, and drops a renderer\'s leases only when its document is replaced or it dies', async () => { + const { screenInterest } = await import('@main/sessions/screenInterest.js') + const manager = new EventEmitter() + Object.assign(manager, { + getScreenSnapshot: () => ({ plain: 'now', markdown: 'now', recent: 'now', recentMarkdown: 'now' }), + }) + registerSessionIpc(manager as never, {} as never, new SessionFeedTap(manager as never)) + harness.routed.mockClear() + const sender = Object.assign(new EventEmitter(), { id: 4242 }) + const lease = harness.handlers.get('session:screen-lease')! + + // An opening debug panel is right at once, even for an idle backend: + // the current screen goes down the ordinary session:screen path. + lease({ sender }, 'pane', 'doc-1') + expect(screenInterest.wants('pane')).toBe(true) + // The same aliased wire payload the recover seed sends (recent/markdown + // equal to plain/markdown are dropped on the wire, #746). + expect(harness.routed).toHaveBeenCalledWith('pane', 'session:screen', { sessionId: 'pane', plain: 'now', markdown: 'now' }) + + // Any navigation, including one the window blocks, leaves the page and + // its leases alone: only a new document's lease or death drops them. + sender.emit('did-start-navigation', { isMainFrame: true, isSameDocument: false }) + expect(screenInterest.wants('pane')).toBe(true) + // The release is the caller's own (another webContents cannot end it). + const release = harness.handlers.get('session:screen-release')! + release({ sender: { id: 1 } }, 'pane', 'doc-1') + expect(screenInterest.wants('pane')).toBe(true) + release({ sender }, 'pane', 'doc-1') + expect(screenInterest.wants('pane')).toBe(false) + + // A real reload whose new page never opens a debug panel: its preload's + // load-time announcement alone retires the old page's leases, or the + // heaviest IPC stream would forward forever (steering q15). + const announce = harness.handlers.get('session:screen-document')! + lease({ sender }, 'old-page', 'doc-1') + announce({ sender }, 'doc-2') + expect(screenInterest.wants('old-page')).toBe(false) + // Re-announcing the live document (nothing reloaded) keeps its leases, + // and the dead page's late release cannot touch them. + lease({ sender }, 'pane', 'doc-2') + announce({ sender }, 'doc-2') + release({ sender }, 'pane', 'doc-1') + expect(screenInterest.wants('pane')).toBe(true) + sender.emit('destroyed') + expect(screenInterest.wants('pane')).toBe(false) + }) + + it('leases a session with no screen yet without sending an empty frame', async () => { + const manager = new EventEmitter() + Object.assign(manager, { getScreenSnapshot: () => null }) + registerSessionIpc(manager as never, {} as never, new SessionFeedTap(manager as never)) + harness.routed.mockClear() + harness.handlers.get('session:screen-lease')!({ sender: Object.assign(new EventEmitter(), { id: 77 }) }, 'fresh', 'doc') + expect(harness.routed).not.toHaveBeenCalledWith('fresh', 'session:screen', expect.anything()) + }) +}) + +describe('session:get-screen-debug (#762)', () => { + it('answers with main\'s latest raw screen and the recorded tail history', async () => { + const { screenTailHistory } = await import('@main/sessions/screenInterest.js') + const manager = new EventEmitter() + Object.assign(manager, { + getScreenSnapshot: () => ({ plain: 'latest', markdown: 'latest', recent: 'latest\nmore', recentMarkdown: 'latest\nmore' }), + }) + registerSessionIpc(manager as never, {} as never, new SessionFeedTap(manager as never)) + screenTailHistory.record('debug-pane', 'first frame') + screenTailHistory.record('debug-pane', 'second frame') + try { + const answer = await harness.handlers.get('session:get-screen-debug')!({}, 'debug-pane') as { + screen: { recent: string } | null; samples: Array<{ content: string }> + } + expect(answer.screen?.recent).toBe('latest\nmore') + expect(answer.samples.map(sample => sample.content)).toEqual(['first frame', 'second frame']) + } finally { + screenTailHistory.forget('debug-pane') + } + }) +}) diff --git a/src/main/ipc/session.ts b/src/main/ipc/session.ts index 8e63dd3dc..6a514dabc 100644 --- a/src/main/ipc/session.ts +++ b/src/main/ipc/session.ts @@ -32,6 +32,9 @@ import { windowIdFor, } from '@main/window/windowRegistry.js' import type { SessionWindowLease } from '@main/window/sessionWindowRouter.js' +import { screenInterest, screenTailHistory } from '@main/sessions/screenInterest.js' +import type { AgentScreenSnapshot } from '@shared/types/session.js' +import type { ScreenTailSample } from '@shared/debug/screenTail.js' // One secret per app process is enough for correlation inside that run's // incident journal, and unlike a bare SHA-256 it prevents an exported bundle @@ -249,11 +252,56 @@ export function registerSessionIpc( manager.detachAgentPty(sessionId) }) + // #762. Live `session:screen` frames are forwarded only while a renderer + // holds a lease for that session (see sessions/screenInterest.ts for why + // nothing else needs them). The acquire also pushes the current screen down + // the ordinary `session:screen` path, the same seed `session:recover` sends, + // so an opening debug panel is correct at once even for an idle backend that + // emits no further frame, and the renderer needs no second way to apply a + // screen. Leases are owned by the calling webContents and document (see + // sessions/screenInterest.ts for why not a navigation event), and dropped + // when the webContents is destroyed, because a renderer that dies never + // runs its cleanup. + const leaseOwnersWatched = new Set() + const watchLeaseOwner = (sender: Electron.WebContents): number => { + const owner = sender.id + if (!leaseOwnersWatched.has(owner)) { + leaseOwnersWatched.add(owner) + sender.once('destroyed', () => { + screenInterest.dropOwner(owner) + leaseOwnersWatched.delete(owner) + }) + } + return owner + } + // Sent by the preload on every page load, whether or not the page ever + // leases: it is what retires a reloaded page's leases (steering q15). + ipcMain.handle('session:screen-document', (evt, document: string): void => { + screenInterest.enterDocument(watchLeaseOwner(evt.sender), document) + }) + ipcMain.handle('session:screen-lease', (evt, sessionId: string, document: string): void => { + screenInterest.acquire(watchLeaseOwner(evt.sender), sessionId, document) + const screen = manager.getScreenSnapshot(sessionId) + if (screen) sendToSessionWindow(sessionId, 'session:screen', aliasScreenSnapshotForWire({ sessionId, ...screen })) + }) + ipcMain.handle('session:screen-release', (evt, sessionId: string, document: string) => { + screenInterest.release(evt.sender.id, sessionId, document) + }) + // Debug bundles read the screen on demand instead of holding a lease: the + // latest raw snapshot plus the tail history main records from every frame. + ipcMain.handle('session:get-screen-debug', (_evt, sessionId: string): { + screen: AgentScreenSnapshot | null + samples: ScreenTailSample[] + } => ({ + screen: manager.getScreenSnapshot(sessionId), + samples: screenTailHistory.samples(sessionId), + })) + ipcMain.handle( 'session:input', (_evt, sessionId: string, data: string, pasteId?: string) => { // Optional pasteId journals THIS write into the per-paste debug - // dump. Only set by the Agent Code paste flow (claudePaste.ts) — + // dump. Only set by the composer's paste flow (useComposerKeybinds) — // never set on keystrokes, agent-pty bridging, or other normal // I/O. Pairs against the renderer's IPC:write:* events by sha8 // + byte count, same way dictation pairs renderer-produced @@ -277,7 +325,7 @@ export function registerSessionIpc( // between and reports the wrong cause — the same misdiagnosis this // replaces, just narrower. const deliveryInFlight = manager.isDeliveryInFlight(sessionId) - // `pasteId` is set only by the Agent Code paste flow (claudePaste.ts) and + // `pasteId` is set only by the composer's paste flow (useComposerKeybinds) and // never by keystrokes, so it is also the exact renderer-side attribution // signal required by SessionManager's prompt-delivery ownership fence. const attributedPasteId = typeof pasteId === 'string' && pasteId.length > 0 @@ -398,25 +446,6 @@ export function registerSessionIpc( }, ) - // Event-driven paste-submit (Track C of the paste-submit harness PR). - // Renderer's claudePaste.ts invokes this AFTER writing the bracketed - // paste payload but BEFORE writing `\r`. We resolve as soon as - // Claude's TUI renders `[Pasted text #N]`, falling back to a 2 s - // timeout if the placeholder never appears (future Claude UI rename - // insurance). See `claudePaste.ts` and - // `packages/claude-code-headless/src/ClaudeCodeHeadless.ts:awaitPastePlaceholder` - // for the full rationale chain. - ipcMain.handle( - 'claude:await-paste-placeholder', - async ( - _evt, - sessionId: string, - opts?: { timeoutMs?: number; pollIntervalMs?: number }, - ) => { - return manager.awaitClaudePastePlaceholder(sessionId, opts) - }, - ) - ipcMain.handle( 'session:load-older-history', async ( diff --git a/src/main/sessionManager.ts b/src/main/sessionManager.ts index bf57217a2..0dd19d36c 100644 --- a/src/main/sessionManager.ts +++ b/src/main/sessionManager.ts @@ -287,8 +287,7 @@ export interface SessionManager { // for the migration rationale. // // Local aliases used below just narrow AgentSession for the -// duck-typed-optional-method call sites (awaitPastePlaceholder, -// awaitReadyForPrompt), so the callers keep reading naturally. +// duck-typed-optional-method call sites (awaitReadyForPrompt), so the callers keep reading naturally. type AgentSessionLike = AgentSession type RegistryLifecycle = { @@ -4390,45 +4389,6 @@ export class SessionManager extends EventEmitter { } } - /** - * Claude-specific accessor for the paste-submit event-driven path - * in `src/renderer/.../claudePaste.ts`. Returns the live ClaudeSession - * cast through `unknown` because AgentSessionLike doesn't (and - * shouldn't) expose `awaitPastePlaceholder` — that's a Claude-only - * affordance and adding it to the cross-provider interface would - * force every other runtime to ship a no-op stub. - * - * Returns `null` for missing sessions or non-Claude kinds. Callers - * MUST treat null as a benign "couldn't reach this session" and - * fall through to whatever non-event-driven path they were using - * before; the absence of a Claude session is not an error worth - * crashing over. - */ - async awaitClaudePastePlaceholder( - sessionId: string, - opts?: { timeoutMs?: number; pollIntervalMs?: number }, - ): Promise< - | { kind: 'appeared'; waitedMs: number } - | { kind: 'timeout' } - | { kind: 'no-headless' } - | { kind: 'no-session' } - > { - const entry = this.sessions.get(sessionId) - if (!entry || entry.kind !== 'claude') return { kind: 'no-session' } - // The cross-provider AgentSessionLike interface doesn't carry - // `awaitPastePlaceholder`; ClaudeSession does. We assert via a - // structural duck-type so misconfigured Claude provider builds - // (a future ClaudeSession that loses the method) surface as - // 'no-session' rather than a TypeError. - // No cast: awaitPastePlaceholder is now a typed optional on - // AgentSession (see @shared/types/session.ts). - const session = entry.session - if (typeof session.awaitPastePlaceholder !== 'function') { - return { kind: 'no-session' } - } - return session.awaitPastePlaceholder(opts) - } - /** * Ask the provider to scroll its own transcript view to the newest message * (#843). Only providers whose TUI pages its transcript on the alternate diff --git a/src/main/sessions/forwarder.test.ts b/src/main/sessions/forwarder.test.ts index dd2a65c4f..6b2134f48 100644 --- a/src/main/sessions/forwarder.test.ts +++ b/src/main/sessions/forwarder.test.ts @@ -1,4 +1,5 @@ import { EventEmitter } from 'node:events' +import { readFileSync } from 'node:fs' import { expect, it, vi } from 'vitest' import type { SessionManager } from '@main/sessionManager.js' import type { LspManager } from '@main/lspManager.js' @@ -11,6 +12,7 @@ vi.mock('@main/window/windowRegistry.js', () => ({ vi.mock('@main/subagents/index.js', () => ({ SubAgentWatcherManager: class { observeParentEntry() {} stop() {} stopAll() {} } })) import { wireSessionForwarder } from './forwarder.js' +import { screenInterest, screenTailHistory } from './screenInterest.js' import { SessionFeedSource } from '@main/remote/SessionFeedSource.js' import { SessionFeedTap } from './sessionFeedTap.js' @@ -101,3 +103,54 @@ it('broadcasts a managed-skill warning to every window, carrying domain names on manager.removeAllListeners() } }) + +it('forwards screen frames only to a leased session, and records every frame for debug bundles (#762)', () => { + // session:screen was 93% of recorded IPC bytes and nothing live reads it. + // An unleased session's frames stay in main; the tail history still sees + // them, because a debug bundle must not depend on a panel having been open. + const manager = new EventEmitter() as SessionManager & EventEmitter + manager.list = () => [] + const sent: string[] = [] + wire.receive = (channel, payload) => { + if (channel === 'session:screen') sent.push((payload as { plain: string }).plain) + } + const forwarder = wireSessionForwarder(manager, new EventEmitter() as LspManager) + // A real frame: the recorded Claude Code 2.1.278 screen from the #1113 + // capture (idle, then with a composer draft). What the gate forwards is + // exactly what the provider painted. + const recorded = JSON.parse(readFileSync( + new URL('../../../testing/fixtures/image-absorption/wrapped-image-pill-2026-09-21.json', import.meta.url), 'utf8', + )) as { deliveries: { wrapped: { baseline: { screen: string }; after: { screen: string } } } } + const idle = recorded.deliveries.wrapped.baseline.screen + const drafted = recorded.deliveries.wrapped.after.screen + // `recent` is the wider window (Codex scrollback) and is what the history + // must record, so it differs from `plain` here to pin that (#1236 review C). + const frame = (screen: string) => ({ sessionId: 'pane', plain: screen.slice(-200), markdown: screen, recent: screen, recentMarkdown: screen }) + try { + manager.emit('screen', frame(idle)) + forwarder.flush() + expect(sent).toEqual([]) + + screenInterest.acquire(7, 'pane', 'doc') + manager.emit('screen', frame(drafted)) + forwarder.flush() + expect(sent).toEqual([drafted.slice(-200)]) + + screenInterest.release(7, 'pane', 'doc') + manager.emit('screen', frame(idle)) + forwarder.flush() + expect(sent).toEqual([drafted.slice(-200)]) + + // Every frame reached the tail history, forwarded or not (idle, drafted, + // idle again: the dedupe only collapses consecutive repeats). + expect(screenTailHistory.samples('pane')).toHaveLength(3) + expect(screenTailHistory.samples('pane')[1]!.content.length).toBeGreaterThan(drafted.slice(-200).trimEnd().length) + manager.emit('removed', { sessionId: 'pane' }) + forwarder.flush() + expect(screenTailHistory.samples('pane')).toEqual([]) + } finally { + screenInterest.dropOwner(7) + wire.receive = () => {} + manager.removeAllListeners() + } +}) diff --git a/src/main/sessions/forwarder.ts b/src/main/sessions/forwarder.ts index 6afc7267a..4d6424c4c 100644 --- a/src/main/sessions/forwarder.ts +++ b/src/main/sessions/forwarder.ts @@ -14,6 +14,7 @@ import { sendToSessionWindow, } from '@main/window/windowRegistry.js' import { SessionFeedTap } from '@main/sessions/sessionFeedTap.js' +import { screenInterest, screenTailHistory } from '@main/sessions/screenInterest.js' // Session event forwarder — the desktop WINDOW SINK over the SessionFeedTap. // @@ -32,7 +33,10 @@ import { SessionFeedTap } from '@main/sessions/sessionFeedTap.js' // `emptyRuntime()` for an unrecognized id, so a misrouted event grows a // ghost runtime rather than being ignored. // 2. The IPC channel names (`session:`). -// 3. The screen alias (#746), an IPC-edge byte optimisation. +// 3. The screen alias (#746), an IPC-edge byte optimisation, and the +// screen-interest gate (#762): frames go only to a renderer that holds a +// lease (screenInterest.ts), while every frame still feeds the debug +// screen-tail history. // 4. Broadcast-only events that are machine-wide rather than per session. // // terminal-data and agent-pty-data are intentionally separate channels from @@ -63,8 +67,17 @@ export function wireSessionForwarder( // `removed` is a cleanup signal the tap needed for its own buffers; the // desktop learns removal from workspace state, and never had an IPC // channel for it. - if (channel === 'removed') return + if (channel === 'removed') { + screenTailHistory.forget(payload.sessionId) + return + } if (channel === 'screen') { + const frame = payload as { sessionId: string } & AgentScreenSnapshot + // Recorded for EVERY frame, forwarded or not: debug bundles read this + // history, and it must not depend on whether a panel happened to be + // open (#762, screenInterest.ts). + screenTailHistory.record(frame.sessionId, frame.recent) + if (!screenInterest.wants(frame.sessionId)) return // WHY the alias happens here and not in the tap or the manager (#746): // the remote server and the recorder-independent readers take the full // payload; only the renderer IPC edge pays structured-clone bytes for @@ -72,11 +85,7 @@ export function wireSessionForwarder( // back. (The session recorder taps this send, so recordings carry the // wire form; replay treats screen frames as no-op ticks and never reads // the fields.) - sendToSessionWindow( - payload.sessionId, - 'session:screen', - aliasScreenSnapshotForWire(payload as { sessionId: string } & AgentScreenSnapshot), - ) + sendToSessionWindow(frame.sessionId, 'session:screen', aliasScreenSnapshotForWire(frame)) return } sendToSessionWindow(payload.sessionId, `session:${channel}`, payload) diff --git a/src/main/sessions/screenInterest.test.ts b/src/main/sessions/screenInterest.test.ts new file mode 100644 index 000000000..cf1b27732 --- /dev/null +++ b/src/main/sessions/screenInterest.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from 'vitest' + +import { ScreenInterest } from './screenInterest.js' + +// #762: screen frames leave main only while some renderer holds a lease. The +// failure this guards is a count that drifts: too low stops a debug panel's +// frames, too high forwards the 93%-of-IPC-bytes stream again forever. +describe('ScreenInterest', () => { + it('wants a session while any lease on it is held, across owners', () => { + const interest = new ScreenInterest() + interest.acquire(1, 's', 'doc') + interest.acquire(2, 's', 'doc') + interest.release(1, 's', 'doc') + expect(interest.wants('s')).toBe(true) + interest.release(2, 's', 'doc') + expect(interest.wants('s')).toBe(false) + }) + + it('ignores a release its owner does not hold, so it cannot cancel another owner', () => { + const interest = new ScreenInterest() + interest.acquire(1, 's', 'doc') + interest.release(2, 's', 'doc') + interest.release(1, 's', 'doc') + interest.release(1, 's', 'doc') + expect(interest.wants('s')).toBe(false) + interest.acquire(2, 's', 'doc') + interest.release(1, 's', 'doc') + expect(interest.wants('s')).toBe(true) + }) + + it('never lets a release for a session the owner does not hold cancel another owner', () => { + // Owner 1 holds 'a' only. Its release of 'b' must not go negative on its + // own books and take owner 2's 'b' lease with it. + const interest = new ScreenInterest() + interest.acquire(1, 'a', 'doc') + interest.acquire(2, 'b', 'doc') + interest.release(1, 'b', 'doc') + expect(interest.wants('b')).toBe(true) + expect(interest.wants('a')).toBe(true) + }) + + it('drops a webContents\' leases when a new document takes one, and only then', () => { + // A reload never runs the old page's cleanup; the new page's first lease + // proves the old document is gone. A blocked navigation keeps the page and + // its document id, so nothing is dropped (#1236 review). + const interest = new ScreenInterest() + interest.acquire(1, 'old-only', 'first-load') + interest.acquire(1, 'shared', 'first-load') + interest.acquire(1, 'shared', 'reloaded') + expect(interest.wants('old-only')).toBe(false) + expect(interest.wants('shared')).toBe(true) + // The dead document's late release cannot touch the new one's lease. + interest.release(1, 'shared', 'first-load') + expect(interest.wants('shared')).toBe(true) + interest.release(1, 'shared', 'reloaded') + expect(interest.wants('shared')).toBe(false) + }) + + it('drops everything an owner held when it goes away, and only that', () => { + // A renderer that reloads or crashes never releases; main drops its + // leases instead (ipc/session.ts), and another window's must survive. + const interest = new ScreenInterest() + interest.acquire(1, 'a', 'doc') + interest.acquire(1, 'a', 'doc') + interest.acquire(1, 'b', 'doc') + interest.acquire(2, 'b', 'doc') + interest.dropOwner(1) + expect(interest.wants('a')).toBe(false) + expect(interest.wants('b')).toBe(true) + interest.dropOwner(1) + expect(interest.wants('b')).toBe(true) + }) +}) diff --git a/src/main/sessions/screenInterest.ts b/src/main/sessions/screenInterest.ts new file mode 100644 index 000000000..c62abaa48 --- /dev/null +++ b/src/main/sessions/screenInterest.ts @@ -0,0 +1,104 @@ +import { ScreenTailHistory } from '@shared/debug/screenTail.js' + +// Which sessions a renderer currently wants live `session:screen` frames for. +// +// WHY frames are opt-in (#762): `session:screen` carried 93% of recorded IPC +// bytes (16,728 frames x 8.8 KB in one 123-minute recording) and arrived 6-10 +// times a second per busy session, every one structured-cloned, dispatched and +// committed into the renderer's runtime map. A map of every consumer found that +// nothing live needs them: +// - main already reads the screen itself for prompt delivery, paste +// confirmation and readiness (`session.snapshotScreen()`); +// - the composer's slash picker is owned by the conditions channel +// (`claude.slash-picker`, applyConditionSnapshot); +// - the Enter-baseline `latestScreenRef` and the renderer paste helpers +// were dead code; +// - ReaderView no longer has a screen fallback (#855). +// What remains are debug surfaces (DebugPanel, the dev modules), which take a +// lease while they are open, and debug bundles, which ask main directly +// (`session:get-screen-debug`) and read the screen-tail history recorded here. +// +// WHY leases are owned by a webContents and not just counted: a renderer that +// reloads or crashes never runs its cleanup, and a bare counter would then +// forward that session's frames forever. Dropping an owner's leases when its +// webContents is destroyed, or when a NEW document in it starts taking leases, +// keeps the count honest. (A leaked lease would only cost bytes, never +// correctness, but the whole point here is the bytes.) +// +// WHY a document id and not a navigation event (#1236 review): the app window +// blocks every main-frame `will-navigate` (appWindow.ts), and Chromium reports +// `did-start-navigation` BEFORE the throttle that emits and blocks +// `will-navigate`. Dropping on navigation start therefore also dropped every +// lease when a stray link click was blocked and the page never changed, so an +// open debug panel silently went stale. The preload mints one id per loaded +// document, ANNOUNCES it as soon as it loads (enterDocument), and sends it +// with every lease. A different id proves the old document is gone, whatever +// event order Electron uses; messages from one renderer reach main in order, +// so the announcement lands after the old page's last message and before the +// new page's first lease. The announcement matters as much as the lease id: a +// reloaded page that never opens a debug panel would otherwise leave the old +// page's leases forwarding the heaviest IPC stream forever (steering q15). + +export class ScreenInterest { + private readonly byOwner = new Map>() + private readonly totals = new Map() + private readonly documents = new Map() + + /** A document loaded in `owner`; everything an earlier document held is dead. */ + enterDocument(owner: number, document: string): void { + const current = this.documents.get(owner) + if (current !== undefined && current !== document) this.dropOwner(owner) + this.documents.set(owner, document) + } + + acquire(owner: number, sessionId: string, document: string): void { + this.enterDocument(owner, document) + let leases = this.byOwner.get(owner) + if (!leases) { + leases = new Map() + this.byOwner.set(owner, leases) + } + leases.set(sessionId, (leases.get(sessionId) ?? 0) + 1) + this.totals.set(sessionId, (this.totals.get(sessionId) ?? 0) + 1) + } + + release(owner: number, sessionId: string, document: string): void { + // A release from a document that no longer owns the leases has nothing + // of its own to release. + if (this.documents.get(owner) !== document) return + const leases = this.byOwner.get(owner) + const held = leases?.get(sessionId) ?? 0 + // A release the owner does not hold (double release, or one that raced + // dropOwner) must not decrement someone else's lease. + if (!leases || held <= 0) return + if (held === 1) leases.delete(sessionId) + else leases.set(sessionId, held - 1) + if (leases.size === 0) this.byOwner.delete(owner) + this.decrementTotal(sessionId, 1) + } + + dropOwner(owner: number): void { + this.documents.delete(owner) + const leases = this.byOwner.get(owner) + if (!leases) return + this.byOwner.delete(owner) + for (const [sessionId, held] of leases) this.decrementTotal(sessionId, held) + } + + wants(sessionId: string): boolean { + return (this.totals.get(sessionId) ?? 0) > 0 + } + + private decrementTotal(sessionId: string, by: number): void { + const next = (this.totals.get(sessionId) ?? 0) - by + if (next > 0) this.totals.set(sessionId, next) + else this.totals.delete(sessionId) + } +} + +/** The process-wide instances: the forwarder consults and feeds them, the + * session IPC handlers mutate and read them. Module singletons for the same + * reason windowRegistry is one: both sides are wired in different files and + * there is exactly one main process. */ +export const screenInterest = new ScreenInterest() +export const screenTailHistory = new ScreenTailHistory() diff --git a/src/main/window/sessionRoutingComposition.test.ts b/src/main/window/sessionRoutingComposition.test.ts index f3b04c53c..c4347dd67 100644 --- a/src/main/window/sessionRoutingComposition.test.ts +++ b/src/main/window/sessionRoutingComposition.test.ts @@ -38,13 +38,20 @@ vi.mock('@main/subagents/index.js', () => ({ import * as registry from './windowRegistry.js' import { wireSessionForwarder } from '@main/sessions/forwarder.js' +import { screenInterest } from '@main/sessions/screenInterest.js' import { registerSessionIpc } from '@main/ipc/session.js' import { registerSessionRoutingIpc } from '@main/ipc/sessionRouting.js' import { abandonPendingBequest, recordPendingBequest, registerWindowIpc } from '@main/ipc/window.js' let manager: SessionManager & EventEmitter let forwarder: ReturnType +// These cases are about ROUTING (which window may see a session's frames), so +// every session here holds a screen lease: since #762 an unleased session's +// frames never leave main, which would make "nothing was sent" true for the +// wrong reason. The lease owner id is not a real webContents. +const TEST_SCREEN_OWNER = -1 beforeEach(() => { + for (const sessionId of ['pane', 'refused-close', 'admission-pane']) screenInterest.acquire(TEST_SCREEN_OWNER, sessionId, 'test-document') registry.resetWindowRegistryForTests() harness.handlers.clear() harness.built.length = 0 @@ -73,6 +80,7 @@ beforeEach(() => { registerSessionRoutingIpc(manager, forwarder) }) afterEach(() => { + screenInterest.dropOwner(TEST_SCREEN_OWNER) forwarder.flush() manager.removeAllListeners() registry.resetWindowRegistryForTests() diff --git a/src/preload/api/session.ts b/src/preload/api/session.ts index 0659b4b86..cdee73c95 100644 --- a/src/preload/api/session.ts +++ b/src/preload/api/session.ts @@ -5,9 +5,11 @@ import type { PromptDeliveryOptions, PromptDeliveryResult } from '@shared/types/ import { subscribe } from '@preload/api/ipc.js' import { expandScreenSnapshotFromWire } from '@shared/types/session.js' -import type { AgentScreenSnapshotWire } from '@shared/types/session.js' +import type { AgentScreenSnapshot, AgentScreenSnapshotWire } from '@shared/types/session.js' +import type { ScreenTailSample } from '@shared/debug/screenTail.js' import type { TerminalForegroundEvent, TerminalForegroundState } from '@shared/types/terminalForeground.js' import type { + SessionExitEvent, SessionHistoryChunk, SessionKind, @@ -38,6 +40,14 @@ import type { SessionProviderSessionChangedEvent, } from '@preload/api/types.js' +// One id per loaded document: preload runs again on every reload, so a new id +// tells main the previous document's screen leases are dead (#762, see +// main/sessions/screenInterest.ts for why this replaced a navigation event). +const screenLeaseDocument = globalThis.crypto.randomUUID() +// Announce the document at load, not at first lease: a reloaded page that +// never leases must still retire the previous page's leases (steering q15). +void ipcRenderer.invoke('session:screen-document', screenLeaseDocument).catch(() => undefined) + type SessionScreenWireEvent = Omit & AgentScreenSnapshotWire // Session lifecycle + I/O bridge methods. @@ -129,6 +139,19 @@ export const sessionApi = { detachAgentPty: (sessionId: string): Promise => ipcRenderer.invoke('session:agent-pty-detach', sessionId), + // #762: live `session:screen` frames are forwarded only while a lease is + // held (debug surfaces). Acquire also sends the current screen as one + // ordinary session:screen event. + acquireScreenLease: (sessionId: string): Promise => + ipcRenderer.invoke('session:screen-lease', sessionId, screenLeaseDocument), + + releaseScreenLease: (sessionId: string): Promise => + ipcRenderer.invoke('session:screen-release', sessionId, screenLeaseDocument), + + /** The latest screen and main's screen-tail history, for debug bundles. */ + getScreenDebug: (sessionId: string): Promise<{ screen: AgentScreenSnapshot | null; samples: ScreenTailSample[] }> => + ipcRenderer.invoke('session:get-screen-debug', sessionId), + // --- Per-session I/O --- // // Optional `pasteId` correlates this write against the per-paste @@ -167,23 +190,6 @@ export const sessionApi = { resize: (sessionId: string, cols: number, rows: number): Promise => ipcRenderer.invoke('session:resize', sessionId, cols, rows), - // Event-driven paste-submit primitive. See - // src/renderer/.../claudePaste.ts and - // packages/claude-code-headless/src/ClaudeCodeHeadless.ts. - // Resolves when Claude's TUI renders `[Pasted text #N]`, or after - // the configured timeout. Renderer treats every non-'appeared' - // outcome as "fall through to the wall-clock submit path." - awaitClaudePastePlaceholder: ( - sessionId: string, - opts?: { timeoutMs?: number; pollIntervalMs?: number }, - ): Promise< - | { kind: 'appeared'; waitedMs: number } - | { kind: 'timeout' } - | { kind: 'no-headless' } - | { kind: 'no-session' } - > => - ipcRenderer.invoke('claude:await-paste-placeholder', sessionId, opts), - loadOlderHistory: (params: { kind: AgentProviderKind cwd: string diff --git a/src/preload/api/types.ts b/src/preload/api/types.ts index c49c0aaf6..3959f1cf6 100644 --- a/src/preload/api/types.ts +++ b/src/preload/api/types.ts @@ -382,7 +382,7 @@ export type DictationDebugEvent = DictationDebugEventInput & { // because the file is local 0o600 and the whole point is to see // what the user actually pasted vs. what reached Claude. export type PasteDebugLayer = - | 'RENDER' // composer keydown, state snapshot, call into claudePaste fn + | 'RENDER' // composer keydown, state snapshot, composer submit | 'IPC' // renderer-side IPC write (paste payload, submit \r) | 'PTY' // main-side PTY write (sha8 + byte count) — pairs with IPC | 'SCREEN' // [Pasted text #N] placeholder observed to appear / clear diff --git a/src/providers/claude/runtime/claudeSession.ts b/src/providers/claude/runtime/claudeSession.ts index 431364113..ec30cd5ea 100644 --- a/src/providers/claude/runtime/claudeSession.ts +++ b/src/providers/claude/runtime/claudeSession.ts @@ -737,26 +737,6 @@ export class ClaudeSession extends EventEmitter { return this.headless?.getScreenMarkdown() ?? '' } - /** - * Poll the live Claude TUI screen for `[Pasted text #N]` and resolve - * the instant it appears. Delegates to ClaudeCodeHeadless; see the - * method comment there for why this is load-independent and why a - * timeout fallback is mandatory. Used by claudePaste.ts to send `\r` - * exactly when Claude has visibly committed the paste — the - * primary path of the paste-submit fix. - * - * Returns `{ kind: 'no-headless' }` when the headless instance has - * not been constructed yet (start() not called or session torn down) - * so the renderer can fall through to the wall-clock path without - * waiting for the full 2 s timeout. - */ - async awaitPastePlaceholder( - opts?: { timeoutMs?: number; pollIntervalMs?: number }, - ): Promise<{ kind: 'appeared'; waitedMs: number } | { kind: 'timeout' } | { kind: 'no-headless' }> { - if (!this.headless) return { kind: 'no-headless' } - return this.headless.awaitPastePlaceholder(opts ?? {}) - } - async resolveCondition( action: ConditionCustomAction, ): Promise { diff --git a/src/providers/opencode/renderer/composerSubmit.test.ts b/src/providers/opencode/renderer/composerSubmit.test.ts index bc5a24ab4..2cd0c4e07 100644 --- a/src/providers/opencode/renderer/composerSubmit.test.ts +++ b/src/providers/opencode/renderer/composerSubmit.test.ts @@ -22,7 +22,6 @@ describe('opencodeComposerSubmit', () => { send: vi.fn(), deliverPrompt: vi.fn(async () => delivery), pasteId: 'paste-1', - getScreen: () => undefined, } await expect(opencodeComposerSubmit(io)).rejects.toMatchObject({ @@ -44,7 +43,6 @@ describe('opencodeComposerSubmit acceptance', () => { send: vi.fn(), deliverPrompt: vi.fn(async () => ({ ok: true as const, acceptance })), pasteId: 'paste-1', - getScreen: () => undefined, } await expect(opencodeComposerSubmit(io)).resolves.toEqual(acceptance) diff --git a/src/providers/registry.renderer.capabilities.ts b/src/providers/registry.renderer.capabilities.ts index 8b860ba61..3980b7df1 100644 --- a/src/providers/registry.renderer.capabilities.ts +++ b/src/providers/registry.renderer.capabilities.ts @@ -326,7 +326,6 @@ export type ComposerSubmitIo = { send: (data: string, pasteId?: string) => Promise deliverPrompt: (prompt: string, imagePaths?: string[]) => Promise pasteId: string - getScreen: () => string | undefined } // Does a tool name identify Agent Code's OWN orchestration spawn verb? The diff --git a/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx b/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx index 5b5074133..6015463e6 100644 --- a/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx +++ b/src/renderer/src/app/shell/TerminalDimensionOwnership.renderer.test.tsx @@ -127,6 +127,12 @@ vi.mock('@renderer/features/debug/devDebugConfig', () => ({ describe('terminal dimension ownership across main-surface takeovers', () => { beforeEach(() => { + // The mounted DebugPanel takes a screen lease (#762); the bridge is the + // only window.api this suite's surfaces touch. + Object.defineProperty(window, 'api', { + configurable: true, + value: { acquireScreenLease: vi.fn(async () => {}), releaseScreenLease: vi.fn(async () => {}) }, + }) harness.paneMounts = {} harness.paneUnmounts = {} const runtime = emptyRuntime() @@ -184,6 +190,7 @@ describe('terminal dimension ownership across main-surface takeovers', () => { afterEach(() => { cleanup() + Reflect.deleteProperty(window, 'api') }) it('hands dimension ownership to the inline terminal while Settings hides the retained pane terminal', async () => { diff --git a/src/renderer/src/features/debug/devModules/ClaudePasteDetection/module.tsx b/src/renderer/src/features/debug/devModules/ClaudePasteDetection/module.tsx index a8e478dac..031038a66 100644 --- a/src/renderer/src/features/debug/devModules/ClaudePasteDetection/module.tsx +++ b/src/renderer/src/features/debug/devModules/ClaudePasteDetection/module.tsx @@ -19,6 +19,7 @@ import type { DevDebugModule, DevDebugModuleProps } from '@renderer/features/deb import type { PasteDebugSession } from '@preload/api/types' import { buildLifecycle, buildStats } from './timeline' +import { useScreenLease } from '@renderer/features/debug/useScreenLease' // The real submit-detection rules, mirrored from claude-code-headless so we can // watch them fire against the same screen text the parser sees: @@ -53,6 +54,8 @@ function matches(value: string, pattern: string, flags: string): boolean { } function ClaudePasteDetection({ sessionId, runtime, kind }: DevDebugModuleProps) { + // Screen frames arrive only while leased (#762, useScreenLease). + useScreenLease(sessionId) const plain = runtime.screen ?? '' const markdown = runtime.screenMarkdown ?? '' const [sessions, setSessions] = useState([]) diff --git a/src/renderer/src/features/debug/devModules/Conditions/module.tsx b/src/renderer/src/features/debug/devModules/Conditions/module.tsx index 1d8476081..1aadd7498 100644 --- a/src/renderer/src/features/debug/devModules/Conditions/module.tsx +++ b/src/renderer/src/features/debug/devModules/Conditions/module.tsx @@ -33,6 +33,7 @@ import type { ClaudeAskUserQuestionState, ProviderConditionSnapshot, } from '@shared/types/providerConditions' +import { useScreenLease } from '@renderer/features/debug/useScreenLease' export const conditionsDebugModule: DevDebugModule = { id: 'conditions', @@ -135,6 +136,8 @@ const AUQ_SCREEN_PROBES = [ ] function ConditionsDebug({ sessionId, runtime, kind }: DevDebugModuleProps) { + // Screen frames arrive only while leased (#762, useScreenLease). + useScreenLease(sessionId) const snapshot = runtime.conditions const conditionKeys = snapshot ? Object.keys(snapshot.conditions) : [] const attention = dispatchAttentionLabelFromConditions(snapshot) diff --git a/src/renderer/src/features/debug/devModules/HeadlessSnapshotProbe/module.tsx b/src/renderer/src/features/debug/devModules/HeadlessSnapshotProbe/module.tsx index fd6853709..9b4eece50 100644 --- a/src/renderer/src/features/debug/devModules/HeadlessSnapshotProbe/module.tsx +++ b/src/renderer/src/features/debug/devModules/HeadlessSnapshotProbe/module.tsx @@ -1,6 +1,7 @@ import { useMemo, useState } from 'react' import type { DevDebugModule, DevDebugModuleProps } from '@renderer/features/debug/devModules/types' +import { useScreenLease } from '@renderer/features/debug/useScreenLease' export const headlessSnapshotProbeModule: DevDebugModule = { id: 'headless-snapshot-probe', @@ -10,6 +11,8 @@ export const headlessSnapshotProbeModule: DevDebugModule = { } function HeadlessSnapshotProbe({ sessionId, runtime, kind }: DevDebugModuleProps) { + // Screen frames arrive only while leased (#762, useScreenLease). + useScreenLease(sessionId) // Wrap-tolerant by default (#1113), so the probe agrees with what // `placeholderCount` counts in production rather than missing every pill the // composer happened to wrap. diff --git a/src/renderer/src/features/debug/renderTrace.renderer.test.ts b/src/renderer/src/features/debug/renderTrace.renderer.test.ts index 61bff63a2..b13b69eb8 100644 --- a/src/renderer/src/features/debug/renderTrace.renderer.test.ts +++ b/src/renderer/src/features/debug/renderTrace.renderer.test.ts @@ -70,4 +70,19 @@ describe('renderTrace', () => { expect(exportDebugTraceFiles(sessionId)).toEqual([]) }) + + it('writes the screen samples main recorded into the bundle trace (#762)', () => { + // The samples now come from main (window.api.getScreenDebug); dropping + // the argument at the call site shipped empty screen traces while every + // test stayed green (#1236 review C). + const sessionId = `trace-screen-${crypto.randomUUID()}` + const samples = [ + { id: '0-a', seq: 0, ts: 1, tsIso: new Date(1).toISOString(), hash: 'a', lineCount: 1, content: 'first' }, + { id: '1-b', seq: 1, ts: 2, tsIso: new Date(2).toISOString(), hash: 'b', lineCount: 1, content: 'latest' }, + ] + const files = exportDebugTraceFiles(sessionId, samples) + expect(readJsonl<{ content: string }>(files, 'trace/screen/tail-samples.jsonl').map(sample => sample.content)) + .toEqual(['first', 'latest']) + expect(files.find(file => file.name === 'trace/screen/latest-tail.txt')?.content).toBe('latest') + }) }) diff --git a/src/renderer/src/features/debug/renderTrace.ts b/src/renderer/src/features/debug/renderTrace.ts index cb72ffa21..39ccda267 100644 --- a/src/renderer/src/features/debug/renderTrace.ts +++ b/src/renderer/src/features/debug/renderTrace.ts @@ -1,4 +1,10 @@ import { sanitizeHtml } from '@renderer/lib/sanitizeHtml' +import { + hashText, + SCREEN_MAX_SAMPLES, + SCREEN_TAIL_LINES, + type ScreenTailSample, +} from '@shared/debug/screenTail' type TraceReason = 'initial' | 'mutation' | 'screen' | 'manual' @@ -28,28 +34,15 @@ type HtmlCheckpoint = { content: string } -type ScreenTailSample = { - id: string - seq: number - ts: number - tsIso: string - hash: string - lineCount: number - content: string -} - type SessionTrace = { sessionId: string startedAt: number htmlCommits: HtmlCommit[] htmlCheckpoints: HtmlCheckpoint[] - screenSamples: ScreenTailSample[] nextHtmlSeq: number - nextScreenSeq: number lastHtml: string lastHtmlHash: string | null lastHtmlCommitId: string | null - lastScreenHash: string | null } type BundleFile = { @@ -61,8 +54,6 @@ const HTML_TRACE_DIR = 'trace/html' const SCREEN_TRACE_DIR = 'trace/screen' const HTML_CHECKPOINT_EVERY = 20 const HTML_MAX_COMMITS = 200 -const SCREEN_MAX_SAMPLES = 300 -const SCREEN_TAIL_LINES = 50 const traces = new Map() @@ -74,32 +65,16 @@ function getTrace(sessionId: string): SessionTrace { startedAt: Date.now(), htmlCommits: [], htmlCheckpoints: [], - screenSamples: [], nextHtmlSeq: 0, - nextScreenSeq: 0, lastHtml: '', lastHtmlHash: null, lastHtmlCommitId: null, - lastScreenHash: null, } traces.set(sessionId, trace) } return trace } -function hashText(text: string): string { - let h1 = 0xdeadbeef ^ text.length - let h2 = 0x41c6ce57 ^ text.length - for (let i = 0; i < text.length; i++) { - const ch = text.charCodeAt(i) - h1 = Math.imul(h1 ^ ch, 2654435761) - h2 = Math.imul(h2 ^ ch, 1597334677) - } - h1 = Math.imul(h1 ^ (h1 >>> 16), 2246822507) ^ Math.imul(h2 ^ (h2 >>> 13), 3266489909) - h2 = Math.imul(h2 ^ (h2 >>> 16), 2246822507) ^ Math.imul(h1 ^ (h1 >>> 13), 3266489909) - const n = 4294967296 * (2097151 & h2) + (h1 >>> 0) - return n.toString(16).padStart(13, '0') -} function nowIso(ts: number): string { return new Date(ts).toISOString() @@ -129,20 +104,6 @@ function buildDelta(base: string, next: string, baseHash: string): TextDelta { } } -function sanitizeScreenText(text: string): string { - return text - .replace(/\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])/g, '') - .replace(/\r\n?/g, '\n') - .replace(/[^\S\n\t]+$/gm, '') - .replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, '') - .trimEnd() -} - -function tailLines(text: string, count: number): string { - const lines = text.split('\n') - return lines.length <= count ? text : lines.slice(lines.length - count).join('\n') -} - function pruneHtmlTrace(trace: SessionTrace): void { if (trace.htmlCommits.length <= HTML_MAX_COMMITS) return const overflow = trace.htmlCommits.length - HTML_MAX_COMMITS @@ -216,57 +177,38 @@ export function recordHtmlTraceSnapshot( pruneHtmlTrace(trace) } -export function recordScreenTailSnapshot(sessionId: string, screenText: string): void { - const trace = getTrace(sessionId) - const content = tailLines(sanitizeScreenText(screenText), SCREEN_TAIL_LINES) - if (!content) return - - const hash = hashText(content) - if (hash === trace.lastScreenHash) return - - const ts = Date.now() - const seq = trace.nextScreenSeq++ - trace.screenSamples.push({ - id: `${seq.toString(36)}-${hash}`, - seq, - ts, - tsIso: nowIso(ts), - hash, - lineCount: content.split('\n').length, - content, - }) - trace.lastScreenHash = hash - - if (trace.screenSamples.length > SCREEN_MAX_SAMPLES) { - trace.screenSamples.splice(0, trace.screenSamples.length - SCREEN_MAX_SAMPLES) - } -} - -export function exportDebugTraceFiles(sessionId: string): BundleFile[] { +/** + * @param screenSamples main's screen-tail history for this session (#762: + * recorded in main from every frame, since the renderer no longer receives + * them; fetched with window.api.getScreenDebug). + */ +export function exportDebugTraceFiles(sessionId: string, screenSamples: readonly ScreenTailSample[] = []): BundleFile[] { const trace = traces.get(sessionId) - if (!trace) return [] + if (!trace && screenSamples.length === 0) return [] + const htmlCommits = trace?.htmlCommits ?? [] + const htmlCheckpoints = trace?.htmlCheckpoints ?? [] const capturedAt = Date.now() const manifest = { schemaVersion: 1, sessionId, - startedAt: trace.startedAt, - startedAtIso: nowIso(trace.startedAt), + startedAt: trace?.startedAt ?? null, + startedAtIso: trace ? nowIso(trace.startedAt) : null, capturedAt, capturedAtIso: nowIso(capturedAt), html: { - commits: trace.htmlCommits.length, - checkpoints: trace.htmlCheckpoints.length, + commits: htmlCommits.length, + checkpoints: htmlCheckpoints.length, checkpointEvery: HTML_CHECKPOINT_EVERY, maxCommits: HTML_MAX_COMMITS, - latestHash: trace.lastHtmlHash, + latestHash: trace?.lastHtmlHash ?? null, }, screen: { - samples: trace.screenSamples.length, + samples: screenSamples.length, tailLines: SCREEN_TAIL_LINES, maxSamples: SCREEN_MAX_SAMPLES, - latestHash: trace.lastScreenHash, - mode: 'deduped snapshots; no commit chain', + latestHash: screenSamples.at(-1)?.hash ?? null, + mode: 'deduped snapshots; no commit chain; recorded in main', }, } @@ -277,22 +219,22 @@ export function exportDebugTraceFiles(sessionId: string): BundleFile[] { }, { name: `${HTML_TRACE_DIR}/commits.jsonl`, - content: trace.htmlCommits.map(commit => JSON.stringify(commit)).join('\n') + - (trace.htmlCommits.length ? '\n' : ''), + content: htmlCommits.map(commit => JSON.stringify(commit)).join('\n') + + (htmlCommits.length ? '\n' : ''), }, { name: `${HTML_TRACE_DIR}/checkpoints.jsonl`, - content: trace.htmlCheckpoints.map(checkpoint => JSON.stringify(checkpoint)).join('\n') + - (trace.htmlCheckpoints.length ? '\n' : ''), + content: htmlCheckpoints.map(checkpoint => JSON.stringify(checkpoint)).join('\n') + + (htmlCheckpoints.length ? '\n' : ''), }, { name: `${SCREEN_TRACE_DIR}/tail-samples.jsonl`, - content: trace.screenSamples.map(sample => JSON.stringify(sample)).join('\n') + - (trace.screenSamples.length ? '\n' : ''), + content: screenSamples.map(sample => JSON.stringify(sample)).join('\n') + + (screenSamples.length ? '\n' : ''), }, ] - const latestScreen = trace.screenSamples[trace.screenSamples.length - 1] + const latestScreen = screenSamples[screenSamples.length - 1] if (latestScreen) { files.push({ name: `${SCREEN_TRACE_DIR}/latest-tail.txt`, diff --git a/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts b/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts new file mode 100644 index 000000000..5d6124223 --- /dev/null +++ b/src/renderer/src/features/debug/saveDebugBundle.renderer.test.ts @@ -0,0 +1,46 @@ +import { expect, it } from 'vitest' + +import { emptyRuntime } from '@renderer/session-runtime/state' +import { assembleAndSaveDebugBundle, buildStateSnapshot } from './saveDebugBundle' + +// #762: the renderer's screen copy only moves while a debug surface holds a +// lease, so the bundle's state snapshot takes the screen from MAIN when it +// has one, and falls back to the renderer copy only when main's read failed +// (#1236 review C: an unconditional fallback survived every test). +it('takes the screen fields from main, falling back to the renderer copy only without one', () => { + const runtime = { ...emptyRuntime(), screen: 'stale', screenMarkdown: 'stale', recentScreen: 'stale', recentScreenMarkdown: 'stale' } + const fromMain = buildStateSnapshot(runtime, { plain: 'live', markdown: 'live md', recent: 'live recent', recentMarkdown: 'live recent md' }) + expect(fromMain).toMatchObject({ screen: 'live', screenMarkdown: 'live md', recentScreen: 'live recent', recentScreenMarkdown: 'live recent md' }) + expect(buildStateSnapshot(runtime, null)).toMatchObject({ screen: 'stale', recentScreen: 'stale' }) +}) + +// The whole consumer seam, through the real bundle assembly: main's screen +// and tail history must land in the saved files (#1236 review C: dropping +// the samples at this call site survived every test). window.api is the only +// mocked edge. +it('saves the screen and tail history main recorded into the bundle', async () => { + const saved: Array<{ name: string; content: string }> = [] + const originalApi = window.api + window.api = { + ...originalApi, + flushPerformance: async () => {}, + getPerformanceSnapshot: async () => null, + readProxyEvents: async () => null, + getScreenDebug: async () => ({ + screen: { plain: 'main screen', markdown: 'main screen', recent: 'main recent', recentMarkdown: 'main recent' }, + samples: [{ id: '0-a', seq: 0, ts: 1, tsIso: new Date(1).toISOString(), hash: 'a', lineCount: 1, content: 'recorded in main' }], + }), + saveDebugBundle: async (input: { files: Array<{ name: string; content: string }> }) => { + saved.push(...input.files) + return { bundlePath: '/tmp/bundle' } + }, + } as never + try { + await assembleAndSaveDebugBundle({ sessionId: 'bundle-pane', runtime: emptyRuntime(), kind: 'claude' }) + } finally { + window.api = originalApi + } + expect(saved.find(file => file.name === 'trace/screen/latest-tail.txt')?.content).toBe('recorded in main') + const state = JSON.parse(saved.find(file => file.name.endsWith('state-snapshot.json'))!.content) as { recentScreen: string } + expect(state.recentScreen).toBe('main recent') +}) diff --git a/src/renderer/src/features/debug/saveDebugBundle.ts b/src/renderer/src/features/debug/saveDebugBundle.ts index 99fb82a98..21b69fd79 100644 --- a/src/renderer/src/features/debug/saveDebugBundle.ts +++ b/src/renderer/src/features/debug/saveDebugBundle.ts @@ -7,8 +7,9 @@ import { commandTargetSessionId } from '@renderer/workspace/hook/selectors/comma import { exportDebugTraceFiles, recordHtmlTraceSnapshot, - recordScreenTailSnapshot, } from '@renderer/features/debug/renderTrace' +import type { AgentScreenSnapshot } from '@shared/types/session' +import type { ScreenTailSample } from '@shared/debug/screenTail' import { buildCommittedAssistantText, buildSemanticRenderUnits, @@ -270,8 +271,10 @@ function buildRenderDiagnostics(runtime: SessionRuntime, kind: string): Record { +// Tail-truncated to SCREEN_TAIL_LINES. Taken from MAIN's latest snapshot +// when available (#762): the renderer's copy only moves while a debug +// surface holds a screen lease, so it is usually stale or empty. +export function buildStateSnapshot(runtime: SessionRuntime, mainScreen: AgentScreenSnapshot | null): Record { const { entries: _entries, toolUseIndex: _toolUseIndex, @@ -296,10 +299,10 @@ function buildStateSnapshot(runtime: SessionRuntime): Record { return { ...rest, - screen: tailLines(screen, SCREEN_TAIL_LINES), - screenMarkdown: tailLines(screenMarkdown, SCREEN_TAIL_LINES), - recentScreen: tailLines(recentScreen, SCREEN_TAIL_LINES), - recentScreenMarkdown: tailLines(recentScreenMarkdown, SCREEN_TAIL_LINES), + screen: tailLines(mainScreen?.plain ?? screen, SCREEN_TAIL_LINES), + screenMarkdown: tailLines(mainScreen?.markdown ?? screenMarkdown, SCREEN_TAIL_LINES), + recentScreen: tailLines(mainScreen?.recent ?? recentScreen, SCREEN_TAIL_LINES), + recentScreenMarkdown: tailLines(mainScreen?.recentMarkdown ?? recentScreenMarkdown, SCREEN_TAIL_LINES), // Counts are preserved so you can see "there were 2145 entries // in the feed" without having to open the JSONL. _counts: { @@ -488,7 +491,11 @@ export async function assembleAndSaveDebugBundle(params: { const html = capturePaneHtml(sessionId) recordHtmlTraceSnapshot(sessionId, html.raw, 'manual') - recordScreenTailSnapshot(sessionId, runtime.recentScreen) + // Main owns the screen now (#762): its latest snapshot and the tail history + // it records from every frame. A failed read degrades to the renderer's copy + // and no samples, never to a failed bundle. + const screenDebug: { screen: AgentScreenSnapshot | null; samples: ScreenTailSample[] } = + await window.api.getScreenDebug(sessionId).catch(() => ({ screen: null, samples: [] })) // Manual bundles include a bounded proxy tail because a human just // asked for a forensic snapshot. Autosave bundles intentionally skip @@ -511,7 +518,7 @@ export async function assembleAndSaveDebugBundle(params: { }, { name: FILE_NAMES.state, - content: JSON.stringify(buildStateSnapshot(runtime), null, 2), + content: JSON.stringify(buildStateSnapshot(runtime, screenDebug.screen), null, 2), }, { name: FILE_NAMES.feedDebug, @@ -542,7 +549,7 @@ export async function assembleAndSaveDebugBundle(params: { ...(proxySection?.sessionMeta ? [{ name: FILE_NAMES.proxySessionMeta, content: proxySection.sessionMeta }] : []), - ...exportDebugTraceFiles(sessionId), + ...exportDebugTraceFiles(sessionId, screenDebug.samples), ...(performanceSnapshot?.files ?? []), ] diff --git a/src/renderer/src/features/debug/ui/DebugPanel.tsx b/src/renderer/src/features/debug/ui/DebugPanel.tsx index 04390e8ce..3570477dd 100644 --- a/src/renderer/src/features/debug/ui/DebugPanel.tsx +++ b/src/renderer/src/features/debug/ui/DebugPanel.tsx @@ -4,6 +4,7 @@ import { useEffect, useMemo, useState } from 'react' import type { SessionRuntime } from '@renderer/workspace/workspaceStore' import type { Entry } from '@shared/types/transcript' import { AgentInlineTerminal } from '@renderer/features/debug/ui/AgentInlineTerminal' +import { useScreenLease } from '@renderer/features/debug/useScreenLease' // DebugPanel — inline diagnostic overlay showing the raw state of the // focused pane. Toggled via "Toggle Debug Panel" in the command palette. @@ -35,6 +36,9 @@ export function DebugPanel({ onClose, }: Props) { const [rawTerminalOpen, setRawTerminalOpen] = useState(false) + // The raw and markdown screen below need live frames, which main forwards + // only while a debug surface holds a lease (#762). + useScreenLease(sessionId) // WHY disable the inline xterm while the pane itself is terminal: // AgentInlineTerminal is not a passive transcript viewer; it attaches to and // resizes the same provider PTY as AgentTerminalLeaf. If both are mounted, diff --git a/src/renderer/src/features/debug/useScreenLease.renderer.test.tsx b/src/renderer/src/features/debug/useScreenLease.renderer.test.tsx new file mode 100644 index 000000000..f4311906c --- /dev/null +++ b/src/renderer/src/features/debug/useScreenLease.renderer.test.tsx @@ -0,0 +1,27 @@ +import { renderHook } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' + +import { useScreenLease } from './useScreenLease' + +// #762: a debug surface receives screen frames only while it holds a lease. +// A lease that is never released keeps the 93%-of-IPC-bytes stream flowing +// for that session; one released early blanks the panel. +afterEach(() => { Reflect.deleteProperty(window, 'api') }) + +it('holds one lease per mounted session and moves it when the session changes', () => { + const acquireScreenLease = vi.fn(async () => {}) + const releaseScreenLease = vi.fn(async () => {}) + Object.defineProperty(window, 'api', { configurable: true, value: { acquireScreenLease, releaseScreenLease } }) + + const { rerender, unmount } = renderHook(({ id }: { id: string | null }) => useScreenLease(id), { initialProps: { id: 'a' as string | null } }) + expect(acquireScreenLease.mock.calls).toEqual([['a']]) + + rerender({ id: 'b' }) + expect(releaseScreenLease.mock.calls).toEqual([['a']]) + expect(acquireScreenLease.mock.calls).toEqual([['a'], ['b']]) + + rerender({ id: null }) + unmount() + expect(releaseScreenLease.mock.calls).toEqual([['a'], ['b']]) + expect(acquireScreenLease).toHaveBeenCalledTimes(2) +}) diff --git a/src/renderer/src/features/debug/useScreenLease.ts b/src/renderer/src/features/debug/useScreenLease.ts new file mode 100644 index 000000000..a3ecd0049 --- /dev/null +++ b/src/renderer/src/features/debug/useScreenLease.ts @@ -0,0 +1,22 @@ +import { useEffect } from 'react' + +/** + * Receive live `session:screen` frames for this session while mounted. + * + * WHY a lease (#762): main forwards screen frames only to a renderer that + * asked for them, because nothing live reads them and they were 93% of IPC + * bytes (src/main/sessions/screenInterest.ts). Debug surfaces that show the + * raw screen take a lease for as long as they are open; main seeds the + * current screen on acquire, so the surface is right even when the backend + * is idle. Main also drops a renderer's leases on reload, so a missed + * release can only cost bytes until then, never correctness. + */ +export function useScreenLease(sessionId: string | null | undefined): void { + useEffect(() => { + if (!sessionId) return + void window.api.acquireScreenLease(sessionId) + return () => { + void window.api.releaseScreenLease(sessionId) + } + }, [sessionId]) +} diff --git a/src/renderer/src/workspace/hook/actions/agentIndexNavigation.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/agentIndexNavigation.renderer.test.tsx index cbed3dfd3..e97bac952 100644 --- a/src/renderer/src/workspace/hook/actions/agentIndexNavigation.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/agentIndexNavigation.renderer.test.tsx @@ -44,7 +44,6 @@ function makeRefs(state: WorkspaceState): WorkspaceRefs { seenUuidsRef: ref({}), historyWindowsRef: { current: {} } as never, historyAwaitingTurnStartRef: { current: new Set() } as never, - latestScreenRef: ref({}), undoStackRef: ref(new UndoCloseStack()), bootstrapTimersRef: ref(new Map()), persistedFeedDebugIdRef: ref({}), diff --git a/src/renderer/src/workspace/hook/actions/pane.ts b/src/renderer/src/workspace/hook/actions/pane.ts index f94fcdcd3..477308994 100644 --- a/src/renderer/src/workspace/hook/actions/pane.ts +++ b/src/renderer/src/workspace/hook/actions/pane.ts @@ -80,7 +80,6 @@ function forgetClosedSessionDebugState(refs: WorkspaceRefs, sessionId: SessionId // liveEntryWindow.ts). Every site that deletes/resets a session's seen // set must clear it too. clearLiveEntryWindowSession(sessionId) - delete refs.latestScreenRef.current[sessionId] // Render traces are intentionally not stored in SessionRuntime: they are // large, debug-only forensic buffers populated by DOM/screen capture paths // that do not need to re-render the app. That module-level map must still diff --git a/src/renderer/src/workspace/hook/actions/paneRecoveryOwnership.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/paneRecoveryOwnership.renderer.test.tsx index 764c8ea17..7047c6fc3 100644 --- a/src/renderer/src/workspace/hook/actions/paneRecoveryOwnership.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/paneRecoveryOwnership.renderer.test.tsx @@ -44,7 +44,6 @@ function renderPaneActionsHarness( latestStateRef: ref(state), latestRuntimesRef: ref(runtimes), seenUuidsRef: ref>>({}), - latestScreenRef: ref>({}), undoStackRef: ref(new UndoCloseStack()), // Close operations release a pending bootstrap debounce for each session // they end, as sessionActions.killSession always did (#886 round 2). @@ -124,7 +123,6 @@ describe('pane recovery ownership', () => { latestStateRef: ref(state), latestRuntimesRef: ref(runtimes), seenUuidsRef: ref>>({}), - latestScreenRef: ref>({}), undoStackRef: ref(new UndoCloseStack()), bootstrapTimersRef: ref(new Map()), } as unknown as WorkspaceRefs diff --git a/src/renderer/src/workspace/hook/actions/session.ts b/src/renderer/src/workspace/hook/actions/session.ts index 2afa69d97..b23f65a3e 100644 --- a/src/renderer/src/workspace/hook/actions/session.ts +++ b/src/renderer/src/workspace/hook/actions/session.ts @@ -1134,7 +1134,6 @@ export function useSessionActions( // Live-window bookkeeping follows the seen-uuid lifecycle (see // liveEntryWindow.ts: trimmed ⊆ ever-seen must hold). clearLiveEntryWindowSession(sessionId) - delete refs.latestScreenRef.current[sessionId] // If a bootstrap debounce was in flight for this session, // cancel it — the session is gone; firing the deferred // bootstrapping→false flip later would be a no-op against a @@ -1146,7 +1145,7 @@ export function useSessionActions( refs.bootstrapTimersRef.current.delete(sessionId) } }, - [refs.bootstrapTimersRef, refs.latestScreenRef, refs.seenUuidsRef, setRuntimes, setState], + [refs.bootstrapTimersRef, refs.seenUuidsRef, setRuntimes, setState], ) // Kills the current session in the focused leaf and spawns a new @@ -1457,7 +1456,6 @@ export function useSessionActions( }) delete refs.seenUuidsRef.current[oldId] clearLiveEntryWindowSession(oldId) - delete refs.latestScreenRef.current[oldId] return newId } finally { @@ -1466,7 +1464,6 @@ export function useSessionActions( }, [ refs.latestRuntimesRef, - refs.latestScreenRef, refs.seenUuidsRef, refs.stateRef, setRuntimes, @@ -1540,7 +1537,6 @@ export function useSessionActions( delete refs.seenUuidsRef.current[oldId] clearLiveEntryWindowSession(oldId) - delete refs.latestScreenRef.current[oldId] try { const kind: SessionKind = meta.kind ?? DEFAULT_PROVIDER @@ -1679,7 +1675,6 @@ export function useSessionActions( [ refs.dangerousAgentsRef, refs.latestRuntimesRef, - refs.latestScreenRef, refs.seenUuidsRef, refs.stateRef, refs.useProxyStreamingRef, @@ -1742,7 +1737,6 @@ export function useSessionActions( // on the live path while older-history kept releasing them — the // trimmed ⊆ ever-seen invariant (liveEntryWindow.ts) would be gone. clearLiveEntryWindowSession(sessionId) - delete refs.latestScreenRef.current[sessionId] setRuntimes(prev => { const current = prev[sessionId] ?? emptyRuntime() @@ -1765,7 +1759,6 @@ export function useSessionActions( }, [ refs.bootstrapTimersRef, - refs.latestScreenRef, refs.seenUuidsRef, refs.stateRef, setRuntimes, diff --git a/src/renderer/src/workspace/hook/actions/sessionRecovery.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/sessionRecovery.renderer.test.tsx index 58f042b71..1847cdd3a 100644 --- a/src/renderer/src/workspace/hook/actions/sessionRecovery.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/sessionRecovery.renderer.test.tsx @@ -426,7 +426,6 @@ describe('useSessionActions recovery retry', () => { useProxyStreamingRef: ref(false), defaultBuiltInMcpDomainsRef: ref([]), seenUuidsRef: ref({}), - latestScreenRef: ref({}), } as unknown as WorkspaceRefs const setState = (next: WorkspaceState | ((prev: WorkspaceState) => WorkspaceState)) => { state = typeof next === 'function' ? next(state) : next diff --git a/src/renderer/src/workspace/hook/actions/sessionReplacementHandoff.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/sessionReplacementHandoff.renderer.test.tsx index 78953875e..ffdf3e637 100644 --- a/src/renderer/src/workspace/hook/actions/sessionReplacementHandoff.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/actions/sessionReplacementHandoff.renderer.test.tsx @@ -71,7 +71,6 @@ describe('renderer session replacement handoff', () => { seenUuidsRef: ref({}), historyWindowsRef: { current: {} } as never, historyAwaitingTurnStartRef: { current: new Set() } as never, - latestScreenRef: ref({}), undoStackRef: ref(new UndoCloseStack()), bootstrapTimersRef: ref(new Map()), persistedFeedDebugIdRef: ref({}), diff --git a/src/renderer/src/workspace/hook/actions/testing/paneActionsHarness.tsx b/src/renderer/src/workspace/hook/actions/testing/paneActionsHarness.tsx index 9bc60ab9c..ae67475c5 100644 --- a/src/renderer/src/workspace/hook/actions/testing/paneActionsHarness.tsx +++ b/src/renderer/src/workspace/hook/actions/testing/paneActionsHarness.tsx @@ -43,7 +43,6 @@ export function makeRefs(state: WorkspaceState): WorkspaceRefs { seenUuidsRef: ref({}), historyWindowsRef: { current: {} } as never, historyAwaitingTurnStartRef: { current: new Set() } as never, - latestScreenRef: ref({}), undoStackRef: ref(new UndoCloseStack()), bootstrapTimersRef: ref(new Map()), persistedFeedDebugIdRef: ref({}), diff --git a/src/renderer/src/workspace/hook/index.ts b/src/renderer/src/workspace/hook/index.ts index 61fe731ec..16ddb621d 100644 --- a/src/renderer/src/workspace/hook/index.ts +++ b/src/renderer/src/workspace/hook/index.ts @@ -1051,7 +1051,6 @@ export function useWorkspace( setReaderModeTarget, toggleReaderMode, setReaderModeSession, - latestScreenRef: refs.latestScreenRef, getRuntime, // Patch a session's runtime (sessionId, partial). Used by TileLeaf for // pane toasts and the prompt-suggestion chip's apply/dismiss handlers. diff --git a/src/renderer/src/workspace/hook/ipc/testing/workspaceRefsForTest.ts b/src/renderer/src/workspace/hook/ipc/testing/workspaceRefsForTest.ts index 947f28186..981814b13 100644 --- a/src/renderer/src/workspace/hook/ipc/testing/workspaceRefsForTest.ts +++ b/src/renderer/src/workspace/hook/ipc/testing/workspaceRefsForTest.ts @@ -22,7 +22,6 @@ export function makeWorkspaceRefsForTest(state: WorkspaceState): WorkspaceRefs { seenUuidsRef: ref({}), historyWindowsRef: { current: {} } as never, historyAwaitingTurnStartRef: { current: new Set() } as never, - latestScreenRef: ref({}), undoStackRef: ref(new UndoCloseStack()), bootstrapTimersRef: ref(new Map()), persistedFeedDebugIdRef: ref({}), diff --git a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.renderer.test.tsx b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.renderer.test.tsx index 7eff7fc3b..7abb65b0f 100644 --- a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.renderer.test.tsx @@ -889,6 +889,35 @@ describe('useIpcSubscriptions with an injected SessionFeed', () => { expect(runtimes.s1?.screen).toBe('hello world') expect(runtimes.s1?.recentScreen).toBe('hello world') + + // #762: the picker a screen frame carries is NOT applied. The conditions + // channel owns runtime.picker (see the first case in this file); a second + // writer could re-show a picker the conditions had just cleared, and + // frames now arrive only while a debug surface holds a lease. + act(() => { + fake.emitScreen({ + sessionId: 's1', + plain: 'hello again', + markdown: 'hello again', + recent: 'hello again', + recentMarkdown: 'hello again', + picker: { visible: true, items: [{ name: '/help', description: 'stale' }] }, + } as never) + }) + expect(runtimes.s1?.screen).toBe('hello again') + expect(runtimes.s1?.picker.visible).toBe(false) + + // A frame whose visible screen is unchanged but whose scrollback grew + // must still apply (#1236 review C: the no-op bail on `screen` alone + // survived). + act(() => { + fake.emitScreen({ + sessionId: 's1', plain: 'hello again', markdown: 'hello again', + recent: 'earlier line\nhello again', recentMarkdown: 'earlier line\nhello again', + picker: { visible: false, items: [] }, + }) + }) + expect(runtimes.s1?.recentScreen).toBe('earlier line\nhello again') }) it('treats provider readiness as versioned state, never as process activity', () => { @@ -977,14 +1006,12 @@ describe('useIpcSubscriptions with an injected SessionFeed', () => { draftInput: 'renderer-local draft survives', } let runtimes: Record = { s1: conflictedRuntime } - let refsForAssertion: WorkspaceRefs | null = null function Harness(): React.JSX.Element { const refs = useRef(null) if (refs.current === null) { refs.current = makeRefs(state) refs.current.latestRuntimesRef.current = runtimes - refsForAssertion = refs.current } useIpcSubscriptions( fake, @@ -1032,7 +1059,6 @@ describe('useIpcSubscriptions with an injected SessionFeed', () => { // workspace's text and mutate hidden semantic/debug state. Quarantine must // return before every state/ref write until matching ownership is proven. expect(runtimes.s1).toBe(conflictedRuntime) - expect(refsForAssertion!.latestScreenRef.current.s1).toBeUndefined() }) it('retains the retired run across exit and replaces it only on the next started event', () => { diff --git a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts index 4fff9e6b9..7c48f18fa 100644 --- a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts +++ b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts @@ -37,7 +37,6 @@ import { import { applyPromptSuggestionToRuntime } from '@renderer/workspace/hook/ipc/applyPromptSuggestionToRuntime' import { summarizeSemanticEventForDebug } from '@renderer/session-runtime/semantic/summarize' import { isSemanticRawCaptureEnabled } from '@renderer/session-runtime/semantic/rawCapture' -import { recordScreenTailSnapshot } from '@renderer/features/debug/renderTrace' import { isCodexRolloutEntry, isOptimisticCodexUserEntry, @@ -64,7 +63,6 @@ import { } from '@renderer/session-runtime/ingest/committedRecords' import { emitRendererMemoryGauges } from '@renderer/performance/memoryInstrumentation' import { loadInitialHistoryForSession } from '@renderer/workspace/hook/actions/initialHistory' -import { pickerEqual } from '@renderer/workspace/layout/helpers' import { ghostsFromSemanticTurn, sweepGhosts, @@ -410,7 +408,7 @@ const WALL_CLOCK_MS_FLOOR = 1_000_000_000_000 // // Handlers intentionally stay inline here rather than broken into // one-per-event files because they share refs, setters, and -// bookkeeping state (seenUuidsRef, latestScreenRef, bootstrapTimersRef) +// bookkeeping state (seenUuidsRef, bootstrapTimersRef) // — breaking them apart would mean passing the whole ctx into each, // gaining nothing but file count. If a handler ever grows to >200 // lines AND has no cross-handler state, it's a candidate for @@ -702,18 +700,18 @@ export function useIpcSubscriptions( }) const offScreen = feed.onSessionScreen( - ({ sessionId, plain, markdown, recent, recentMarkdown, picker }) => { + ({ sessionId, plain, markdown, recent, recentMarkdown }) => { if (quarantinesSessionFeed(sessionId)) return const startedAt = performance.now() - // latestScreenRef is the synchronous source of truth for - // the Enter-baseline capture in TileLeaf — always update - // it, even when we bail on React state below. - // Use `recent` (wider window) so the baseline includes any - // assistant text that may have already scrolled out of the - // viewport. The baseline comparison is the basis for "is - // the streaming card stale?", which depends on seeing the - // same marker the streaming extractor will see. - refs.latestScreenRef.current[sessionId] = recent + // WHY only the screen strings are applied here (#762): frames now + // arrive only while a debug surface holds a screen lease, so this + // handler feeds DebugPanel and the dev modules, nothing live. The + // frame's `picker` is NOT written: the composer's slash picker is + // owned by the conditions channel (applyConditionSnapshot), and a + // second writer here was split authority that could re-show a + // picker the conditions had just cleared. The Enter-baseline ref and + // the screen-tail trace this used to feed are gone: the baseline was + // dead code, and main records the trace from every frame. setRuntimes(prev => { const current = prev[sessionId] ?? emptyRuntime() @@ -730,11 +728,7 @@ export function useIpcSubscriptions( // every no-op frame. This is the difference between // "scheduled work on every frame" and "scheduled work // only when the screen actually changed". - if ( - current.screen === plain && - current.recentScreen === recent && - pickerEqual(current.picker, picker) - ) { + if (current.screen === plain && current.recentScreen === recent) { const durationMs = performance.now() - startedAt if (durationMs > 8) { perf.metric('workspace.ipc.screen.noop.slow', durationMs, 'sample', { @@ -747,7 +741,6 @@ export function useIpcSubscriptions( const changed: string[] = [] if (current.screen !== plain) changed.push('screen') if (current.recentScreen !== recent) changed.push('recent') - if (!pickerEqual(current.picker, picker)) changed.push('picker') // Screen frames can differ only by transient TUI chrome // (cursor blink, spinner tick, timestamp) while the // visible transcript is unchanged. We still commit the @@ -767,8 +760,7 @@ export function useIpcSubscriptions( const chromeTickOnly = changed.every(k => k === 'screen' || k === 'recent') && changed.length > 0 && - recent.length === current.recentScreen.length && - pickerEqual(current.picker, picker) + recent.length === current.recentScreen.length const nextBody = { ...current, @@ -776,7 +768,6 @@ export function useIpcSubscriptions( screenMarkdown: markdown, recentScreen: recent, recentScreenMarkdown: recentMarkdown, - picker, // activityStatus is owned by the process-state IPC // handler below — it carries the provider-correct verb // (Claude's spinner verb, Codex's bottom-row text). @@ -795,22 +786,16 @@ export function useIpcSubscriptions( summary: `screen update · ${changed.join(', ')}`, data: { changed, - pickerVisible: picker.visible, - pickerCount: picker.items.length, recentLength: recent.length, }, }, ) - if (!chromeTickOnly) { - recordScreenTailSnapshot(sessionId, recent) - } const durationMs = performance.now() - startedAt if (durationMs > 8) { perf.metric('workspace.ipc.screen.apply.slow', durationMs, 'sample', { sessionId, changed, recentLength: recent.length, - pickerVisible: picker.visible, }) } return { diff --git a/src/renderer/src/workspace/hook/persistence/codexLiveContinuity.renderer.test.tsx b/src/renderer/src/workspace/hook/persistence/codexLiveContinuity.renderer.test.tsx index baaf759fd..dd18bdbd0 100644 --- a/src/renderer/src/workspace/hook/persistence/codexLiveContinuity.renderer.test.tsx +++ b/src/renderer/src/workspace/hook/persistence/codexLiveContinuity.renderer.test.tsx @@ -211,7 +211,6 @@ function makeRefs(state: WorkspaceState, runtimes: Record> - latestScreenRef: MutableRefObject> undoStackRef: MutableRefObject bootstrapTimersRef: MutableRefObject>> persistedFeedDebugIdRef: MutableRefObject> @@ -105,7 +104,6 @@ export function useWorkspaceRefs( const seenUuidsRef = useRef>>({}) const historyWindowsRef = useRef>({}) const historyAwaitingTurnStartRef = useRef>(new Set()) - const latestScreenRef = useRef>({}) const undoStackRef = useRef(new UndoCloseStack()) const bootstrapTimersRef = useRef>>(new Map()) const pendingAdoptionWindowIdsRef = useRef([]) @@ -147,7 +145,6 @@ export function useWorkspaceRefs( // Latest screen per session — mirrored from state into a ref so // the Enter handler in TileLeaf can capture a baseline // synchronously. - latestScreenRef, // Undo-close stack — mutable ref because the stack is imperative // (push/pop) and we don't want React re-renders on every close. diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx index 8e64c95a6..77e6622ca 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TileLeaf.tsx @@ -59,8 +59,9 @@ import { const MAX_TRACKED_VISIBLE_SUBMIT_SURFACES = 2_048 -// Claude paste-state-machine constants + helpers moved to -// ./TileLeaf/claudePaste.ts. Image helpers moved to +// Claude paste detection lives in @shared/claude/pasteConfirm and runs in +// main's prompt delivery (the renderer paste helpers were removed as dead +// code in #762). Image helpers moved to // ./TileLeaf/claudeImages.ts. Label helpers moved to // ./TileLeaf/labels.ts. See those files for the full rationale on // the paste debounce, the image size/format gates, and the pane diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf/claudePaste.ts b/src/renderer/src/workspace/tile-tree/TileLeaf/claudePaste.ts deleted file mode 100644 index 05e6ffc9e..000000000 --- a/src/renderer/src/workspace/tile-tree/TileLeaf/claudePaste.ts +++ /dev/null @@ -1,219 +0,0 @@ -// Claude's TUI has its own paste-state machine. Large input and -// bracketed paste do NOT go straight from "bytes arrived" to "submit -// immediately" — Claude buffers every `isPasted` chunk and flushes on a -// ~100ms debounce (`PASTE_COMPLETION_TIMEOUT_MS` in claude-code-src). -// INSIDE that debounce window a `pastePendingRef` guard swallows every -// subsequent keystroke — INCLUDING our submit `\r` — as more paste -// content. So if the Enter lands before the debounce fires, it becomes -// literal text, no turn starts, and the user has to press Enter again. -// That is the exact user-facing bug: -// -// 1. the pasted prompt appears in Claude's composer, -// 2. Claude flips into a "working" looking state, -// 3. but no real turn starts, -// 4. and the NEXT Enter finally submits the already-buffered prompt. -// -// We fix this at the shell boundary instead of forking Claude's paste -// logic: deliver the paste in TWO phases — first the bracketed payload, -// then a separate submit `\r` only AFTER we can see Claude's composer -// has visibly absorbed the paste (the debounce has provably fired). -// -// ---------------------------------------------------------------------------- -// THE SIGNAL IS A CONTENT MATCH, NOT A CLOCK. -// ---------------------------------------------------------------------------- -// -// We compare what the user submitted against what is actually in the -// headless composer, via the live screen snapshot (`latestScreenRef`), -// and send `\r` the instant the composer reflects the paste. Two -// screen-truth manifestations, because Claude renders pastes two ways: -// -// * COLLAPSE: a large paste becomes a `[Pasted text #N]` placeholder. -// Signal: a NEW placeholder appeared vs. the pre-paste baseline. -// * INLINE: a medium paste (≈100 chars up to Claude's collapse -// threshold) is inserted as raw text. NO placeholder ever renders — -// this is the band that issue #279 / #90 kept failing on, because -// the old detector only watched for the placeholder, timed out, and -// blind-fired `\r` straight into the debounce. Signal: the paste's -// tail newly appears in the composer. -// -// Both are load-independent: 10ms or 10s, we wait for the *condition*. -// -// The wall-clock wait survives ONLY as a rare safety floor — if neither -// signal materializes within a short bound (a future Claude UI rename, -// or a screen snapshot we can't read), we send `\r` anyway so submit -// degrades rather than hangs. It must never again be the primary path: -// real production traces show the timer races even at 1000ms under load, -// and "there's no value of T that's correct under all load conditions." -// -// Why these numbers: -// - 100 chars: empirical lower bound where Claude's paste ACCUMULATOR -// engages, independent of the inline-vs-collapse cutoff. Below this we -// send raw `text + \r` in one write (the accumulator doesn't engage, -// so the `\r` is safe). -// - 125 ms floor: slightly above Claude's 100 ms debounce. Consulted -// ONLY when the content match can't be made (the rare safety net). -// - 500 ms detection bound: well past the 100 ms accumulator window, so -// a real absorption always wins first; short enough that the rare -// safety-net path still feels responsive. - -import { sha8Web } from '@shared/code/sha8' -import { - CLAUDE_PASTE_THRESHOLD, - placeholderCount, - pasteTailNeedle, - pollPasteAbsorbed, -} from '@shared/claude/pasteConfirm' -import type { PasteAbsorbedOutcome } from '@shared/claude/pasteConfirm' - -// The paste-commit DETECTION (placeholder-or-inline) now lives in the shared -// module that BOTH this desktop composer and the runtime delivery path consume -// — killing the drift that had left the runtime on a placeholder-only probe. -// Re-exported here so existing importers keep their `@renderer/.../claudePaste` -// import path. The battle-scarred TIMING constants below are unchanged. -export { CLAUDE_PASTE_THRESHOLD, placeholderCount, pasteTailNeedle } -export type { PasteAbsorbedOutcome } - -export const CLAUDE_PASTE_SUBMIT_DELAY_MS = 125 -export const CLAUDE_IMAGE_PATH_SUBMIT_DELAY_MS = 750 -export const CLAUDE_PASTE_EVENT_DRIVEN_TIMEOUT_MS = 500 -export const CLAUDE_PASTE_POLL_INTERVAL_MS = 10 - -function wait(ms: number): Promise { - return new Promise(resolve => setTimeout(resolve, ms)) -} - -// SHA-256 prefix used as a fingerprint to correlate paste payloads -// across the renderer→IPC→main→PTY chain in the per-paste debug -// journal. crypto.subtle is the standard renderer-side digest API; -// 4 bytes is more than enough for cross-process pairing (collision -// probability is negligible for a single paste's chunk set). -const sha8 = sha8Web - -// Content-match detection (placeholderCount / pasteTailNeedle / -// pollPasteAbsorbed) moved verbatim to @shared/claude/pasteConfirm so the -// runtime delivery path shares the exact same logic. Imported + re-exported at -// the top of this file. - -export type ClaudePasteSendFn = (data: string, pasteId?: string) => Promise - -export type ClaudePasteOptions = { - /** Renderer-minted UUID for the per-paste debug journal. When set, - * every PTY write in this paste emits an IPC:write event to the - * journal. */ - pasteId?: string - /** Event-driven submit for Claude paste-like text. `getScreen` returns the - * live headless TUI snapshot so we can confirm the composer absorbed the - * paste before sending Enter. Optional in the type only because lower-level - * helpers are also used by Codex and image-path flows that do not need it. */ - eventDriven?: { enabled: boolean; getScreen: () => string | undefined } -} - -export async function sendBracketedPaste( - send: ClaudePasteSendFn, - payload: string, - opts?: ClaudePasteOptions, -): Promise { - const pasteId = opts?.pasteId - if (pasteId) { - window.api.recordPasteDebugEvent(pasteId, { - layer: 'IPC', - event: 'write:paste-payload', - data: { bytes: payload.length, sha8: await sha8(payload) }, - }) - } - await send(`\x1b[200~${payload}\x1b[201~`, pasteId) -} - -export async function sendBracketedPasteThenSubmit( - send: ClaudePasteSendFn, - payload: string, - delayMs = 0, - opts?: ClaudePasteOptions, -): Promise { - const pasteId = opts?.pasteId - const eventDriven = opts?.eventDriven - - // Single-write fast path. Used by callers that don't care about the - // accumulator race (e.g. Codex, which has its own paste-state machine that - // does NOT exhibit this bug). On Claude this branch is only taken when - // `delayMs <= 0` AND event-driven is off — the composer call site always - // sets event-driven, so in production this is codex-only. - if (delayMs <= 0 && !eventDriven?.enabled) { - if (pasteId) { - window.api.recordPasteDebugEvent(pasteId, { - layer: 'IPC', - event: 'write:paste-and-submit-single', - data: { bytes: payload.length, sha8: await sha8(payload) }, - }) - } - await send(`\x1b[200~${payload}\x1b[201~\r`, pasteId) - return - } - - // Capture the composer baseline BEFORE writing the paste so the detector - // keys on the placeholder/inline-text *transition*, not a stale match. - const baselineScreen = eventDriven?.enabled ? (eventDriven.getScreen() ?? '') : '' - - await sendBracketedPaste(send, payload, opts) - - if (eventDriven?.enabled) { - const outcome = await pollPasteAbsorbed(eventDriven.getScreen, baselineScreen, payload, { - timeoutMs: CLAUDE_PASTE_EVENT_DRIVEN_TIMEOUT_MS, - pollIntervalMs: CLAUDE_PASTE_POLL_INTERVAL_MS, - }) - if (pasteId) { - window.api.recordPasteDebugEvent(pasteId, { - layer: 'SCREEN', - event: outcome.kind === 'absorbed' ? 'paste:absorbed' : 'paste:absorb-timeout', - data: - outcome.kind === 'absorbed' - ? { waitedMs: outcome.waitedMs, via: outcome.via } - : {}, - }) - } - if (outcome.kind === 'absorbed') { - if (pasteId) { - window.api.recordPasteDebugEvent(pasteId, { - layer: 'IPC', - event: 'write:submit-cr', - data: { strategy: `event-driven:${outcome.via}`, waitedMs: outcome.waitedMs }, - }) - } - await send('\r', pasteId) - return - } - // Absorption not detected within the safety bound. Fall through to the - // wall-clock floor — the rare safety net, NOT the primary path. - } - - if (delayMs > 0) await wait(delayMs) - if (pasteId) { - window.api.recordPasteDebugEvent(pasteId, { - layer: 'IPC', - event: 'write:submit-cr', - data: { strategy: 'wall-clock-floor', delayMs }, - }) - } - await send('\r', pasteId) -} - -export async function sendClaudeDraftText( - send: ClaudePasteSendFn, - text: string, - opts?: ClaudePasteOptions, -): Promise { - if (text.length === 0) return - const isPasteLike = text.includes('\n') || text.length > CLAUDE_PASTE_THRESHOLD - if (isPasteLike) { - await sendBracketedPaste(send, text, opts) - await wait(CLAUDE_PASTE_SUBMIT_DELAY_MS) - return - } - await send(text, opts?.pasteId) -} - -export function buildClaudeImagePastePayload(text: string, imagePaths: string[]): string { - if (imagePaths.length === 0) return text - if (text.trim().length === 0) return imagePaths.join('\n') - return `${imagePaths.join('\n')}\n${text}` -} diff --git a/src/renderer/src/workspace/tile-tree/TileLeaf/useComposerKeybinds.ts b/src/renderer/src/workspace/tile-tree/TileLeaf/useComposerKeybinds.ts index 6159282fe..9d3dec7b1 100644 --- a/src/renderer/src/workspace/tile-tree/TileLeaf/useComposerKeybinds.ts +++ b/src/renderer/src/workspace/tile-tree/TileLeaf/useComposerKeybinds.ts @@ -9,7 +9,7 @@ import { isSessionExited } from '@renderer/workspace/providerSessionIdentity' // stable across providers so historical dumps stay comparable. The // actual submit routing moved to the provider composerSubmit // capability (#394 phase 2c-4). -import { CLAUDE_PASTE_THRESHOLD } from '@renderer/workspace/tile-tree/TileLeaf/claudePaste' +import { CLAUDE_PASTE_THRESHOLD } from '@shared/claude/pasteConfirm' import { getRendererProviderCapabilities } from '@providers/registry.renderer.capabilities' import { DEFAULT_PROVIDER, @@ -35,7 +35,8 @@ import { reportLifecycle } from '@renderer/lifecycle/report' // Escape forwards, Shift+Enter inserts a literal newline. // The submit path forks by provider because Claude and // Codex have DIFFERENT paste + submit timing requirements -// (see claudePaste.ts for the debounce story). +// (see @shared/claude/pasteConfirm and main's +// providers/claude/runtime/promptDelivery.ts for the paste story). // 3. Bash-style prompt history — Up/Down cycle through previous // prompts when the composer is empty. Modifier combos fall // through to the PTY-forward path so OS line navigation @@ -357,7 +358,6 @@ export function useComposerKeybinds({ () => workspace.ensureSessionLive(sessionId, 'tile-leaf.deliver-retry'), ), pasteId, - getScreen: () => workspace.latestScreenRef.current[sessionId], }) // Accepted: the sent prompt leaves the draft, keeping anything another // writer added during the send (draftAfterAcceptance). diff --git a/src/shared/claude/pasteConfirm.ts b/src/shared/claude/pasteConfirm.ts index ce3db288e..e60ef4f6a 100644 --- a/src/shared/claude/pasteConfirm.ts +++ b/src/shared/claude/pasteConfirm.ts @@ -1,6 +1,6 @@ -// Claude paste-commit detection — THE ONE implementation, shared by both the -// desktop composer (renderer: claudePaste.ts) and the remote/runtime delivery -// path (main: providers/claude/runtime/promptDelivery.ts). +// Claude paste-commit detection — THE ONE implementation, used by prompt +// delivery in main (providers/claude/runtime/promptDelivery.ts), which every +// composer, remote and runtime prompt goes through. // // WHY this module exists (the bug it kills): // Claude's TUI buffers bracketed-paste bytes on a ~100ms accumulator; an @@ -32,7 +32,7 @@ * engages, so a plain `text + \r` in one write is safe and needs no * confirmation. Above it, OR with any newline, the paste route + this * detection is mandatory. Empirical lower bound; single source for every - * `isPasteLike` check (previously duplicated in claudePaste.ts, + * `isPasteLike` check (previously duplicated in the renderer's paste helpers, * composerSubmit.ts, useComposerKeybinds.ts, and promptDelivery.ts). */ export const CLAUDE_PASTE_THRESHOLD = 100 diff --git a/src/shared/debug/screenTail.test.ts b/src/shared/debug/screenTail.test.ts new file mode 100644 index 000000000..b00c3844f --- /dev/null +++ b/src/shared/debug/screenTail.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from 'vitest' + +import { SCREEN_MAX_SAMPLES, SCREEN_TAIL_LINES, ScreenTailHistory } from './screenTail.js' + +// The debug bundle's trace/screen/tail-samples.jsonl, now recorded in main +// (#762). Same contract as the renderer version it replaced: tail only, +// deduplicated, bounded, forgettable. +describe('ScreenTailHistory', () => { + it('keeps the last lines of each distinct screen, once', () => { + let now = 1_000 + const history = new ScreenTailHistory(() => now) + const long = Array.from({ length: SCREEN_TAIL_LINES + 10 }, (_, i) => `line ${i}`).join('\n') + history.record('s', `\u001b[31m${long}\u001b[0m `) + now += 1 + history.record('s', long) + const [sample, ...rest] = history.samples('s') + expect(rest).toEqual([]) + expect(sample!.lineCount).toBe(SCREEN_TAIL_LINES) + expect(sample!.content.startsWith('line 10')).toBe(true) + expect(sample!.content).not.toContain('\u001b') + }) + + it('caps the samples per session and forgets a closed session', () => { + const history = new ScreenTailHistory(() => 0) + for (let i = 0; i < SCREEN_MAX_SAMPLES + 5; i += 1) history.record('s', `frame ${i}`) + const samples = history.samples('s') + expect(samples).toHaveLength(SCREEN_MAX_SAMPLES) + expect(samples[0]!.content).toBe('frame 5') + history.forget('s') + expect(history.samples('s')).toEqual([]) + }) +}) diff --git a/src/shared/debug/screenTail.ts b/src/shared/debug/screenTail.ts new file mode 100644 index 000000000..83d94b617 --- /dev/null +++ b/src/shared/debug/screenTail.ts @@ -0,0 +1,97 @@ +// Screen-tail history for debug bundles: a bounded, deduplicated sample of +// the last SCREEN_TAIL_LINES of each session's terminal screen over time. +// +// WHY this lives in shared code and is recorded in MAIN (#762): it used to be +// recorded in the renderer from every `session:screen` frame. That IPC was +// 93% of recorded IPC bytes and is now only forwarded to a renderer that +// asked for it (see src/main/sessions/screenInterest.ts), so the renderer no +// longer sees every frame. Main does, in the forwarder sink, so the history +// is recorded there and handed to the debug bundle on request. The format is +// unchanged: bundles still carry trace/screen/tail-samples.jsonl. + +export const SCREEN_TAIL_LINES = 50 +export const SCREEN_MAX_SAMPLES = 300 + +export type ScreenTailSample = { + id: string + seq: number + ts: number + tsIso: string + hash: string + lineCount: number + content: string +} + +/** A fast, non-cryptographic 53-bit string hash (cyrb53), hex. Used to + * deduplicate identical samples and to label trace content. */ +export function hashText(text: string): string { + let h1 = 0xdeadbeef ^ text.length + let h2 = 0x41c6ce57 ^ text.length + for (let i = 0; i < text.length; i++) { + const ch = text.charCodeAt(i) + h1 = Math.imul(h1 ^ ch, 2654435761) + h2 = Math.imul(h2 ^ ch, 1597334677) + } + h1 = Math.imul(h1 ^ (h1 >>> 16), 2246822507) ^ Math.imul(h2 ^ (h2 >>> 13), 3266489909) + h2 = Math.imul(h2 ^ (h2 >>> 16), 2246822507) ^ Math.imul(h1 ^ (h1 >>> 13), 3266489909) + const n = 4294967296 * (2097151 & h2) + (h1 >>> 0) + return n.toString(16).padStart(13, '0') +} + +export function sanitizeScreenText(text: string): string { + return text + .replace(/\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])/g, '') + .replace(/\r\n?/g, '\n') + .replace(/[^\S\n\t]+$/gm, '') + .replace(/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/g, '') + .trimEnd() +} + +export function tailLines(text: string, count: number): string { + const lines = text.split('\n') + return lines.length <= count ? text : lines.slice(lines.length - count).join('\n') +} + +type SessionTail = { samples: ScreenTailSample[]; nextSeq: number; lastHash: string | null } + +export class ScreenTailHistory { + private readonly sessions = new Map() + + constructor(private readonly now: () => number = Date.now) {} + + record(sessionId: string, screenText: string): void { + const content = tailLines(sanitizeScreenText(screenText), SCREEN_TAIL_LINES) + if (!content) return + const hash = hashText(content) + let tail = this.sessions.get(sessionId) + if (!tail) { + tail = { samples: [], nextSeq: 0, lastHash: null } + this.sessions.set(sessionId, tail) + } + if (hash === tail.lastHash) return + const ts = this.now() + const seq = tail.nextSeq++ + tail.samples.push({ + id: `${seq.toString(36)}-${hash}`, + seq, + ts, + tsIso: new Date(ts).toISOString(), + hash, + lineCount: content.split('\n').length, + content, + }) + tail.lastHash = hash + if (tail.samples.length > SCREEN_MAX_SAMPLES) { + tail.samples.splice(0, tail.samples.length - SCREEN_MAX_SAMPLES) + } + } + + samples(sessionId: string): ScreenTailSample[] { + return [...(this.sessions.get(sessionId)?.samples ?? [])] + } + + /** Per-session arrays are bounded, but a map of closed sessions is not. */ + forget(sessionId: string): void { + this.sessions.delete(sessionId) + } +} diff --git a/src/shared/types/providerConfig.ts b/src/shared/types/providerConfig.ts index dd7847c9c..b2c95e540 100644 --- a/src/shared/types/providerConfig.ts +++ b/src/shared/types/providerConfig.ts @@ -362,8 +362,8 @@ export type MainProviderConfig = { * The io bag deliberately passes the AgentSession plus a bound * write-with-liveness function rather than the SessionManager: * providers must not depend on the manager (dependency arrow), and - * the typed optionals they need (awaitReadyForPrompt / - * awaitPastePlaceholder) live on AgentSession since phase 2a. + * the typed optional they need (awaitReadyForPrompt) lives on + * AgentSession since phase 2a. */ /** * Optional provider-owned history source, for providers whose durable @@ -409,8 +409,8 @@ export type MainProviderConfig = { * The io bag deliberately passes the AgentSession plus a bound * write-with-liveness function rather than the SessionManager: * providers must not depend on the manager (dependency arrow), and - * the typed optionals they need (awaitReadyForPrompt / - * awaitPastePlaceholder) live on AgentSession since phase 2a. + * the typed optional they need (awaitReadyForPrompt) lives on + * AgentSession since phase 2a. */ deliverPrompt: (io: PromptDeliveryIo) => Promise } diff --git a/src/shared/types/session.ts b/src/shared/types/session.ts index 3d400370f..2264ac2af 100644 --- a/src/shared/types/session.ts +++ b/src/shared/types/session.ts @@ -521,7 +521,7 @@ export interface AgentSessionEmitter { * * Optional methods encode capabilities that are legitimately * provider-specific today. The manager already duck-types them - * (sessionManager awaitClaudePastePlaceholder / resolveCondition / + * (sessionManager resolveCondition / * awaitCodexReadyForPrompt) — declaring them optional here just makes * that duck-typing type-checked. Phase 2c generalizes these into a * `promptDelivery` capability shape; the individual optional methods @@ -584,16 +584,6 @@ export interface AgentSession extends AgentSessionEmitter { * not one. Never throws. */ jumpToLatest?(): Promise<{ ok: true } | { ok: false; reason: string }> - /** Optional (Claude today): wait for the bracketed-paste placeholder - * to appear before firing Enter. See sessionManager.ts:952. */ - awaitPastePlaceholder?( - opts?: { timeoutMs?: number; pollIntervalMs?: number }, - ): Promise< - | { kind: 'appeared'; waitedMs: number } - | { kind: 'timeout' } - | { kind: 'no-headless' } - > - /** Optional (Claude today): a synchronous read of the live plain-text TUI * screen (headless snapshotPlain). The prompt-delivery path polls this to * detect the paste-commit *transition* — a NEW `[Pasted text #N]`