Step 21 (S2): persistent SSH sessions and reattach #39
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
PLAN step 21: SSH sessions survive a closed client or a dropped tunnel, and any authorized web, CLI or MCP client can reattach by session id and output offset, under the owner, target and bounded-replay rules of contracts §3.11. This is a security surface, so it runs on Sol.
Step 21 start: branch
ssh-reattach, basedbd66850f8a5d6190e65b4bd364e075950033425. Contract-first commit records the remaining client behavior and detached-session authorization in §3.11.Gap analysis against §3.11:
crates/ssh/src/session.rskeeps persistent PTYs after attachment drop, has 1 MiB per-session replay and 64 MiB aggregate budget, idle reaper, 64 total / 16 per-user / 16 per-host limits, open-key dedupe, owner-kind-target binding and one-writer takeover.crates/server/src/stream.rsvalidates a reattach against durablestream_sessions; stream OPEN usessession_id/resume_fromand returnsoutput_offset/replay_start/gap. The existing real-sshd product test covers close then reattach and attached-token revocation. The sessions API lists SSH rows and closes them. CLI already hasssh --attachwith a required host, andsessionslisting.ssh --attach <id>without a host, CLI resume-offset option, and all four MCP session tools. No real-sshd tests explicitly kill a CLI process, drop its network transport mid-output, or cross between web-style stream, CLI and MCP.resource_exhausted/not_founderrors. Contract is explicit on one writer and takeover, so no decision is needed.Contract commit: see branch head after this comment. I will build only the gaps above; no web files.
Finding:
refresh_sessionrevokes the old access-token id and issues a new one in the same refresh family. A detached-session check against only the original token id would end ordinary long-lived SSH sessions at credential refresh, despite unchanged grants. The implementation resolves the latest active token in that family for the detached sweep, then checks both current role and token grants. A revoked/expired family has no live successor and ends the session. §3.11 now states this explicitly.Dependency finding: at base
dbd6685,crates/agent/src/lib.rsstill describes its framed tunnel OPEN path as used by a future network tunnel; the current server stream routes through the embedded infraternal agent. Step 19 owns remote tunnel transport in parallel. I added a real-sshd test that drops the entire client WebSocket while output is pending and reconnects by session id/offset. A real remote-tunnel-drop test needs step 19 integrated and belongs in the step-26 combined end-to-end gate; this branch cannot truthfully claim that network hop was exercised.Verification so far (branch head
e56f582):scripts/verify fastpassed after the four clippy findings were fixed. Verbatim gate lines:Focused real-sshd product test (includes the new transport-drop and detached-token-revocation cases) passed:
test host_trust_stream_replay_resize_close_and_changed_key ... SSH_STREAM_ROUND_TRIP samples=40 p50_ms=1.256 p99_ms=2.124, then1 passed; 0 failed. CLI/MCP real integration and full integration gate are next; load5 must fall below 8 before the full gate.Focused compiled CLI/MCP integration first run failed before reaching the new SSH cases, in the existing container exec PTY timing assertion at
crates/infraternal/tests/cli_mcp.rs:2332. It expected initial24 80output but observed only47 123twice after SIGWINCH; load5 was about 9 during the run. This is the load-sensitive timing class tracked by #31, not an SSH assertion. Per the job rule I will rerun this single test directly once when load5 is below 8 and quote both outcomes. I have not changed the unrelated exec test.Required single direct retry of
cli_and_mcp_use_the_public_http_contractalso failed at the existing container exec PTY resize assertion, afterexercise_cli_sshhad returned successfully (that SSH helper runs near the start of this test, before the failing exec section). First run: line 2332 expected24 80, observed only47 123twice; second run: line 2336 expected47 123, observed only24 80twice. The opposite observations under concurrent VM load show the test's current timing sensitivity (#31). I did not rerun it again or modify the unrelated exec path. The focused real-sshdssh_producttest passed independently.Final-head verification (
b11e0d8):scripts/verify fastpassed again after fixture run IDs and MCP read annotations. Verbatim output:Full
scripts/verify integrationstarted after load5 fell below 8 and the other release build finished.Full
scripts/verify integrationwas started with load5 below 8 after the previous release build ended. It passed release build, real HTTP, and Docker/rootless Podman socket checks, then stopped at an existing engine PTY cleanup assertion before CLI/SSH steps. Verbatim gate lines:The failed case was
pty_session_drop_and_close_terminate_exec_processes_on_all_real_engines; atcrates/engine/tests/integration.rs:875, a Podman shell PID remained in the process table after drop. The test suite result was10 passed; 1 failed; 0 ignored, under load5 above 10 after other jobs started concurrently. This is outside step 21's owned files. I will run that single case directly once below load5=8 and quote the result; I will not change the engine adapter from this branch.Required single direct retry of the failed engine cleanup case started at load5=7.84 and failed independently in the same unrelated engine test, at an earlier cleanup assertion. Verbatim result:
First full integration run had
10 passed; 1 failedand found a Podman shell still in the process table at line 875. This direct retry failed at line 743 with unconfirmed exec cleanup even at low load. It is an engine cleanup defect outside this job's owned SSH/server/CLI/MCP files, so I am not changing it here. The full integration gate cannot reach its later SSH steps until that upstream gate is green; focused real-sshd SSH tests are being run separately.Step 21 branch work finished at
b11e0d896e1953ff910504178193872a312ee3ce(ssh-reattach, basedbd6685). No push; worktree clean. The contract, detached-session creator authorization across normal token refresh, CLI attach-by-id/offset, bounded MCP list/attach/send/read, and real-sshd cases for SIGKILL, client socket drop, MCP cross-client reattach, and detached revocation are committed. No web files changed; no screenshots apply.Final
scripts/verify fastoutput, verbatim:Focused real-sshd gates, verbatim result lines:
The compiled CLI/MCP test executed and returned from the new SSH helper on both runs, then failed later in existing exec PTY dimension assertions (first missing initial size, direct retry missing resized size). Both outputs are quoted in earlier comments.
Full
scripts/verify integrationoutput until its stop, verbatim:The engine test suite was
10 passed; 1 failed; the failed Podman PTY cleanup assertion is quoted above. Its required single direct retry at load5=7.84 failed withexec_termination_unconfirmed, also quoted above. This is outside this branch's owned engine files, and the integration script never reached later SSH steps. A real remote-tunnel-drop test remains dependent on step 19's network tunnel transport and should run in step 26 after integration; the current branch tested full client-WebSocket loss against a real sshd. No issues were closed.Independent review-21 started on branch
ssh-reattachatb11e0d896e1953ff910504178193872a312ee3ce, against basedbd6685. I will inspect the step 21 diff and contracts, run targeted verification in a throwaway worktree, and report findings with evidence. The review worktree remains read-only.Review findings in progress:
list_ssh_sessionscollects the full/sessionsresponse (crates/infraternal/src/mcp.rs:712); storage selects every owned row withoutLIMIT(crates/server/src/storage/mod.rs:1733). Exited rows persist, so the MCP list has unbounded result size despite the 64 live-session cap. I am treating this as a bounded-output contract defect.ssh_producttest failed on a clean branch run atcrates/server/tests/ssh_product.rs:648: immediately after receiving channel CLOSE, the database still reportedAttachedinstead ofDetached. I am checking whether this is an async state-observation race and will record the final conclusion in findings.md.NotFound). This confirms that test detects the intended regression.Independent review-21 verdict: request changes. Full findings with severity, file:line, scenario and fix are in
/mnt/hdd/infraternal/jobs/review-21/findings.md.Blocking findings:
list_ssh_sessionsis unbounded because/sessionsfetches every owned row, including retained exited rows (mcp.rs:712,storage/mod.rs:1733); ordinary attach/detach lacks a session-specific audit record (stream.rs:1232,:2348,:2369). The new real-sshd test also has an immediateDetachedassertion race atssh_product.rs:648: it failed on the clean branch, then passed in the throwaway checkout with a bounded wait.Confirmed: CLI attach-by-id/offset and MCP attach/send/read code paths, refresh lineage and fail-closed revocation/role checks, same-owner/kind/target authorization on server and infraternal agent, one-writer takeover, bounded replay/read/send, session quotas and idle expiry. A same-owner AI agent token needs explicit
hosts:readandssh:connectgrants to reach a person's SSH session. Abrupt client WebSocket loss was tested against real sshd; the remote infraternal-agent tunnel hop depends on step 19. Two tests were proven sensitive to their fixes in the throwaway checkout: disabling refresh-lineage fallback failedstorage.rs:344withNotFound; disabling detached-session grant-loss termination failedssh_product.rs:696with the one-second revocation timeout (after a test-only wait for the state update).Podman gate diagnosis, tagged load-flake with uncertainty: the unchanged engine test
pty_session_drop_and_close_terminate_exec_processes_on_all_real_enginesfailed on this branch atintegration.rs:875(PID remained) and on direct retry at:743(exec_termination_unconfirmed) per #39. On detacheddbd6685, the same single test passed when started at load5 8.39:test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 10 filtered out; finished in 44.31s. The base pass excludes a deterministic branch-only engine regression; the exact load threshold is not established. Re-run the full integration gate after the step 11c cleanup path is stabilized.Review
scripts/verify fastoutput, verbatim:Review worktree
ssh-reattachwas not edited. No issue closed.fix-21 started on branch ssh-reattach at base
b11e0d896e. Fetched and merged current dev; already up to date. Scope: review findings 1–3 only; crates/engine and finding 4 excluded.Finding 1: stream_sessions had an unbounded owner fetch. GET /sessions now reads at most 51 indexed rows, returns up to 50 with next_cursor, and offers exact owner/grant checked GET /sessions/{id}; CLI and MCP use exact lookup for attach. The web Terminals route is currently a placeholder with no API caller. Existing index stream_sessions_owner_created_idx matches the cursor order.
Finding 2: attach, takeover, and detach now use a transaction to commit state and actor-attributed audit together; migration 0016 retains the current attachment token id. Storage regression covers takeover, ordinary detach, and audit-failure rollback.
Finding 3: real-sshd test polls for Detached with a one-second bound before revocation.
Regression mutation proof on
d90cdc7(all mutations restored; tree clean): removing both SQL LIMIT 51 clauses made session_pages_are_bounded_and_attachment_audit_is_atomic fail at storage.rs:83 (expected [51,51,21]); changing stream.session.attach to attach_removed made the same test fail at storage.rs:108; replacing the bounded Detached poll with the original immediate assertion made host_trust_stream_replay_resize_close_and_changed_key fail on attempt 1 at ssh_product.rs:681 (Attached != Detached). The unmutilated storage test, HTTP page test, and real-sshd test passed. scripts/verify fast passed all 15 steps; integration waits for another job release build to finish and load5 below 8.Integration gate on
551d063was started at load5=5.66. It passed release build, real HTTP, Docker/rootless Podman pings, engine adapters, and bounded log snapshots, then failed at TypeScript stream client against Docker and Podman. Exact failure: stream_e2e.rs:366stream close left an exec shell in podman; the process table showed/bin/shstill present. This is a different test from the two named known flakes. Other integration jobs were concurrent at the time; I am waiting for resource contention to clear before a direct reproduction. No crates/engine changes.Direct retry of the failed, non-enumerated integration test after other integration jobs ended passed verbatim:
The first gate failure is therefore timing-sensitive under concurrent live-engine runs, not yet proven branch-caused; it is distinct from the two named known flakes. I will rerun the full integration gate after load5 falls below 8.
Second full integration gate on
551d063started at load5=7.47. It passed release build, HTTP, Docker/Podman pings, engine adapters, bounded log snapshots, and the previously failing TypeScript stream test. It then failed in stream_regressions.rs:755:authorization_loss_terminates_stalled_stdin_on_both_enginesreportedsleep did not start on docker;psshowed/bin/shbut nosleep 3600within five seconds. Eleven other tests in that suite passed. This is another non-enumerated timing failure under concurrent browser load. No engine or test edits. I will run the failed test directly after the browser suite ends and load falls.Direct retry of the second integration failure passed verbatim:
Both non-enumerated live-test failures reproduced only under concurrent load and passed direct retries; I will make one more full integration attempt when load5<8 and no other heavy suite is active.
Fix round head:
551d063562on ssh-reattach. No push. Findings 1–3 fixed and regression mutations proved; finding 4 and crates/engine untouched. The web Terminals route had no GET /sessions caller on this branch. The commit is clean.Verbatim scripts/verify fast output:
Verbatim scripts/verify integration attempt 1 output (started load5=5.66):
Verbatim scripts/verify integration attempt 2 output (started load5=7.47):
Both non-enumerated timing failures passed direct retries, quoted in preceding comments. Integration is not green; do not merge on these results alone. A third attempt was stopped during web build before live tests when its own prestart load5 reading was 8.31, above this job's cutoff. Other jobs currently have concurrent release builds and browser suites; a clean third attempt is pending.
Fix-round handoff at head
551d063562, branch ssh-reattach, clean worktree; no push. The three assigned findings and regression mutations are complete.scripts/verify fastpassed all 15 steps (verbatim output above). Both completescripts/verify integrationattempts failed in different non-enumerated live tests under concurrent load; each failed test passed its direct retry (verbatim output above). Neither failure is being labeled one of the two named known flakes. A third attempt was stopped during web build because its own prestart load5 was 8.31. As of this handoff, other jobs keep load5 above 8, so the required isolated full integration pass is pending. Do not merge until that gate passes or the owner accepts the documented limitation.progress.mdis ignored and contains this one-page handoff. Finding 4 and crates/engine remain untouched.Verification started on branch
ssh-reattachat base/head551d0635628e357dca68f7228221ff6eb35de070. I’m checking findings 1–3 from the review against the fix round, including test mutation proofs and web/audit consumers, then I’ll merge currentdevand run the requested gates. Integration will wait until load5 is below 8, with two-minute polling for up to 90 minutes.Findings 1–2 verification on
551d063(throwaway/home/kayg/wt/verify-21-tmp):SqliteStorage::owned_stream_sessionsusesLIMIT 51; the HTTP handler emits at most 50 rows and returns a cursor. The storage test seeded 121 rows. Fixed version:1 passed; 0 failed; 21 filtered out. Removing the twoLIMIT 51clauses made that test fail:left: [121, 71, 21],right: [51, 51, 21]. The HTTP page/authorization test passed:1 passed; 0 failed; 3 filtered out.session_id,attachment_id, andreason; targets are session id and host/engine target. The actor helper reconstructs user/token IDs, token kind, and auth source from the token row; no bearer, SSH key, command, environment value, or terminal data is serialized. Fixed storage test:1 passed; 0 failed; 21 filtered out. Renaming attach and ordinary-detach actions in the throwaway made it fail at the audit count (left: 1,right: 4).The Terminals route is still a placeholder (
packages/web/src/routes/terminals/+page.svelte); there is no web caller ofGET /sessionsto regress. MCP and CLI consumeSessionListResponse, and MCP forwardsnext_cursor.Finding 3 is closed. In
ssh_product.rs, after the channel CLOSE frame is received, the test polls forDetachedfor up to one second before revoking the creator token; this matches the server’s order of queueing CLOSE beforerelease_session_pairpersists detach metadata. The ignored real-sshdtest was run with--ignored --exact:1 passed; 0 failed; 0 ignored; 0 measured; 7 filtered out. In the throwaway, replacing the poll with the prior immediate assertion and adding a test-only 500 ms delay after CLOSE was queued made the assertion fail withleft: Attached,right: Detached. The test-ownedTestSshdcleanup reaps its listener/watchdog and marked PTYs on unwind.Integration attempt 1 failed at the pre-existing Podman exec cleanup test (#42); the gate stopped before server/MCP integration tests. At failure observation 11:28:49,
uptimewasload average: 9.55, 8.84, 9.09(load5 8.84). The branch changes nocrates/enginefiles.Gate failure output:
Required direct retry (one run, same real-engine test):
Verdict: NOT VERIFIED. Step 21 findings 1–3 are closed, but integration has a blocker in the new CLI SSH reattach path. No fix was made because the failure is not caused by merging
dev, and this job only permits merge-caused fixes.Merge head:
551d0635628e357dca68f7228221ff6eb35de070.origin/devwas fetched atdbd66850f8a5d6190e65b4bd364e075950033425, already an ancestor; merge reportedAlready up to date.No push.fastexited 0. Verbatim gate output:Integration attempt 1 stopped at the known Podman exec cleanup flake (#42); it reported 10 passed and 1 failed. The failure and the required direct retry (
1 passed; 0 failed) are quoted in my preceding #39 comment. Load5 at failure observation was 8.84.Integration attempt 2 started at load5 6.84 and passed through the server resource API, then failed in
cli_and_mcp_use_the_public_http_contract. The gate first observed the Podman exec exit-code assertion (Some(7)instead ofSome(23)) at load5 9.48. As required, I ran this test directly once. That retry reached the Step 21 CLI SIGKILL/SSH replay case and failed atcli_mcp.rs:2887because the post-kill output marker was missing. Load5 at that failure observation was 9.06. This is the blocker; no second direct retry was run.Verbatim output from integration attempt 2:
Verbatim direct retry output:
Starting fix-21b on branch
ssh-reattach, base head551d0635628e357dca68f7228221ff6eb35de070. I am tracing the replay-marker scenario, then will measure the requested 20-run branch loop and 10-run devdbd6685loop before choosing a product fix or deterministic test correction.Starting fix-21b on branch
ssh-reattach, base head551d0635628e357dca68f7228221ff6eb35de070. I am tracing the replay-marker scenario, then will measure the requested 20-run branch loop and 10-run devdbd6685loop before choosing a product fix or deterministic test correction.Reproduction evidence before the deterministic test change: one full current-branch run failed at the reported
__CLI_SSH_REPLAY_AFTER__assertion. The focused SIGKILL loop passed 20/20 (0/20 failures; load averages at start 8.21/6.29/6.94 and end 9.91/9.08/8.11). Ondevdbd6685, the SIGHUP loop passed 9/10; the single failure timed out waiting for the CLI to exit after captured output already containedAFTER, so it was not a marker assertion failure (load start 8.34/9.24/8.46, end 7.59/7.86/8.04). Incrates/ssh/src/session.rs, output is appended to the replay buffer before it is fanned out. The test reattached immediately after killing the CLI, five seconds before the remote command wroteAFTER; I am treating this as a test race and verifying with an observable-state wait.Root cause: (b), a test race. The CLI test killed its client after
BEFOREappeared, then reattached immediately while the remote shell was still insleep 5; it did not wait forAFTERto be produced. The PTY harness also stopped draining shortly after client exit.crates/ssh/src/session.rsappends each output chunk to the replay buffer before fan-out (record_output, lines 790–805). The deterministic test now waits for detached state, waits for a unique remote progress file written afterAFTER, confirms the actual bytes with bounded MCP reads, and then requires the CLI reattach to replay them before the shell exits.Reproduction evidence (load averages shown as 1/5/15 minutes):
7.73, 8.12, 8.74.8.21, 6.29, 6.94and ended9.91, 9.08, 8.11.devdbd6685SIGHUP loop: 0/10 marker failures; 9/10 completed, and the other run hit the PTY harness's CLI-exit timeout after captured output already containedAFTER. Load started8.34, 9.24, 8.46and ended7.59, 7.86, 8.04.2.01, 3.25, 5.06and ended7.25, 5.28, 5.42.Head:
fed23a4818ed53a78872b61143ddacccbd5a649donssh-reattach(not pushed). Both gates passed; output quoted verbatim:Starting merge job on branch
ssh-reattach, based atfed23a4818ed53a78872b61143ddacccbd5a649d.I will integrate the current
devhead into step 21, regenerate OpenAPI and the API client if needed, verify that remote SSH remains unsupported while reattach and attachment audit semantics remain intact, then run the requested gates. No push.Merge finding:
crates/server/tests/migrations.rswas the only content conflict. The merged migration history includes remote agents at0015and session attachment audit at step 21's0016; the migration test helper now expects the contiguous sequence1..=16.scripts/verify fastcontains the append-only migration gate and will be run on the merge result.Fast gate passed on the merge result before commit. Verbatim
scripts/verify fastoutput:Merge commit:
d9197837c5adb20ad0da2a6b8872ed2826a92aee(parentsfed23a4andb63a853; worktree clean). At 14:41 UTC the integration load gate reported load510.46; I am following the requested two-minute polling interval until it is below 8, for up to 60 minutes.Correction to the prior timestamp:
uptimedisplayed 14:41 CEST, which was 12:41 UTC; load5 then was10.46. At the 12:43:49 UTC poll, load5 was8.97, still above the< 8integration threshold.At 12:50:56 UTC, load5 was
6.38(below 8), so I startedscripts/verify integrationon merge commitd9197837c5adb20ad0da2a6b8872ed2826a92aee.Merge job complete. Head:
d9197837c5adb20ad0da2a6b8872ed2826a92aee(parentsfed23a4818ed53a78872b61143ddacccbd5a649dandb63a853186e046a774ff367ee74f9f4d175e58a7). The worktree is clean; no push.Conflict resolution:
crates/server/tests/migrations.rswas the only content conflict. The merged helper and upgrade assertions expect migrations1..=16, preserving0015_remote_agents.sqland step 21's0016_session_attachment_audit.sql.stream.rs, CLI/MCP registration, contracts, and generated files merged automatically. I regenerateddocs/openapi.jsonwithinfraternal openapi,packages/api-client/src/schema.tswithopenapi-typescript, and the generated client with its generator script.Coexistence check: remote-host SSH OPEN still returns
unsupported, asserted by the tunnel integration test. Step 21's reattach authorization still checks owner, kind, and target; attachment state and audit writes remain transactional and covered by storage and SSH product tests. The integration gate passed the SSH product path and remote mTLS infraternal agent enrollment/exec/revoke path.scripts/verify fast(exit 0), verbatim output:scripts/verify integration(exit 0), verbatim output: