Self-host Dormouse: service-based remote Host and review hardening - #416
Merged
Conversation
An assistant-run playbook for self-hosting the Dormouse `server` behind
Tailscale. Above the fold it covers the only path that exists today: build
the current checkout into a self-contained release under Application
Support, run it from a macOS LaunchAgent bound to loopback, and put
`tailscale serve` in front for private HTTPS at the laptop's tailnet name.
The always-on cloud relay (DigitalOcean + continuous deployment from `main`)
is designed but unbuilt, so it lives under `## Future` as the
`always-on-relay` scope per the AGENTS.md spec-lifecycle conventions.
Notes on two choices the runbook makes:
- The installed service listens on 3100, not 3000, because `dev:server` and
`dev:pocket-server` both take 3000 on the same laptop that runs the
installed copy.
- It requires adding `DORMOUSE_BIND_HOST`. `server/src/index.ts` calls
`serve({ fetch, port })` with no hostname today, so the server binds every
interface; the local install must not expose plaintext 3100 to the LAN or
the tailnet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The server always speaks plain HTTP and expects a TLS proxy in front. When
that proxy is local — `tailscale serve` on the same laptop — the listen
interface becomes a security boundary: `serve({ fetch, port })` with no
hostname bound every interface, so the plaintext port was reachable from the
LAN and from the tailnet itself, bypassing the proxy.
`DORMOUSE_BIND_HOST` closes that. Unset still binds everything, which is what
a container wants (the namespace is the boundary and the port is published
explicitly), so this is additive.
Env parsing moves out of the entrypoint into `server/src/config.ts` so the
mapping is testable without binding a port. `bind-host.test.mjs` spawns the
real entrypoint and asserts both halves: loopback answers and a non-loopback
address does not when the var is set, and the unbound default still serves
every interface when it isn't.
Also corrects the runbook's claim about VS Code Hosts. The blocker is not the
webview CSP: `enableRemoteHost` is passed only by `standalone/src/main.tsx`,
so the shared entrypoint the extension renders never loads the relay,
enrollment, or pairing modules. A VS Code Host is a feature, not a build flag.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Remote hosting was standalone-only, and not by any deliberate design: nothing in `lib/src/remote/host/` is Tauri-specific, but `enableRemoteHost` was passed only by `standalone/src/main.tsx`, so the entrypoint the VS Code webview renders never loaded the relay, enrollment, or pairing modules at all. Turning the flag on is not enough. Standalone is one webview per app; VS Code is many webviews over one extension host, and that breaks two assumptions the Host stack was built on. Storage. Enrollment and the ACL persist through `local-json-store`, which means `localStorage`. That is wrong twice in VS Code: webview `localStorage` is not the persistence story here, and `hostToken` is a bearer credential granting the `/ws/host` socket. `local-json-store` now takes per-prefix backend claims, and the webview hands `dormouse.remote-host.` to the extension host — enrollment to SecretStorage, ACL to globalState, both prefix-gated and size-capped so a webview can never reach unrelated extension state. Since the store API is synchronous by contract, `main.tsx` hydrates it into memory alongside `resumeOrRestore` before anything reads it. Election. Every webview mounts the same Wall, so each would start its own RemoteHost against the same enrollment, displace the others on the single socket, and arm its own alarm push. The extension host arbitrates a named `remote-host` lease — it is the only party that sees every webview and outlives each one — and re-offers it on dispose, so closing one Dormouse view hands the Host to another open one instead of dropping it until reload. Activation starts un-owned wherever a lease exists, so two webviews racing to mount cannot both activate before the first answer arrives. CSP. The webview `connect-src` had no remote origin, so the socket could not open regardless. The sources are now baked in at build time by a new esbuild wrapper, defaulting to the SaaS origin, with the same `DORMOUSE_REMOTE_CONNECT_SRC` per-build opt-in the standalone binary already uses. Host lifetime is "while a Dormouse view exists" — `retainContextWhenHidden` is already set on both hosting modes, so hiding the panel keeps it connected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixes from a four-angle review of the previous commit. The one that mattered: `activation.ts` called `claimSingleton` through a detached reference, so `this` was undefined and the first real webview would have thrown inside the adapter — after `owned = false` was already set, leaving a permanently disabled Host. The adapter binds every method reached that way and this one was missing from the list; the tests mock the platform as an object literal, so they could not see it. Now called as `platform.claimSingleton(…)`. Deduplication the previous commit set out to do and then didn't: the store prefix and enrollment key were re-declared in the extension with "Mirrors …" comments, though `lib/src/remote/host/store.ts` exists precisely to be the one definition. Both are imported now, and `store.ts` owns `ENROLLMENT_KEY` so the extension can have it without dragging `server-lib-common` into its bundle. Likewise `DEFAULT_REMOTE_CONNECT_SRC`, which had been copied into the new esbuild wrapper: the two Hosts keep their different substitution mechanisms but now share one definition in `scripts/csp-defaults.mjs`, so changing the SaaS origin cannot ship one Host pointed at the old one. `claimSingleton` also added a second `message` listener with its own copy of the auth guard, never removed, paying a token check on every `pty:data`. It now registers in a Map dispatched from the constructor's existing listener, so re-claiming replaces rather than stacks. Simplifications: the lease's `holds` set was derivable, so one module-level `singletonHolders` map holds the answer instead of N per-claimant sets; `readStore` no longer re-proves the prefix it already checked, and guards before the keychain read rather than discarding it after; the prefix registry is a Map with first-match instead of a sorted array with longest-match, since claims are documented as non-overlapping. Two real holes the review surfaced, both now closed: the console `enroll()` started a Host without consulting the lease, and the test that claimed to cover repeated grants never exercised the lease at all (it left `claimSingleton` unset, so both calls hit an early return). The console hook also outlives `vi.resetModules()`, which was letting one test call the previous module's closure. Also: restored em-dashes mangled in `vscode-ext/package.json`, fixed the `watch` script so `--watch` reaches esbuild instead of a trailing `cp`, and documented that the lease is per-window while the enrollment it guards is machine-wide — two windows still elect one holder each. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The lease hands the Host between webviews, but each webview hydrated the store once at boot and served every read from that snapshot. So a webview that mounted before another approved a pairing would, on taking the lease, load its stale ACL, start from it, and write the full record list back — dropping the pairing from globalState permanently, not just for the session. `clearEnrollment` had the mirror-image problem: one webview deletes the secret, another keeps `hostToken` in cache and starts a Host against a revoked enrollment. Committed writes are now broadcast to every webview as `store:changed` and applied to each cache, which is what makes the spec's claim — that closing one Dormouse view hands the Host to another — true regardless of when the others booted. `writeStore` returns whether it wrote so only real changes are announced. The broadcast includes the writer: re-applying your own write is a no-op, and skipping self would mean identifying it. Also from the review: - `store:write` and `singleton:claim` trusted their payload. `WebviewMessage` is a claim about the sender, not a runtime check, and a non-string key threw inside `allowed()` as an unhandled rejection instead of a refused write. Both now validate, matching what `store:read` already did. - The boot read inherited `requestResponse`'s 1s default while being gated on a `SecretStorage` unlock. A cold keychain could blow through it, installing an empty cache and leaving the Host silently un-enrolled — indistinguishable from never having enrolled. Budget is now 10s and a miss warns. The `try`/`catch` around it was dead: `requestResponse` resolves null rather than rejecting. - `esbuild.mjs` had no equivalent of the standalone drift guard, so losing the `define` would surface as a ReferenceError in `getWebviewHtml` and an empty webview, with nothing failing at build time. It now asserts the placeholder is gone from the bundle and the resolved sources are present. - Two doc comments pointed at a spec heading that does not exist, and the code map listed `scripts/esbuild.mjs` inside the `src/` tree it is not in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two real holes in the previous push. A broadcast that arrived while a webview was still hydrating was dropped: `applyStoreChange` walks the cache map, and the prefix only enters it once the read resolves. That is not a narrow window — the host snapshots `globalState` before it waits on the keychain, so another webview can commit a pairing that the in-flight snapshot cannot contain, and the widened timeout made the gap bigger. Reached through boot, it is the same permanent pairing loss the broadcast was added to prevent. Changes with no cache yet are now buffered with their value (a deletion has to survive too) and applied on top of the snapshot before it goes live. Raising the read budget to 10s also raised the blank-webview ceiling to 10s, because `main.tsx` gated `render` on it. The ordering constraint was never "hydrated before first paint" — it is "hydrated before anything reads a `dormouse.remote-host.` key", which happens when `installRemoteHostConsoleHook` runs, downstream of render in a lazily-mounted component. Boot now starts the read and publishes it via `setHostStoreReady`; `RemotePairingModalHost` awaits `hostStoreReady()` before installing. The terminal paints on `resumeOrRestore` alone. Also: the code-map entry closed the tree with a second terminator, one comment said `store:read` was below when it is above, and the `enroll` comment promised a handoff nothing performs — nothing signals the current holder, so the Host starts on the next lease grant or reload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The webview lease was per-extension-host, and VS Code runs one per window. With two windows open, each elected its own Host, both connected `/ws/host` with the same enrollment, and the server closed the displaced socket — whose `close` handler reconnects with backoff and displaces the other one. That is not a degraded mode, it is an endless fight, with each window arming its own alarm push. Multiple windows are a normal way to use VS Code, so window-local arbitration was never enough. A window may now grant the role only while it holds a lease recorded in the extension's `globalStorageUri`: per-extension, shared by every window, and with no cross-window change event to depend on, so ownership is a heartbeat with a TTL rather than a flag. The holder re-stamps every 5s and a record unstamped for 15s is free, which is what recovers the role from a window killed without running its disposables. A clean dispose unlinks the record, and a filesystem watcher lets the next window take over without waiting for its poll. Two cases the rules have to get right, both tested: a fresh claim is confirmed by re-reading, because two windows can judge the same record stale in the same instant and both write, and the loser must not believe it won; and a heartbeat stamped far in the future is treated as stale, or a clock jump would lock every window out until the skew elapsed. This makes the revocation path load-bearing. Losing the window lease is not just losing the right to be re-offered the role — the webview holding it is told `held: false` and stops its Host. A `/simplify` pass had flagged that branch as dead and suggested a grant-only protocol; keeping it was right. The decision logic and the cycle are pure and live in lib so the concurrency cases are testable without a filesystem; `window-lease.ts` is the fs and timers around them. Nothing starts until a webview first claims `remote-host`, so a user who never enrolls a Host never sees the file or the timer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The phone could only ever see one webview's terminals. Each webview is its own JS realm with its own xterm registry, so `collectDirectorySnapshot` listed the local registry and `surface.attach` resolved against it — meaning the bottom panel, and every editor tab, were invisible to each other. The lease made that deterministic rather than fixing it: one webview holds the Host, and its panes were the whole world. Most of what was needed already existed. `pty:input` and `pty:resize` go straight to `ptyManager`, ungated by webview ownership, so the Host could already drive a sibling's PTY; and pane ids carry a random suffix, so surface ids are unique across webviews without namespacing. The only real gap in the transport was streaming: `pty:data` reached the owning webview only. A webview may now subscribe to a PTY it does not own, tracked separately from `ownedPtyIds` so it never affects union status, `killOnDispose`, or ownership. Semantic events stay owner-only — they maintain the owner's pane state, and a subscriber wants bytes, not a second copy of that state. The extension host brokers the rest, being the only party that sees every webview: it fans a directory request out to the others and settles when they have all answered or a 1s budget expires, and it routes a surface op to whichever webview owns the id. Every webview installs a responder regardless of whether it is the Host, so its terminals are reachable from whichever one is; the responder is a registry lookup, the directory collector, and a resize, with none of the relay or enrollment machinery behind it. Attach and resize on a foreign surface go to the owner rather than to the PTY, because attach-is-the-resize has to drive the live xterm or the owning pane's view drifts from the size the phone set. The directory emits twice — local entries immediately, then merged once peers answer — so the phone never waits on a round trip to see the panes that are already here. This is the within-window tier. Reaching other windows needs a channel between extension host processes; the lease holder is the natural broker and `dor`'s control socket is the pattern, but none of that is built. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tier 1 gave the phone every terminal in one window. This gives it the machine, which is the case that actually matters — several windows open at once is the normal way to work, and until now the other ones were invisible. There is no shared process to broker through: VS Code runs one extension host per window. So the window holding the Host lease listens on a local socket and the others connect to it. The lease makes that one-directional — the webview lease is gated on the window lease, so the broker window is always the Host window, and a peer window only ever answers. Roles follow the lease. Acquire it and the window serves, publishing a mode-0600 rendezvous file naming the socket path and a token; lose it and the window tears the server down and connects as a client. Clients watch that file so a handover does not wait out the reconnect backoff. Sockets live in the temp dir because macOS caps a unix socket path near 104 bytes and the extension's globalStorage path is most of that by itself. A peer answers a directory or surface request by running its own *in-window* fan-out, never the cross-window one, or a request would loop back out. That is also why the link is injected with what it needs instead of importing the router that imports it. Once an attach succeeds the broker records which window owns that PTY: an id says nothing about where it lives, and input and resizes have to reach that window. Input and resize consult the table and fall back to the local manager; a subscribe asks the owning window to stream, and those bytes are injected into the subscriber's ordinary pty:data path, so the Host webview cannot tell a remote terminal from a local one. When a peer disconnects every PTY behind it is dropped and reported exited — a terminal in a closed window is gone, and a later write must not go into a dead socket. The framing and the routing table are pure and live in lib, so a split frame, a malformed frame, a peer that never terminates one, and a peer vanishing mid-attach are covered by tests rather than by reasoning. The sockets themselves are not — vscode-ext has no test runner. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three features have now landed with their I/O halves untested, on the grounds that `vscode-ext` had no runner. That turned out to be a thin excuse: most modules worth testing import `vscode` as `import type`, which erases, and the only runtime use in that graph is the output channel `log.ts` opens. So vitest plus a four-line stub is the whole setup. `peer-link.ts` did import `pty-manager` — and therefore node-pty — for exactly two calls, while injecting everything else. Those two move into `PeerLinkDeps` with the rest, which removes the last obstacle and drops an inconsistency the module already had. The tests are the ones that need real I/O, since the pure halves are already covered in lib: two lease instances contending over a real directory and handing over on dispose, and a broker and a peer over a real socket covering the rendezvous handshake, PTY routing, streaming, unsubscribe, token rejection, and what a disconnect does to in-flight terminals. One process plays two windows via `vi.resetModules()` and a dynamic import. The socket test immediately earned its keep: `startServer` runs fire-and-forget from the lease callback, so a failed rendezvous write surfaced as an unhandled rejection rather than a logged error. An unwritable globalStorage should mean no peers, not a crashed extension host — it now catches, logs, and tears the half-started server down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
From a four-angle review. Two of these are security properties, not tidiness. The peer-link socket guarded its token with `!==`, under a comment claiming parity with the `dor` control socket — which deliberately uses a constant-time compare, because `!==` leaks the token byte-by-byte to a co-resident process that can time the response. It now compares the same way. That file is CommonJS and the shared protocol module has to stay Node-free for the webview, so this is a second copy rather than an import; the comment now says so instead of claiming a reuse that does not exist. The rendezvous file carrying that token was written plain and chmod-ed 0600 afterwards, leaving it world-readable in between, and non-atomically — a reader catching the truncated window fell into the 2s reconnect backoff. It is now written 0600 to a temp file and renamed into place, which fixes both. A surface request that nobody in this window owned always burned the full 1s budget, because non-owners answered by staying silent. Since the common case for a miss is "it lives in another window", that was a second of latency on the path that matters most. Every webview now answers, and the broker settles as soon as they all have — the same shape the directory fan-out already had. A webview disposing mid-fan-out now releases surface requests too, not only directory ones. The window lease's watcher fired on the holder's own heartbeat, so the holder re-ticked on every write it made. Re-ticking there is wrong regardless of how much it actually costs: only a window waiting for the lease needs the accelerator. Overlapping ticks also shared one temp filename per window, so a collision failed the rename and dropped the role; writes are now uniquely named and cycles cannot overlap. A test pins the holder to its heartbeat rate. Honest note: that test passes against the old code too — the loop did not reproduce here — so this is a correctness fix, not a measured one. Deduplication the three reviewers agreed on: `PeerSurfaceResult` and the attach/detach/resize union were declared three times and inlined four more, and the reply budget twice with a comment asking to keep them in sync. All now live once in the protocol module, which the webview can import because it has no Node dependencies. `PeerRouteTable` was a Map with four delegating methods; only `forgetPeerRoutes` had behavior, so that is all that remains. Also: dead `resetWindowLeaseForTest`, an `ack` frame nothing correlated, an unused `LeaseState.dir`, a `stopped` flag that duplicated `state !== current`, five needless exports and five identity-arrow wrappers, a doubled size read in `#beginAttach`, a rendezvous watcher left running after a window became the broker, and a nested role switch flattened. The two new extension suites now share their fixtures instead of each defining `waitFor` and a temp dir. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`directory` and `surfaceOp` were each declared three times — in `PeerBridge`, in the webview↔extension message union, and in the cross-window frames — and implemented six. Adding a third peer operation meant editing nine files, none of it about the new operation. The tell was `PeerBridge.directory(): Promise< unknown[]>`: the platform layer was transporting a `DirectoryEntry[]` it refused to name, because naming it would have proved the operation does not belong there. There is now one operation: `(op, params)` in, zero or more results back. `op` is opaque to the adapter, to the extension-host broker, and to the socket, because *what* a peer may be asked belongs to the remote Host and not to any transport. The map with the real types — `directory` and `surfaceOp`, their params and results — lives in `remote/host/peer-surfaces.ts` next to the responder that answers them. A new operation is one entry there plus its caller. Absence is the miss. A webview that owns nothing the request named answers with no results, which deletes the `ok` flag from three layers and makes every field of a result that does arrive required, instead of an optional the caller had to `?? 0` its way past. The broker gains one fan-out rule where it had two, and the in-window and cross-window tiers are now asked at once rather than in series: what is asked about lives in exactly one place, so serial asking only meant paying a hung tier's budget before reaching the tier that owns the answer. The one thing a transport still reads out of an answer is a `ptyId`, and that is named as such (`routedPtyId`) rather than left implicit in a surface-op branch: an answer claiming a PTY is the only way the cross-window broker can learn which window that PTY lives in, and every later write, resize, and subscribe depends on knowing. `claimSingleton` and `peers` also collapse into one optional member. They carried near-identical doc comments — "only hosts that can show several webviews over one backend" — because they are two facets of one precondition, and nothing stopped a host implementing half of it. Now a host either has peers to elect among and ask, or it has neither. Subscribing returns its own unsubscribe, so a caller cannot leak a stream by losing the id it opened one with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`remote-api.ts` carried a `SurfaceTarget` union, a `targetSize`, a `#resizePeer`, and peer branches in `#attach`, `#beginAttach`, `#resize`, and `#teardownAttachment` — all to answer "is this pane in my webview or a sibling's". That is a fact about VS Code webview hosting, and it is not a protocol-v1 concept: `docs/specs/remote-api.md` does not mention webviews at all. The asymmetry was the tell. The same feature already makes a foreign PTY *transparent* for everything else: `pty:data` from another window is injected into the ordinary data path, and `pty:input` / `pty:resize` route by table before falling back to the local manager, so `#write` has zero branches. Only surface resolution had been pushed up into the protocol layer. `resolveSurface(surfaceId, size)` now answers with a `SurfaceHandle` — its `ptyId`, the size it stands at, a `resize`, a `release` — or `null` if nobody owns it. `#attach` is one `await`, `#resize` is `handle.resize(...)`, teardown is `handle.release()`, and the word "peer" no longer appears in the protocol layer's code. The size travels with the resolve because attach-is-the-resize: a sibling has to apply it inside the attach round trip, since there is no reaching into its xterm afterwards without a second one. A local pane is left alone there and resized by the caller, which subscribes to the PTY first so a synchronous repaint is not lost — the handle reports the size as it stands and the caller reconciles, so both paths keep exactly the sequence of round trips they had. This makes local attach asynchronous too, which is the honest shape: a pane in another window *is* a round trip away, and the alternative was one path that answered synchronously and one that did not. The suite that pinned the synchronous shape now awaits; nothing else about it changed, which is the point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Attach: `#lifecycleGeneration` only advanced on dispose, so two attaches in one session shared a generation. A peer surface resolves over a socket round trip while a local one resolves on the next microtask, so an older attach could land last and take the attachment from the newer one, breaking last-attach-wins. Bump per attach; the superseded one is answered with an error rather than left pending on the client. Peer link: `startServer` claimed the server slot only after an await, and nothing rechecked it before publishing. A lease flipping back to client mid-startup either left a rendezvous naming a just-unlinked socket — every peer dialing it, failing, and backing off — or made a window that lost the lease serve as broker. Claim it in the same tick as the guard, recheck before the rename, and abandon the half-started server without touching whoever holds the role now. Rendezvous watcher: no 'error' listener, so an async FSWatcher failure was rethrown and killed the extension host. Same directory and same hazard the lease watcher was hardened against in 559dacf; reuse its helper.
Cleanup pass over the previous commit, no behavior change beyond the two noted below. peer-link: `startServer` had three staleness branches with three cleanup shapes; now one `abandon()` closure gives back exactly what the attempt claimed, and `stopServer` shares its close-and-unlink with it instead of spelling it out a second time. `setPeerLinkRole` records the role it is transitioning into, so the client branch no longer installs the rendezvous watcher when the lease flipped back to broker while it was standing down — a broker watching wakes itself on its own writes. The `fs.watch` dance the last commit copied out of window-lease is now one `watch-dir-file.ts` owning both failure modes, which also stops peer-link importing the lease module it is deliberately decoupled from. remote-api: `#lifecycleGeneration` became `#attachGeneration`, since the attach epoch is all it tracks — the bump in `dispose()` was dead, `#disposed` is checked first and never cleared. Tests: the flip-back test asserts nothing is left behind at all (rendezvous, socket, temp) rather than one missing file, with sockets pointed at the test's own directory; shared fixtures replace the copied registry and gate setup. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Assigning `undefined` back sets the literal string "undefined", so the next test's mkdtemp tried to create `undefined/dormouse-ext-…`. macOS always has a TMPDIR to put back, which is why this only showed up on the Linux runner. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # docs/specs/server.md # server/src/index.ts
RemoteApiSession now speaks protocol-v1 against a HostSurfaceProvider and nothing else: where a surface lives, how a PTY is read, written, and resized, and when the directory could differ are all provider calls, so the session no longer imports the platform adapter, the stores, or document. The webview-resident binding of that seam (xterm registry + peer bridge) is assembled inline in activation.ts, since it is exactly the part a Node-resident Host replaces. The directory now emits one snapshot per collect: the provider answers for every reachable surface, so there is no longer a subset that is known sooner than the rest, and the old local-then-merged double emit existed only because the peer round trip was visible from the session. Session tests run against a fake provider (7 cases grow to 26, covering the attach-generation guard, the same-size bounce edge at one row, and release-on-stale-resolve); peer-surfaces tests keep exercising the interim webview provider end to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The interrupted simplify list completes: the pairing-queue seed re-arms on every enrolled transition (a window joining mid-pairing shows the modal), VAPID env parsing joins the server's config module with its both-or-neither rule tested, RemoteHost's webview-era option defaults become required (localStorage leaves both Node bundles entirely), the notify topic collapses to a bare ping end to end, one serial-queue helper replaces three lib copies, one FakeSocket serves three test suites, webview directory notifies coalesce on the trailing edge, the adapter's nine listens register in parallel, persistent is a required store fact, and push-devices can clear without dropping its refresher. From the review of the Codex commits: authorization gains a generation guard so an older connect2 evaluation resolving late can neither answer nor re-open the gate a newer attempt closed — remote-security-model.md now states the rule it previously implied; the state-dir chmod no longer fails a durable save on modeless filesystems; a link that can never settle refuses queued commands immediately; a destroyed client socket counts as unsettled; and the exit-during-deferred-resize attach answer is pinned by a test and recorded in remote-api.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fail-closed state reads: only a missing file (or a corrupt one, still loudly) may read as empty — any other read failure now rejects loads and refuses writes, so a transient EACCES at boot can no longer be memoized into an empty snapshot whose next save wipes every pairing. Lifecycle mutations persist before they believe: clearEnrollment deletes from the store before stopping the Host, enroll saves before starting, the enrollment fetch times out at 10s inside the serialized queue, and re-enrolling over a running Host emits the enrolled:false edge the webview gates re-arm on. The peer link survives its edges: a listening server keeps a permanent error handler (an accept-time EMFILE previously killed the extension host), a route is never claimed for a PTY this window owns (colliding cold-restored pane ids could steal the broker's own shell), the broker role is answered only once confirmed (no zombie service inside the reclaim verification window), an unwritable token store stands down once instead of retrying forever, a dropping window settles its in-flight asks, and a result frame is accepted only from the window its request was put to. Late ask answers now trigger a directory re-collect instead of leaving an idle machine's phone blank; never-enrolled windows answer the read-only commands with the canonical idle shapes (parity-tested against a real un-enrolled service); blank PORT is unset and PORT=0 is refused; the extension's scripts build server-lib-common like standalone's do; and an unparseable connect-src override fails the build with the same grammar the runtime enforces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying mouseterm with
|
| Latest commit: |
5ce9dce
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f3eb8e90.mouseterm.pages.dev |
| Branch Preview URL: | https://tailnet-deploy.mouseterm.pages.dev |
dormouse-bot
left a comment
dormouse-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.
I got here on 99787fb with a finding on stillOurs comparing inode numbers alone, and 2bd97f0 landed the same fix while I was writing it up — so this is just confirmation plus one nit.
Confirming the fix independently, since the rollup still shows the old red Build & Test: on an ext4 /tmp (what the runners use) I measured 20/20 inode-number reuse across unlink-then-rebind, and the exact corpse → rm → bind sequence the reclaim path takes reported { dead: '89566', now: '89566', same: true } — so the old now.ino === mine.ino really would have read a competitor's replacement socket as our own, and does not answer broker while a reclaimed bind is still unverified was failing deterministically rather than flaking. On this head pnpm exec vitest run test/peer-link.test.ts is 44/44 in ~6s, down from a 20s run dominated by that test's waitFor burning its whole budget. Adding dev alongside ctimeNs is the better call — I had only reached for birthtimeNs.
Worth a second look while you're in there: re-binds when the socket it reclaimed is unlinked out from under it was passing vacuously under the old comparison — its injected competing reclaim never fired, so it only ever asserted against the uncontested path. It exercises the real scenario now, but it was green throughout, which is the kind of test that is worth knowing can go quiet.
Nothing else surfaced. I read the service/provider seam, the state store, the connect-src matcher, the peer handshake, and traced the failure paths through RemoteApiSession#beginAttach; the unwinding is consistent and the fail-closed choices are deliberate.
Co-authored-by: dormouse-bot <ned.twigg+dormouse-bot@diffplug.com>
nedtwigg
marked this pull request as ready for review
August 20, 2026 00:45
dormouse-bot
left a comment
dormouse-bot
left a comment
Collaborator
There was a problem hiding this comment.
Full pass now that this is out of draft. My draft-mode notes on stillOurs and the vacuously-passing re-binds when the socket it reclaimed is unlinked out from under it still stand as written — nothing since 2bd97f0 touched either. One inline suggestion below, plus one observation.
The directory concatenates duplicate surface ids that askBothTiers deliberately disambiguates. askBothTiers handles the "Duplicate Workspace in New Window" case for the mutating path — probe resolve read-only, then send the attach only to the window carrying that provider-local PTY key. But collectDirectory returns [...local, ...remote] with no dedup, and buildDirectorySnapshot sets paneRef and surfaceId both to the pane id, so in that same scenario two entries with identical surfaceId, paneRef, and title reach the phone. directorySessionItems / directoryWallSessions then key the picker by that id, and PocketWall's auto-select takes attachableEntries[0], so which row the user picks is not what decides which window they get. Worth a line in docs/specs/vscode.md → "Peer surfaces across windows" if it's a knowing beta gap rather than something to collapse in collectDirectory.
…ctory The VS Code store now applies the same fail-closed rule as the file store: a rejected SecretStorage read is forgotten rather than memoized, because a locked or keyring-less keychain says nothing about what the store holds — and a memoized rejection left an enrolled window silently Host-less for its whole life, since onDidChange only fires on a write and nothing else ever retried. The directory deduplicates by surfaceId, keeping the first answer: duplicated cold-restored windows can hold panes with identical ids, and two identical rows made the phone's picker a lottery over which window an attach reached. First-from-the-concatenation is the same owner the attach path's read-only resolve probe selects, so the row shown is the surface attached. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dormouse-bot
approved these changes
Aug 20, 2026
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.
Supersedes #398 with the final review fixes.
Summary
This completes the beta-quality self-host path: run the coordinating server behind Tailscale and reach terminals owned by either the standalone sidecar or VS Code extension hosts.
Final review fixes
Each finding is isolated in its own commit:
Verification
pnpm testpnpm buildBoth pass locally. The specs and targeted regression suites cover the four review races and validation cases.
Beta caveat
The automated path is green; the live enroll → pair → attach flow still merits a human smoke test on a real tailnet before promoting this beyond beta.