diff --git a/.project-docs/30-worklog/tasks/20260827-x01-crash-cleanup-7d4b1e92.md b/.project-docs/30-worklog/tasks/20260827-x01-crash-cleanup-7d4b1e92.md new file mode 100644 index 00000000..14fc61e2 --- /dev/null +++ b/.project-docs/30-worklog/tasks/20260827-x01-crash-cleanup-7d4b1e92.md @@ -0,0 +1,77 @@ +# Task: Remediate X-01 crashed CDP cleanup + +## Identity + +- Task ID: 20260827-x01-crash-cleanup-7d4b1e92 +- Mode: Feature +- Branch: codex/20260827-x01-crash-cleanup-7d4b1e92-x01-crash-cleanup-7d4b1e92 +- Worktree: D:\Datas\OthersProjects\makelore-x01-crash-cleanup-7d4b1e92 +- Base commit: 23c49aa3d28730be657387cf9c061fb3a3bb53a8 +- Owner: codex +- Status: Ready for Integration + +## Scope + +- Bound best-effort preview-data cleanup CDP commands in + `electron/agent-browser/module.ts`, preserving normal removal, child release, + and auto-attach ordering. +- Add the focused crashed-renderer/never-settling-removal regression in + `tests/unit/agent-browser-core.test.ts`. +- Own only this task record and the two files above; do not modify the + coordinator, open a PR, or perform live acceptance. + +## Intent And Constraints + +- X-01 packaged evidence showed a crashed renderer can leave + `Page.removeScriptToEvaluateOnNewDocument` pending forever, blocking the next + public data-enabled `open` during cleanup. +- Race each cleanup command against the existing bounded best-effort cleanup + convention (1,000 ms), swallow command failure/timeout, and clear timers. +- Preserve successful cleanup ordering and the existing preview token + invalidation semantics; avoid a broad CDP refactor. + +## Planning Gate + +- Result: Passed. +- Evidence: concurrent task gate started this isolated feature task from exact + base `23c49aa3d28730be657387cf9c061fb3a3bb53a8`; current task record, + project-memory entry documents, coordinator scope, lifecycle/X-01 pointers, + and relevant Agent Browser seams were read; no conflicting owned path was + found. + +## Plan + +1. Add a narrow bounded debugger-cleanup helper and apply it to preview script + removal, child waiting-target release, and cleanup auto-attach restoration. +2. Prove a never-settling removal response cannot block crash invalidation and + the next data-enabled open. +3. Run focused tests, typecheck, scoped lint, project-docs gates, and commit + once from the exact base. + +## Outcome + +- Added a 1,000 ms timer-bounded best-effort debugger command helper and used it + for preview script removal, document-child waiting-target release, and + cleanup `Target.setAutoAttach`. Normal successful cleanup retains its prior + ordering. A never-settling script-removal regression now proves renderer crash + invalidation does not block the next data-enabled open. + +## Verification + +- PASS: `corepack pnpm vitest run tests/unit/agent-browser-core.test.ts` (78/78). +- PASS: `corepack pnpm run typecheck`. +- PASS: `corepack pnpm exec eslint electron/agent-browser/module.ts tests/unit/agent-browser-core.test.ts`. +- PASS: `git diff --check`. +- PASS: `check_doc_drift.py --task-id 20260827-x01-crash-cleanup-7d4b1e92`. +- The isolated worktree required `corepack pnpm install --frozen-lockfile + --offline`; it changed no tracked dependency files. + +## Follow-ups + +- No live/E2E run was requested for this bounded X-01 remediation; coordinator + should rerun live acceptance after integrating the commit. + +## Promotion Candidates + +- None; this is a feature-task implementation with no canonical project-memory + change. diff --git a/electron/agent-browser/module.ts b/electron/agent-browser/module.ts index fc028cb4..6da3d036 100644 --- a/electron/agent-browser/module.ts +++ b/electron/agent-browser/module.ts @@ -36,6 +36,7 @@ const DEFAULT_CDP_TIMEOUT_MS = 10_000; const MAX_CDP_TIMEOUT_MS = 30_000; const OPEN_TIMEOUT_MS = 30_000; const RENDERER_PRIME_URL = 'about:blank'; +const PREVIEW_CLEANUP_TIMEOUT_MS = 1_000; const PUBLISH_PREFLIGHT_TIMEOUT_MS = 30_000; const PUBLISH_PREFLIGHT_SETTLE_MS = 500; const PUBLISH_PREFLIGHT_VIEWPORTS = [ @@ -927,21 +928,17 @@ export class AgentBrowserModule { record.previewDataRequested = false; const scripts = binding.scripts.splice(0); await Promise.all(scripts.map(async ({ identifier, sessionRef }) => { - try { - await record.view.webContents.debugger.sendCommand( + await bestEffortDebuggerCommand(() => record.view.webContents.debugger.sendCommand( 'Page.removeScriptToEvaluateOnNewDocument', { identifier }, sessionRef, - ); - } catch { - // The debugger or child target may already be detached. - } + )); if (sessionRef) { - await record.view.webContents.debugger.sendCommand( + await bestEffortDebuggerCommand(() => record.view.webContents.debugger.sendCommand( 'Runtime.runIfWaitingForDebugger', undefined, sessionRef, - ).catch(() => undefined); + )); } })); if ( @@ -949,11 +946,11 @@ export class AgentBrowserModule { && !record.view.webContents.isDestroyed() && record.view.webContents.debugger.isAttached() ) { - await record.view.webContents.debugger.sendCommand('Target.setAutoAttach', { + await bestEffortDebuggerCommand(() => record.view.webContents.debugger.sendCommand('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: false, flatten: true, - }).catch(() => undefined); + })); } if (record.previewData === binding) record.previewData = undefined; })(); @@ -1994,6 +1991,20 @@ function delay(milliseconds: number): Promise { }); } +async function bestEffortDebuggerCommand(operation: () => Promise): Promise { + let timer: ReturnType | undefined; + try { + await Promise.race([ + Promise.resolve().then(operation).catch(() => undefined), + new Promise((resolvePromise) => { + timer = setTimeout(resolvePromise, PREVIEW_CLEANUP_TIMEOUT_MS); + }), + ]); + } finally { + if (timer) clearTimeout(timer); + } +} + function isTimeoutError(error: unknown): boolean { return error instanceof Error && error.message === 'PUBLISH_PREFLIGHT_TIMEOUT'; } diff --git a/tests/unit/agent-browser-core.test.ts b/tests/unit/agent-browser-core.test.ts index 896f9630..f67ac474 100644 --- a/tests/unit/agent-browser-core.test.ts +++ b/tests/unit/agent-browser-core.test.ts @@ -924,6 +924,54 @@ describe('AgentBrowserModule', () => { expect(adapter.views).toHaveLength(2); }); + it('bounds crashed preview cleanup so a subsequent data-enabled open completes', async () => { + vi.useFakeTimers(); + try { + const adapter = new FakeAdapter(); + const preview = new FakePreviewDataSession(); + adapter.onCreate = (view) => { + view.webContents.debugger.responders.set('Page.addScriptToEvaluateOnNewDocument', async () => ({ + identifier: 'script-root', + })); + }; + const module = new AgentBrowserModule(adapter, { previewDataSession: preview }); + + await module.open({ + projectId: 'clock', + projectPath, + url: 'http://127.0.0.1:4173/', + injectProjectData: true, + }); + const crashedView = adapter.views[0]; + crashedView.webContents.debugger.responders.set( + 'Page.removeScriptToEvaluateOnNewDocument', + async () => await new Promise(() => undefined), + ); + + crashedView.webContents.emit('render-process-gone', {}, { reason: 'crashed' }); + expect(preview.invalidations).toContain('browser_crashed'); + expect(preview.getInjectionValue()).toBeNull(); + + const reopening = module.open({ + projectId: 'clock', + projectPath, + url: 'http://127.0.0.1:4173/', + injectProjectData: true, + }); + await vi.advanceTimersByTimeAsync(1_000); + + await expect(reopening).resolves.toMatchObject({ + state: 'attached', + generation: 2, + }); + expect(preview.invalidations).toContain('browser_crashed'); + expect(preview.opens).toHaveLength(2); + expect(adapter.views).toHaveLength(2); + } finally { + vi.useRealTimers(); + } + }); + it('replaces a data-enabled browser when a later ordinary open omits the opt-in', async () => { const adapter = new FakeAdapter(); const preview = new FakePreviewDataSession();