Repository navigation
feat(agent): run the extension UI as a plain web page for e2e testing - #1117
Nithin (felarof99) wants to merge 4 commits into
Conversation
Run apps/agent's React UI as an ordinary localhost web page (no extension load) wired to the real apps/server over HTTP/SSE — for fast iteration and agent-browser e2e tests. - web/: standalone Vite server (:5300) serving the existing entrypoints/app - shim installs @webext-core/fake-browser as chrome/browser + a thin chrome.browserOS stub (real MCP port via getPref; localStorage pref store) - no server change needed: isTrustedAppOrigin already trusts localhost:* - scripts: web / web:build / web:prepare; deps: vite, @vitejs/plugin-react, @webext-core/fake-browser Production entrypoints/app/* are untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Empty or non-numeric VITE_BROWSEROS_MCP_PORT now falls back to 9100 instead of yielding 0/NaN. Extracted resolveMcpPort() helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR adds
Confidence Score: 3/5The web harness is dev-only and does not touch production extension paths, but the MCP port seeding logic can leave the harness silently connected to the wrong server across runs. The MCP port stored in localStorage is never overwritten when VITE_BROWSEROS_MCP_PORT changes between dev sessions. A developer who first ran with port 9100 and then restarts the server on 9200 will see the harness make requests to the old port with no error or warning. packages/browseros-agent/apps/agent/web/browseros-stub.ts — the PrefStore constructor port-seeding guard needs to always honour the build-time env var over a stale stored value. Important Files Changed
Sequence DiagramsequenceDiagram
participant Dev as Developer / agent-browser
participant Vite as Vite (localhost:5300)
participant Shim as web/shim.ts + browseros-stub.ts
participant App as entrypoints/app (React UI)
participant Server as apps/server (real)
Dev->>Vite: open http://localhost:5300
Vite->>Shim: import './shim' (ES module, depth-first)
Shim->>Shim: install fakeBrowser as globalThis.chrome/browser
Shim->>Shim: seed chrome.browserOS stubs
Shim->>Shim: seed MCP port from VITE_BROWSEROS_MCP_PORT
Vite->>App: import entrypoints/app/main
App->>Shim: chrome.browserOS.getPref(mcp_port)
Shim-->>App: real port value
App->>Server: HTTP/SSE to real server
Server-->>App: streamed agent reply
App-->>Dev: rendered chat UI
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/browseros-agent/apps/agent/web/browseros-stub.ts:35-42
**Stale MCP port not refreshed when env var changes**
`PrefStore` only seeds `MCP_PORT_PREF` when the stored value is `null`, so any cached value from a previous session is silently reused. If a developer restarts the harness with a different `VITE_BROWSEROS_MCP_PORT` (e.g. because the server moved from 9100 to 9200), the old port stays in `localStorage` and the harness silently continues to target the wrong server — no error, no redirect, just a failed SSE connection.
The build-time env var should always take precedence over the persisted value. The seed guard should be split: if `import.meta.env.VITE_BROWSEROS_MCP_PORT` is explicitly set, write it unconditionally; otherwise fall back to the stored value or the compile-time default.
### Issue 2 of 2
packages/browseros-agent/apps/agent/web/vite.config.ts:19-20
The `#imports` alias maps the entire WXT virtual module to `@wxt-dev/storage`. If any production app file later imports something other than storage from `#imports` (e.g. `import { browser } from '#imports'`), Vite will silently resolve it to `@wxt-dev/storage`, returning `undefined` for unknown named exports — no build error, only a runtime crash. A comment marking this assumption is worth making explicit so a future importer knows it will break the web harness silently.
```suggestion
// WXT's #imports virtual module — ONLY `storage` is imported from it in the
// current codebase. If any file adds `import { browser } from '#imports'` or
// similar, this alias will silently resolve to @wxt-dev/storage (which does
// not export `browser`), causing a runtime crash with no build error.
{ find: '#imports', replacement: '@wxt-dev/storage' },
```
Reviews (1): Last reviewed commit: "fix(agent): harden web-harness MCP port ..." | Re-trigger Greptile |
| constructor() { | ||
| this.values = this.load() | ||
| // seed the server port so getAgentServerUrl() resolves to the real server | ||
| if (this.values[MCP_PORT_PREF] == null) { | ||
| this.values[MCP_PORT_PREF] = resolveMcpPort() | ||
| this.persist() | ||
| } | ||
| } |
There was a problem hiding this comment.
Stale MCP port not refreshed when env var changes
PrefStore only seeds MCP_PORT_PREF when the stored value is null, so any cached value from a previous session is silently reused. If a developer restarts the harness with a different VITE_BROWSEROS_MCP_PORT (e.g. because the server moved from 9100 to 9200), the old port stays in localStorage and the harness silently continues to target the wrong server — no error, no redirect, just a failed SSE connection.
The build-time env var should always take precedence over the persisted value. The seed guard should be split: if import.meta.env.VITE_BROWSEROS_MCP_PORT is explicitly set, write it unconditionally; otherwise fall back to the stored value or the compile-time default.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/agent/web/browseros-stub.ts
Line: 35-42
Comment:
**Stale MCP port not refreshed when env var changes**
`PrefStore` only seeds `MCP_PORT_PREF` when the stored value is `null`, so any cached value from a previous session is silently reused. If a developer restarts the harness with a different `VITE_BROWSEROS_MCP_PORT` (e.g. because the server moved from 9100 to 9200), the old port stays in `localStorage` and the harness silently continues to target the wrong server — no error, no redirect, just a failed SSE connection.
The build-time env var should always take precedence over the persisted value. The seed guard should be split: if `import.meta.env.VITE_BROWSEROS_MCP_PORT` is explicitly set, write it unconditionally; otherwise fall back to the stored value or the compile-time default.
How can I resolve this? If you propose a fix, please make it concise.| // WXT's #imports virtual module — only `storage` is imported from it | ||
| { find: '#imports', replacement: '@wxt-dev/storage' }, |
There was a problem hiding this comment.
The
#imports alias maps the entire WXT virtual module to @wxt-dev/storage. If any production app file later imports something other than storage from #imports (e.g. import { browser } from '#imports'), Vite will silently resolve it to @wxt-dev/storage, returning undefined for unknown named exports — no build error, only a runtime crash. A comment marking this assumption is worth making explicit so a future importer knows it will break the web harness silently.
| // WXT's #imports virtual module — only `storage` is imported from it | |
| { find: '#imports', replacement: '@wxt-dev/storage' }, | |
| // WXT's #imports virtual module — ONLY `storage` is imported from it in the | |
| // current codebase. If any file adds `import { browser } from '#imports'` or | |
| // similar, this alias will silently resolve to @wxt-dev/storage (which does | |
| // not export `browser`), causing a runtime crash with no build error. | |
| { find: '#imports', replacement: '@wxt-dev/storage' }, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browseros-agent/apps/agent/web/vite.config.ts
Line: 19-20
Comment:
The `#imports` alias maps the entire WXT virtual module to `@wxt-dev/storage`. If any production app file later imports something other than storage from `#imports` (e.g. `import { browser } from '#imports'`), Vite will silently resolve it to `@wxt-dev/storage`, returning `undefined` for unknown named exports — no build error, only a runtime crash. A comment marking this assumption is worth making explicit so a future importer knows it will break the web harness silently.
```suggestion
// WXT's #imports virtual module — ONLY `storage` is imported from it in the
// current codebase. If any file adds `import { browser } from '#imports'` or
// similar, this alias will silently resolve to @wxt-dev/storage (which does
// not export `browser`), causing a runtime crash with no build error.
{ find: '#imports', replacement: '@wxt-dev/storage' },
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The web/ harness serves global.css from a Vite root of web/, so Tailwind v4 auto-detection only scanned web/ and generated none of the app's utility classes — the page rendered unstyled (e.g. the logo ballooned full-screen). Add explicit @source directives for the app source dirs. No-op for the extension build, where these dirs are already auto-detected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The harness persists prefs to localStorage and only seeded the server port
when absent, so a previously-cached port (e.g. the 9100 default) stuck even
after relaunching with a different VITE_BROWSEROS_MCP_PORT — the UI kept
hitting the wrong port ("Failed to fetch"). The port is harness config, so
the env var now always wins; other prefs (providers, etc.) still persist.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks so much for contributing to BrowserOS! 🙏 We're doing some housekeeping on our PR queue and closing out PRs that have been open for more than 7 days. This isn't a reflection on your work — we really appreciate the time and effort you put in. If this is still relevant, please feel free to reopen it or open a fresh PR and we'll take another look. Thanks again for being part of the project! 🚀 |
The rule was lowest-numbered-not-in-flight, which reliably picks the item everybody else skipped: browseros-ai#1111, architectural, and no bee ever came back from it. Task length against success rate is R² = 0.83 - near-certain under four human-minutes, under one in ten past four hours - so narrow and verifiable is where the success is, not a preference. The sort now counts the files named under the boundary heading, treats a directory as large, and uses the number only to break a tie. Driven both ways: by size: Chose browseros-ai#1117, fileCount 1 by number: Chose browseros-ai#1111, fileCount none One line of sort between them, same epic, same command. On a task that fits, the circle went further than it ever has with nobody naming anything: 14:27:50 queen.choose Chose browseros-ai#1117 out of 24 14:27:51 queen.delegate 14:30:28 Committed 1 file(s) 14:31:13 Reviewer returned 2 verdict(s) for 3 criterion(s) 14:31:14 running -> awaitingReview It stops there and should: two of three answered is not a pass. Closes Trinity-S-AI-DNA/trios#1163
… I cannot say why A correction first: last pass I wrote that the name rule never matches. Wrong. browseros-ai#1158 narrows to acceptanceBlockReasonDistinguishingEmptyAnswers, a name its body carries, and the range starts at that declaration. What it does not do is fire for browseros-ai#1117, which names requestReviewerVerdicts and whose declaration is at line 4392. Two explanations tested by driving: two names in one body confuse it - refuted, one name is still silent backticks are not read as prose - refuted, prose is still silent A self-check went in - the first line of the range must declare the matched name - and changed nothing observable, consistent with the match never being wrong when it does happen. Where it stands: one issue in four gets a range, that range is right, three get silence, and no pointer is wrong. But the mechanism is not predictable from reading a spec, so naming the function is not yet a reliable way to aim a bee. Refs Trinity-S-AI-DNA/trios#1176
Two passes of guessing from outside ended by making the mechanism report itself.
Both answers were bad:
browseros-ai#1117 identifiers: a paragraph of Russian prose
browseros-ai#1158 identifiers: awaitingReview | return | acceptanceBlockReason… | …
matched: return
The first extracted nothing usable, so there was never anything to compare. The
second matched on the keyword return, which appears in almost every function.
The single hit across five measurements was luck; the real score was zero, and
every earlier report of one-in-four was counting it.
Now: identifier-shaped tokens only, Swift keywords dropped, paths dropped.
browseros-ai#1158 matched acceptanceBlockReasonDistinguishingEmptyAnswers, 4703-4853
browseros-ai#1117 identifiers: ChatViewModel — still silent
browseros-ai#1158 matches for the right reason, the first true match this feature has made.
browseros-ai#1117 stays silent because a bare lowerCamelCase word in prose is not picked up -
one more step, and the first in six passes that is diagnosable rather than
guessed.
Closes Trinity-S-AI-DNA/trios#1177
Closes Trinity-S-AI-DNA/trios#1178
…f four The last thing in the way was that a function name written as a bare word inside Russian prose was invisible. browseros-ai#1117 says "Вердикты запрашивает и разбирает requestReviewerVerdicts" and only ChatViewModel came out, pulled from the boundary path. Extraction now reads the whole body for Latin runs of six or more characters carrying an interior capital. Before and after, driven, in this cycle's record: before browseros-ai#1117 identifiers: ChatViewModel silent after browseros-ai#1117 matched requestReviewerVerdicts 4430-4680 browseros-ai#1158 matched acceptanceBlockReason… 4703-4853 browseros-ai#1156, browseros-ai#1165 silent Two right, none wrong, two silent, and the silences are honest: neither body names a function. Six passes ago this had never made a correct match and its one apparent hit was the keyword return. The difference was not a better guess. It was making the mechanism log what it extracted, which turned two passes of hypotheses into two passes of reading. Closes Trinity-S-AI-DNA/trios#1179
…rrect
First full pass with nobody naming anything:
22:54:38 queen.choose Chose browseros-ai#1117 out of 40
22:54:39 queen.delegate
22:54:39 queen.brief.narrowed lines 4430-4680
22:57:57 queen.review.verdicts asked=3 parsed=3 recorded=3 unchecked=none
22:57:58 queen.auto_accept.gated - one criterion not met
She chose the issue, aimed the brief at the right function, opened the work, and
the reviewer answered every criterion. The stop is a judgement about the work
rather than a hole in the machine: the unmet criterion asks for a test that
fails when the distinction collapses, and nobody has written it.
What moved to get there: the loop dead-ended at "no committed files", which made
work already done by an earlier pass indistinguishable from a bee that did
nothing. browseros-ai#1117 was in fact finished three passes ago and so could never be
accepted however good its verdicts were. Acceptance now turns on the verdicts,
and on the file count only when they are missing.
Choose, aim, delegate, review, decide - the whole chain, once through. The
decision was no, and it was the right no.
Closes Trinity-S-AI-DNA/trios#1180
browseros-ai#1117's third criterion says a check must break if the distinction collapses. That check exists at ChatSSEEndToEndTest.swift:5597. The reviewer marked the criterion unmet and was right within what it had been given: the boundary is rings/SR-02 and the test file is not in it. It was asked about something nobody showed it. The shape is everywhere here - nearly every issue in this epic ends with "the check breaks if…", and none of them could be confirmed. Now a criterion mentioning a check brings the matching test files with the diff, narrowed to the regions naming the identifiers: queen.review.testsShown - 19 regions Nineteen pieces of test the reviewer had never been shown. The circle did not finish this time: the worker failed and acceptance skipped with "state is failed, not awaitingReview", which is the right refusal for a run that died. The gap is closed and observable; the pass it was meant to unblock still has not run to the end. Closes Trinity-S-AI-DNA/trios#1181
… of three
Reading its actual answer settled three passes of guessing at the parser:
"The file contents provided are empty placeholders (no criteria names found
to search for in this file), so I can only work from the diff."
Not silent, not ignoring the format - it had no code. The assembler looked for
criteria names in the file, found none, sent a placeholder, and the reviewer did
detective work on the diff and answered in narrative. Last pass demanded a
stricter format from it, which was the symptom: nothing answers in one line
about a thing it was not shown.
A file with no name match now shows its narrowed region, or its first three
hundred lines saying so; a placeholder is kept only for a file that is not
there. Across four runs:
parsed 0 of 3 -> 1 of 3 -> 2 of 3 -> 3 of 3
Acceptance still refuses - browseros-ai#1117 was finished three passes ago so nothing is
committed - and no new crash report through two more circles.
Closes Trinity-S-AI-DNA/trios#1184
Closes Trinity-S-AI-DNA/trios#1183
A run logged three verdicts of three, and two minutes later the sweep asked
about all three again. The sweep only asks about unanswered criteria, so none of
the three had been stored.
recordVerdict guarded on acceptanceCriteria.contains(criterion) - an exact
string match - and returned false without a word when the reviewer's label
differed by a quote or a trimmed tail. The counter said parsed, which counts
what was read rather than what was kept.
The comparison is normalised now, exact first, and a label matching nothing is
written down:
queen.verdict.unmatched - received "a criterion nobody set",
expected "make check passes"
queen.review.verdicts - parsed=1 recorded=1
The unmatched line comes from the suite's own fixture, which is how it is
proven; in the live run parsed and recorded agree, which they never did before.
Still short of a merge: browseros-ai#1117 has nothing to commit and the reviewer's coverage
swings between one and three across runs.
Closes Trinity-S-AI-DNA/trios#1185
…e is one The browseros-ai#1090 timeline already carries each sub-issue's body, so the forty five reads per launch collapse to one and the last gh invocation leaves these paths. A token is now also looked for in ~/.trios/config.json under TRIOS_GITHUB_TOKEN, where this project already keeps provider keys. A person puts it there, not the Queen. Without one the forge allows sixty requests an hour and a few relaunches spend it - which is the HTTP 403 the launch hits today. Also settled an old mystery: five browseros-ai#1117 checks that went red and green for no reason I could name. The controlled difference is the app. With trios running, 5 of 594 fail; with it stopped, the same code passes. The test writes to $PROJECT_DIR/.trinity/logs and so does the release build, so they share one store. Filed as browseros-ai#1213.
Three cycles running I built instruments and checked instruments while the work stood still. Today I took the first of the thirteen behavioural issues - browseros-ai#1183, the reviewer answers at length and parsing extracts nothing - and checked it the way its own criteria demand: by running it. Two real delegations with review on browseros-ai#1117, back to back: parsed=3 recorded=3 each time, three of three, and not one unparsed-line event. Closed. Twelve remain, and each needs its own run. It will not go faster, and it should not: that is exactly what separates them from the ones a search can settle.
…does Bee browseros-ai#1128's interface-divergence watchdog ran before the contract, with a stated reason: criteria are judged inside a tree, and a tree that does not compile is no place to judge them. True of criteria. Not true of the contract's own refusals - "the reviewer was asked and gave no answer" is a fact about the reviewer, established without any tree at all. Running the watchdog first replaced that answer with "does not build together" and lost the distinction browseros-ai#1117 exists to keep: asked-and-silent is not never-checked. Both requirements are right; only the order was wrong. Refusals that need no tree now come first, and the watchdog guards the case where the contract is satisfied and something is about to land. Two smaller ones from the same live run: - releaseWorktree refused a path recorded before worktrees were scoped by variant, and would have refused it on every sweep forever. A path that does not exist is now simply forgotten. - the e2e script's worktree cleanup sat after the run under `set -e`, so a failing suite skipped it - which is exactly when leftover checkouts are most likely. 760 e2e checks, 0 checkouts left behind.
…ros-ai#1176) (#307) Second pass. The first pass put the self-check in and then watched it change nothing - and said so - because the check compared a range's first line against the name extracted from that same line. Tautology: rule 1's window is clamped to the declaration line by construction, so the guard could never fire and removing it could not break anything. Criterion 4 was unmeetable as wired. The re-measurement found what actually produces wrong ranges on today's file, and it is not rule 1: 1. maskComments opened block comments inside string literals. A branch glob quoted at line 6864 - "No empty queen/* branch ..." - blanked the literal view from there to the end of the file: 6 726 lines of evidence invisible. Corroboration saw no mentions past the glob, browseros-ai#1158's guard and neighbour tied at zero, rule 1 went silent, and rule 4 answered with 6005-6304 - the middle of handleWorkerFinished, a function that spec never named. The confident wrong range browseros-ai#1176 is about, produced by the wrong rule through a blind view. maskComments tracks strings now, using the same model maskCommentsAndStrings always had. 2. The self-check moved to where ranges are born: every rule's answer passes through one finishing step - the first line of the returned range must declare a function, and the name rule must land on the very name it matched. A window that capping slid into the middle of a body begins nowhere in particular, and beginning nowhere in particular is silence. The check is live: queen.review.verdicts sits 322 lines into the 376-line requestReviewerVerdicts, its window would start at 7508, and the check silences it. Delete the two guards in finished() and that replay row goes red - verified from both sides; that is the criterion-4 property. The re-recording (identifiers unchanged; the boundary file moved to 13 590 lines): browseros-ai#1158 points at autoAcceptIfUnambiguous - the neighbour corroborates 0, the subject 2, measured not assumed; browseros-ai#1117 points at requestReviewerVerdicts (7432-7731, beginning at its declaration); browseros-ai#1156's clue moved upstream into settleCharacterCountVerdicts and the recording follows the clue, not the old address; browseros-ai#1166 unchanged at chooseNextOpenIssue. Expected gains declarationOrSilence - the issue's own acceptance, verbatim: point at the named function, or say nothing; a range that begins anywhere else is a FAIL. A returned range now counts as ok only when it BEGINS at the named function's declaration line and stays inside its extent - "somewhere inside" was the loophole the mid-body ranges walked through. One follow-up outside this task's boundary: the Linux module copy at agent-server/queen-core/Sources/QueenCore/QueenLocalisation.swift is byte-compared by make queen-core-sync and now differs until someone copies rings/SR-00 over it. The boundary names one file, so this task did not touch it. Co-authored-by: Trinity Bee <bee@trinity.local>
Summary
apps/agent/web/— a standalone Vite server (localhost:5300) that runs the existing extension UI (entrypoints/app) as a plain web page, nochrome-extension://load.apps/serverover the production HTTP/SSE path — a true integration target (real UI + transport + agent loop + tools), with only boot-timechrome.*plumbing faked.agent-browserfor e2e; example script (web/e2e/chat.e2e.sh) + runbook (web/README.md) included.Design
The UI boots via a shim (
web/shim.ts) that installs@webext-core/fake-browserasglobalThis.chrome/browser(in-memory storage/tabs/runtime) plus a thinchrome.browserOSstub (web/browseros-stub.ts). The stub's only truthful call isgetPref('browseros.server.mcp_port'), returning the real server port fromVITE_BROWSEROS_MCP_PORT— so the app's existinggetAgentServerUrl()resolution targets the real server with zero app-code changes. No server change was needed:isTrustedAppOriginalready trusts anyhttp://localhost:*origin and CORS is permissive.import.meta.env.DEV(true under Vite) enables all capability features, so the UI renders without login. Productionentrypoints/app/*are untouched;webscripts auto-runcodegen+wxt prepareso the standalone build reuses WXT's exact TS setup.Test plan
bun run dev:watch(real BrowserOS + server; note the MCP port)cd apps/agent && VITE_BROWSEROS_MCP_PORT=<port> bun run web→ open http://localhost:5300 → agent UI rendersweb/e2e/chat.e2e.shwithagent-browser(fill refs fromsnapshot -i)Verified locally:
vite buildpasses; runtime boot viaagent-browsermounts React and renders the real UI;getPref(mcp_port)→9100,runtime.lastError→undefined,getManifest().version→0.0.0-web; biome clean.🤖 Generated with Claude Code