feat: stealth launch option + adopt the plugin's own running browser session (0.3.0) - #1
Conversation
…session
- `stealth: false` config -> `--args --disable-blink-features=AutomationControlled`
at launch, the same flag core's agent_browser plugin uses for its stealth option
and nothing more. Google refuses to sign an account in from a CDP-driven Chrome
("this browser or app may not be secure"), which is what stopped the one-time
eBay sign-in for a Google-federated account.
- The "daemon already running" guard misfired on the plugin's OWN `ebay` session:
the daemon outlives the agent process, so every fresh Browser instance (agent
restart, subagent tool rebuild) raised and told the operator to `close --all`,
killing the session it was protecting. A clean launch now records its options
in `<profile>/.protoagent-ebay-session.json`; a later instance adopts a matching
session, names the differing option otherwise, and only treats a marker-less
daemon as foreign. Errors point at `close --session ebay`, never `--all`.
- `ebay_session_status` names the stealth fix when sign-in fails and stealth is off.
- SKILL.md no longer tells the agent "there is no Amazon data here" while
`amazon_price_check` exists; it says what that number is (asking, not sold).
- 0.3.0. 126 host-free tests (was 114); ruff check + format clean.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X1Mou2gxw6Wz9dLCmVocoL
There was a problem hiding this comment.
QA panel review — WARN
code-review-structural · head 288302f8a2e3 · formal
Low-risk change: a single minor correctness issue in browser.py's close() path. The fix is straightforward — check the _run result (or wrap in try/except) before calling _clear_marker(). No panel disagreement; the lone finding was confirmed by the verifier. Verification was diff-only (direct file reads unavailable), so the structural pass was effectively skipped — the finding's grounding rests on the diff hunk alone.
Findings
| Severity | Location | Finding | Verified | |
|---|---|---|---|---|
| 🟡 | minor | browser.py |
close() unconditionally clears the launch marker after _run(self._cmd("close")) without checking whether the close succeeded; if the daemon is still running, t… | confirmed |
findings JSON (machine-readable)
[
{
"file": "browser.py",
"line": 0,
"severity": "minor",
"category": "correctness",
"claim": "close() unconditionally clears the launch marker after _run(self._cmd(\"close\")) without checking whether the close succeeded; if the daemon is still running, the next ensure_session() finds no marker and misidentifies our own session as a foreign one, raising a confusing 'foreign daemon' error.",
"evidence": "def close(self) -> None:\n _run(self._cmd(\"close\"), timeout=self.timeout_s)\n self._clear_marker()\n self._session_ready = False",
"verdict": "confirmed",
"note": "Code confirmed in PR diff: close() discards _run's result and unconditionally calls _clear_marker(). Since _run returns a result (not raises) per the ensure_session() pattern, a failed close still clears the marker, causing _adopt_running_session() to hit the marker-is-None branch on the next call."
}
]There was a problem hiding this comment.
Promoting the WARN verdict for head 288302f8a2e3: all checks terminal-green, zero unresolved review threads. (approve-on-green)
Open findings carried by this approval — non-blocking, but they did not go away:
- minor
browser.py:0— close() unconditionally clears the launch marker after _run(self._cmd("close")) without checking whether the close succeeded; if the daemon is still running, the next ensure_session() finds no marker
Approving a WARN does not resolve its findings (issue #22).
…arker
Review (adversarial pass + agent-browser 0.27.1 source) showed the first cut's model
of the CLI was wrong. A URL-less `open` with flags makes the CLI send TWO launches:
the full-options one, then the parsed command — an explicit launch carrying only
`headless`. The daemon reconciles launch options by hash, so it relaunches Chrome a
second time, on a throwaway temp profile. That was this plugin's launch step through
0.2.0: every window it ever opened was replaced within seconds by one on a profile
nobody was signed in to, which is why the profile dir never held a cookie. Verified
live by watching Chrome's --user-data-dir.
- ensure_session = `open about:blank --profile --headed [--args …] --session ebay`
(one launch; the daemon starts / reuses / relaunches on its own).
- The same flags ride on every `open <url>`, core's proven pattern: a fresh process,
a subagent's tool set, a config change or a dead daemon all converge on a browser
with our options. Stealth now actually reaches the browser.
- The launch marker / adopt / session-list machinery is gone — the daemon's own
reconciliation makes it unnecessary (and its "different options" branch would have
told the operator to close a browser the daemon had just corrected).
- The CLI's "ignored: daemon already running" warning is client-side noise; no
longer treated as an error. Nothing is written into the profile dir.
- `_as_bool` for headed/stealth ("false" from a form is False). Manifest no longer
says `close --all`. `_StubBrowser` carries session/stealth; no getattr crutches.
- 127 host-free tests; ruff check + format clean.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X1Mou2gxw6Wz9dLCmVocoL
|
Redesign after review (head Live-verified on the running |
There was a problem hiding this comment.
QA panel review — PASS
code-review-structural · head cb6ca3457731 · formal
Low-risk change: all four LLM finders and the structural pass found no defects. The prior-round minor finding about close() unconditionally clearing the launch marker was confirmed moot by the verifier — the code path it described no longer exists in the current head. No panel disagreement. No coverage gaps: the structural pass ran and returned clean. Nothing to fix.
No findings — the review came back clean.
findings JSON (machine-readable)
[]- `_as_bool("")` returns the default: a blank `headed` field from a form must not
mean headless (eBay refuses headless browsers).
- `ebay_session_status` hint says the relaunch follows the config reload, matching
the manifest and skill.
- README/manifest: two instances that need different profiles need different
`session` names — a session reconciles to whichever instance opened it last.
- Tidy one test assertion.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X1Mou2gxw6Wz9dLCmVocoL
There was a problem hiding this comment.
QA panel review — PASS
code-review-structural · head e043044c247b · formal
Low-risk change: the only surviving finding is a nit-level docstring inconsistency in _as_bool where the documented falsy-string set omits "off". Fix first: add "off" to the docstring (or drop it from the code set if it was accidental). The panel did not disagree on any point. Verification confirmed the single finding without modification. No structural pass was skipped.
Findings
| Severity | Location | Finding | Verified | |
|---|---|---|---|---|
| ⚪ | nit | tools.py:183 |
_as_bool's docstring lists only "false"/"no"/"0" as the falsy strings, but the implementation also treats "off" as False, so the documented contract understate… | confirmed |
findings JSON (machine-readable)
[
{
"file": "tools.py",
"line": 183,
"severity": "nit",
"category": "conventions",
"claim": "_as_bool's docstring lists only \"false\"/\"no\"/\"0\" as the falsy strings, but the implementation also treats \"off\" as False, so the documented contract understates the accepted set (and no test covers \"off\").",
"evidence": "Docstring: `\"\"\"A YAML/console flag: real bools pass through; the strings \"false\"/\"no\"/\"0\" mean False.\"\"\"` \u2014 but the code is `return text not in {\"false\", \"no\", \"0\", \"off\"}`, which also maps \"off\" to False; the test test_string_flags_from_a_settings_form_are_read_as_booleans exercises \"false\"/\"true\"/\"yes\"/\"\" but never \"off\".",
"verdict": "confirmed",
"note": "Verified in the PR diff: the docstring lists three falsy strings (\"false\", \"no\", \"0\") while the code's set includes a fourth (\"off\"); the test exercises \"false\", \"true\", \"yes\", and \"\" but never \"off\". All three sub-claims are accurate."
}
]Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1Mou2gxw6Wz9dLCmVocoL
There was a problem hiding this comment.
QA panel review — PASS
code-review-structural · head 5238ea6224bc · formal
The change is low-risk: it adds a stealth launch option, refactors flag assembly into _launch_flags/_open_cmd, retires the now-obsolete _IGNORED_MARKER check in favour of the new daemon-reconciliation model, and updates the docs to match. No defects surfaced across the panel or the verifier's structural pass. The two prior-round items (a minor on browser.py close-marker handling and a nit on _as_bool's docstring) were both below the blocker/major threshold and do not carry forward. Nothing to fix before merge.
No findings — the review came back clean.
findings JSON (machine-readable)
[]
Why
merchantAgent has never returned a live eBay sold comp. Two causes, both in this plugin's launch path:
navigator.webdriver, and Google's sign-in page answers "this browser or app may not be secure". The eBay account signs in through Google, so the profile never got signed in (zero eBay cookies from Aug 5 until this week).ebaysession daemon outlives the agent process. After a restart, or from a subagent that built its own tool set,ensure_sessionre-ranopen --profile … --headed, the CLI printed⚠ --profile, --headed ignored: daemon already running, and the plugin raised — telling the operator toagent-browser close --all, which kills the very session it was protecting (and every other plugin's browser). Hit twice in one evening.What
stealth: false(manifest, ships off) →--args --disable-blink-features=AutomationControlledat launch. Same flag core'sagent_browserplugin uses for its stealth option; nothing else changes. Documented in README and the skill as a Google-sign-in fix, not eBay evasion.<profile>/.protoagent-ebay-session.jsonwith{session, profile, headed, stealth, launched_at}. When the CLI reports the daemon already running, the plugin reads it back: same options → adopt the session (log line, no error); different options → error naming exactly which option differs; no/garbled marker → the old foreign-daemon error. Every message saysclose --session ebay, never--all.close()removes the marker. No profile configured → no identity at stake → unchanged pass-through.ebay_session_statusadds the stealth instruction tonext_stepwhen not signed in and stealth is off; drops it once on.amazon_price_check— it now says what the Amazon number is (asking, not sold). Also documents the new "different options" error.Tests
126 host-free tests (was 114),
ruff check+ruff format --checkclean. New: foreign daemon still refused (and--allabsent from the message), own session adopted across a fresh instance, option drift named (stealth), garbled marker = no marker, close clears the marker, no-profile pass-through, stealth flag present only when asked and still session-scoped, unwritable profile dir doesn't fail the launch, config → Browser wiring, manifest ships stealth off,session_statushint on/off/signed-in. The CLI stub now puts the warning on stderr with exit 0, which is what the real CLI does (verified live against agent-browser 0.27.1).Not in this PR
browser.py.🤖 Generated with Claude Code
https://claude.ai/code/session_01X1Mou2gxw6Wz9dLCmVocoL