Repository navigation
Conversation
state_show returned a preformatted summary string while the CLI read summary.cookies and summary.origins as numbers, so every file reported 0 cookies and 0 origins. Return the counts as an object instead, matching the summary shape doctor already uses. The same payload also carries filename and path, which the download formatter claims first, so state show printed a completed-download line instead of the summary. Render the summary before the generic path branches and keep the preformatted string working for older daemons.
Contributor
|
@Bella-rab is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
Other tests in this module build fixtures under tempfile::tempdir(). process::id() is constant within the test binary, so the previous name could collide between tests and leave the file behind on failure.
This branch has not been deployed
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
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.
Summary
state showreports the wrong summary in both output modes.The daemon built
summaryas a preformatted string ("12 cookies, 3 origins"), but the CLI readsummary.cookiesandsummary.originsas numbers. Those lookups always returnedNone, so every state file printedCookies: 0andOrigins with localStorage: 0regardless of its contents.While verifying, the text path turned out to be wrong for a second reason. A state-show payload carries
filenameandpath, and the download formatter claims any payload with those fields. It runs earlier inprint_primary_response, sostate showprinted a completed-download line and never reached the summary at all:Changes
cli/src/native/state.rs:summaryis now an object withcookiesandoriginscounts. That matches the shapedoctoralready uses and what the CLI was written to read.cli/src/output.rs: the summary is extracted intoformat_state_show_text, and thestate_showbranch moves ahead of the genericpath/filenamebranches. A preformatted string summary is still rendered, so a CLI pointed at an older daemon keeps working.Verification
Built the binary and ran it against a fixture holding 2 cookies and 1 origin.
Before:
After:
--jsonnow returns"summary":{"cookies":2,"origins":1}.Tests:
cargo test --bin agent-browser format_state_show(5 passed), covering count rendering, the encrypted marker, a legacy string summary, the non-state payload, and the payload that used to match the download branch.cargo test --bin agent-browser state::(19 passed), including a new daemon-side test that keepssummarystructured.cargo test --bin agent-browser: 1918 passed, 4 failed. The 4 failures are pre-existing on Windows and unrelated to this change:install::tests::download_bytes_connection_refused_includes_detailsandsession_settings::tests::*assert a localized error string and/pathseparators, andnative::tab_binding::tests::test_pinned_owner_dead_or_missing_pid_is_freeshells out tops. The same 4 failed before this change.cargo fmt -- --checkis clean.Why the contract changed on the daemon side
Fixing only the CLI would have left
--jsonconsumers reading a sentence where they expect counts. The object shape is the one the CLI already assumed and the onedoctorreturns, so the daemon is the side that drifted.Note
I did not find an open issue for this specific report. #2455 is a different
state listproblem (a blank modification date) and is already claimed by #2457.Follow-up commit
74c31eemoves the new daemon-side test ontotempfile::tempdir(), matching the other tests in that module.process::id()is constant within the test binary, so the previous fixed filename could collide between tests and survive a panic.