fix: survive Chrome's consent prompt instead of wedging on it - #18
Merged
Conversation
…ompt Ensure checked for a running daemon, found none, and spawned one — with nothing making that path single-file. Several chrome-cdp processes starting at once therefore each found nothing and each spawned a daemon, and every spawned daemon attaches to Chrome, raising its own "Allow remote debugging?" prompt. That prompt is browser-MODAL, so stacked prompts are not a slower version of one prompt: the visible dialog need not be the one holding input, and the browser looks frozen with no button that responds. It happened to a real user's session during this work — several review agents each ran the binary, and their attaches raced. The unlink was the other half. Outside a lock, a late caller's os.Remove(sockPath) can delete a socket a sibling daemon has just bound, orphaning a live daemon that no client can reach. An exclusive flock now covers spawn-and-wait, with a re-check inside it so callers that queued behind the holder find the daemon it started instead of duplicating it. The wait is deliberately unbounded: the holder may be waiting out a prompt the user has not clicked, and blocking behind that is correct — spawning our own would add to the pile. The regression test runs eight concurrent Ensure calls against a fake spawn it can count. Without the lock: 8 daemons, so 8 prompts. With it: 1. The connect timeout message now says the prompt may be hiding behind the window and that Chrome accepts no other input until it is answered, since "did you click Allow" is not much help when the reason you have not is that you cannot see it. The skill says the same. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Written from a controlled reproduction of the wedge that froze a real browser during this work, which contradicted the first diagnosis. Stacked prompts are real and #17 fixes them, but a SINGLE unanswered prompt wedges Chrome just as thoroughly, so serialising the spawn is necessary and not sufficient. The mechanism, measured: while consent is pending, TCP connects immediately and the WebSocket upgrade HANGS — it never completes and never refuses. The client therefore has no error to classify, only silence, which is why the failure surfaces as an undifferentiated timeout. Three defects follow: the daemon gives up in ten seconds and abandons the prompt it raised, so a later click grants consent to a connection that no longer exists; ten seconds is not a human timescale for a browser-modal dialog that can sit behind the window; and `doctor` reports "attach ready" from the port file without ever probing, so the one command whose job is to answer "can I connect?" said yes while every connection was hanging. Also recorded because it cost time: /json/version returning 404 is NOT a consent signal — it returns 404 in this connection mode either way, and an early reading of it as pending-consent was wrong. The proposal is to wait for the consent rather than abandon it, classify the pending state distinctly, make doctor probe, and lead the docs with the launch flag that never prompts. The pending state is just a listener that accepts and stalls, so nearly all of it is testable with net.Listen and no browser — which matters, because reproducing this by hand wedged a real browser twice and must not become the regression test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
While Chrome holds its "Allow remote debugging?" consent prompt it does not refuse the connection: it accepts the TCP connect and then holds the WebSocket upgrade open, saying nothing. There is no error to classify, only silence — so a boolean "reachable" puts that silence in the same bucket as a refused port, and the tool cannot tell "nothing is listening" (fast, real failure) from "waiting for a human" (not a failure at all). This adds the observation the distinction needs: one upgrade, classified as refused / pending / ready, with a dial timeout, a threshold past which silence counts as the consent signature, and a total budget the same socket is held open across. Holding it matters — an answer that arrives after we hung up is an orphaned prompt, which is how clicking Allow came to grant consent to a connection that no longer existed. Nothing uses it yet. Note the deliberate absence of any /json/version check: that endpoint returns 404 on the chrome://inspect path whether or not consent has been granted, so detection built on it would be wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The daemon used to dial, hit a ~10s deadline while Chrome held its consent dialog, write a generic error and exit — leaving the modal on screen with nothing behind it. Clicking Allow then granted consent to a connection that no longer existed, and because the prompt is browser-modal the whole browser stayed frozen. Ten seconds is not a human timescale for a dialog that can sit behind the window. So the ladder now decides on the three-way probe state, and an open port with a hanging upgrade is its own rung: consent_pending, distinct from "enable the toggle". The daemon holds that upgrade open for consent_timeout (default 120s, flag > env > file), publishes a marker beside its socket the moment it starts waiting — it is detached, so a file is the only channel it has — and Ensure moves its own deadline when it sees one. A refused endpoint is untouched by all of this and still fails in milliseconds, which is the property that makes a two-minute wait safe to have. consent_pending is a new error.code on the EXISTING exit 3: callers branch on the code, and the number is contract. doctor now probes. It used to read DevToolsActivePort and report "Path B attach ready" without ever connecting, and during the reproduction it said ready while every connection was hanging — a diagnostic that reports readiness it never verified sends the user looking anywhere but the dialog. It prefers a running daemon (already holding a verified connection, and asking it raises no prompt), falls back to one upgrade after saying it is about to, and takes --no-probe for a user who wants a diagnosis and not a connection. Every failure message now leads with --remote-debugging-port, which never prompts, and offers chrome://inspect second with a note that it prompts on every fresh attach. Recommending the toggle first walked each new user straight into this. Tests are stub listeners that accept and stall — the manual reproduction wedged a real browser twice and must not be the regression test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every setup path in the docs sent the user to chrome://inspect first, which is the one route that raises a browser-modal consent prompt on every fresh attach — so the documentation itself walked each new reader into the failure. The flag goes first now, the toggle second with what it costs stated plainly. Also documents what the prompt actually does (modal to the whole browser, can hide behind the window, blocks all other input), the consent_pending code on exit 3, --consent-timeout and the consent_timeout config key, and doctor's three states plus the fact that it answers through a running daemon rather than opening a connection of its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An explicit --port names an http:// endpoint, and the upgrade probe would have handshaked against "/" on it — never a 101, so the healthiest possible setup (the launch flag this RFC now recommends to everyone) would classify as refused and fall through to "enable the toggle". Resolve it through /json/version first, which is the same lookup chromedp's remote allocator does, so the probe and the attach agree on the endpoint. That API locates a browser and can never classify one: it answers identically whether or not consent is pending, which is the trap this RFC records. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sanketsudake
changed the title
RFC-0013 item 3 is "doctor reports readiness it never verified". The fix removed the unverified claim at the port-file level and reintroduced it one level up: the daemon path emitted state:ready on `running: true` alone, and `running` came from dialling the Unix socket and closing it. Three claims stacked — TryConnect's, a swallowed StatusInfo error, and a __status that hardcoded "connected": true — so the trigger is ordinary. Start a daemon, quit Chrome: the chromedp connection is dead, Serve holds the listener for another 30 minutes, and doctor answers ok:true / ready / exit 0 while every subsequent command fails. VS-5 asks for the ready case to be established by a completed round trip, not by a file or a socket. __status now reports `connected` as the answer to the List it already made, Status propagates StatusInfo's error instead of discarding it, and doctor requires that positive evidence before claiming ready. The same payload was also a privacy leak. doctor blanket-copied the daemon's status map, which carried []target.Info — every open tab's title and full URL. SKILL.md makes `doctor --json` step 1 of every agent session, so each session pulled OAuth callbacks, reset tokens and internal hostnames into the transcript before a tab had even been selected. The status payload now carries a target_count, and doctor copies an allowlist rather than the map. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The probe expects one line of HTTP back. It read with an unbounded
bufio.Reader.ReadString('\n') and set no read deadline, so the only ceiling on
either was the caller's total wait — 120s in the daemon.
Anything on the machine that can bind the loopback debug port can therefore
answer with a stream that never contains a newline. Measured against a local
listener doing exactly that: 18 GB of heap in six seconds, ~3 GB/s, and the
daemon's full budget would reach hundreds of gigabytes.
An 8 KiB LimitReader ends that stream at the limit, where the missing newline
becomes an ordinary refusal. The read deadline is the same bound expressed in
time, so the goroutine cannot outlive the wait even if nobody closes the socket
— but reaching it is reported as SILENCE rather than as a failed handshake,
because classifying a deadline as "refused" would turn a Chrome still holding
its consent prompt into a dead endpoint.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ResolveWSURL declared "no endpoint" whenever /json/version did not answer 200. But RFC-0013's fourth observation is that on the chrome://inspect toggle path /json/version 404s regardless of consent state — the toggle exposes the WebSocket without the HTTP JSON API. So the lookup failing carried no information, and treating it as failure blinded the tool on the only path that raises the dialog. The result: `chrome-cdp --port 9222 <verb>` against a toggle-path Chrome holding an unanswered prompt classified the endpoint as refused, fell to InstructToggle, and told the user "Chrome is running but not debug-enabled" — about a Chrome that is debug-enabled and is showing them the consent dialog at that moment. They were sent to re-enable a setting that was already on. A failed lookup now falls back to the ws:// root of the same host:port. In the granted case that costs nothing (an endpoint that will not upgrade there is classified refused, which is where it already was); in the pending case the hang becomes visible, which is the one unambiguous consent signature there is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The value crossed four layers and three of them read it differently. chrome.Connect mapped <= 0 to DefaultConsentTimeout; daemon.Ensure took the zero literally; main forwarded CHROME_CDP_CONSENT_TIMEOUT to the daemon only when > 0. So with consent_timeout = "0s" (or a negative value) the daemon waited the full 120s while its client gave up at 10s and printed "still waiting ... after 0s" — which is the orphaned-prompt failure this parameter exists to prevent, restored through its zero value. Normalisation now happens where flag, environment and config file actually resolve: config.ResolveFrom/FromEnv for the file and the environment (including the daemon subprocess's own resolution), and App.connOpts for the flag, which is the one path config resolution never sees. Both call the same chrome.ClampConsentTimeout, so no two layers can drift. The range is clamped as well as the zero. Below one second the wait is not a wait; above ten minutes it stops being a timeout, because the daemon spawn lock is held for its duration and an inherited CHROME_CDP_CONSENT_TIMEOUT=8760h would have blocked every other invocation for a year. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
doctor called browser.FindPortFile("") and never consulted a.port, so
`doctor --port 9333` read the DevToolsActivePort file, probed whichever Chrome
that file happened to name, and reported THAT browser healthy. Every other verb
resolves --port ahead of the port file, which makes doctor the one command that
can answer a question about a different browser than the one asked about — and
the one command whose entire job is to be believed.
Endpoint resolution moves into browser.FindEndpoint, shared by chrome.Connect
and doctor so the command that diagnoses the connection and the command that
makes it cannot disagree about where Chrome is. doctor also resolves the
WebSocket URL before probing, which an http:// endpoint from --port requires
and which it previously never did.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Each of these ends with the user being told about a consent prompt that has nothing to do with what happened. A daemon that published its .pending marker and then died left Ensure with no exit but the deadline: ~130s of waiting for a process that no longer existed. spawnDaemon called Process.Release() and discarded the handle, so no liveness check was possible. It now keeps a handle and reaps the child in the background, which is both the reaping and the signal — kill(pid, 0) would not have worked, because a dead child is a zombie until it is waited for, and a zombie answers a liveness signal perfectly well. Child gone with no .err is an immediate daemon_error. lockSpawn blocked in complete silence. A second command run during a pending prompt hung for over two minutes with no output — the same "my tool has frozen and I do not know why" that US-2 exists to end, reached through the fix for US-2. It now tries the lock non-blocking first purely so contention can be named, then blocks as before. RunDaemon reported nothing for anything that failed AFTER the connect. The daemon is detached, so its stderr is never read: a failed net.Listen exited silently, and Ensure — seeing the pending marker and no .err — reported the consent prompt for a bind failure. The RFC's own darwin sun_path note makes that reachable. Bind failures now write the .err sidecar with a daemon_error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng on #17 made the FIRST attach single-file, and VS-7 has held since: never two prompts at once. US-5 — "at most one consent request" — did not. Behind the spawn lock each queued caller in turn cleared the previous verdict, spawned its own daemon and raised its own prompt at a browser that was already holding one. Eight concurrent commands against an unanswered dialog came to about seventeen minutes and eight sequential prompts: one at a time, which is not the same as one. A consent_pending verdict written by the previous holder is now inherited rather than re-derived, for five seconds. Queued callers are released within milliseconds of it being written, so that is long enough to drain a queue and short enough that a user who has just found the dialog and clicked Allow is not told to go looking for it again. Ensure also takes a context. It had none, and chrome.Connect discards its own by design, so --timeout stopped applying the moment a command needed a connection: a caller queued behind a holder sitting on an unanswered prompt inherited that wait with no way to say otherwise. The flock wait now runs on its own goroutine and hands the lock back if the caller has given up, rather than being abandoned holding it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The "browser-modal / BEHIND the window / accepts no other input / not a crash" paragraph was hand-written in five places: chrome.consentPendingMsg, chrome.connectFailMsg, the daemon's wait notice, Ensure's give-up message, and doctor's consent_pending state. It had already drifted — one copy said "behind" where the others shouted it, one said "blocks all other input" — and two test files asserted on a hardcoded substring list that one of those five copies would have failed. browser.ConsentPromptAdvice is now the one authored version, with each call site composing its own prefix, exactly as browser.EnableAdvice already does. The two assertion lists collapse to a single strings.Contains against the const, which is the part that matters: a list of substrings cannot tell five paragraphs apart, and that is how they drifted in the first place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Options.OnConsentPending was assigned in exactly one place — the daemon — and the --no-daemon connector passed a ConsentTimeout with no hook at all. So `--no-daemon` waited out the prompt in complete silence: up to two minutes with the browser frozen and nothing written to stderr. RFC-0013's US-2 asks to be told a consent prompt is pending WHILE it is pending, and this is the one path where nothing else can do it — there is no daemon to publish a .pending marker and no Ensure to read one. No test caught it, because every test that exercises the hook supplies its own. The options now come from directConnectOptions, which exists as a named function so the hook can be asserted on at all: cmd/chrome-cdp had no tests, which is part of why an unwired seam went unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
browser.Upgrade's doc comment states the governing model: a probe that connects, learns the answer and hangs up has spent the user's click on a connection nobody kept. ProbeWS does exactly that on every outcome including WSReady, so doctor's `ready` was falsified by the act of producing it — on the chrome://inspect path the next command is a fresh attach and prompts again. Of the two ways to make the comment and the code agree, doctor takes the honest-output one. Handing the live socket to the connection it just proved possible is the other, and doctor is the wrong place for it: it is a diagnostic, it was not asked to connect, and it has nothing to hand the socket to. So the ready verdict now says the probe's connection was closed, that the next command may prompt again, and that the daemon is how to be asked once per session — and ProbeWS's own comment says why it hangs up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
AwaitUpgrade was a clamp, two timers, two selects, a remainder computation and
a `rest <= 0` branch, with the WSPending exit written three times. One loop with
two independent timers is the same behaviour in a third of the lines — except
for the case the branch had and the loop does not.
When PendingAfter >= Total the remainder came out <= 0 and the function
returned WSPending having never looked at the answer channel again. That is
every doctor probe, because ProbeWS passes the same value for both; and
onPending runs in between, which is not instantaneous — the daemon's writes a
file. Anything the endpoint delivered during it was discarded unread and its
socket closed, so a completed handshake was reported as a pending consent
prompt. The loop consults the channel once more before giving up, and the read
deadline now sits just past the budget rather than exactly on it, so an answer
landing as the wait ends is still readable rather than being cut off by the
backstop meant to protect against a runaway goroutine.
The three positional durations become UpgradeTimings{PendingAfter, Total}:
they were easy to swap and the consequences are not symmetric. dialTimeout
leaves the signature entirely — two packages had independently declared the
same two seconds for the same loopback dial, and no caller has information that
would make a different value right.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
internal/browser's own package doc says it is "deliberately free of chromedp so it unit-tests without a live browser". That claim was true of a DevToolsActivePort parser and a decision table. It did not survive the probe: a TCP dialer, an HTTP client, a hand-rolled RFC 6455 handshake, a reader goroutine, and a live net.Conn handed across the package boundary for internal/chrome to close. The socket work moves to internal/chrome/probe.go, next to the connection it exists to serve and to the code that owns that net.Conn's lifetime. What stays behind is what the doc actually describes: WSState (the vocabulary a probe answers in), Probe, Action, DecideConnection, the endpoint resolution, and the two advice constants. Both package docs now say what their package is. The tests move unchanged — they are net.Listen-based and need no browser in either package. `settle` had to be renamed on the way in, which is its own small argument for the move: internal/chrome already had a settle(), and every "settle" in this package means "wait until it stops moving" while the probe's meant "close it or keep it". It is now `upgraded`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There were four ways to name the same three answers: browser.WSState's constants, browser.Action, doctor's own stateNoEndpoint/stateConsentPending/ stateReady, and a bare "unverified" string literal. Two of them already disagreed — WSState.String() returned "refused" where doctor's envelope said "no_endpoint" — and the only thing keeping that from shipping as two names for one state was that WSState.String() had no callers at all. That is not a design, it is a coincidence. WSState.String() is now the wire value, doctor derives its `state` field from the probe's answer in one assignment instead of three, and the switch carries only the prose and the exit code that genuinely differ per outcome. "unverified" becomes a named constant and is documented as the one value with no WSState behind it, because it is the absence of a probe rather than the result of one. doctor's --help promised three states while four shipped; it now names all four. Action.String() goes: no caller anywhere, and the branch had grown a case on it that nothing would ever have printed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The consent-pending state is a TCP listener that accepts and says nothing, so every scenario in RFC-0013 is reproducible with net.Listen and no browser — which matters more than usual here, because reproducing it by hand wedged a real user's Chrome twice and a test that needs a human to click a browser-modal dialog is not a test. Three packages needed the same three listeners and each had grown its own. stallListener existed twice with different signatures; answerListener and lateAnswerListener were the same function; stubEndpoint was a third variant with a port file bolted on — around 170 lines, with separately maintained comments explaining the same thing three times. Only two of the three counted accepted connections, which is the assertion that proves the property the whole RFC turns on: every connection to the debug endpoint is a consent request, so "how many did we open" is the question being asked. internal/probetest gives all three Stall, Answer, Closed, the connection count, the answered-on-a-live-socket flag, and the port file. It cannot live in chrometest, which imports internal/chrome — one of the packages that needs it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The daemon's pending-marker test sat a 3s consent budget and a 3s poll around a 2s threshold living in another package, which the test had no way to reach. Measured: 3.05s against a marker at ~2.0s — a 1.5x margin on exactly the kind of timing this repo has been bitten by four times on macOS CI. The threshold is now chrome.Options.ConsentPendingAfter, alongside the ConsentTimeout and OnConsentPending it belongs with. The test sets 100ms for a 30x margin, and both the mutable package var and the shrinkPendingThreshold helper delete themselves. The probe's own announce test asserted `pendingAt > 400ms` against a 100ms timer: an absolute wall-clock ceiling on a timer plus a goroutine wakeup, on CI boxes that are not idle. The property is an ordering — the announcement lands during the wait, not on the way out — and its sibling in the connect test already expressed it that way with `pendingAt > elapsed/2`. Now both do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Handshake verification. The Sec-WebSocket-Key was generated with crypto/rand and then never checked against Sec-WebSocket-Accept, so "ready" meant nothing more than "the status line said 101" — a listener replying "HTTP/9.9 101 whatever" passed. Chrome's debug port is a loopback port any local process can bind, and being told ready by something that is not a WebSocket server is how the probe hands chromedp a socket that will never speak CDP. The accept key is now computed and compared, and Upgrade: websocket is required. probetest's stubs complete the handshake properly, so they are testing the thing. Endpoint validation. ResolveWSURL returned any webSocketDebuggerUrl verbatim and http.Client follows up to ten redirects, so a request about 127.0.0.1 was verified to come back with ws://10.1.2.3:4444/pwned — the URL the probe dials and chromedp attaches to. Redirects are no longer followed, and the answer is only accepted if it names the endpoint that was asked (scheme ws/wss, no userinfo, same port, same host modulo two spellings of loopback). Also: attach() now gets the RESOLVED ws URL, so chromedp does not repeat the lookup and the comment claiming the probe and the attach agree on what they are talking to is true; Probe.PortFileWS becomes Endpoint, which is what it holds half the time, and DecideConnection drops the guard that restated its own precondition; Upgrade.Close loses the nil-receiver half that cannot fire; the .pending payload says who it is for; spawn_test uses wg.Go; and three comments that narrated the change rather than the result are cut to what stays true. One correction to the review: Action.String() was reported as having no callers and is kept, because %v in the ladder's table test is one — without it a failure reads "= 4, want 2". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
doctor's four states and what each is worth; that a merely-running daemon is not evidence of a connection; that it honours --port; that a probe-derived `ready` costs the consent click it just spent; and that it reports a tab COUNT rather than the tab list, which is the part the Agent Skill most needed to know since it runs `doctor --json` before anything else. Also: consent_timeout's clamp and why "0s" cannot mean "do not wait"; the notices on the --no-daemon and queued-caller paths; and the two verification scenarios the review added — a bounded handshake read, and a 101 that is not a completion of our own handshake. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
Review battery results — 4 passes, 23 commits, all findings addressedRan security-review, code-review, simplify/deslop and thermo-nuclear as parallel read-only passes, then applied the consolidated findings. The battery earned its keep: the fix reproduced the bug it was written to fix. The critical one
That is "reporting readiness it never verified" — RFC-0013's own finding — moved up one layer from the port file to the daemon socket. Now: A privacy leak in the command agents run first
An unbounded read on hostile input
Everything else
Deviations worth reading
Gate
|
…apture macOS CI failed the --scale assertion again: a 0.5 recording of a pinned 800x600 viewport came back a square 300x300. Reproduced locally at about one run in eight. Chasing it found the viewport is not stable enough to be the reference. It was observed reverting BETWEEN two adjacent recordings — the scale-1 capture saw the pinned 800x600 and the scale-0.5 one saw 600x600 — with both CDP's visual viewport and the page's own innerWidth/innerHeight agreeing on the wrong value, so a cross-check between them does not catch it either. A test that compares one capture against another is therefore measuring window stability, not --scale. So the claim is decomposed instead. Live: Chrome honours the cap it was given, asserted against the max box RecordStart already reports rather than against a second recording. Pure: the cap is round(viewport x scale), now scaleBox with a table test and no browser at all. Together those are VS-8, and neither half is hostage to a viewport that moves. The recorder also now cross-checks CDP's visual viewport against the page's own before sizing. That does not fix this failure, but the two measures were separately observed disagreeing after a resize, and sizing a whole recording from a box that never existed gives every frame the wrong aspect. Verified: 10 consecutive TestRecordLive runs green, where the original failed 1 in 8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI: "--scale 0.5 produced a 207x207 frame against the 378x207 box it asked Chrome for". The frame was correct and the assertion was wrong. record.go says so directly, on the field the test was reading: the caps are "not a promise about the frames — a screencast frame is never UPSCALED to fill them, so the real dimensions are whatever the compositor produces WITHIN this box". I asserted equality against a documented upper bound, and it held only while the window happened not to move between the box being computed and the frame being produced. 207x207 inside a 378x207 box is Chrome behaving as specified on a window that resized underneath it. That is Chrome's business, not this feature's. What is ours to guarantee is that the cap is honoured AS a cap and that a scaled recording still produces pixels, so that is what the live test now checks. The arithmetic making the cap half the viewport is TestScaleSizesTheBox, with no browser involved. Better failure messages are what found this: naming the box alongside the frame turned three rounds of guessing at viewport timing into one reading of the contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sanketsudake
added a commit
that referenced
this pull request
Jul 27, 2026
All thirteen are implemented and merged, so the index and each header now say so and link the PR that did it. RFC-0002, 0003, 0004 and 0011 also name the follow-up PRs that materially changed them, since a reader chasing why the shipped behaviour differs from the proposal should not have to find that by bisecting. The folder is now a design record rather than a to-do list, and the README says which it is. The gap analysis is left in the present tense of when it was written — the reasoning is the point, and rewriting it into the past would lose why that ordering was chosen. RFC-0013's references to #17 are replaced: that PR was superseded by #18, which carries its commits, so the links pointed at a closed PR. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements RFC-0013. Stacked on #17, which serialises the spawn — necessary, but not sufficient, as the reproduction showed.
Developed entirely against
net.Listenstubs. No browser was driven to build this, deliberately: reproducing this by hand wedged a real Chrome twice, and a defect whose reproduction breaks the user's browser must not have a manual regression test.A
boolwas the bug"Refused" and "accepted, then silent" were the same value. So a hanging upgrade — which is exactly what Chrome does while it waits for consent — could only ever surface as an undifferentiated timeout, and the CLI had nothing to tell the user beyond "cannot reach the endpoint".
It is now three-way —
WSRefused/WSPending/WSReady— andDecideConnectiongains aConsentPendingaction ahead ofInstructToggle. That function is pure with a table-driven test, so the whole diagnosis lands red before anything connects.Telling pending from refused
A TCP dial plus one WebSocket handshake on a socket we own, with three timings:
onPendingduring the wait, so the user is told while it is still happeningconsent_timeout/json/versionis deliberately not used as a signal — it 404s whether or not consent has been granted, which is the wrong turn the RFC records so nobody rebuilds it.chromedp can't do this itself: a context timeout on its first
Runtears down the browser it just allocated, so the classification has to happen on a separate socket. That socket is held until the chromedp attach returns, so the consent just granted is still live when the attach arrives.Waiting, and saying so
consent_timeout(default 120s;--consent-timeout> env > config > built-in) rides onConnOptsto the daemon.The daemon is detached, so it publishes
<socket>.pendingthe moment it starts waiting.Ensuresees that, prints a notice naming the modal dialog, and moves its own 10s deadline to the full budget — otherwise the client declares a live daemon dead and we are back to an orphaned prompt, just more slowly.result.CodeConsentPendingmaps to the existingExitConnection(3) — a new code, not a new number. The message says the prompt is browser-modal, may be behind the window, and that Chrome accepts no other input until it is answered. That last clause is the one a user cannot deduce, and is why a frozen browser reads as a crash.doctornow verifies what it claimsIt previously read the port file and reported "debug endpoint reachable — Path B attach ready" with a
ws://URL, without ever connecting. During the reproduction it said "ready" while every connection was hanging.via: daemon,probed: false, zero connections to Chrome (VS-6 counts them)ready/consent_pending(exit 3) /no_endpoint(exit 3)--no-probestate: "unverified"and connects to nothingThe old unverified "ready" claim is no longer reachable — you either get a verified answer or an explicitly unverified one.
One extra defect found on the way
An explicit
--portyields anhttp://endpoint, which the new upgrade probe would have handshaked against/— so the very flag path this RFC recommends would have classified as refused. Fixed withResolveWSURL(via/json/version, the same lookup chromedp does), with tests.Docs
--remote-debugging-port=9222skips consent entirely, so it now leads indocs/cli-reference.md, thedoctoroutput, and the agent skill's setup step. Thechrome://inspecttoggle comes second, with a note that it prompts on every fresh attach — it was previously presented as the primary route, which walked every new user into this failure.Scope note
VS-3 stops at classification. A stub listener is a socket, not a browser, so "the connection succeeds" end-to-end would need a fake Chrome speaking CDP. The test asserts what the defect actually was: the endpoint's late answer landed on a still-open socket,
Connectdid not return before it, and it was not misreported asconsent_pending.Testing
gofmt,go vet,go test -race ./...all clean, including ~112s of live Chrome. VS-7 (8 concurrent callers → 1 daemon) still green.