Repository navigation
Conversation
agent-browser types and presses into whatever holds the keyboard focus. It asks the target to take the focus first, but an element that takes none (a heading, a label) leaves it where it was: text typed into a heading lands in the field filled before it, and a key pressed on one presses whatever button last had the focus. Browser::perform now checks first, for Focus, Fill, Type with a target and Press with a target: it takes the focus off whatever holds it, gives it to the target, marks the elements holding the focus with a fresh random value and reads that attribute back on the target through the engine, which resolves the target in its own frame. A focus that landed elsewhere fails Error::FocusMissed (NOT_ACTIONABLE on the wire, naming only the tag that holds it) with nothing typed or pressed. A focus inside a cross-origin frame goes ahead, since only the target can have put it there; a locator's fill, which only the engine resolves, gets the focus taken off first. Press gains an optional target (contract 2.17), so a key can be checked like typing; the browser tool maps its selector onto it. The check composes existing engine commands because the engine's own fix cannot land in the agent-browser fork for now; when the engine verifies the focus itself, this can go.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Comment |
A whitespace key is refused as invalid before the focus check runs, so the test now presses Enter on the element whose focus misses.
main took contract 2.17 for StartTask.site and the SiteMemoryHost epoch, so the optional Press target moves to 2.18.
Tiny Sweeper review
Last completed reportTiny Sweeper reviewTiny Sweeper reviewed this change across 5 lane(s) and found 0 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Incomplete Review snapshot
Completeness: Incomplete FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. FindingsNo active actionable findings. Lower-confidence notes
Could not review: crates/tinycomputer/src/tinybus_module/tinybus_module_tests/browser_tests.rs, docs/crates/tinycomputer-bus/browser.md, docs/technical/specs/desktop-module-contract.md Before merge
How this fits togetherflowchart LR
n0["wait_for<br/>changed"]:::changed
n1["every_variant<br/>changed"]:::changed
n2["Error<br/>changed"]:::changed
n3["Fake<br/>changed"]:::changed
n4["Value"]:::impacted
n5["...fore_the_page_claim_nothing_was_delivered"]:::impacted
n6["locator"]:::impacted
n7["to_action"]:::impacted
n8["Err"]:::impacted
n0 -->|uses| n4
n0 -->|calls| n8
n1 -->|uses| n2
n3 -->|uses| n4
n5 -->|calls| n1
n5 -->|tests| n1
n5 -->|uses| n2
n7 -->|calls| n6
n7 -->|calls| n8
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinycomputer/src/tinybus_module/tinybus_module_tests/browser_tests.rs, docs/crates/tinycomputer-bus/browser.md, docs/technical/specs/desktop-module-contract.md.
$0.0039 · 1,907,308 in / 48,012 out · 323,712 cached (17%) · ladder/vectors-oai3, deepseek-v4.1-flash, · 739 embedded
critique: $0.0028 · 1,108,753 in / 36,550 out · 205,824 cached (19%) · deepseek-v4.1-flash,
security: $0.0006 · 633,389 in / 7,160 out · 106,752 cached (17%) · deepseek-v4.1-flash
tests: $0.0001 · 99,764 in / 1,045 out · 6,912 cached (7%) · deepseek-v4.1-flash
description: $0.0000 · 45,573 in / 553 out · 4,224 cached (9%) · deepseek-v4.1-flash
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
2 participants
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
agent-browser types and presses into whatever holds the keyboard focus. It asks the target to take the focus first, but an element that takes none (a heading, a label) leaves the focus where it was. Text typed "into" a heading then lands in the field filled before it, a card number say, and a key pressed on one presses whatever button last had the focus. A security review of an agent driving
BrowserPerformfound both.Browser::performnow checks first, forFocus,Fill,Typewith a target, andPresswith a target (new, below). The check lives insessions/focus.rs:focuscommand.data-tc-focus), then reads that attribute on the target withgetattribute. The engine resolves the target in its own frame, so only the target, or a shadow host around it, carries the mark. The marks come off again either way.Error::FocusMissed, and nothing is typed or pressed. On the wire this isNOT_ACTIONABLE. The error names only the tag that holds the focus, never page text, and its suggestion is to act on the field itself.Pressgains an optionaltarget(contract 2.18), so a key can be checked the same way. Thebrowsertool maps itsselectoronto it.Why in the adapter: the right home for this is the engine: one function that focuses and compares
activeElementin a single call. That fix cannot land in the agent-browser fork for now, so this composes the commands the engine already exposes. When the engine verifies the focus itself, this check can go.Related issue
None.
API or behavior changes
Action::Pressgainstarget: Option<Target>, with#[serde(default)], and is omitted on the wire whenNone. Untargeted presses serialize exactly as before.Action::Press { key }with a struct literal must addtarget: None.Focus,Fill, andTypewith a ref or selector target now failNOT_ACTIONABLE(Error::FocusMissed) when the focus does not land on the target. Before, they reported success with the keys gone elsewhere.Fillcannot be checked, since only the engine resolves the locator. It gets the focus taken off first, so its text lands in the field or nowhere.TypeandPresswithout a target are unchanged: they go wherever the focus is.Error::FocusMissed { focused }. The enum is#[non_exhaustive], so this isn't breaking.Validation
Commands actually run, with their outcome:
cargo fmt --all -- --check: clean.cargo clippy --locked -p tinycomputer-bus -p tinycomputer-browser --all-targets -- -D warnings -A unused-imports: clean.-A unused-importsis only for theCENTRE_JSimport insurface/mod.rs, which is unused in a default-feature test build. That was already the case onmain; this change doesn't touch it.--all-featuresover the whole workspace.cargo build --all-targets --all-features: not run. The module crate links both engines, and this machine didn't have the disk space to build them in debug. CI runs it.cargo test --locked -p tinycomputer-bus -p tinycomputer-browser: browser 177 passed (12 new), plus 2 doctests; bus 227 passed, plus 34 doctests.cargo test --all-features, including the module crate's new bus test, was not run locally, for the same reason. CI runs it.RUSTDOCFLAGS="-D warnings" cargo doc --no-deps -p tinycomputer-bus -p tinycomputer-browser: clean.No live Chrome run, and no live test yet. The
live_pagehelper is private to the sight tests. A follow-up could share it and add alive_test: a page with a heading and a focused card field, then aTypeon the heading's ref, expectingFocusMissedwith the card field untouched.Tests
sessions_tests/focus_tests.rs(12 tests) covers:Focusrefused;PageFocus, defaulting to the target) and answers the focus check before a test's own script. So the existing surface tests, which answer everyevaluate, keep running unchanged.error_tests.rscoversFocusMissed's wire name, envelope and suggestion.interaction_tests.rschecks that a targeted press is the plain key command.action_tests.rs);tool_tests.rs);tinybus_module_tests/browser_tests.rssends a press with a target over the in-memory bus. It checks that the press is refusedNOT_ACTIONABLEwith the suggestion, and is never sent.Documentation
docs/crates/tinycomputer-browser/interacting.md: a new section, "Keys go only where an action points", plus the press section.docs/crates/tinycomputer-bus/browser.md,docs/crates/tinycomputer-bus/versioning.md.docs/technical/specs/desktop-module-contract.md: the 2.18 note.Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the description