Fleet thin client: #792 slices + #1260 + #1258 (→ feature/free-tier-fleet) - #1266
Merged
jeonghun-jj-lee merged 23 commits intoSep 19, 2026
Merged
Conversation
The client-role never-fork decision was mac-only: isFleetClientGuard early-returned on process.platform !== "darwin", so a linux/WSL client silently cold-spawned a local engine (the ADR-0005 split-brain, #1227). Extract the platform-agnostic decision divertToFleetRelay(binary, state) into fleet_topology.ts (the role read is already OS-neutral) and drop the darwin early-return. Never-fork now holds on mac, linux, and WSL alike, asserted by a non-darwin fixture + a source-guard against the return.
…ck (AC1/AC2/AC6)
A fleet client runs the amicode_service as a relay with NO local engine
(never-fork). Thread a `client` flag through createAmicodeService → the
FleetPlane:
- AC6: suppress the standalone→engine mode flip for a client (getMode stays
fleet) and emit the relay's OWN named hub-down 503 ("fleet-hub-down" +
pointer) instead of "engine upstream not available" — there is no local
engine to be unavailable, and no silent local fall-through. The engine-armed
machine keeps its existing behavior.
- AC1/AC2 (already-built, now pinned for the client config): UI served locally
(zero assets cross the WAN), host sessions via the proxied data plane, and
the credential translated (client mint stripped, hub mint attached).
The client relay refuses to START when its pinned version and the host's version (GET /global/health → version) disagree beyond a defined tolerance — an ACTIONABLE message naming both versions and the fix, never a confusing generic downstream timeout. Protects the host-API-shape assumptions #1262 / #1264 build on (moved here from #1265 — it is a relay-start gate). - fleet_version_skew.ts: pure versionSkewVerdict (tolerance exact|minor|major, default minor = major.minor must agree), relayVersionGate (start|refuse), hostVersionProbe (bounded, honest-null on failure). - Wired into startAmicodeService: a skewed host returns undefined (no boot); matching — or an unreadable host version (deferred to hub-down) — boots.
…n (AC5) #792 Amendment B's churn objection: an extension-host relay accumulating SSE upstreams on every window reload (a hub connection storm). Answer it with the lazy 1:1 pass-through it always was — plus the missing teardown: when the CLIENT disconnects (a reloaded window closing its stream), destroy the host upstream. So N reloads re-join each endpoint ONCE, never N× concurrent. - HubProxy + EngineProxy: res.on("close") → upstream.destroy() (skipping a normal writableFinished completion). - A client-initiated abort sets clientAborted so it never records a no-response outcome (a reload must not falsely drive the hub-down posture). Asserted by a concurrency-counting stub host: N reloads → N joins, maxConcurrent === 1; the posture stays fleet, no_response_streak 0.
The guard backstop was darwin-only in two places. Make it cross-platform: - install.sh: enrollment writes the platform-correct VS Code settings path — macOS Application Support; linux/WSL ~/.config/Code/User, plus the Remote-WSL/ SSH server-side ~/.vscode-server/data/Machine/settings.json when present — so amicode.opencodeBinary/opencodePort are set on a client of any OS. The guard --check host verification runs on every OS now (no non-darwin skip); the launchd TUNNEL stays darwin-only (the linux tunnel is #1260). Packaged copy kept byte-identical. - fleet_health.ts: checkFleetGuard/Settings/Role RUN on linux (only checkFleetTunnel stays darwin-specific); the aggregate standalone-floor is cross-platform. - extension.ts: the activation-time drift warning fires on linux too (the plist read stays darwin-only; the tunnel check self-skips).
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…cal dispatch (AC1/AC2/AC5) In fleet CLIENT mode the host owns all /amicode/* state. dispatch() now bypasses the ENTIRE local /amicode/* dispatch (the exact-match route table AND the :262 catch-all) so a REGISTERED route (GET /amicode/problems, POST /amicode/connections) no longer shadows the fleet branch — the request routes to the host's authoritative amicode_service (reads via HubProxy, non-GETs via the write-failure contract). /amicode/fleet/* stays LOCAL (the client's own posture/mode/staging honesty surface, #1261 AC6). Gated on the client role: standalone AND the engine-armed base machine are byte-identical. Extends #1261's stub-host relay test: a registered /amicode/* GET returns the host sentinel (AC1); the fleet honesty surface stays local (AC1); a mutation lands + is accepted host-side with the hub mint translated (AC2); standalone still serves /amicode/* locally (AC5).
…ency pin (AC3/AC4)
The write-failure contract's deriveRefetchPath was session-shaped: an
/amicode/* write fell through to /${segs[0]} = /amicode, dropping the mutated
family and yielding a refetch that hides a partial write. Add
deriveAmicodeRefetchPath — the /amicode/* variant that refetches the FAMILY
GET twin (POST /amicode/connections/credential → GET /amicode/connections),
and dispatch to it from deriveRefetchPath. So a proxied /amicode/* non-GET
resolves through delivered/failed/ambiguous with an /amicode/*-appropriate
refetch, never a raw proxy that hides a partial write (AC3).
Concurrency policy (AC4): LAST-WRITE-WINS with whole-request atomic delivery.
The write pipeline buffers each body fully and sends it in ONE request, so two
clients' bodies never interleave; the single-threaded host applies whole
requests, so the resource holds exactly one client's COMPLETE value. No
lost-update DETECTION (no CAS/ETag) — a client that read-modify-wrote stale
state can silently lose. A two-client concurrent-POST test pins it.
… (AC1/AC2/AC3) The #1261 relay proxied HTTP + SSE, but both proxies are body-pipe-only and the relay's HTTP server had no `upgrade` handler — so the integrated terminal (the engine's WS PTY route, GET /pty/:id/connect) was dead on a thin client: the handshake fell through to dispatch → the body-pipe HubProxy, which drops `Connection`, so the host never upgraded (answered 426). Extend the SAME fleet branch (fleetPlane.client === true && routingMode === "fleet") with an upgrade path: - server.ts: an `upgrade` listener on the relay's http.Server. A fleet CLIENT tunnels the upgrade to the host engine via HubProxy.handleUpgrade; every other posture (engine-armed base, standalone, no fleet plane) destroys the socket — the prior no-listener behavior, so loopback/never-fork is unchanged. - hub_proxy.ts: HubProxy.handleUpgrade re-implements credential translation for the 101/raw-socket path (the body-pipe proxies have no upgrade path to reuse). It forwards the engine's OWN 101 verbatim — status line + rawHeaders (the engine-computed Sec-WebSocket-Accept) + the head buffer — NEVER a synthesized 101; keeps Connection/Upgrade/Sec-WebSocket-* so the host upgrade fires; strips the client mint + ?auth_token= and attaches the hub mint; preserves ?ticket=/?cursor=/Origin. The host's PTY route authorizes by ticket OR Basic. - Clean teardown (AC3): any end/close/error on either side destroys BOTH sockets (pipe with { end: false }) so a dropped WS never leaks the counterpart via a half-open FIN; a pending pre-upgrade drop aborts the upstream request. - server.ts stop(): closeAllConnections() so a live WS/PTY tunnel at shutdown can't wedge server.close() on a hijacked socket. New test fleet_ws_upgrade.test.ts: a WS-echo test against a stub WebSocket upstream (no `ws` dep — hand-rolled framing). The stub returns a SENTINEL Sec-WebSocket-Accept a synthesized 101 could never produce, so the client observing it proves verbatim 101 forward. Covers handshake + both-way frames (AC1), the PTY route with hub-mint translation + ?ticket=/?cursor=/Origin preserved (AC2), leak-free teardown on an isolated host (AC3), and the scope-fence that a non-fleet-client relay never proxies the upgrade.
…undary dedupe (AC1/AC2/AC3/AC4)
The client relay's event stream sent no cursor and the engine's multiplexed
streams emit id: undefined, so a tunnel blip dropped every event in the gap.
The engine's per-session route /api/session/{id}/event?after=<seq> is resumable
(replay durable events after an aggregate seq) but unused.
Make the relay resume it losslessly, client-side (no engine change):
- SessionEventResume tracks the last delivered aggregate seq PER SESSION
(per aggregate, never one global cursor), carried across relay reconnects.
- HubProxy injects ?after=<seq> on a per-session reconnect and routes the SSE
body through a byte-verbatim dedupe/track filter (a replayed seq at or below
the resume point is dropped — idempotent resume, no double-delivery).
- SSE headers are flushed on connect so a fully-caught-up resume still opens
the stream (Node sends headers lazily; a zero-byte resume would otherwise
hang the client).
- The non-resumable /event and /global/event streams have no cursor form and
are LEFT AS-IS (never given ?after=, never assumed resumable) — documented
out of scope in ADR 0024; making them resumable is separate engine work
coordinated with #775, not forked from it.
Only armed for a fleet CLIENT (never-fork); the engine-armed base machine
falls back to its local engine on hub-down, so its steady state is unchanged.
Tests (test/fleet_sse_resume.test.ts, reusing #1261's stub-host harness):
AC1 drop-mid-stream loses zero events, gap replayed, boundary deduped;
AC2 per-session cursors resume independently (A@3, B@7);
AC3 /event + /global/event never given ?after=; AC4 steady-state byte-verbatim.
…y (AC1/AC2/AC4/AC5)
…p — single writer, transition-only (AC1/AC3)
…e-topology-reader guard (AC5)
…ed render (AC1/AC2/AC3) The relay's FleetPostureDetector computes the hub-up-but-slow DEGRADED steady state the extension's own up/down probe cannot; #780 left feeding it into the posture state file as an explicit seam. Wire it THROUGH #780's single writer: - fleet_posture_feed.ts: the RELAY→WRITER seam. Maps the relay posture vocabulary (fleet|degraded|standalone) onto #780's PostureFacts and drives the SAME FleetPostureStateWriter — no fs write of its own, no second writer. Transition-only is the writer's own signature discipline. degraded is hub-UP-but-slow (reachable=true), never rendered as fell-back. - extension.ts: checkFleet's fleet/standalone writes and the manual goStandalone fallback both go through the seam (recordPostureState) — one fact-builder, the one writer. goStandalone now persists standalone posture so the render stops claiming 'attached' the instant the user chooses standalone (AC1: never a stale attached claim). - stack_state.test.ts: locks the honest DEGRADED render (machine, hub, reachable-but-slow, timestamped, never as-if-attached) and OS-neutral copy (no launchd/plist) — coverage the relay-distinctive state lacked. #780's renderPostureLines already handled it; these guard it. AC1 render for fleet/standalone/missing/corrupt/stale was #780's; this adds the degraded-state coverage + the relay feed. The client-relay boot that co-locates the detector with checkFleet is a later #792 world-merge.
The pluggable-transport seam (ADR 0024) as a new module, fully unit-tested: - FleetTransportProvider interface (the Data Contract the tailscale/direct slices consume): resolveBaseUrl() -> URL|undefined, health() -> status. undefined base URL = the honest 'no forward bound' (hub-down), never a fallback to another provider's URL. - createSshProvider: the current launchd/systemd -L forward behind the seam. resolveBaseUrl() is the loopback hub URL (read late); health() is an active probe (reachable+latency+version | unreachable+reason). (AC2) - resolveFleetTransportKind: the amicode.fleetTransport selector — default ssh, independently disableable, a NAMED not-ok for a disabled/unshipped/unknown kind, NEVER a silent cross-provider fallback. (AC3) - transportHealthToOutcome: the SHARED drop-vs-slow-link posture contract this slice OWNS — maps any provider's health into the existing FleetPostureDetector (reachable -> responded -> p95 decides degraded; unreachable -> no-response -> streak decides hub-down); the provider kind rides the reason. (AC4) - sshForwardArgs + systemdTunnelUnit: the ssh provider's OS forms — the Linux systemd form of the launchd plist, loopback-only -L on both ends (ADR 0002/0005). - hubUrlStringFromProvider: the byte-identical adapter back to the getUrl seam. Provider-conformance tests reuse the #792 relay stub-hub contract (test/support/ stub_hub.ts). 23 tests.
…no fallback (AC1/AC3) startAmicodeService constructs the transport provider from the fleetTransport.kind setting (default ssh) and derives hub.getUrl from provider.resolveBaseUrl() through the byte-identical adapter: - unset/ssh reproduces today's launchd-forward behavior exactly — the 13 existing activation-wiring tests stay green (the no-regression gate). (AC1) - a not-yet-shipped provider (tailscale/direct) binds NO base URL -> the honest 'hub upstream not available' 503, NEVER a dial of the configured ssh URL (no silent cross-provider fallback). (AC3) - the boot log names the transport selection outcome (never a silent no-op). The late-bound hub-URL getter is preserved (a de-armed activation still yields the honest upstream absence). 3 new end-to-end wiring tests; 16 total in the file.
… (AC1) - package.json declares amicode.fleetTransport (enum ssh|tailscale|direct, default ssh) — the one knob; the description states the no-fallback law and that unset/ssh reproduces today's behavior. - extension.ts reads it (readFleetTransportOption) and passes fleetTransport.kind to both startAmicodeService call sites. Empty/unset -> the wiring defaults ssh.
…ifecycle The Data Contract lists resolveBaseUrl() + health() + start()/stop(). Add the lifecycle members to the provider interface. The ssh tunnel is OS-managed (launchd/systemd, installed by the fleet installer) in this build, so its start()/stop() are honest no-ops that defer to the service manager — the seam members exist so the tailscale/direct slices (which own their lifecycle) implement them behind the SAME interface. 24 tests.
…bind preservation (AC tailscale slice) The host runs `tailscale serve` fronting its loopback service; the client resolves the host's MagicDNS origin as the base URL. createTailscaleProvider implements the SAME FleetTransportProvider interface as ssh (resolveBaseUrl → the MagicDNS origin, read LATE; health() an active probe; start/stop honest no-ops — serve is host-side, tailscaled is a system daemon). MagicDNS + the `tailscale serve` mapping are modeled behind injectable seams (no `tailscale` binary in CI), exactly as the ssh provider models its `-L` forward. - resolveFleetTransportKind ships `tailscale` in the available set; it is independently disableable. `direct` remains the unshipped seam member that pins the no-cross-provider-fallback law. - transportForSelection maps a selection → ITS OWN provider (tailscale → the tailscale provider, never a silent ssh substitution); the host wiring uses it. - tailscaleServeMapping fronts the MagicDNS origin onto a LOOPBACK target (127.0.0.1), so the host engine/service bind stays loopback and the mutation guard never trips — asserted TOGETHER with MagicDNS resolution in the bind-host guard suite, with the rejected direct-tailnet-bind (100.x) contrast. - health() reuses transportHealthToOutcome (the ONE shared posture contract); an unreachable host is the honest hub-down, never a reroute to ssh. - #775 front-door composition: the serve target is the same loopback front-door endpoint the ssh forward reaches, never a bypass. - Refactor: extracted the shared probeTransportHealth (ssh + tailscale).
…robe, no tunnel lifecycle (AC direct)
…anti-substitution (AC direct / shared AC)
…e / systemd WantedBy+Restart, run the #955 runner not a bespoke serve (one writer, ADR 0005)
…only) — launchd/systemd-user unit runs the #955 runner with RunAtLoad+KeepAlive, one writer (ADR 0005), never-fork gate on the parsed role (ADR 0023)
jeonghun-jj-lee
marked this pull request as ready for review
September 19, 2026 02:50
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
1 participant
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.
Single integration PR for the fleet thin-client campaign, targeting
feature/free-tier-fleet(NOTmain). Every slice was implemented TDD (RED→GREEN) and gate-verified by the director (typecheck + full suite) before banking; slices merged sequentially on this one branch.Design of record: ADR 0024 (
docs/adr/0024-fleet-thin-client-pluggable-transport.md, already on base).Slices — all green (director-verified)
/amicode/*to the host (host owns all state); last-write-wins concurrency,/amicode/*refetch variant.?after=cursor resume + boundary dedupe).~/.amico/ops/fleet/posture-state.json) + plugin render (sole-writer discipline). [Aaron's issue — implemented here as the Fleet thin client — Slice 5: honest degraded posture (consume #780) #1265 prerequisite; review welcome.]amicode.fleetTransportknob (defaultssh);ssh(launchd/systemd forward behind the seam),tailscale(tailscale serve+ MagicDNS, loopback preserved),direct(supplied URL). All three selectable + anti-substitution-safe.install.shcopies (byte-identical).Test posture
test/ops/skill_freshness_orchestrator.test.ts(SKILL_FRESHNESS_SEARCH_ROOTS, Drift-lint false positives: bang boundary, comment-scope extraction, ext scan, search roots #1002), present at base269c653c— zero new failures introduced. Typecheck exit 0.Known integration seam — the #792 world-merge (flagged by #1261 AND #1265) — NOT in this PR
extension.tsclient-activation boot still frames the tunnel engine origin directly (the fleet-client branch boots no relay; basestartAmicodeServiceboots the relay with no fleet option). The relay data plane + posture feed + transport are built, typed, tested, and wired at the correct seams, but no running process co-locates them for a client yet. The last-mile "boot + frame the local relay for a client" wiring is the remaining integration step. Deliberately not faked — a synthesized live relay poll would read the hub's posture, not the client's (a dishonest posture), which is exactly what this work refuses to do. Recommend a dedicated follow-up issue for the activation-boot world-merge.Other documented follow-ups (additive — not regressions)
fleet_health.tscheckFleetTunnelstill self-skips Linux;systemdTunnelUnit/fleet_hub_servicerenderers exist and (for the hub service) are installer-wired, but the tunnel's Linux health-check arm isn't wired.checkFleetpoll still uses the old inline probe; rewiring it totransport.health()is a clean shared follow-up (posture already works via the hub proxy's data-plane outcomes).Part of #792. Do not merge to
main— target isfeature/free-tier-fleet, merge is the maintainer's call.