Repository navigation
e2e: a screen's page says what it loaded - #790
Conversation
agent-smoke's first screen page on preview (run e5415519fd, 2026-10-31) never ran its script: no agent named, its HTML's own status, for 90 s, while every page opened after it connected. Nothing said which of its requests failed. Each screen page (agent-smoke's three, bot-mode's) now opens watched until it connects or PAGE passes, and a FAIL's detail carries what it said: each request and its answer (a status, a failure, or none), each socket, each error answer's body (the cell's refusal or the bridge's) with its cf-ray, its console, its exceptions and the browser's log. Watching stops there (Browser::unwatch): an open screen's socket streams every frame. A ticket's one-time secret is never said. Diagnostics only: no check changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
alexlwn123
left a comment
There was a problem hiding this comment.
Fail for now at fcec0464b9cbd63c5479886965480dfbde849569.
The load diagnostics fit the stated purpose, but their output does not preserve the ticket-secret boundary.
Blocking:
crates/e2e/src/lanes/agent_smoke.rs:463copiesLog.entryAdded.entry.urlverbatim intoloading.log. When the initial/__ticket?t=...&next=...navigation fails (for example an edge 502 before redemption), Chrome's failed-resource log carries that URL. Onlyrequests()strips its query; this log path bypasses that sanitizer, andSuite::failprints the resulting JSON. The ticket can still be usable:PAGEis 90 seconds, whileTICKET_TTL_MSis 120 seconds and redemption is what consumes it. This exposes a computer sign-in credential in failure logs, contrary to docs/secrets.md and this PR's explicit no-ticket-secret promise.
Redact the ticket before any diagnostic string leaves loading, covering the log URL and occurrences in free-form log text, console/exception text, and response bodies, rather than only the request summary. Add a failed-ticket Log.entryAdded fixture and assert the complete serialized loading detail contains no ticket value. The existing test covers only requests() and cannot catch this bypass.
Validation: cargo test -p fragment-e2e a_page_says_each_request_and_its_answer -- --nocapture passes; git diff --check passes. CI remains in progress. This finding is based on the source/data flow, not a live browser reproduction: Chrome is unavailable here. The referenced /Users/futurepaul/dev/finite/engineering-style/engineering-style.md is also absent. No hosted/paid run was performed.
The implementation size otherwise looks appropriate for the diagnostic scope; no additional complexity finding.
|
Preview run from this branch (
|
Review (alexlwn123): only requests() dropped a ticket's query; the browser's log entry for a failed ticket navigation carried its URL verbatim, as could its text, the console, an exception or an error's body. A ticket whose navigation failed is still good (PAGE is 90 s, a ticket lives 120 s), so a FAIL could have printed a sign-in. open_screen takes the secret from the ticket's URL it minted, and said_loading (now pure) says it as <ticket> in every string it returns, before any is cut to BODY_MAX (a cut first could keep part of it). Its test reads the whole detail, as Suite::fail prints it, for any 12 characters of the secret; it fails when the cut comes first and when nothing is hidden. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks, that's right, and it's fixed in 9c4a731.
🤖 Generated with Claude Code |
alexlwn123
left a comment
There was a problem hiding this comment.
Pass at 9c4a73129e28cf6d4a1a7ce0849c88e0151acdc6.
The earlier ticket-secret blocker is fixed: browser log URLs/text, console output, exceptions, request summaries and error bodies redact the minted ticket before diagnostic truncation. The new test checks the serialized failure detail, including a secret spanning the truncation boundary. The ticket issuer generates hex, so the extracted query value matches its diagnostic representation.
The change fits the screen-load diagnostic purpose, and the small pure formatter is an appropriate way to test the credential boundary. I found no remaining blocking regression or additional complexity concern.
Validation: cargo test -p fragment-e2e a_page_says -- --nocapture passed (2 tests); git diff --check HEAD~2 HEAD passed. CI remains in progress. No live browser or hosted/paid run performed in this review. The referenced engineering-style file is unavailable on this machine.
On preview, agent-smoke's first screen page (run
e5415519fd, 2026-10-31) never ran its script. For 90 s it named no agent and kept its HTML's own "Connecting to the screen…", while every page opened after it connected and drew. Its FAIL had nothing to say about which request failed.What changes: each screen page (agent-smoke's three, bot-mode's one) is opened watched (
open_watched) until it connects orPAGEpasses. A FAIL's detail now carriesloading:failed net::ERR_…, orno answer;socket /p/6080/control?… 101, closed);cf-rayto find it in the Worker's logs;Watching stops there (
Browser::unwatch, which turns off itsNetworkevents): an open screen's socket streams every RFB frame as an event. A ticket's one-time secret is never said: its query is dropped.Only diagnostics: no check, wait or bound changed.
Tests:
a_page_says_each_request_and_its_answer(a ticket's redirect, a module answered 502, one failed, one never answered, a socket, the edge id's header in any case, and no ticket secret). Clippy is clean forfragment-e2e. Its proof on preview is the next hosted agent-smoke run from this branch (reported in a comment here).🤖 Generated with Claude Code