fix(hub): route interactive SSH prompts by owner identity, not bare host (#43)#49
Merged
Merged
Conversation
…on state machine (#43) Replace ad-hoc per-host acquire/waiter bookkeeping with a single SessionAcquisition owner (session-acquisition.ts) that serializes the lifecycle: - One acquisition per host, refcounted by per-spawn leases; the shared connect is aborted only when the last lease leaves (no leader-disconnect cancelling followers). - Single authority: the acquisition exists only while CONNECTING; it is deleted on a guarded commit (map-identity + state + not-aborted + session-current), so a late connect cannot revive a closed session and fast-path never coexists with an acq. - AbortSignal threaded through connect AND reconnect; closeSession/shutdown abort in-flight attempts; reconnect re-checks session currency before wiring (no revive). - Idempotent lease release on every exit path (no underflow, no orphaned session leak). - All map/prompt removals are identity-guarded; auth/host-key/agent-verify prompts are promptId-keyed with an owner, and re-target to a live lease-holder when the prompt owner disconnects instead of failing the shared connect. - SshAgent: reject (no unverified-binary fallback) on deploy failure; reject fast when the agent channel closes before HELLO instead of hanging. Design: docs/plans/session-acquisition-redesign.md.
…nt-verify response path (#43) Thread client.id through the full WS → session-manager → SshConnectionManager call chain for HOST_VERIFY_RESPONSE and AGENT_BINARY_VERIFY_RESPONSE. Make clientId a required (non-optional) parameter in both handleHostVerifyResponse and handleAgentVerifyResponse at every layer, and replace the optional guard (`clientId !== undefined && ...`) with an unconditional ownership check (`pending.clientId !== clientId`) that mirrors the existing SEC-003 pattern used for handleAuthPromptResponse. Stale or re-targeted clients are silently rejected; the pending entry and its trust decision remain intact until the correct owner responds. Tests: 6 new deterministic cases covering correct-owner accept, wrong-owner reject (pending entry intact, resolveSpy not called), and post-re-target old-owner reject / new-owner accept — for both host-key and agent-binary verify paths.
… reviewed (codex+copilot), multi-package scope
…ost (#43) Concurrent SSH connects, reconnects, and per-client test-connects now deliver their interactive prompts (passphrase, host-key TOFU, agent-binary verify, elevation) to the correct live client and dismiss stale dialogs — even when a client connects or disconnects in the middle of a multi-prompt sequence. Replaces the bare-hostId prompt maps with a PromptContext model: each prompt is owned by its acquisition / test / elevation / reconnect context and keyed by a unique promptId; the client is a mutable route. On owner-client disconnect the whole remaining prompt sequence retargets to a live lease-holder (or fails cleanly so followers retry), and the hub sends PROMPT_CANCEL so the dead client's dialog dismisses. Stale or rogue responses (wrong route, old delivery epoch) are rejected. Coordinated across packages: - shared: optional promptId+deliveryEpoch on AUTH_PROMPT/RESPONSE and a new PROMPT_CANCEL message. PROTOCOL_VERSION unchanged — additive/optional, keeps back-compat with cached web clients (the version equality check guards only the Rust agent HELLO handshake). - hub: PromptContext ops (open/prompt/respond/retarget/clearContext/ clientDisconnect) with store-before-send, terminal-CLOSED, deterministic route chooser, delivery-epoch and owner-id lifecycle guards; host-verify, agent-verify, passphrase, elevation, test-connect and reconnect all migrated onto them; the three legacy prompt maps removed; removeClient/closeSession collapsed onto clientDisconnect/clearContext. - web: prompt stores queue by promptId, echo promptId+epoch, handle PROMPT_CANCEL to drop a stale dialog. Preserves the original per-prompt timeouts (host-key / agent verify 30s, passphrase 120s, elevation 60s) and the 15-minute passphrase cache.
…ailConnectingAcq Every leader pre-commit failure path (connect catch, commit-guard failure, generic catch) now routes through one caller-local helper that fails the acquisition (Acq.fail -> rejects waiters + removes the acq so followers never hang), clears its PromptContext, and releases the leader lease. Closes the class the #43 gate surfaced (a catch that forgot to fail the acq, wedging followers). Behavior-preserving; existing acquisition tests unchanged.
…mpt stores auth-prompt / host-verify / agent-verify duplicated queue logic, which produced two #43 gate findings (response applied to the wrong prompt after the queue advanced; rememberSession leaking across prompts). The queue mechanics — FIFO + dedup-by-promptId, head computed, head-guarded actions (withHeadPrompt), removeByPromptId, PROMPT_CANCEL — now live in one composable. Store-local bits (WS payloads, rememberSession, deliveryEpoch echo, accessors) stay per-store; public store surface unchanged (no component/session.ts edits). Behavior-preserving.
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.
Closes #43.
What & why
The coalesced multi-client SSH connect issues interactive prompts (passphrase, host-key TOFU,
agent-binary verify, elevation). The previous handling conflated each prompt's identity, owner, route,
and lifecycle onto
(hostId → {resolve, clientId}), producing a long tail of cross-client edges thatnever converged (prompts bound to a disconnected leader, TEST_CONNECT clobbering a session prompt,
prompts surviving session close, stale owners answering after re-target).
This PR makes the acquisition the prompt owner and the client a mutable route: a
PromptContext { id, kind, hostId, routeClientId, prompts }, keyed by a uniquepromptId, with lifecycle cleared byowner-id. Every interactive prompt path (passphrase, host-key, agent-verify, elevation, test-connect,
reconnect) routes by the current live route and retargets — or fails cleanly — on owner disconnect; the
hub emits
PROMPT_CANCELso a stale dialog dismisses, and stale/rogue responses (wrong route, olddelivery epoch) are rejected.
Cross-package
promptId+deliveryEpochonAUTH_PROMPT/AUTH_PROMPT_RESPONSE, newPROMPT_CANCELmessage.PROTOCOL_VERSIONunchanged — additive/optional, back-compat with cached webclients (the version equality check guards only the Rust-agent HELLO handshake).
PromptContextops (open/prompt/respond/retarget/clearContext/clientDisconnect) withstore-before-send, terminal-CLOSED, deterministic route chooser, delivery-epoch, owner-id lifecycle,
and KIND discipline (test/elevation contexts never retarget to / cache for / cross-answer an unrelated
client). Reconnect routes to clients engaged with the session's channels. The three legacy prompt maps
are removed;
removeClient/closeSessioncollapse ontoclientDisconnect/clearContext.promptId, echopromptId+epoch, bind every response to therendered
promptId(no-op if not the queue head), scoperememberSessionper-prompt, and handlePROMPT_CANCEL.Original per-prompt timeouts (host-key / agent verify 30 s, passphrase 120 s, elevation 60 s) and the
15-minute passphrase cache are preserved. The validated session-acquisition state machine is untouched.
Verification
clean. No new skips.
caught 11 real defects the green suite missed because it exercised functions, not the production entry
points (WS handler dropping
promptId; elevation secrets cached as SSH passphrase; non-sessioncontexts retargeting to unrelated clients; acquisition wedging on a pre-commit exception with joined
followers; queued-prompt response races;
rememberSessionleaking across prompts). All fixed withregression tests at the production entry point.
Gate status (degraded GATE_OK — documented)
The final gate on the merged HEAD returned codex = CLEAN; copilot was exogenously unavailable
(transient outage) that run. Per the gate's set-union semantics a single CLEAN satisfies when the other
engine is exogenously down. copilot returned CLEAN on the immediately-prior HEAD, which differs from
the final HEAD only by a 3-file, web-only
rememberSessionper-prompt scoping fix that codex reviewedCLEAN.
Deferred follow-ups (tracked locally, low priority)
failConnectingAcq(acq, err)helper.usePromptQueue<TEntry>composable for the three web prompt stores.Both surfaced by codex as architecture signals; deferred to keep this PR scoped (no current correctness
gap — the specific defects are fixed).