Repository navigation
feat(extension): debugger-free page control so Browser Hand works in Search - #33
autohub-claude[bot] wants to merge 1 commit into
Conversation
…Search
Problem: every page operation went through chrome.debugger. Search (macOS,
WebKit-based, Chrome-extension compatible) stubs chrome.debugger.attach/detach/
sendCommand to refuse with "debugger.attach isn't available in Search", so
doctor reported path_b_required and named-tab creation, snapshot, click, fill,
evaluate and screenshot all failed, although the relay and tab list worked.
Sites also increasingly block sessions with a debugger attached.
Fix:
- Debugger-free transport is now the default. Page.navigate runs through
chrome.tabs.update (waits for load, reports loadComplete), Runtime.evaluate
through chrome.scripting MAIN world, screenshots through captureVisibleTab.
Snapshot/click/type/fill all ride on evaluate, so they work unchanged.
- Auto-detect a stubbed or missing chrome.debugger (probe attach on tabId -1;
match "isn't available") and never attempt CDP there, even in debugger mode.
- Strict-CSP pages (no unsafe-eval) fall back to chrome.userScripts.execute,
which injects script text instead of eval'ing it; the manifest now declares
userScripts. Without it the error says how to turn it on.
- Tab bootstrap: tab grouping is cosmetic, so a missing chrome.tabGroups no
longer fails Target.createTarget or group requests.
- Popup "Page control" selector opts into the CDP transport; doctor reports
transport {mode, effective, debuggerAvailable}.
- CLI skips waiting for Page.loadEventFired when the navigate already settled.
Expected improvement: doctor reaches tab_bootstrap_works in Search and the full
operation set works there, with no debugging banner for sites to detect.
Known limits without the debugger: screenshot --full-page is viewport only, and
CSP-strict pages need user scripts allowed.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
autohub-claude
Bot
added
babysit:active
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3603bf602b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
| }, 150); | ||
| } | ||
| return { frameId, loaderId, ...(loaded ? { loadComplete: true } : {}) }; |
There was a problem hiding this comment.
In the default scripting transport, a navigation that takes longer than 20 seconds exits the polling loop without loadComplete or errorText, and no subsequent completion event is emitted: the synthetic Page.loadEventFired is scheduled only inside if (loaded), while the background tab-update listener only re-registers the tab. Consequently, runCurrentOperation waits another full event timeout (30 seconds by default), even if the page finishes loading immediately afterward, then reports success even if it never finished. Return an explicit timeout error or continue observing the tab so a later completion can resolve the CLI's wait.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed against the code, and intentionally not changed in this PR because the fix needs a decision the PR does not specify.
What happens today: when navigateViaTabs reaches its 20s poll deadline with loaded === false, it returns no loadComplete and no errorText and emits no Page.loadEventFired. The CLI then sees navigationSettled() as false and waits the full event timeout before continuing. A page that finishes at second 21 therefore costs up to the extra wait instead of resolving on completion. (Reporting success after that timeout is pre-existing CLI behavior: tool.js already does waitForEvent("Page.loadEventFired", ...).catch(() => null) on the debugger transport too. What is new here is only that the scripting transport never emits the late event.)
No test or documented invariant covers the >20s path, so this is an unmade policy choice rather than a breach of an existing contract.
Decision needed (owner): for a navigation still loading after 20s in the scripting transport, should it
- (A) keep observing the tab and emit the synthetic
Page.loadEventFiredwhen its status reachescomplete(parity with CDP; no new failure mode), or - (B) return an explicit timeout
errorText? This is a behavior change: for a freshly created tabopencloses the target on anyerrorText(tool.js, thenavError && createdTargetbranch), so slow-loading pages would be torn down.
Leaving this thread open until that is decided.
autohub-claude
Bot
added
babysit:active
|
Babysit stopped —
Verified head: |
autohub-claude
Bot
added
babysit:blocked
Summary
Every Browser Hand page operation went through
chrome.debugger. The "Search" browser (macOS, WebKit-based, Chrome-extension compatible) stubschrome.debugger.attach/detach/sendCommandto refuse withdebugger.attach isn't available in Search, sodoctorreportedpath_b_requiredand named-tab creation, snapshot, click, fill, evaluate and screenshot all failed even though the relay and tab list worked. Sites also increasingly block sessions that have a debugger attached.This makes a debugger-free transport the default and auto-detects a stubbed or missing
chrome.debugger, so no debugging banner is shown and nothing depends on the debugging port.Changes
extension/services/TabManager.ts,CDPRouter.ts,StateManager.ts):Page.navigate→chrome.tabs.update(waits for load, reportsloadComplete);Runtime.evaluate→chrome.scriptingin the MAIN world; screenshots →captureVisibleTab. Snapshot, click, type and fill all ride on evaluate, so they work unchanged.chrome.debugger.attach({tabId: -1})and treats "isn't available" as a stub; CDP is never attempted there, even if debugger mode is selected.unsafe-eval) fall back tochrome.userScripts.execute. The manifest now declaresuserScripts; without it the error says how to enable it.chrome.tabGroupsno longer failsTarget.createTargetor group requests.doctorreportstransport {mode, effective, debuggerAvailable, debuggerUnavailableReason}; the CLI skips waiting forPage.loadEventFiredwhen the navigate already settled (cli-js/src/tool.js).skills/browser-hand/SKILL.md(new "Page control" section),references/troubleshooting.md(path_b_required),README.md.Test plan
npm run build:relay— cleancd extension && npx vitest run) — 71/71, includes newscripting-transport.test.tscli-jstests (npm testincli-js/) — 162/162, includes newnavigation-settled.test.jsnpm run build:extension— builds;dist/chrome-mv3/manifest.jsondeclaresuserScriptsnpm run doctoragainst Search with this build — not run (see below)daemon/cli) — not applicable, untouchedNotes for reviewers
Not verified inside Search itself. The implementing run could not reload the extension in the real Search app (no CLI reload; driving its Settings UI was unsafe). Live checks ran in a disposable Chrome for Testing loading this build, with a copy of Search's
chrome.debuggerstub prepended (attachrejects withdebugger.attach isn't available in Search,getTargetsreturns[]) andtabGroups/userScriptsremoved. On that surface:doctor→tab_bootstrap_workswithtransport.effective: scripting;open --page-namecreated then reused;snapshot/fill/type/click/evaluate/gotosucceeded on gym 01 withwindow.__oracle().ok === true;screenshotproduced a real PNG; shadow-DOM (gym 08) and iframe (gym 09) snapshots listed all fields; the debugger opt-in path still passes in stock Chrome. Those live results come from the implementing run's report; the unit tests and builds above were re-run independently before opening this PR.Unverified assumptions in Search's WebKit engine:
tabs.captureVisibleTab(not referenced in Search's shim, so it must be native; screenshots depend on it),scripting.executeScriptin the MAIN world, anduserScripts.execute.Verify in Search:
npm install && cd extension && npm run buildin the main checkout (Search installs fromextension/dist/chrome-mv3).userScriptspermission if prompted.npm run doctor→ expecttab_bootstrap_workswithtransport.effective: scripting.open,snapshot,screenshotagainstextension/challenges/01-hello-form.html. Ifscreenshotfails,captureVisibleTabis the first suspect.Review focus
userScriptsis now declared in the manifest; existing installs may be asked to re-approve. On stock Chrome, CSP-strict pages also need "Allow User Scripts" on.captureVisibleTabpath inCDPRouter.captureScreenshot, which activates the tab inside its window and restores the previous one afterwards (nochrome.windows.update, no OS focus). That code predates this PR; it is now the default screenshot path in scripting mode rather than a fallback, so it runs far more often.screenshot --full-pagecaptures the viewport only; CSP-strict pages needuserScripts;goto's responsetarget.titlecan show the previous page's title.npx tsc --noEmitinextension/reports 3 errors, all in the untouched__tests__/CDPRouter.test.ts(loosely typedchrome.tabGroupsmocks); none in files this PR changes.daemon/clijobs, so the extension andcli-jschecks above are local-only.Implementation harness: Claude Code (Sonnet 5.5) via the autohub-claude GitHub App.
🤖 Generated with Claude Code