Repository navigation
fix: remove console browser tool - #1131
Conversation
|
Greptile (@greptileai) review |
|
Claude (@claude) review |
|
Claude finished Nikhil (@shadowfax92)'s task in 2m 8s —— View job Code Review
Overall: clean, complete removal. No blocking issues. The PR fully de-wires Correctness
Test changes
Minor observations (non-blocking)
SummaryFull removal executed cleanly — runtime hooks, registry, label maps, system prompt, limits, and tests all updated consistently. The |
✅ Tests passed — 1004/1010
|
Greptile SummaryThis PR performs a clean, full removal of the
Confidence Score: 5/5The removal is complete and consistent across all layers — no dangling imports, no orphaned constants, and no active code paths reference the deleted tool. Every reference to get_console_logs in active code has been removed. BrowserSession accepts an optional hooks parameter that correctly defaults to {}, so the simplified constructor call is safe. CONSOLE_META_CHAR is retained because logger.ts still uses it for unrelated log truncation. Tests are updated with negative assertions to guard against accidental re-introduction. The change is well-scoped and has no side effects on remaining browser tooling. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Before
B1[Browser] -->|creates| CC[ConsoleCollector]
B1 -->|onSessionAttached| LE[Log.enable + attach]
B1 -->|onPageDetached| DT[detach]
CC -->|buffers| CE[ConsoleEntry buffer]
T1[get_console_logs tool] -->|calls| B1
R1[registry] -->|registers| T1
P1[prompt.ts] -->|documents| T1
end
subgraph After
B2[Browser] -->|no hooks| BS[BrowserSession]
R2[registry] -->|no get_console_logs| R2
P2[prompt.ts] -->|evaluate_script only| P2
end
CC -.->|deleted| X1((removed))
T1 -.->|deleted| X2((removed))
Reviews (1): Last reviewed commit: "fix: remove console tool prompt referenc..." | Re-trigger Greptile |
Greptile SummaryThis PR removes the
Confidence Score: 5/5This is a clean, complete removal with no stale references remaining and thorough test coverage confirming the tool's absence. Every call site — session hooks, registry entry, prompt text, limit constants, labels, and test fixtures — has been updated. The retained CONSOLE_META_CHAR constant continues to serve the unrelated logger truncation path. No dead code is left behind and the negative-assertion tests guard against accidental re-introduction. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Browser constructor] -->|before| B[new ConsoleCollector created]
A -->|before| C[BrowserSession with session hooks]
C -->|onSessionAttached| D[session.Log.enable + collector.attach]
C -->|onPageDetached| E[collector.detach]
B --> F[CDP events buffered]
F --> G[getConsoleLogs on Browser]
G --> H[get_console_logs MCP tool]
A2[Browser constructor] -->|after| C2[BrowserSession no hooks]
C2 -.->|removed| D2[Log.enable / collector.attach]
B2[ConsoleCollector REMOVED] -.->|removed| F2[CDP event buffers REMOVED]
F2 -.->|removed| G2[getConsoleLogs REMOVED]
G2 -.->|removed| H2[get_console_logs tool REMOVED]
style B fill:#ffcccc
style D fill:#ffcccc
style E fill:#ffcccc
style F fill:#ffcccc
style G fill:#ffcccc
style H fill:#ffcccc
style A2 fill:#ccffcc
style C2 fill:#ccffcc
Reviews (2): Last reviewed commit: "fix: remove console tool prompt referenc..." | Re-trigger Greptile |
Third instance of one shape in a day. The autonomous tick chose a candidate, `delegate` refused it because another live task already owns those files - correct - and the tick ended there instead of trying the next one. Same as "already delegated", same as the review sweep that only ran on a finish. Filtered with the very policy that would otherwise refuse it (`QueenDelegationPolicy.conflictingTasks`), so the selection and the refusal cannot disagree. Driven in release: she skipped browseros-ai#1131 as already running, skipped browseros-ai#1133 and browseros-ai#1169 as already done, chose browseros-ai#1132, approved it herself - and then met the boundary conflict. That refusal is what this removes from the path. Also visible in the same run and worth recording: all five parked tasks now carry COMPLETE verdicts - 4/4, 4/4, 2/2, 3/3, 4/4, where two of them were 0/4 and 0/2 before the key guard landed. The reviewer was never broken; it had nothing to answer with.
The dev registry held nineteen tasks and fourteen failures. browseros-ai#1127 had been attempted seven times, browseros-ai#1129 five, browseros-ai#1128 four - every one the same brief against the same issue. Nothing counted the attempts and nothing told the next worker that anyone had been there before it, so a retry was not a second attempt; it was the first attempt run again by someone who did not know. The reason it could not count is that the registry had one word for two different endings. Ten of those fourteen had `streamOutcome: open` and no completed turns - the signature of a process that went away under the worker, which on this machine tonight was me rebuilding the app. The other four had done real work. Both were spelled `failed`, and afterwards nothing could tell them apart. Counting the first group would retire issues for the crime of being open during a rebuild; not counting the second lets one run forever. So the ending is now recorded rather than inferred later. `reconcileOrphaned- Workers` already knew it was reconciling a dead process and threw that away; it writes `interrupted`. The review path had the failure text in hand and stored only the state; it classifies from what the runner measured. An interruption never counts against an issue. Two real failures and the issue stops being chosen - `queen.choose.exhausted` says so with the kinds named, because a third identical attempt is evidence about the issue, not the bees. And when a retry does happen, the brief opens with what the earlier attempts ran into and says not to repeat the approach. That is the half that makes it a second attempt. Driven, not read. Live in the dev app: `Skipping browseros-ai#1131: 2 attempts have already failed on their own merits (workedButFailed, workedButFailed)`, and she took browseros-ai#1132 instead. Three tasks orphaned by my own restarts were recorded `interrupted` and did not count. 805 e2e checks in 71 scenarios, up from 790. Both new guards proven red: making interruptions count breaks four of them, dropping the briefing from the worker's brief breaks two.
…its default The activation window arrived (the browseros-ai#1131 bee settled at its send-back ceiling) and every pending fix was proven on the wire in one pass: the heal sweep read 45 conversations on its own the moment the key answered; keychain.read.served_late fired three times, serving settlements the old shape would have refused for a minute; settle tails measured up to 215.6 seconds with OSStatus 0 - securityd here is slow, never broken. Two probe defects blocked the last proof and are fixed here. ChatProbe never read TRIOS_<PROVIDER>_API_KEY from the environment - only as a JSON key inside config files - so `make chat-probe VARIANT=release` printed "key: MISSING" about a key the running app held in its cache, on the very machine whose config.json carries the documented zero-length values. Env is now the probe's first source, mirroring the app's own chain. And the probe now prints the usage frame it receives, so a probe run IS the wire proof for the spend pipeline: PASS in 3701 ms usage: 18308 in / 45 out tokens (usage SSE frame received) With that printed, usage emission flips to ON by default (opt-out TRIOS_EMIT_USAGE=0). The env allowlist stays opt-in until a worker bash call is observed under it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…and the allowlist earns its default Three wire proofs collected in one worker turn. queen.worker.expensive fired for the first time in its existence: task browseros-ai#1131 recorded 10,650,930 input / 73,416 output tokens, priced at $6.55 by ModelPricing - the spend pipeline is live end to end, and the $10/day SwarmBudget gate can finally trip. queen.pr.gate_administrative fired at 17:38:56Z naming its check ("#74 is red only on administrative check(s) (cla); nothing a worker can fix") - no wake, no illegal transition. And the turn's finishing commit (queen.branch.committed 17:56:19Z) landed with the env allowlist active on the live server: ten variables are enough for real work, so the scrub flips to ON by default (TRIOS_BASH_ENV_ALLOWLIST=0 is the opt-out). make spend's note stops claiming the server "does not emit usage yet" - that cause died this evening; the 49 zero-token tasks are turns that finished before emission went live, and the note now says so. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The daily budget gate lived only on NEW dispatches, and the automatic review sweep's send-back restarted workers straight past it - measured on 2026-08-21, the first day tokens were real: two automatic returns of browseros-ai#1131 burned $7.60 of the $10 day through a path no gate watched, while delegateIssueToWorker would already have refused fresh work. The sweep's send-back now defers when the day is spent, with the same shape as its existing slot deferral: the task keeps its place in the review queue, queen.review.send_back_deferred names the measured spend, and nothing is killed. The operator's own /review reject is deliberate and stays ungated. Not proven on the wire yet: the running binary predates this commit, the third browseros-ai#1131 turn is in flight, and the gate only shows itself when a sweep meets an exhausted day - queued for the next relaunch window. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…a loss The preservation pattern generated its own noise: browseros-ai#1131's branches (r4 with the supervisor's preservation commit, r5 with the bee's three) raised urgent unrecordedWork the pass after their record was cancelled with a written reason - the checker saw commits HEAD does not hold on a terminal record and concluded nobody was looking, unable to see that the cancel reason and the archive stamp ARE the record of the looking. unrecordedWork now also defers to recordArchived, joining branchMissing and commitMissing: a stamped record's parked branches read as archivedRecordDrift (stale bucket, no repair proposed). An UNarchived record's parked commits still alarm - nobody has decided about those, and that alarm is the reason failed is not archivable in the first place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…erprint now survives them The seal computed the boundary fingerprint and kept it in a process-local dictionary: 0 of 58 tasks in the live store carried treeStateFingerprint, and this app relaunches constantly, so browseros-ai#1126/browseros-ai#1131 worked until the next restart and then went blind, silently. The registry now records the fingerprint on the task and persists; the e2e suite proves it on the wire - a full /verify through the command path, then the store FILE decoded as a fresh reader (ISO8601, no shared state), 971/971 checks green twice. build.sh survived the day's Xcode update (6.0.3 -> 6.3.3) by naming three breaks: XCTest moved out of the SDK (conditional -F at all three import sites, derived from xcrun), a neighbour's foreign-toolchain swiftmodule, and the module vanishing while SwiftPM still said Build complete - the toolchain probe now repairs all three signatures and falls back to the vendored halves on the neighbour-wipe signature.
1 participant
Summary
Design
This is a full removal rather than a hidden compatibility path: Browser no longer creates a console collector or session hooks, the MCP registry no longer exposes get_console_logs, and prompt/label/test fixtures no longer advertise the removed tool.
Test plan