Skip to content

Workspaces 10/10: harness alert stores through the sidecar - #623

Draft
nedtwigg wants to merge 28 commits into
workspaces-move-verbfrom
workspaces-harness
Draft

Workspaces 10/10: harness alert stores through the sidecar#623
nedtwigg wants to merge 28 commits into
workspaces-move-verbfrom
workspaces-harness

Conversation

@nedtwigg

Copy link
Copy Markdown
Member

The browser harness now sends alert_command and applies the sidecar stores' broadcasts like the Tauri adapter, so the seed-once and delta rules are exercised there. Simulating several windows in the harness is not built.

Based on workspaces-move-verb.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PkPyEFCxiPo5UFeju5Ya9u

The harness applied watched-command and settings changes to its own
AlertManager and ignored the sidecar's broadcasts, so the seed-once and
delta rules the shipped app relies on were never exercised there. It now
sends alert_command like the Tauri adapter and applies the stores'
broadcasts, with the harness forwarding the command to the sidecar.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PkPyEFCxiPo5UFeju5Ya9u
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 10, 2026

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: fc8a4eb
Status: ✅  Deploy successful!
Preview URL: https://90745b10.mouseterm.pages.dev
Branch Preview URL: https://workspaces-harness.mouseterm.pages.dev

View logs

@nedtwigg
nedtwigg added this pull request to stack #624 September 10, 2026 21:44

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback on work in progress — not a merge verdict. Mark the PR ready when you want the full review.

The seed's reply is now the only thing that populates the harness's AlertManager, and the harness can miss it. Before this change alertSetWatchedCommands / alertPublishSettings applied locally, so the manager always had the rules. Now the round trip is the only path: initAlertStateReceiver registers the handlers and immediately POSTs the two seeds, and the canonical snapshot comes back over the SSE stream. But BrowserSidecarHost.init() (standalone/src/browser-sidecar-host.ts, the method that does this.events = new EventSource(url)) resolves as soon as the EventSource is constructed, not when it is open — nothing awaits onopen. If the seed POST reaches the bridge before the /events request registers in sseClients, broadcast() fans out to nobody, the reply is gone, and since WatchedCommandHost.initialize only republishes when something asks again, the harness runs with an empty WATCHING rule set until the user toggles a rule by hand — silently, and only in the dev harness the spec rule you added exists to keep faithful. The same gap reopens on any EventSource auto-reconnect: nothing replays the stores afterwards. Resolving init() on the open event closes the startup half.

Tauri has no equivalent gap — listenToWindow registration is awaited before boot proceeds — so this is the one place where the harness's answer to the two-store rule differs from the shipped one.

The new test covers about half of what its name claims. It asserts the outbound commands and the alert:watchedCommands handler fan-out, but not that the broadcast reaches alertManager (setWatchedCommands / applySettings), and the alert:settings branch — including its if (!settings) return guard — is never delivered. Worth a deliver("alert:settings", …) with an onAlertSettings assertion beside the existing one.

Comment thread standalone/src/browser-sidecar-adapter.test.ts Outdated
nedtwigg and others added 2 commits September 10, 2026 17:32
The seed round trip is now the only thing that populates the harness's
AlertManager, and BrowserSidecarHost.init() resolved as soon as the
EventSource was constructed. If the seed POSTs reached the bridge before
the GET /events registered in sseClients, the broadcast fanned out to
nobody and the harness ran with an empty rule set until a manual toggle.

init() now settles on the stream's open event, rejects on an error
before that (or after a bounded wait) and closes the stream, and reports
every later open through onReconnect. The adapter remembers the two
seeds it last sent and re-offers them on a reconnect, which makes the
sidecar republish both stores: a repeat seed is refused as a seed but
still answered with the canonical snapshot.

The adapter test also asserted a settings field the wire never carries
and never checked that a broadcast reached the manager; it now uses
speakEnabled, spies on setWatchedCommands / applySettings, and delivers
alert:settings including its empty-payload guard.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RChsJ5rMUMyfu22UZDfUus
Carries every stage's review fixes to the tip. The replay-rebuilds-alerts
test seeds through the sidecar path this stage installs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RChsJ5rMUMyfu22UZDfUus

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback on work in progress — not a merge verdict. Mark the PR ready when you want the full review.

The seed-before-stream gap from the last round is closed, and the replay's premise checks out: WatchedCommandHost.initialize and AlertSettingsHost.initialize both call publish() outside the initialized guard, so a repeat seed really does come back as the canonical snapshot.

init() can now reject, and its only caller can't show that. bootstrap() in standalone/src/main.tsx ends with createRoot(document.getElementById("root")!).render(…) and is invoked as a bare bootstrap(); with no .catch(), so a rejected host.init() aborts before the render: the harness is a blank page whose only explanation is an unhandled rejection in devtools. The old init() never rejected — a pre-open failure logged and EventSource retried on its own until the bridge answered. Either keep that self-healing (inline below) or catch in bootstrap() and put the reason on screen; as it stands the new failure mode is loud in code and silent to whoever ran pnpm innerdogfood.

The reconnect hook covers the alert stores but not the registry. dormouse://workspaces rides the same stream — broadcast('sidecar', { event: 'dormouse://workspaces', … }) in standalone/scripts/dev-agent-browser.mjs — so a drop stales it the same way, and installWorkspaceRegistry already holds the re-fetch it would need (host.invoke<WorkspaceRegistrySnapshot>("workspace_registry")). Inert while the harness is one window, but it is the gap the seed replay just closed, one subscriber away.

Comment thread standalone/src/browser-sidecar-host.ts
Comment thread docs/specs/transport.md Outdated
**An unauthorized caller gets the same `404 not found` as an unknown path**, so the port does not identify itself. The harness prints the token and a ready-made `curl` on startup.

The harness **may omit** native-only desktop chrome (window controls, update checks) but **must preserve** every `PlatformAdapter` contract the app uses — PTY, control-request, clipboard, iframe-proxy, Burrow, agent-browser. It **must mirror** standalone's Session-persistence answer ("The governing rule"): the same `persistsSession`, one `PersistedWindow` per window (in `localStorage` rather than the Rust file store), and the same agent-recovery *claim* against a per-run temp state directory. **The harness must never capture**: a reload there is a live resume over PTYs that survive it, and capture is a quit-only step (`docs/specs/standalone.md` → "Agent recovery"). **Tauri APIs must not be required at static module-evaluation time** when `VITE_DORMOUSE_BROWSER_DEV_HOST` is set — a normal browser loads the page, not the Tauri WebView.
The harness **may omit** native-only desktop chrome (window controls, update checks) but **must preserve** every `PlatformAdapter` contract the app uses — PTY, control-request, clipboard, iframe-proxy, Burrow, agent-browser, and the sidecar-hosted alert stores (`alert_command` in, their broadcasts back; `docs/specs/alert.md`) — so a rule that only holds across the host boundary is exercised rather than answered by a private copy. **`BrowserSidecarHost.init()` resolves on the SSE stream being open, not on its construction**, so a seed cannot precede the stream that carries its reply; after a reconnect the adapter re-sends its last seeds so the stores republish (`standalone/src/browser-sidecar-host.test.ts`). It **must mirror** standalone's Session-persistence answer ("The governing rule"): the same `persistsSession`, one `PersistedWindow` per window (in `localStorage` rather than the Rust file store), and the same agent-recovery *claim* against a per-run temp state directory. **The harness must never capture**: a reload there is a live resume over PTYs that survive it, and capture is a quit-only step (`docs/specs/standalone.md` → "Agent recovery"). **Tauri APIs must not be required at static module-evaluation time** when `VITE_DORMOUSE_BROWSER_DEV_HOST` is set — a normal browser loads the page, not the Tauri WebView.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clause this pins is two rules, and the second lives in the other test: the re-send on reconnect is "re-sends its last seeds when the event stream reconnects" in the adapter test, while the host test covers only the open-before-resolve half. Citing both keeps AGENTS.md -> "House form for rules" ("Name the test that pins a rule") honest, and it still fits the budget you just ratcheted — I checked pnpm lint:specs against the edited line.

Suggested change
The harness **may omit** native-only desktop chrome (window controls, update checks) but **must preserve** every `PlatformAdapter` contract the app uses — PTY, control-request, clipboard, iframe-proxy, Burrow, agent-browser, and the sidecar-hosted alert stores (`alert_command` in, their broadcasts back; `docs/specs/alert.md`) — so a rule that only holds across the host boundary is exercised rather than answered by a private copy. **`BrowserSidecarHost.init()` resolves on the SSE stream being open, not on its construction**, so a seed cannot precede the stream that carries its reply; after a reconnect the adapter re-sends its last seeds so the stores republish (`standalone/src/browser-sidecar-host.test.ts`). It **must mirror** standalone's Session-persistence answer ("The governing rule"): the same `persistsSession`, one `PersistedWindow` per window (in `localStorage` rather than the Rust file store), and the same agent-recovery *claim* against a per-run temp state directory. **The harness must never capture**: a reload there is a live resume over PTYs that survive it, and capture is a quit-only step (`docs/specs/standalone.md` → "Agent recovery"). **Tauri APIs must not be required at static module-evaluation time** when `VITE_DORMOUSE_BROWSER_DEV_HOST` is set — a normal browser loads the page, not the Tauri WebView.
The harness **may omit** native-only desktop chrome (window controls, update checks) but **must preserve** every `PlatformAdapter` contract the app uses — PTY, control-request, clipboard, iframe-proxy, Burrow, agent-browser, and the sidecar-hosted alert stores (`alert_command` in, their broadcasts back; `docs/specs/alert.md`) — so a rule that only holds across the host boundary is exercised rather than answered by a private copy. **`BrowserSidecarHost.init()` resolves on the SSE stream being open, not on its construction**, so a seed cannot precede the stream that carries its reply; after a reconnect the adapter re-sends its last seeds so the stores republish (`standalone/src/browser-sidecar-host.test.ts`, `standalone/src/browser-sidecar-adapter.test.ts`). It **must mirror** standalone's Session-persistence answer ("The governing rule"): the same `persistsSession`, one `PersistedWindow` per window (in `localStorage` rather than the Rust file store), and the same agent-recovery *claim* against a per-run temp state directory. **The harness must never capture**: a reload there is a live resume over PTYs that survive it, and capture is a quit-only step (`docs/specs/standalone.md` → "Agent recovery"). **Tauri APIs must not be required at static module-evaluation time** when `VITE_DORMOUSE_BROWSER_DEV_HOST` is set — a normal browser loads the page, not the Tauri WebView.

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feedback on work in progress — not a merge verdict. Mark the PR ready when you want the full review.

The retry split lands: EventSource keeps its own reconnect for a CONNECTING failure, OPEN_TIMEOUT_MS still bounds it, and FakeEventSource now carries the readyState/CLOSED pair the assertion needs, so "rejects, and stops the stream, when it errors before opening" is pinning something again.

The new startup-failure UI governs Tauri, but only the harness spec says so. void bootstrap().catch(…) in standalone/src/main.tsx is unconditional — it catches a rejected TauriAdapter.init() (its listenToWindow round trips are awaited and can reject) and a rejected restoreWindowOrFresh, not just the harness's stream timeout. The rule you wrote lives in docs/specs/transport.md → "Standalone browser-dev harness", but bootstrap()'s owning spec is docs/specs/standalone.md → "Boot sequence", which declares Source of truth: standalone/src/main.tsx (bootstrap()) and today carries the neighbouring invariant "planArrival never throws into bootstrap()." A refused sole arrival on the boot path renders a fresh one-pane Workspace, never a blank window — the premise this change generalizes. A reader of standalone.md still sees a boot rejection as unhandled, and AGENTS.md → "House form for rules" puts each rule once in the spec that owns it. Either add the step-9 line there and leave transport.md a pointer, or keep the clause harness-local and gate the catch on BROWSER_DEV_HOST.

RegistryWorkspace.ref's null no longer has a producer. standalone/src/workspace-registry.ts declares ref: string | null under /** `workspace:<n>` for a minted id; `null` for one the registry did not mint. */, but Rust never produced it — ref_for in standalone/src-tauri/src/workspaces.rs is Some(ref_number(id).map_or_else(|| format!("workspace:{id}"), …)), pinned by assert_eq!(ref_for("workspace-abc12345-3"), Some("workspace:workspace-abc12345-3".to_string())). The harness's null was the last one, and this commit removes it. The doc comment is the half that was wrong, so tightening to ref: string with a comment naming the opaque-id fallback makes the type match both hosts; leaving it means every consumer keeps narrowing a case neither host can send.

One housekeeping note on the open thread above: its suggestion block reproduces the pre-commit paragraph, so applying it now would silently drop the retry/startup-error clause you just added. Worth re-composing rather than clicking apply.

Comment on lines +68 to +71
mkdir -p standalone/src-tauri/binaries
cp /Users/ntwigg/projects/dormouse/standalone/src-tauri/binaries/node-aarch64-apple-darwin standalone/src-tauri/binaries/
cd standalone/src-tauri
DORMOUSE_NODE_BINARY=/Users/ntwigg/projects/dormouse/standalone/src-tauri/binaries/node-aarch64-apple-darwin cargo test

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only absolute home path checked into the repo, and the recipe is the one a new agent in a fresh worktree runs first: cp reports no such file, DORMOUSE_NODE_BINARY then points at nothing, and cargo test fails with exactly the build-script error section 2 exists to steer around. Parameterizing keeps the warning above intact (the variable names the main checkout, so it can't be the worktree's own binaries/ path).

Suggested change
mkdir -p standalone/src-tauri/binaries
cp /Users/ntwigg/projects/dormouse/standalone/src-tauri/binaries/node-aarch64-apple-darwin standalone/src-tauri/binaries/
cd standalone/src-tauri
DORMOUSE_NODE_BINARY=/Users/ntwigg/projects/dormouse/standalone/src-tauri/binaries/node-aarch64-apple-darwin cargo test
MAIN=/path/to/your/dormouse # the main checkout, never this worktree
mkdir -p standalone/src-tauri/binaries
cp "$MAIN/standalone/src-tauri/binaries/node-aarch64-apple-darwin" standalone/src-tauri/binaries/
cd standalone/src-tauri
DORMOUSE_NODE_BINARY="$MAIN/standalone/src-tauri/binaries/node-aarch64-apple-darwin" cargo test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants