Merge branch 'main' into codex/20260906-project-setup-ux-90fe6cf2-project-setup-ux
This commit is contained in:
@@ -0,0 +1,230 @@
|
||||
# Task: Diagnose agent_browser call failures
|
||||
|
||||
## Identity
|
||||
|
||||
- Task ID: 20260906-agent-browser-failure-a7c91e4d
|
||||
- Mode: Feature
|
||||
- Branch: main
|
||||
- Worktree: D:\Datas\OthersProjects\makelore
|
||||
- Base commit: 5c61110f465cc4172f712ad435c06041c0aed1ec
|
||||
- Owner: developer
|
||||
- Status: Ready for Integration
|
||||
|
||||
## Scope
|
||||
|
||||
- Preserve the completed diagnosis of the parent Pi `agent_browser` failure and use
|
||||
its deterministic reproduction as the implementation baseline.
|
||||
- Restore a lightweight Coding-owned shared browser surface and route Pi opens
|
||||
through a bounded Main/Renderer presentation handshake before inspection actions
|
||||
continue.
|
||||
- Preserve zero browser resources and zero browser polling while the surface is
|
||||
closed; scope diagnostics to explicit Agent or user demand and keep event buffers
|
||||
bounded.
|
||||
- Close the browser on panel dismissal, failed presentation, module/project exit,
|
||||
window hide, or Renderer loss, and preserve structured `AgentBrowserFault` details
|
||||
across the Pi bridge.
|
||||
- Add focused regression coverage plus typecheck, lint, unit, Electron UI, build,
|
||||
and deterministic performance-boundary verification in proportion to the change.
|
||||
|
||||
## Intent And Constraints
|
||||
|
||||
- Preserve ADR-006: Pi `0.84.2` remains the sole Coding runtime; `agent_browser`
|
||||
crosses the generated Makelore extension into the Main-owned browser service.
|
||||
- Keep `agent_browser`, selected-model `makelore_web_search`, and the unrelated
|
||||
release-preflight `electron/agent-browser` module distinct.
|
||||
- Reproduce the exact failure before ranking or testing hypotheses. Redact credentials,
|
||||
headers, prompts, tool arguments/results, account/session identities, and user paths
|
||||
from reported evidence.
|
||||
- Do not restart, abort, recover, or replay a live Conversation, and do not invoke a
|
||||
paid Provider while implementing or verifying the repair.
|
||||
- Establish a red-capable performance baseline before attributing cost. Prefer
|
||||
profiler/timing/resource measurements over broad logging, and distinguish the
|
||||
persistent interactive browser from the short-lived release-preflight browser.
|
||||
- Implement only the smallest replacement surface needed by the current Pi/Main
|
||||
architecture; do not restore OpenCode state, speculative compatibility layers, or
|
||||
the pre-optimization polling behavior.
|
||||
- The active legacy Web Search coordinator owns a separate old-base native-search
|
||||
scope and does not semantically overlap this diagnosis.
|
||||
|
||||
## Outcome
|
||||
|
||||
- Restored a lightweight Coding right-side development-browser panel with an
|
||||
explicit header toggle. An Agent `open` event expands the same panel
|
||||
automatically; manual opening alone creates no `WebContentsView`.
|
||||
- Added an event-driven Main/Pi presentation handshake. Pi opens the browser
|
||||
hidden, requests the Renderer surface, and waits at most five seconds for
|
||||
visible bounds for the same browser generation. Timeout, close, crash,
|
||||
debugger loss, or generation replacement rejects the wait; failed
|
||||
presentation destroys the browser instead of retaining an unusable renderer.
|
||||
- Kept project paths Main-owned for the restored Renderer call sites. The panel
|
||||
sends the active project id under the existing Renderer capability, and the
|
||||
Host route resolves and revalidates the active real path before each operation.
|
||||
- Replaced the single diagnostics toggle with owner leases. Agent diagnostics
|
||||
stay enabled only for the current run, Renderer diagnostics only while the
|
||||
Console/Network drawer is expanded, and the domains plus buffers are released
|
||||
after the last owner exits. Event rendering remains capped at 500 records and
|
||||
reads use a five-second long poll with bounded drain backoff.
|
||||
- Closed the browser during idle `background_sleep` in addition to the existing
|
||||
panel, project, window, Renderer, and application teardown paths. The panel
|
||||
performs no interval polling and installs its document observer and resize
|
||||
observer only while open; it does a single state refresh when the app regains
|
||||
focus after a background close.
|
||||
- Preserved `AgentBrowserFault` code, retryability, generation, and outcome across
|
||||
the Pi extension bridge, so callers receive `VIEWPORT_NOT_READY` and other
|
||||
actionable faults instead of the generic `Bridge request failed` symptom.
|
||||
- Confirmed primary cause: the Pi product-tool path creates the Main-owned browser
|
||||
without a Renderer presentation handshake. `PiAgentBrowserTool.open()` calls
|
||||
`AgentBrowserModule.open()` directly with `visible: false` and no bounds. The
|
||||
resulting live snapshot is therefore `state: attached`, `visible: false`, and
|
||||
`bounds: null`.
|
||||
- `navigate`, `send_cdp`, `read_events`, and `read_payload` enter
|
||||
`requirePresented()` (directly or through `requireAttached()`), which rejects that
|
||||
state with `VIEWPORT_NOT_READY`. This is intentional browser-module behavior: the
|
||||
shared development browser pauses agent debugging while its viewport is hidden.
|
||||
- The required presentation owner no longer exists in the current Coding Renderer.
|
||||
Commit `5a275b9` removed `src/pages/Chat/AgentBrowserPanel.tsx` and its 619-line
|
||||
unit test file as part of the legacy OpenCode removal. That panel was the only code
|
||||
which measured the viewport and called `presentAgentBrowser()` with visible bounds.
|
||||
Current source has no caller of `presentAgentBrowser()` and no consumer of
|
||||
`agent-browser:show`; the installed Renderer bundle likewise contains that event
|
||||
name only as the key/value pair in the generic event map.
|
||||
- The Pi path also bypasses the Host API route that emits `agent-browser:show` after
|
||||
an open. Therefore merely retaining the route event is insufficient: the Pi
|
||||
product-tool open requires the same presentation notification/coordinator, and the
|
||||
current Coding Renderer requires a replacement shared-browser surface.
|
||||
- Confirmed secondary cause of the unhelpful symptom: the extension bridge receives
|
||||
an HTTP 400 from `PiExtensionHost`, whose broad catch preserves only
|
||||
`DevicePackageError`; it replaces `AgentBrowserFault` (including
|
||||
`VIEWPORT_NOT_READY`) with the literal `Bridge request failed`. The generated Pi
|
||||
extension correctly reports the text it receives, so the loss occurs in Main.
|
||||
- The direct Pi browser path was introduced by commit `13ab383` already using
|
||||
`visible: false`. While the old panel still existed, a user-presented viewport
|
||||
could satisfy the gate; after `5a275b9` removed that panel, current supported UI
|
||||
has no way to do so, making the failure deterministic for inspection actions.
|
||||
- Historical performance concern is confirmed, but the deletion itself was not a
|
||||
performance fix. Commit `927e133` first optimized the then-live browser surface:
|
||||
it removed the 2.5-second state interval, replaced the 750 ms event interval with
|
||||
a diagnostics-only 5-second long poll, limited the rendered event tail to 500,
|
||||
activated the document-wide MutationObserver only while the panel was open,
|
||||
disabled Runtime/Log/Network/Page CDP domains by default, cleared their buffers
|
||||
when diagnostics closed, and destroyed the WebContentsView when the panel closed.
|
||||
Five days later `5a275b9` deleted the already-optimized panel as part of the legacy
|
||||
OpenCode hard cutover; its commit message and diff do not identify browser
|
||||
performance as the deletion reason.
|
||||
- A deterministic historical work-budget comparison reproduced the old amplification:
|
||||
a collapsed-but-active browser retained one Chromium renderer, dispatched 24
|
||||
state requests per minute, kept the whole-document observer active, and captured
|
||||
four CDP event domains continuously. An open panel dispatched another 80 event
|
||||
reads per minute even when its diagnostics drawer was collapsed. The `927e133`
|
||||
version passed the corresponding zero-background-work source assertions.
|
||||
- Current source retains the Main-side diagnostics gating and teardown, but has a
|
||||
lifecycle regression around the Pi path. Pi `open` can create a hidden renderer
|
||||
indefinitely; it emits no presentation event, and `codingProducts.sleep()` stops
|
||||
Pi workers/the Agent Server without closing the browser. An executable red test
|
||||
expected browser close during `background_sleep` and observed zero calls. Window
|
||||
hide, renderer exit/navigation, and project deactivation do close it, but merely
|
||||
leaving the Programming module does not.
|
||||
- No preserved profiler capture or hardware-normalized CPU/memory measurement was
|
||||
found for the historical browser. Therefore the verified conclusions are about
|
||||
retained resources and deterministic background work, not a fabricated MB, CPU,
|
||||
startup-time, or frame-time improvement number.
|
||||
- Falsified alternatives:
|
||||
- Tool registration/authentication is not the cause: live `open` and `status`
|
||||
calls cross the extension/Main seam successfully.
|
||||
- General request-schema drift is not the cause: a minimal `read_events` request
|
||||
has the exact supported shape. One historical malformed `read_payload` call is
|
||||
isolated and cannot explain the other failures.
|
||||
- Installed/source component drift is not the cause of this symptom: the running
|
||||
installed package contains the same hidden-open, viewport gate, and bridge mask.
|
||||
- A GPU-process warning is present in the application log, but no evidence links
|
||||
it to these failures; the browser reaches `attached` and the deterministic
|
||||
viewport rejection occurs before actionable CDP work.
|
||||
- The three adopted pre-existing task records remain unchanged. Product changes
|
||||
are limited to the current Agent Browser presentation, diagnostics, and
|
||||
lifecycle path; the isolated release-preflight browsers remain unchanged.
|
||||
|
||||
## Verification
|
||||
|
||||
- Red baseline: the four focused implementation suites initially produced six
|
||||
expected failures and 106 passes for the missing presentation wait, diagnostic
|
||||
ownership, structured bridge fault, and background close behaviors.
|
||||
- Focused post-change verification passed across browser core/routes/panel, Pi
|
||||
product tools/extension host, Coding background sleep, and Coding feature UI;
|
||||
the final panel suite has 3 passing tests, including zero Host API calls while
|
||||
closed and one-shot foreground resynchronization.
|
||||
- `pnpm run typecheck`: passed.
|
||||
- `pnpm run lint:check`: passed with zero errors and five pre-existing warnings in
|
||||
`src/pages/Home/index.tsx` and `src/pages/Makelore/index.tsx`; no changed file
|
||||
produced a warning.
|
||||
- `pnpm test`: 1,913 tests passed and 2 existing tests were skipped across the two
|
||||
repository test phases.
|
||||
- `node ./node_modules/@playwright/test/cli.js test
|
||||
tests/e2e/pi-coding-first-chat.spec.ts`: all 4 Electron E2E tests passed,
|
||||
including the restored browser toggle and diagnostics-off initial state.
|
||||
- `pnpm run build:vite`: passed for Renderer, Electron Main, Preload, and utility
|
||||
worker. Only existing Browserslist, mixed dynamic-import, and chunk-size
|
||||
advisories were emitted.
|
||||
- `pnpm run perf:budget`: passed. Initial JS gzip was 205,846 / 358,400 bytes,
|
||||
initial CSS gzip 24,377 / 30,720 bytes, fonts 1,729,900 / 2,097,152 bytes, and
|
||||
module media 241,362 / 1,048,576 bytes.
|
||||
- `git diff --check`: passed.
|
||||
- Deterministic captured-trace assertion was run twice against the latest local Pi
|
||||
session, without exposing prompts, arguments, results, identities, or paths. Both
|
||||
runs reproduced the same red condition: 3 successful opens and 14 subsequent
|
||||
`Bridge request failed` results.
|
||||
- Minimized captured sequence reproduced the failure as one successful `open`
|
||||
followed by one failed `read_events`. Across the inspected session, 20
|
||||
`agent_browser` results comprised 6 successes (`open`/`status`) and 14 failures
|
||||
(`read_events`, `send_cdp`, `navigate`, and one malformed `read_payload`).
|
||||
- `pnpm exec vitest run tests/unit/pi-extension-bundle.test.ts
|
||||
tests/unit/pi-product-tools.test.ts tests/unit/agent-browser-routes.test.ts
|
||||
--maxWorkers=1` with the repository-pinned pnpm 10.33.4: 3 files, 35 tests passed.
|
||||
These tests confirm registration/bridge schema/route forwarding but mock the
|
||||
browser module and do not exercise the broken cross-layer sequence.
|
||||
- `pnpm exec vitest run tests/unit/agent-browser-core.test.ts -t "keeps a view
|
||||
hidden until open/present receives current bounds" --maxWorkers=1`: 1 passed,
|
||||
77 skipped. It confirms that open without bounds yields `visible: false`,
|
||||
`bounds: null`, and that `sendCdp`/`readEvents` then reject with
|
||||
`VIEWPORT_NOT_READY` until a visible presentation with bounds occurs.
|
||||
- Read-only inspection of the running installed package confirmed all load-bearing
|
||||
code literals and paths: Pi open is hidden, `requirePresented()` emits the
|
||||
viewport fault, Main masks it, and the Renderer bundle has no show-event consumer.
|
||||
- Git history/blame confirmed the Pi path originated in `13ab383` and the presentation
|
||||
surface plus its tests were removed in `5a275b9` without a Coding replacement.
|
||||
- Historical zero-idle budget check: pre-`927e133` intentionally failed with one
|
||||
retained renderer, 24 collapsed-state requests/minute, 80 open-panel event
|
||||
requests/minute, an always-on DOM observer, and always-on diagnostic CDP domains;
|
||||
the `927e133` source passed all seven inverse assertions.
|
||||
- Current lifecycle red test (`debug-agent-browser-background-release.test.ts`,
|
||||
removed immediately after diagnosis) failed as expected: `background_sleep`
|
||||
invoked the browser `close` spy 0 times instead of 1.
|
||||
- Focused current-source verification passed: 2 files and 4 tests covering
|
||||
diagnostics-off first-load events, hidden-until-present behavior, full close
|
||||
teardown, and agent-only hidden open. The temporary diagnostic test was removed;
|
||||
`git status` shows no product-source residue from the investigation.
|
||||
|
||||
## Follow-ups
|
||||
|
||||
- No required implementation follow-up remains.
|
||||
- If hardware-normalized CPU, memory, or frame-time numbers are needed, capture a
|
||||
controlled before/after profile through the existing `app:performance` snapshot.
|
||||
The current evidence deliberately claims deterministic resource/work bounds and
|
||||
bundle-budget compliance, not an invented runtime percentage.
|
||||
|
||||
## Promotion Candidates
|
||||
|
||||
- Target: `.project-docs/30-worklog/current-state.md`
|
||||
- Summary: Pi `agent_browser` now has a Coding-owned visible-surface handshake;
|
||||
presentation failure tears down the browser and structured browser faults cross
|
||||
the Pi bridge unchanged.
|
||||
- Evidence: focused browser/Pi/bridge tests, Electron E2E, and production build.
|
||||
- Future impact: keep the visibility gate and presentation coordinator together;
|
||||
do not reintroduce hidden actionable CDP access.
|
||||
- Target: `.project-docs/30-worklog/current-state.md`
|
||||
- Summary: the restored Agent Browser preserves the post-`927e133` zero-idle
|
||||
lifecycle: no closed-state polling/observers/browser process, owner-scoped
|
||||
diagnostics, bounded event retention, and idle background teardown.
|
||||
- Evidence: panel/core/background regression tests, full unit suite, E2E, and the
|
||||
passing bundle performance budget.
|
||||
- Future impact: future browser UI changes must preserve these deterministic work
|
||||
bounds and keep release-preflight browsers isolated from the interactive one.
|
||||
Reference in New Issue
Block a user