kayg/infraternal
Watch
1
0
Fork
You've already forked infraternal
 0
Code Issues 356 Pull requests Projects Releases Packages Wiki Activity Actions

agent_daemon tests flake under load (resource_unavailable on readiness open, lazy-manager accept timeout) #324

New issue
Open
opened 2026-11-03 08:14:03 +00:00 by kayg · 47 comments
kayg commented 2026-11-03 08:14:03 +00:00
Owner
Copy link

Job J130. crates/infraternal/tests/agent_daemon.rs (11 tests, each runs a python oracle against the real binary) fails intermittently while the VM is loaded, and the flake fails scripts/verify fast for unrelated jobs.

No existing issue covers it: #118 (credit/snapshot/socket tests) and #207 (restart tests) are different tests.

Evidence (J127 gate on dev-02-std, 2026-11-03, ~/worktrees/j127-verify.log lines 1529-1603): 8 passed, 3 failed in 34.86 s.

  • real_binary_rootless_private_readiness_does_not_refuse_person_revocation: terminal.open ... readiness=True returned resource_unavailable (scripts/test-rootless-revoke-readiness.py:31).
  • real_binary_rootless_person_preparation_honors_explicit_state_without_runtime_environment: explicit-state fixed readiness was refused: terminal unavailable / resource_unavailable (scripts/test-rootless-explicit-state.py:43).
  • real_binary_keeps_background_backend_probe_off_the_control_path: TimeoutError in probe_listener.accept() (scripts/verify-agent-lazy-manager.py:61).
    The same tests pass 11/11 alone on the same head (j127-agent-head.log) and on dev 2c58f76b5 (j127-agent-base.log).

Task: reproduce under load, find the root cause (test timing assumption or a real product race), fix the right side, and show 20 consecutive passes under the load that failed before. No bare timeout raise or retry unless the evidence shows the timeout is the contract.

Job J130. `crates/infraternal/tests/agent_daemon.rs` (11 tests, each runs a python oracle against the real binary) fails intermittently while the VM is loaded, and the flake fails `scripts/verify fast` for unrelated jobs. No existing issue covers it: #118 (credit/snapshot/socket tests) and #207 (restart tests) are different tests. Evidence (J127 gate on dev-02-std, 2026-11-03, `~/worktrees/j127-verify.log` lines 1529-1603): 8 passed, 3 failed in 34.86 s. - `real_binary_rootless_private_readiness_does_not_refuse_person_revocation`: `terminal.open ... readiness=True` returned `resource_unavailable` (scripts/test-rootless-revoke-readiness.py:31). - `real_binary_rootless_person_preparation_honors_explicit_state_without_runtime_environment`: `explicit-state fixed readiness was refused: terminal unavailable / resource_unavailable` (scripts/test-rootless-explicit-state.py:43). - `real_binary_keeps_background_backend_probe_off_the_control_path`: `TimeoutError` in `probe_listener.accept()` (scripts/verify-agent-lazy-manager.py:61). The same tests pass 11/11 alone on the same head (`j127-agent-head.log`) and on dev 2c58f76b5 (`j127-agent-base.log`). Task: reproduce under load, find the root cause (test timing assumption or a real product race), fix the right side, and show 20 consecutive passes under the load that failed before. No bare timeout raise or retry unless the evidence shows the timeout is the contract.
kayg commented 2026-11-03 08:14:10 +00:00
Author
Owner
Copy link

Start: branch agent-daemon-flake-j130, base a49fb0f3fb52d7e1a8f5fbd0a5d6e4abf8185768 (origin/dev). Read CLAUDE.md, PROCESS.md, ARCHITECTURE.md, BRIEF-COMMON, #118 and #207.

Plan (estimate about 3 h, so I stop and report at 6 h):

  1. (30 min) Read the three failing oracles and the product path for terminal.open readiness=true -> resource_unavailable. Decide whether the refusal comes from a production deadline (#118 saw a 2 s shim readiness bound under load) or from a test assumption.
  2. (45 min) Reproduce on dev-02-std through build-slot: build once, run agent_daemon with stress-ng / N parallel copies. Record the before pass rate over 20 runs with the machine load.
  3. (60 min) Fix the real cause, test side or product side. Say plainly which. No bare timeout raise or retry unless the evidence makes the timeout the contract.
  4. (30 min) Re-run 20 times under the same load, then scripts/verify fast. Post the verbatim output and the head SHA here.

Evidence so far: J127's log shows three failures in one run (see the issue body). Alone, the same tests pass 11/11 in about 33 s.

Start: branch `agent-daemon-flake-j130`, base `a49fb0f3fb52d7e1a8f5fbd0a5d6e4abf8185768` (origin/dev). Read CLAUDE.md, PROCESS.md, ARCHITECTURE.md, BRIEF-COMMON, #118 and #207. Plan (estimate about 3 h, so I stop and report at 6 h): 1. (30 min) Read the three failing oracles and the product path for `terminal.open readiness=true` -> `resource_unavailable`. Decide whether the refusal comes from a production deadline (#118 saw a 2 s shim readiness bound under load) or from a test assumption. 2. (45 min) Reproduce on dev-02-std through build-slot: build once, run `agent_daemon` with stress-ng / N parallel copies. Record the before pass rate over 20 runs with the machine load. 3. (60 min) Fix the real cause, test side or product side. Say plainly which. No bare timeout raise or retry unless the evidence makes the timeout the contract. 4. (30 min) Re-run 20 times under the same load, then `scripts/verify fast`. Post the verbatim output and the head SHA here. Evidence so far: J127's log shows three failures in one run (see the issue body). Alone, the same tests pass 11/11 in about 33 s.
kayg commented 2026-11-03 09:23:15 +00:00
Author
Owner
Copy link

Finding 1 (evidence, dev-02-std, aarch64, base a49fb0f3f): the flake is not CPU starvation, and it is not one bug.

What does NOT reproduce it

  • 20/20 pass with 40 busy loops pinned on the same 2 cores as the suite (load1 up to 45, runs slowed from 35 s to 46 s). Log: ~/worktrees/j130-out/before40/summary.txt.
  • systemd scope creation: 4 ms idle, 60 ms with 16 parallel callers (the systemd-run --user --scope call in sessions.rs launch). Far from the 2 s HANDSHAKE.
  • inotify (128 per uid): peak 2 during a 4-copy run. ptys: peak 38 of 4096.
  • Typeahead: bash on a fresh pty keeps input written before its first prompt, 0 lost of 240 at delays 0/5/20/100 ms. The real binary, one copy, open then type immediately: 0 lost of 300.

What DOES reproduce it: several copies of the suite at once under the same uid (this is what happens when other jobs run their gates on the same VM; J127's failing run took 34.86 s, the same as a healthy one, so it was not slow).

  • 4 concurrent copies of agent_daemon, 8-12 rounds each: pass 14/16, 10/12, 29/32, 36/40, 35/40, 44/48, 44/48. About 10% of copies fail, always with 1 failing test, at normal duration.
  • Every failure in these runs is in tests that run scripts/test-rootless-revoke-live.py (plain or --expanded), plus once ..._at_full_shim_peer_capacity and once ..._reaps_completed_adopted_children.

Captured stall (fixture patched on the VM only to dump state at the 20 s socket timeout), same shape in all captures (r1c3, r4c4, r10c4, r3c4, r6c4):

  • Fixture order: Attach, Credit(1 MiB), Ack(0), two input frames (seq 0 and 62, 99 bytes), Ack(99), then Output seq 0 (104 bytes: bash's first prompt), a census frame, and then nothing for 20 s.
  • The typed commands DID run: shell.pid holds the PID of the correct bash. So input is not lost.
  • Load at the time of the stall: load1 1.7-3.7; daemon, shims and bash at 0.0% CPU, bash in do_select. So nothing is starved.
  • The attached shim's ring head = 453; the viewer received 104. The unviewed shims have head = 104. After poking the shim with an extra connection, still nothing arrived in 3 s.
  • So 349 bytes of live output (echo, marker, next prompt) sit between the shim ring and the viewer while the viewer holds 1 MiB of unspent credit. This points at the daemon relay (crates/infraternal-agent/src/daemon.rs, the per-stream relay loop and relay::Output state), not at the shim, bash or the test timing. I have not yet identified the exact state transition.

Other signatures in older gate logs (J107, J127, j61b, j62b, j91, appwin-146): terminal.open refused with resource_unavailable (fast, 4-15 s total), and the lazy-manager accept() timeout. I have not reproduced these with concurrent copies (0 in about 150 copy-runs), so they may be separate causes. I will not claim them fixed unless I reproduce them.

Next (about 1.5 h): add temporary tracing to the daemon relay loop on the VM (not committed), capture the relay state (session, pending, snapshot, credit, required, stalled, replay_until) at a stall, then write the failing test first and fix the state transition. If the cause is in the relay state machine, that is product code in the protocol area, so I will say so plainly in the commit. No timeout raise or retry.

Scope note for the orchestrator: the original estimate assumed a test-side timing fix. If this is a product relay bug, the fix needs the strictest review, and the resource_unavailable signatures may need a second job.

Finding 1 (evidence, dev-02-std, aarch64, base a49fb0f3f): the flake is not CPU starvation, and it is not one bug. **What does NOT reproduce it** - 20/20 pass with 40 busy loops pinned on the same 2 cores as the suite (load1 up to 45, runs slowed from 35 s to 46 s). Log: `~/worktrees/j130-out/before40/summary.txt`. - systemd scope creation: 4 ms idle, 60 ms with 16 parallel callers (the `systemd-run --user --scope` call in `sessions.rs` launch). Far from the 2 s `HANDSHAKE`. - inotify (128 per uid): peak 2 during a 4-copy run. ptys: peak 38 of 4096. - Typeahead: bash on a fresh pty keeps input written before its first prompt, 0 lost of 240 at delays 0/5/20/100 ms. The real binary, one copy, open then type immediately: 0 lost of 300. **What DOES reproduce it: several copies of the suite at once under the same uid** (this is what happens when other jobs run their gates on the same VM; J127's failing run took 34.86 s, the same as a healthy one, so it was not slow). - 4 concurrent copies of `agent_daemon`, 8-12 rounds each: pass 14/16, 10/12, 29/32, 36/40, 35/40, 44/48, 44/48. About 10% of copies fail, always with 1 failing test, at normal duration. - Every failure in these runs is in tests that run `scripts/test-rootless-revoke-live.py` (plain or `--expanded`), plus once `..._at_full_shim_peer_capacity` and once `..._reaps_completed_adopted_children`. **Captured stall (fixture patched on the VM only to dump state at the 20 s socket timeout)**, same shape in all captures (r1c3, r4c4, r10c4, r3c4, r6c4): - Fixture order: `Attach`, `Credit(1 MiB)`, `Ack(0)`, two input frames (seq 0 and 62, 99 bytes), `Ack(99)`, then `Output seq 0` (104 bytes: bash's first prompt), a census frame, and then **nothing for 20 s**. - The typed commands DID run: `shell.pid` holds the PID of the correct bash. So input is not lost. - Load at the time of the stall: load1 1.7-3.7; daemon, shims and bash at 0.0% CPU, bash in `do_select`. So nothing is starved. - The attached shim's ring `head` = **453**; the viewer received **104**. The unviewed shims have head = 104. After poking the shim with an extra connection, still nothing arrived in 3 s. - So 349 bytes of live output (echo, marker, next prompt) sit between the shim ring and the viewer while the viewer holds 1 MiB of unspent credit. This points at the daemon relay (`crates/infraternal-agent/src/daemon.rs`, the per-stream relay loop and `relay::Output` state), not at the shim, bash or the test timing. I have not yet identified the exact state transition. **Other signatures in older gate logs (J107, J127, j61b, j62b, j91, appwin-146)**: `terminal.open` refused with `resource_unavailable` (fast, 4-15 s total), and the lazy-manager `accept()` timeout. I have not reproduced these with concurrent copies (0 in about 150 copy-runs), so they may be separate causes. I will not claim them fixed unless I reproduce them. **Next (about 1.5 h)**: add temporary tracing to the daemon relay loop on the VM (not committed), capture the relay state (`session`, `pending`, `snapshot`, `credit`, `required`, `stalled`, `replay_until`) at a stall, then write the failing test first and fix the state transition. If the cause is in the relay state machine, that is product code in the protocol area, so I will say so plainly in the commit. No timeout raise or retry. Scope note for the orchestrator: the original estimate assumed a test-side timing fix. If this is a product relay bug, the fix needs the strictest review, and the `resource_unavailable` signatures may need a second job.
kayg commented 2026-11-03 10:21:34 +00:00
Author
Owner
Copy link

Finding 2 (root cause found and fixed for the revoke-live family; the other signatures are NOT covered).

Root cause: a product race in the daemon relay, not a test timing assumption.
crates/infraternal-agent/src/daemon.rs, terminal_run. The relay splits a shim Output frame at the replay fence (Hello.head). When the shell printed more between that Hello and the shim handling ReadFrom, the shim sends ONE frame with the old head plus the new bytes. The relay forwarded the prefix, sent ReplayComplete, parked the remainder in pending, and fell into select!. There the shim-frame branch is disabled while pending is Some (if pending.is_none()), no deadline is armed (credit is available, so output.pending() returns None), and pending is only taken at the top of the loop. Nothing re-entered it, so the tail stayed parked until an unrelated command, credit grant or ping arrived. User-visible form: a terminal opened and attached shows its first prompt and then looks frozen until a key is pressed.

Evidence (relay trace of the stalled daemon, dev-02-std, base a49fb0f3f):

cmd Credit 1048576
cmd Input seq=0 len=62
cmd Input seq=62 len=37
shim-frame InputAck 99
shim-frame Output seq=0 len=453          <- one frame, 453 bytes
top link=true pending=true out=Output{cursor:0, credit:1048576, ...} replay_until=Some(104) deadline=false
(no further loop iteration)

Viewer received 104 bytes; the attached shim's ring head was 453; the shell had executed the commands (shell.pid held the right bash PID); daemon, shims and bash at 0.0% CPU. Second capture: Output seq=0 len=165, same shape.

Failing test first. daemon_replay_fence_tests.rs (fake shim, Hello.head=104, one 453-byte Output after ReadFrom). Before the fix:
104 of 453 bytes were delivered; the rest of the frame stayed parked with credit available (test result: FAILED, 0 passed; 1 failed). After the fix: ok, and the whole infraternal-agent lib suite passes (134 passed, 0 failed, 9 ignored), including the exact 249/250 ms starvation-boundary tests in relay.rs.

Before / after under the same load (dev-02-std, 4 concurrent copies of the agent_daemon binary per round, 20 rounds, no CPU pinning; base = unmodified a49fb0f3f):

  • before: 70/80 copy-runs passed; 13 of 20 rounds fully clean; longest clean streak 4 rounds. All 10 failures: rootless_person_revocation_stops_existing_shells (6) and rootless_revocation_after_restart_stops_every_owned_terminal (4), each one failing test at normal duration (33-35 s).
  • after (commit 6d1b5e7a9): 80/80 copy-runs passed, 20 of 20 rounds clean.
    Logs: ~/worktrees/j130-out/before20/summary.txt, ~/worktrees/j130-out/after20/summary.txt on dev-02-std.

What this does NOT show. J127's three failing tests (private_readiness_does_not_refuse_person_revocation, person_preparation_honors_explicit_state..., lazy-manager accept() timeout) did not fail even once in about 400 copy-runs under this load, before or after. They are a different signature (terminal.open readiness refused with resource_unavailable, fast, 4-15 s). The fix does not touch that path and I cannot claim it fixes them. Older gate logs on compute01 show a third signature: terminal.login.revoke refused with resource_unavailable at the end of rootless_cleanup_survives_sender_disconnect_before_readiness. Also not reproduced.

Rejected alternatives are in the commit message. Next: scripts/verify fast on compute01 (queued), then a further attempt to reproduce the resource_unavailable signature.

Finding 2 (root cause found and fixed for the revoke-live family; the other signatures are NOT covered). **Root cause: a product race in the daemon relay, not a test timing assumption.** `crates/infraternal-agent/src/daemon.rs`, `terminal_run`. The relay splits a shim `Output` frame at the replay fence (`Hello.head`). When the shell printed more between that Hello and the shim handling `ReadFrom`, the shim sends ONE frame with the old head plus the new bytes. The relay forwarded the prefix, sent `ReplayComplete`, parked the remainder in `pending`, and fell into `select!`. There the shim-frame branch is disabled while `pending` is Some (`if pending.is_none()`), no deadline is armed (credit is available, so `output.pending()` returns None), and `pending` is only taken at the top of the loop. Nothing re-entered it, so the tail stayed parked until an unrelated command, credit grant or ping arrived. User-visible form: a terminal opened and attached shows its first prompt and then looks frozen until a key is pressed. **Evidence (relay trace of the stalled daemon, dev-02-std, base a49fb0f3f):** ``` cmd Credit 1048576 cmd Input seq=0 len=62 cmd Input seq=62 len=37 shim-frame InputAck 99 shim-frame Output seq=0 len=453 <- one frame, 453 bytes top link=true pending=true out=Output{cursor:0, credit:1048576, ...} replay_until=Some(104) deadline=false (no further loop iteration) ``` Viewer received 104 bytes; the attached shim's ring head was 453; the shell had executed the commands (`shell.pid` held the right bash PID); daemon, shims and bash at 0.0% CPU. Second capture: `Output seq=0 len=165`, same shape. **Failing test first.** `daemon_replay_fence_tests.rs` (fake shim, `Hello.head=104`, one 453-byte `Output` after `ReadFrom`). Before the fix: `104 of 453 bytes were delivered; the rest of the frame stayed parked with credit available` (test result: FAILED, 0 passed; 1 failed). After the fix: ok, and the whole `infraternal-agent` lib suite passes (134 passed, 0 failed, 9 ignored), including the exact 249/250 ms starvation-boundary tests in `relay.rs`. **Before / after under the same load** (dev-02-std, 4 concurrent copies of the `agent_daemon` binary per round, 20 rounds, no CPU pinning; base = unmodified a49fb0f3f): - before: **70/80** copy-runs passed; 13 of 20 rounds fully clean; longest clean streak 4 rounds. All 10 failures: `rootless_person_revocation_stops_existing_shells` (6) and `rootless_revocation_after_restart_stops_every_owned_terminal` (4), each one failing test at normal duration (33-35 s). - after (commit 6d1b5e7a9): **80/80** copy-runs passed, 20 of 20 rounds clean. Logs: `~/worktrees/j130-out/before20/summary.txt`, `~/worktrees/j130-out/after20/summary.txt` on dev-02-std. **What this does NOT show.** J127's three failing tests (`private_readiness_does_not_refuse_person_revocation`, `person_preparation_honors_explicit_state...`, lazy-manager `accept()` timeout) did not fail even once in about 400 copy-runs under this load, before or after. They are a different signature (`terminal.open` readiness refused with `resource_unavailable`, fast, 4-15 s). The fix does not touch that path and I cannot claim it fixes them. Older gate logs on compute01 show a third signature: `terminal.login.revoke` refused with `resource_unavailable` at the end of `rootless_cleanup_survives_sender_disconnect_before_readiness`. Also not reproduced. Rejected alternatives are in the commit message. Next: `scripts/verify fast` on compute01 (queued), then a further attempt to reproduce the `resource_unavailable` signature.
kayg commented 2026-11-03 11:15:34 +00:00
Author
Owner
Copy link

Finding 3: session_exit_events (orchestrator request, J62b's failure). Different cause from Finding 2. Not fixed here.

What the failure is. J62b's verify-fast-c.log (dev-02-std, 2026-11-03 10:23), idle_daemon_discovers_unattached_and_adopted_shim_exit_without_requests: the first client.open() (scripts/verify-session-exit-events.py:51) never got its terminal.open Result and died with TimeoutError: timed out. The "1 s" in the report is not the socket timeout: it is the fixture's self.child.wait(timeout=1) (agent_fixture.py:264) that runs after a read has already failed. The socket timeout on that read is 20 s. So this is a terminal.open that produced no Result for 20 s, on a daemon that had just passed its 2.2 s zero-activity idle check. The same test passed in the other 4 J62b logs on the same code.

Why it is not my relay bug. My bug stalls OUTPUT after a successful attach (a parked frame tail); the open Result is delivered. Here the open Result itself is missing. The fix in 6d1b5e7a9 does not touch that path and I do not claim it affects this test.

What I measured (dev-02-std vs compute01, 60 s each, concurrently, a 64-byte write + fsync every 50 ms):

host fsync p50 p99 max samples
dev-02-std 1272 ms 5929 ms 18085 ms 23
compute01 5.7 ms 30 ms 94 ms 1049
(load1 18 on dev-02, 12 on compute01 at the time.) That matches the pattern J62b reported: failures on dev-02 only, 12/12 on compute01. It is also consistent with the owner's earlier diagnosis of dev-02 storage stalls (fsync stalls of up to 16 s tripping wall-clock guards).

What I could NOT show. The stall is bursty: 10 minutes later dev-02 fsync p50 was 10 ms and 120 plain terminal.open calls through the real binary had p50 10 ms, p99 34 ms, 0 refusals. So I have no failing run to tie to a stall, and the plain open path has no explicit sync_all in sessions.rs/daemon.rs/shim session.rs. The link "host storage stall, blocking a metadata operation behind the journal" is a hypothesis supported by the numbers above, not a proven cause. A proof needs a failing run recorded next to an fsync sampler (j130-sample.sh + the fsync probe on dev-02 during a gate), or an injected stall on a private filesystem.

Same hypothesis may explain the resource_unavailable readiness signatures (J107, J127, j61b, appwin-146 logs: all on dev-02 except the compute01 ...sender_disconnect_before_readiness ones) and the lazy-manager accept() timeout, which I could not reproduce either. Unproven.

Not looked at: transient_authority_stall in infraternal-server (auth/socket_tests). I did not open it; I cannot say whether it shares a cause.

Suggested next step (owner's call, scope outside J130): treat dev-02 storage latency as the suspect for the remaining signatures, measure it next to a failing gate, and per the owner's rule replace wall-clock guards in functional tests with progress/ordering assertions, keeping only long hang-detection timeouts. I have not changed any timeout or retry.

Finding 3: `session_exit_events` (orchestrator request, J62b's failure). **Different cause from Finding 2. Not fixed here.** **What the failure is.** J62b's `verify-fast-c.log` (dev-02-std, 2026-11-03 10:23), `idle_daemon_discovers_unattached_and_adopted_shim_exit_without_requests`: the first `client.open()` (`scripts/verify-session-exit-events.py:51`) never got its `terminal.open` Result and died with `TimeoutError: timed out`. The "1 s" in the report is not the socket timeout: it is the fixture's `self.child.wait(timeout=1)` (`agent_fixture.py:264`) that runs after a read has already failed. The socket timeout on that read is 20 s. So this is a `terminal.open` that produced no Result for 20 s, on a daemon that had just passed its 2.2 s zero-activity idle check. The same test passed in the other 4 J62b logs on the same code. **Why it is not my relay bug.** My bug stalls OUTPUT after a successful attach (a parked frame tail); the open Result is delivered. Here the open Result itself is missing. The fix in `6d1b5e7a9` does not touch that path and I do not claim it affects this test. **What I measured (dev-02-std vs compute01, 60 s each, concurrently, a 64-byte write + `fsync` every 50 ms):** | host | fsync p50 | p99 | max | samples | |---|---|---|---|---| | dev-02-std | 1272 ms | 5929 ms | **18085 ms** | 23 | | compute01 | 5.7 ms | 30 ms | 94 ms | 1049 | (load1 18 on dev-02, 12 on compute01 at the time.) That matches the pattern J62b reported: failures on dev-02 only, 12/12 on compute01. It is also consistent with the owner's earlier diagnosis of dev-02 storage stalls (fsync stalls of up to 16 s tripping wall-clock guards). **What I could NOT show.** The stall is bursty: 10 minutes later dev-02 fsync p50 was 10 ms and 120 plain `terminal.open` calls through the real binary had p50 10 ms, p99 34 ms, 0 refusals. So I have no failing run to tie to a stall, and the plain open path has no explicit `sync_all` in `sessions.rs`/`daemon.rs`/shim `session.rs`. The link "host storage stall, blocking a metadata operation behind the journal" is a hypothesis supported by the numbers above, not a proven cause. A proof needs a failing run recorded next to an fsync sampler (`j130-sample.sh` + the fsync probe on dev-02 during a gate), or an injected stall on a private filesystem. **Same hypothesis may explain the `resource_unavailable` readiness signatures** (J107, J127, j61b, appwin-146 logs: all on dev-02 except the compute01 `...sender_disconnect_before_readiness` ones) and the lazy-manager `accept()` timeout, which I could not reproduce either. Unproven. **Not looked at:** `transient_authority_stall` in `infraternal-server` (`auth/socket_tests`). I did not open it; I cannot say whether it shares a cause. **Suggested next step (owner's call, scope outside J130):** treat dev-02 storage latency as the suspect for the remaining signatures, measure it next to a failing gate, and per the owner's rule replace wall-clock guards in functional tests with progress/ordering assertions, keeping only long hang-detection timeouts. I have not changed any timeout or retry.
kayg commented 2026-11-03 11:55:03 +00:00
Author
Owner
Copy link

Status at head 6d1b5e7a9a9d057eb53847b738651347bc4a257a (branch agent-daemon-flake-j130, base a49fb0f3f, one commit). The gate is NOT green. I am stopping here for a decision.

scripts/verify fast, compute01 (x86_64), verbatim:

+---------------------------+--------------+
| Rust gate                 | Result       |
+---------------------------+--------------+
| fmt                       | PASS         |
| VT native fmt             | PASS         |
| CLI plugin compile-out    | PASS         |
| clippy                    | PASS         |
| SSH runtime test helper   | PASS         |
| real-binary fixture paths | PASS         |
| test                      | FAIL (101)   |
| SSH inode reuse fixture   | PASS         |
| shim process-per-user     | PASS         |
| cargo-deny                | PASS         |
| gate architecture         | PASS         |
| static builds             | PASS         |
+---------------------------+--------------+
VERIFY_EXIT=1

The single failing test:

test websocket::writer_tests::authority_watchdog::cap_review_r3::sqlite_busy_timeout_recovers_detached_storm_without_unlocking ... FAILED
panicked at crates/infraternal-server/src/websocket_authority_tests.rs:526:9:
assertion `left == right` failed
  left: 5
 right: 4
test result: FAILED. 748 passed; 1 failed; 57 ignored; finished in 573.52s
  • In the same gate, agent_daemon passed 11/11 (33.94 s) with the fix.
  • That test is in infraternal-server; my commit changes only crates/infraternal-agent/src/daemon.rs and adds daemon_replay_fence_tests.rs. It asserts started == AUTHORITY_READS after polling 32 futures against a 4-connection pool, so it looks like its own race.
  • Alone, on my tree, load1 16.6: 40/40 pass (~/worktrees/j130-out/single40 on compute01). It failed once in the 749-test suite run.
  • Not proven on unmodified origin/dev. Proving it needs a full infraternal-server --lib run (about 10 min) on a base build, and compute01's queue is 6 deep. I did not bypass build-slot for a heavy run. I did not rerun the gate.

Decision needed from the orchestrator / kayg: (a) rerun the gate (the compute01 worktree and warm target, ~/worktrees/j130, 43 GB, are kept for this), or (b) accept the gate row as a known unrelated flake once a base reproduction exists, or (c) a separate job for transient_authority_stall and this test.

Cleanup: dev-02 worktree, target (36 GB) and temp files deleted. compute01 target kept as above. Evidence logs are small and stay in ~/worktrees/j130-out/ on both VMs.

Status at head `6d1b5e7a9a9d057eb53847b738651347bc4a257a` (branch `agent-daemon-flake-j130`, base `a49fb0f3f`, one commit). **The gate is NOT green. I am stopping here for a decision.** `scripts/verify fast`, compute01 (x86_64), verbatim: ``` +---------------------------+--------------+ | Rust gate | Result | +---------------------------+--------------+ | fmt | PASS | | VT native fmt | PASS | | CLI plugin compile-out | PASS | | clippy | PASS | | SSH runtime test helper | PASS | | real-binary fixture paths | PASS | | test | FAIL (101) | | SSH inode reuse fixture | PASS | | shim process-per-user | PASS | | cargo-deny | PASS | | gate architecture | PASS | | static builds | PASS | +---------------------------+--------------+ VERIFY_EXIT=1 ``` The single failing test: ``` test websocket::writer_tests::authority_watchdog::cap_review_r3::sqlite_busy_timeout_recovers_detached_storm_without_unlocking ... FAILED panicked at crates/infraternal-server/src/websocket_authority_tests.rs:526:9: assertion `left == right` failed left: 5 right: 4 test result: FAILED. 748 passed; 1 failed; 57 ignored; finished in 573.52s ``` - In the same gate, `agent_daemon` passed 11/11 (33.94 s) with the fix. - That test is in `infraternal-server`; my commit changes only `crates/infraternal-agent/src/daemon.rs` and adds `daemon_replay_fence_tests.rs`. It asserts `started == AUTHORITY_READS` after polling 32 futures against a 4-connection pool, so it looks like its own race. - Alone, on my tree, load1 16.6: **40/40 pass** (`~/worktrees/j130-out/single40` on compute01). It failed once in the 749-test suite run. - **Not proven on unmodified `origin/dev`.** Proving it needs a full `infraternal-server --lib` run (about 10 min) on a base build, and compute01's queue is 6 deep. I did not bypass build-slot for a heavy run. I did not rerun the gate. Decision needed from the orchestrator / kayg: (a) rerun the gate (the compute01 worktree and warm target, `~/worktrees/j130`, 43 GB, are kept for this), or (b) accept the gate row as a known unrelated flake once a base reproduction exists, or (c) a separate job for `transient_authority_stall` and this test. Cleanup: dev-02 worktree, target (36 GB) and temp files deleted. compute01 target kept as above. Evidence logs are small and stay in `~/worktrees/j130-out/` on both VMs.
kayg commented 2026-11-03 23:03:26 +00:00
Author
Owner
Copy link

J157 start. Branch agent-resource-j157, base origin/dev b0eb5237c (includes J130 6d1b5e7a9 and J155). Worktree on code; builds and repro on dev-02-std through build-slot.

Plan (estimate ~3 h; I stop and report at 6 h):

  1. (30 min) Collect the resource_unavailable / "terminal unavailable" / pid-file failures from J79c, j79d, j146, j62c, j93c logs; map the refusal to its source. open_refusal (daemon.rs:278) turns every io error that is not InvalidInput/InvalidData/PermissionDenied into resource_unavailable, so the refusal itself says nothing; I need the underlying io error.
  2. (60 min) Reproduce on dev-02 under controlled load (parallel agent_daemon copies, CPU/IO stress, shrunk ulimit) with the underlying error logged (VM-only instrumentation, not committed).
  3. (60 min) Fix the root cause on the right side (product or fixture), failing test first where possible. No blind timeout raise or retry.
  4. (30 min) 20/20 under the same load, before/after numbers, scripts/verify fast on dev-02 verbatim.

First observation from the logs: J79c, j79d and j146 (2 runs) fail the SAME three tests together, fast (whole binary 5.3 s / 7.2 s / 20.7 s vs ~35 s healthy): preserves_input_and_output_across_24_abrupt_stops, rootless_person_preparation_honors_explicit_state..., rootless_private_readiness_does_not_refuse_person_revocation. All three open a readiness terminal (terminal.open readiness=true). The person-terminal tests in the same run pass. So this is a fast, systematic refusal on the readiness path, not a slow timeout.

**J157 start.** Branch `agent-resource-j157`, base origin/dev `b0eb5237c` (includes J130 `6d1b5e7a9` and J155). Worktree on `code`; builds and repro on dev-02-std through build-slot. Plan (estimate ~3 h; I stop and report at 6 h): 1. (30 min) Collect the `resource_unavailable` / "terminal unavailable" / pid-file failures from J79c, j79d, j146, j62c, j93c logs; map the refusal to its source. `open_refusal` (`daemon.rs:278`) turns every io error that is not InvalidInput/InvalidData/PermissionDenied into `resource_unavailable`, so the refusal itself says nothing; I need the underlying io error. 2. (60 min) Reproduce on dev-02 under controlled load (parallel `agent_daemon` copies, CPU/IO stress, shrunk ulimit) with the underlying error logged (VM-only instrumentation, not committed). 3. (60 min) Fix the root cause on the right side (product or fixture), failing test first where possible. No blind timeout raise or retry. 4. (30 min) 20/20 under the same load, before/after numbers, `scripts/verify fast` on dev-02 verbatim. First observation from the logs: J79c, j79d and j146 (2 runs) fail the SAME three tests together, fast (whole binary 5.3 s / 7.2 s / 20.7 s vs ~35 s healthy): `preserves_input_and_output_across_24_abrupt_stops`, `rootless_person_preparation_honors_explicit_state...`, `rootless_private_readiness_does_not_refuse_person_revocation`. All three open a **readiness** terminal (`terminal.open readiness=true`). The person-terminal tests in the same run pass. So this is a fast, systematic refusal on the readiness path, not a slow timeout.
kayg commented 2026-11-03 23:30:41 +00:00
Author
Owner
Copy link

J157 finding 1 (evidence, dev-02-std). Where resource_unavailable comes from, and what the failing gates actually hit.

Mapping. open_refusal (crates/infraternal-agent/src/daemon.rs:278) turns every io error except InvalidInput/InvalidData/PermissionDenied into resource_unavailable. So the refusal names no cause. Candidates on the plain/readiness open path (sessions::launch_configured): inotify init/add_watch (EMFILE/ENOSPC), the launcher spawn, and the 2 s HANDSHAKE from spawning the launcher (systemd-run --user --scope …) to the shim's Hello (TimedOut).

Failing cases in the named logs. All resource_unavailable cases are on dev-02: J79c verify-fast-final.log, j79d verify-fast-4.log, j146 verify-6363a04c4{,-run1}.log. Each time it is the same three tests, which fail at their FIRST open: preserves_input_and_output_across_24_abrupt_stops (plain open), rootless_person_preparation_honors_explicit_state… and rootless_private_readiness_does_not_refuse_person_revocation (readiness opens). j146 also has the lazy-manager accept() timeout in the same runs. The "pid-file race" is j93c verify-fast.log, rootless_terminal_reaps_completed_adopted_children: ValueError: invalid literal for int() with base 10: ''. That is a fixture race (it reads job{n}.pid as soon as the file exists, before printf has written it), and it is unrelated to the refusal. compute01's named logs only show J130's relay bug (fixed in 6d1b5e7a9).

The refused opens did spawn their shim, and those shims are still alive hours later. The failed tests left tb<pid>/sessions/<id>.sock + .stop in the shared fixture root. The shims behind them are still running, each with a live -bash child:

shim 2319614 (j146 run1)  start 19:21:17.52  bash 19:21:22.01  gap 4.49 s   scope "Started" 19:21:22.2996, socket bound 19:21:22.3409
shim 2661197 (j146)       start 20:47:24.46  bash 20:47:28.02  gap 3.56 s   scope "Started" 20:47:24.806,  socket bound 20:47:28.348
shim  652358 (j79d)       start 15:39:48.55  bash 15:39:53.66  gap 5.11 s

(start times from /proc/<pid>/stat; the gaps are exact, the absolute times carry ~0.3 s of btime skew.) So inotify and spawn succeeded. The launch then took 3.5–5.1 s from launcher spawn to a running shell, against a 2 s HANDSHAKE. In one case the time was spent before systemd's user manager started the scope; in the other, between the scope start and the shim binding its socket (exec + startup of a 464 MB debug binary). The user journal shows 21 scopes of the j146 suite all started inside 360 ms after the stall: the user manager was blocked and then caught up.

Second defect from the same evidence: when the handshake deadline expires, nothing stops the shim. A refused open leaves an orphan terminal and shell running indefinitely (13 such shims are on dev-02 now, the oldest 21 h old).

Ruled out / measured:

  • inotify max_user_instances is 128 on dev-02 (8192 on compute01). Holding all but 12 instances reproduces the same refusal (Os { code: 24, TooManyOpenFiles } from socket_readiness), and holding all but 24 does not. But these shims prove the failing opens got past inotify, so inotify is not the cause of the gate failures. One caveat on earlier counts: the infraternal binaries are non-dumpable, so a /proc/<pid>/fd count as the same user misses them (J130's "peak 2" has that blind spot).
  • 6 concurrent suite copies on a quiet dev-02: 18/18 pass. Concurrency alone does not reproduce it.
  • Heavy buffered-write load (2× dd conv=fsync): fsync p50 13 s, max 19.6 s, but systemd-run --user --scope true stays under 313 ms. That load gives 1/5 passes, but only with 20 s fixture timeouts, not this refusal.

Next: reproduce the slow launch itself (evicted binary + IO contention), measure the spawn→Hello distribution, then fix: the deadline must cover the real launch path, and a timed-out launch must not leak its shim.

**J157 finding 1 (evidence, dev-02-std).** Where `resource_unavailable` comes from, and what the failing gates actually hit. **Mapping.** `open_refusal` (`crates/infraternal-agent/src/daemon.rs:278`) turns every io error except InvalidInput/InvalidData/PermissionDenied into `resource_unavailable`. So the refusal names no cause. Candidates on the plain/readiness open path (`sessions::launch_configured`): inotify init/add_watch (EMFILE/ENOSPC), the launcher spawn, and the 2 s `HANDSHAKE` from spawning the launcher (`systemd-run --user --scope …`) to the shim's Hello (TimedOut). **Failing cases in the named logs.** All `resource_unavailable` cases are on dev-02: J79c `verify-fast-final.log`, j79d `verify-fast-4.log`, j146 `verify-6363a04c4{,-run1}.log`. Each time it is the same three tests, which fail at their FIRST open: `preserves_input_and_output_across_24_abrupt_stops` (plain open), `rootless_person_preparation_honors_explicit_state…` and `rootless_private_readiness_does_not_refuse_person_revocation` (readiness opens). j146 also has the lazy-manager `accept()` timeout in the same runs. The "pid-file race" is j93c `verify-fast.log`, `rootless_terminal_reaps_completed_adopted_children`: `ValueError: invalid literal for int() with base 10: ''`. That is a fixture race (it reads `job{n}.pid` as soon as the file exists, before `printf` has written it), and it is unrelated to the refusal. compute01's named logs only show J130's relay bug (fixed in `6d1b5e7a9`). **The refused opens did spawn their shim, and those shims are still alive hours later.** The failed tests left `tb<pid>/sessions/<id>.sock` + `.stop` in the shared fixture root. The shims behind them are still running, each with a live `-bash` child: ``` shim 2319614 (j146 run1) start 19:21:17.52 bash 19:21:22.01 gap 4.49 s scope "Started" 19:21:22.2996, socket bound 19:21:22.3409 shim 2661197 (j146) start 20:47:24.46 bash 20:47:28.02 gap 3.56 s scope "Started" 20:47:24.806, socket bound 20:47:28.348 shim 652358 (j79d) start 15:39:48.55 bash 15:39:53.66 gap 5.11 s ``` (start times from `/proc/<pid>/stat`; the gaps are exact, the absolute times carry ~0.3 s of btime skew.) So inotify and spawn succeeded. The launch then took 3.5–5.1 s from launcher spawn to a running shell, against a 2 s `HANDSHAKE`. In one case the time was spent before systemd's user manager started the scope; in the other, between the scope start and the shim binding its socket (exec + startup of a 464 MB debug binary). The user journal shows 21 scopes of the j146 suite all started inside 360 ms after the stall: the user manager was blocked and then caught up. **Second defect from the same evidence:** when the handshake deadline expires, nothing stops the shim. A refused open leaves an orphan terminal and shell running indefinitely (13 such shims are on dev-02 now, the oldest 21 h old). **Ruled out / measured:** - inotify `max_user_instances` is 128 on dev-02 (8192 on compute01). Holding all but 12 instances reproduces the same refusal (`Os { code: 24, TooManyOpenFiles }` from `socket_readiness`), and holding all but 24 does not. But these shims prove the failing opens got past inotify, so inotify is not the cause of the gate failures. One caveat on earlier counts: the infraternal binaries are non-dumpable, so a `/proc/<pid>/fd` count as the same user misses them (J130's "peak 2" has that blind spot). - 6 concurrent suite copies on a quiet dev-02: 18/18 pass. Concurrency alone does not reproduce it. - Heavy buffered-write load (2× `dd conv=fsync`): fsync p50 13 s, max 19.6 s, but `systemd-run --user --scope true` stays under 313 ms. That load gives 1/5 passes, but only with 20 s fixture timeouts, not this refusal. Next: reproduce the slow launch itself (evicted binary + IO contention), measure the spawn→Hello distribution, then fix: the deadline must cover the real launch path, and a timed-out launch must not leak its shim.
kayg commented 2026-11-03 23:53:27 +00:00
Author
Owner
Copy link

J157 finding 2: root cause reproduced and fixed; the 20-round before/after run is in progress. Branch agent-resource-j157, head 33c392fbf.

Reproduction (dev-02, controlled load = a 24 GiB anonymous-memory hog + 2× looping dd … conv=fsync). On unmodified dev, with the open error logged by VM-only instrumentation:

explicit-state fixed readiness was refused: {'error': 'terminal unavailable', 'reason': 'resource_unavailable'}
handshake …/explicit3239916/sessions/d7a5….sock failed after 2.003945192s: Custom { kind: TimedOut, error: "shim socket readiness timed out" }; dir=[]

That is the gate's exact signature: the same readiness/plain-open tests, plus the lazy-manager test in the same run. The underlying error is the 2 s HANDSHAKE that launch_configured applies to the whole launch (systemd-run → scope → exec → shim bind → Hello). dir=[]: the shim had not bound yet.
Loads that do NOT reproduce it: 6 concurrent suite copies (18/18 pass), fsync-only write load (other failures, not this), a cold page cache alone (3/3).

Uncensored latency (instrumented build with the bound lifted to 60 s, same load, 5 suites):

  • launch spawn→Hello: n=110, p50 28 ms, p90 421 ms, p99 1983 ms, max 1983 ms
  • systemctl --user show-environment probe (Manager::detect, also bounded by 2 s): most 2–20 ms, max 4731 ms. Above 2 s, the daemon caches Detached for its whole life.
  • The gate's own failures (finding 1) were 3.5–5.1 s.

Fix (product, because real users run on loaded hosts):

  • 6a143017b agent: a terminal launch waits on its launcher, not the 2 s shim handshake. The launcher's exit before the rendezvous refuses the open at once; once the socket exists, connect keeps the 2 s HANDSHAKE for Hello. LAUNCH = 30 s only detects a hung start, inside the server's 60 s terminal connect budget (#29). Test slow_shim_start_is_not_refused_while_its_launcher_lives is red on the old bound (TimedOut: shim socket readiness timed out) and green now. Two more tests: an early exit refuses without waiting; a detaching launcher's success exit keeps waiting.
  • 33c392fbf agent: a slow init-manager probe no longer selects Detached for the daemon's life (same reasoning, same bound).
  • 412e7b383 test: the adopted-children oracle reads a job pid only once it is written (the j93c pid-file race: printf > file creates the file before writing it).

Not fixed, flagged: a launch that does fail after spawn still leaves its shim running; nothing kills it. The fix makes this rare (only a real failure or a >30 s hang), but it is a separate leak. 13 such shims from today's failed gates are still alive on dev-02. They belong to other jobs, so I am leaving them for the orchestrator. Under the same fsync load, rootless_retirement_releases_person_terminal_records also fails ("fixture terminal did not retire", a 5 s fixture wait behind a directory sync_all while fsync stalls 13–19 s). That is J130's storage-stall family, not this refusal, and it is not changed here.

Next: interleaved before/after, 20 rounds under the same load (~/j157-out/ab20/summary.txt on dev-02), then scripts/verify fast on dev-02.

**J157 finding 2: root cause reproduced and fixed; the 20-round before/after run is in progress.** Branch `agent-resource-j157`, head `33c392fbf`. **Reproduction (dev-02, controlled load = a 24 GiB anonymous-memory hog + 2× looping `dd … conv=fsync`).** On unmodified dev, with the open error logged by VM-only instrumentation: ``` explicit-state fixed readiness was refused: {'error': 'terminal unavailable', 'reason': 'resource_unavailable'} handshake …/explicit3239916/sessions/d7a5….sock failed after 2.003945192s: Custom { kind: TimedOut, error: "shim socket readiness timed out" }; dir=[] ``` That is the gate's exact signature: the same readiness/plain-open tests, plus the lazy-manager test in the same run. The underlying error is the 2 s `HANDSHAKE` that `launch_configured` applies to the whole launch (systemd-run → scope → exec → shim bind → Hello). `dir=[]`: the shim had not bound yet. Loads that do NOT reproduce it: 6 concurrent suite copies (18/18 pass), fsync-only write load (other failures, not this), a cold page cache alone (3/3). **Uncensored latency** (instrumented build with the bound lifted to 60 s, same load, 5 suites): - launch spawn→Hello: n=110, p50 28 ms, p90 421 ms, p99 1983 ms, max 1983 ms - `systemctl --user show-environment` probe (`Manager::detect`, also bounded by 2 s): most 2–20 ms, max **4731 ms**. Above 2 s, the daemon caches `Detached` for its whole life. - The gate's own failures (finding 1) were 3.5–5.1 s. **Fix (product, because real users run on loaded hosts):** - `6a143017b` agent: a terminal launch waits on its launcher, not the 2 s shim handshake. The launcher's exit before the rendezvous refuses the open at once; once the socket exists, `connect` keeps the 2 s HANDSHAKE for Hello. `LAUNCH` = 30 s only detects a hung start, inside the server's 60 s terminal connect budget (#29). Test `slow_shim_start_is_not_refused_while_its_launcher_lives` is red on the old bound (`TimedOut: shim socket readiness timed out`) and green now. Two more tests: an early exit refuses without waiting; a detaching launcher's success exit keeps waiting. - `33c392fbf` agent: a slow init-manager probe no longer selects Detached for the daemon's life (same reasoning, same bound). - `412e7b383` test: the adopted-children oracle reads a job pid only once it is written (the j93c pid-file race: `printf > file` creates the file before writing it). **Not fixed, flagged:** a launch that does fail after spawn still leaves its shim running; nothing kills it. The fix makes this rare (only a real failure or a >30 s hang), but it is a separate leak. 13 such shims from today's failed gates are still alive on dev-02. They belong to other jobs, so I am leaving them for the orchestrator. Under the same fsync load, `rootless_retirement_releases_person_terminal_records` also fails ("fixture terminal did not retire", a 5 s fixture wait behind a directory `sync_all` while fsync stalls 13–19 s). That is J130's storage-stall family, not this refusal, and it is not changed here. Next: interleaved before/after, 20 rounds under the same load (`~/j157-out/ab20/summary.txt` on dev-02), then `scripts/verify fast` on dev-02.
kayg commented 2026-11-04 01:11:43 +00:00
Author
Owner
Copy link

J157 finding 3: before/after under the reproducing load. Interleaved (before = dev b0eb5237c, after = 33c392fbf; the later fmt-only fixup leaves the shipped code the same), 20 rounds, the full agent_daemon binary each time, under one continuous load: a 24 GiB anon-memory hog + 2× looping dd bs=1M count=6000 conv=fsync (dev-02, load1 6–19). Log: dev-02:~/j157-out/ab20/.

before after
rounds with a resource_unavailable refusal 5/20 (r2, r7, r12, r15, r19) 0/20
person_preparation_honors_explicit_state… failed 4 1 (r12, see below)
private_readiness_does_not_refuse_person_revocation failed 4 1 (r12)
keeps_background_backend_probe_off_the_control_path (lazy manager) failed 3 0
preserves_input_and_output_across_24_abrupt_stops failed 2 (refused open) 2 (TimeoutError on a 20 s fixture socket send, r2/r12)
rootless_retirement_releases_person_terminal_records failed 4 5 (fixture terminal did not retire)
fully clean suites 12/20 14/20

Honest reading. This load is harsher than the gate's: fsync p50 is 13 s. Besides the refusal, it trips 20 s/5 s wall-clock fixture waits on BOTH sides. That is J130's storage-stall family (finding 3 above), and it is not this bug. After r12 failed 9 tests with TimeoutError in 24 s: a whole-machine stall while I was copying 760 MB into the gate target (my perturbation, on the after side by chance). So under this load the target signature goes from 5/20 to 0/20, but the suite is not 20/20 on either side.

Next: the same interleaved 20 rounds under gate-like load (a looping cargo test --workspace --no-run rebuild beside another job's real verify fast that is running on dev-02 now, no synthetic hog/dd), to get the 20/20 number under the load the gates actually had (~/j157-out/gl20/).

**J157 finding 3: before/after under the reproducing load.** Interleaved (`before` = dev `b0eb5237c`, `after` = `33c392fbf`; the later fmt-only fixup leaves the shipped code the same), 20 rounds, the full `agent_daemon` binary each time, under one continuous load: a 24 GiB anon-memory hog + 2× looping `dd bs=1M count=6000 conv=fsync` (dev-02, load1 6–19). Log: `dev-02:~/j157-out/ab20/`. | | before | after | |---|---|---| | rounds with a `resource_unavailable` refusal | **5/20** (r2, r7, r12, r15, r19) | **0/20** | | `person_preparation_honors_explicit_state…` failed | 4 | 1 (r12, see below) | | `private_readiness_does_not_refuse_person_revocation` failed | 4 | 1 (r12) | | `keeps_background_backend_probe_off_the_control_path` (lazy manager) failed | 3 | **0** | | `preserves_input_and_output_across_24_abrupt_stops` failed | 2 (refused open) | 2 (`TimeoutError` on a 20 s fixture socket send, r2/r12) | | `rootless_retirement_releases_person_terminal_records` failed | 4 | 5 (`fixture terminal did not retire`) | | fully clean suites | 12/20 | 14/20 | **Honest reading.** This load is harsher than the gate's: fsync p50 is 13 s. Besides the refusal, it trips 20 s/5 s wall-clock fixture waits on BOTH sides. That is J130's storage-stall family (finding 3 above), and it is not this bug. After r12 failed 9 tests with `TimeoutError` in 24 s: a whole-machine stall while I was copying 760 MB into the gate target (my perturbation, on the `after` side by chance). So under this load the target signature goes from 5/20 to 0/20, but the suite is not 20/20 on either side. Next: the same interleaved 20 rounds under gate-like load (a looping `cargo test --workspace --no-run` rebuild beside another job's real `verify fast` that is running on dev-02 now, no synthetic hog/dd), to get the 20/20 number under the load the gates actually had (`~/j157-out/gl20/`).
kayg commented 2026-11-04 01:34:39 +00:00
Author
Owner
Copy link

J157 progress. Head fe53807b7 (4 commits on b0eb5237c, all pushed):

  1. 412e7b383 test: adopted-children oracle reads a job pid only once it is written (the j93c pid-file race).
  2. 6baf8fc29 agent: a terminal launch waits on its launcher, not the 2 s shim handshake (product).
  3. 7d03f6be2 agent: a slow init-manager probe no longer selects Detached for the daemon's life (product).
  4. fe53807b7 test: the retired-records oracle waits for retirement with a hang bound, not a 5 s race (fixture).

Why (4). Under gate-like load (a looping cargo test --workspace --no-run rebuild next to another job's verify fast), the after side still failed rootless_retirement_releases_person_terminal_records in 2 of the first 7 rounds with "fixture terminal did not retire". I measured retirement (the oracle with the bound lifted and the time printed) under that load: n=48, p50 0.22 s, p90 1.56 s, max 5.09 s against a 5 s fixture bound. Retirement is asynchronous shim work behind a directory sync_all; no viewer waits on it. The bound is now 30 s, above dev-02's worst measured fsync stall (19.6 s). It only detects a hang.

Partial gate-load data on the launch fix alone (rounds 1–7, ~/j157-out/gl20-partial-launchfix-only/): before r2 failed exactly like the gates (4 tests in 18.4 s, all resource_unavailable). After: 0 refusals.

Now running: a fresh interleaved 20-round before/after under the same gate-like load, with after = fe53807b7 (~/j157-out/gl20/). Then verify fast on dev-02.

**J157 progress.** Head `fe53807b7` (4 commits on `b0eb5237c`, all pushed): 1. `412e7b383` test: adopted-children oracle reads a job pid only once it is written (the j93c pid-file race). 2. `6baf8fc29` agent: a terminal launch waits on its launcher, not the 2 s shim handshake (product). 3. `7d03f6be2` agent: a slow init-manager probe no longer selects Detached for the daemon's life (product). 4. `fe53807b7` test: the retired-records oracle waits for retirement with a hang bound, not a 5 s race (fixture). **Why (4).** Under gate-like load (a looping `cargo test --workspace --no-run` rebuild next to another job's `verify fast`), the `after` side still failed `rootless_retirement_releases_person_terminal_records` in 2 of the first 7 rounds with "fixture terminal did not retire". I measured retirement (the oracle with the bound lifted and the time printed) under that load: n=48, p50 0.22 s, p90 1.56 s, **max 5.09 s** against a 5 s fixture bound. Retirement is asynchronous shim work behind a directory `sync_all`; no viewer waits on it. The bound is now 30 s, above dev-02's worst measured fsync stall (19.6 s). It only detects a hang. Partial gate-load data on the launch fix alone (rounds 1–7, `~/j157-out/gl20-partial-launchfix-only/`): before r2 failed exactly like the gates (4 tests in 18.4 s, all `resource_unavailable`). After: 0 refusals. Now running: a fresh interleaved 20-round before/after under the same gate-like load, with `after` = `fe53807b7` (`~/j157-out/gl20/`). Then `verify fast` on dev-02.
kayg commented 2026-11-04 02:55:17 +00:00
Author
Owner
Copy link

J157 finding 5: dropped the Manager::detect change. It needs a decision. It is parked on agent-resource-j157-detect (7d03f6be2).

Gate-like load A/B (interleaved, 20 rounds: a looping cargo test --workspace --no-run rebuild beside another job's real verify fast; before = dev, after = launch fix + detect change + fixture fixes; ~/j157-out/gl20/):

before after
rounds with a resource_unavailable refusal 1/20 (r1: 4 tests failed in 16 s, the gates' exact shape) 0/20
rounds with 20 s fixture TimeoutErrors 0/20 3/20 (r11: 4 tests, r13: 9 tests, r17: 9 tests, all waiting on terminal.open)
clean suites 16/20 17/20

What the after-side timeouts are. In both r13 and r17, the user journal shows the systemd user manager started NO scope for 24 s (02:12:07→02:12:31) and 27 s (02:23:33→02:24:00). With no tests running, the same load gives the same kind of stall: systemd-run --user --scope true p50 15 ms, but max 11.0 s, at the same second as an 11.2 s fsync stall (fsync max 48.5 s in that 10-minute probe, ~/j157-out/probe-gateload.txt). So dev-02's storage stalls also stall systemd's user manager. My code does not cause them.

Why only the after side showed them. The old detect gives up at 2 s and caches Detached for the daemon's life. Detached shims are forked directly and never go through the manager again, so in the old code a manager stall at daemon start is absorbed silently. With the detect change, the first open waits for the stalled manager (bound 30 s), and the fixture's 20 s per-read timeout fires first.

The trade-off (owner's call, not this job's):

  • Keep the 2 s fallback (dev, and now this branch): terminals keep opening during a manager stall, but on that daemon every terminal is launched without its own scope. A rootless infraternal agent that starts on a loaded host keeps that placement until it restarts. Measured: probe 2–20 ms typically, up to 4.73 s under memory + fsync load.
  • Wait for the manager (the parked commit): placement is always correct, but opens wait out a manager stall (up to 30 s) and the gate's 20 s fixture reads fail during such stalls on dev-02.
  • A third option for the owner: on a probe timeout, use Detached for that open but do not cache it, so the next open probes again. That changes the "probe once, never re-probed" contract in sessions.rs.

Branch now 485d2bf38: 412e7b383 (pid-file fixture), 6baf8fc29 (launch waits on its launcher), 485d2bf38 (retired-records fixture). The launch fix alone is never worse for the tests: a manager stall of 2–20 s used to refuse the open and now succeeds; a stall over 20 s fails both before and after. Running now on dev-02: verify fast at 485d2bf38 and 20 rounds of the final head under the same gate-like load (~/j157-out/after20/).

**J157 finding 5: dropped the `Manager::detect` change. It needs a decision.** It is parked on `agent-resource-j157-detect` (`7d03f6be2`). **Gate-like load A/B** (interleaved, 20 rounds: a looping `cargo test --workspace --no-run` rebuild beside another job's real `verify fast`; `before` = dev, `after` = launch fix + detect change + fixture fixes; `~/j157-out/gl20/`): | | before | after | |---|---|---| | rounds with a `resource_unavailable` refusal | 1/20 (r1: 4 tests failed in 16 s, the gates' exact shape) | **0/20** | | rounds with 20 s fixture `TimeoutError`s | **0/20** | **3/20** (r11: 4 tests, r13: 9 tests, r17: 9 tests, all waiting on `terminal.open`) | | clean suites | 16/20 | 17/20 | **What the after-side timeouts are.** In both r13 and r17, the user journal shows the systemd user manager started NO scope for 24 s (02:12:07→02:12:31) and 27 s (02:23:33→02:24:00). With no tests running, the same load gives the same kind of stall: `systemd-run --user --scope true` p50 15 ms, but **max 11.0 s**, at the same second as an 11.2 s fsync stall (fsync max 48.5 s in that 10-minute probe, `~/j157-out/probe-gateload.txt`). So dev-02's storage stalls also stall systemd's user manager. My code does not cause them. **Why only the after side showed them.** The old `detect` gives up at 2 s and caches `Detached` for the daemon's life. Detached shims are forked directly and never go through the manager again, so in the old code a manager stall at daemon start is absorbed silently. With the detect change, the first open waits for the stalled manager (bound 30 s), and the fixture's 20 s per-read timeout fires first. **The trade-off (owner's call, not this job's):** - Keep the 2 s fallback (dev, and now this branch): terminals keep opening during a manager stall, but on that daemon every terminal is launched without its own scope. A rootless infraternal agent that starts on a loaded host keeps that placement until it restarts. Measured: probe 2–20 ms typically, up to 4.73 s under memory + fsync load. - Wait for the manager (the parked commit): placement is always correct, but opens wait out a manager stall (up to 30 s) and the gate's 20 s fixture reads fail during such stalls on dev-02. - A third option for the owner: on a probe timeout, use Detached for that open but do not cache it, so the next open probes again. That changes the "probe once, never re-probed" contract in `sessions.rs`. **Branch now** `485d2bf38`: `412e7b383` (pid-file fixture), `6baf8fc29` (launch waits on its launcher), `485d2bf38` (retired-records fixture). The launch fix alone is never worse for the tests: a manager stall of 2–20 s used to refuse the open and now succeeds; a stall over 20 s fails both before and after. Running now on dev-02: `verify fast` at `485d2bf38` and 20 rounds of the final head under the same gate-like load (`~/j157-out/after20/`).
kayg commented 2026-11-04 04:05:10 +00:00
Author
Owner
Copy link

J157 finish. Branch agent-resource-j157, head 485d2bf38166ad7bd6a46063db0f80687f2543f8: 3 commits on origin/dev b0eb5237c. Not merged, not deployed. The Manager::detect change is parked on agent-resource-j157-detect (7d03f6be2) and needs a decision (finding 5).

Commits

  • 412e7b383 test: the adopted-children oracle reads a job pid only once it is written (the j93c "pid-file race", a pure fixture race).
  • 6baf8fc29 agent: a terminal launch waits on its launcher, not the 2 s shim handshake (product fix). This is the root cause of the gates' resource_unavailable / "terminal unavailable": on a loaded host, process creation through the user manager + exec + shim startup took 3.5–5.1 s, and the 2 s HANDSHAKE refused healthy launches (and left their shells running). Real users on busy hosts hit the same refusal.
  • 485d2bf38 test: the retired-records oracle waits for retirement with a hang bound, not a 5 s race (fixture; retirement measured at up to 5.09 s under gate load).

Before / after: what the fix removes

run (dev-02) before (dev) after
memory + fsync load, 20 interleaved rounds 5/20 rounds with resource_unavailable 0/20
gate-like load, 20 interleaved rounds 1/20 (4 tests refused in 16 s, the gates' exact shape) 0/20
gate-like load, final head, 20 rounds (n/a) 0/20
during my verify fast, final head, 20 rounds (n/a) 0/20; 20/20 suites clean (load1 0.7–15)

Across 80 runs of the fixed builds, no open was refused with resource_unavailable.

The 20/20 target is NOT met under gate-like load. Final head under gate-like load: 14/20 suites clean. Every failure is a 20 s fixture TimeoutError (r1: 10 tests, r7: 10, r9: 9, r16: 9, r13: 2, r20: 1). They come from 20–45 s whole-host stalls on dev-02. In those windows the user manager starts no scope for 24–47 s, and one shim took 24 s from exec to binding its socket (scope started 02:12:07, socket bound 02:12:30). The fsync probe with no tests running shows the same stalls (fsync max 48.5 s, systemd-run --scope max 11.0 s). In such a stall, the old build refuses at 2 s and the new one waits until the fixture's 20 s read times out, so both fail. A stall of 2–20 s now succeeds where it used to fail.
Open doubt, said plainly: in the interleaved gate-load run, these mass timeouts landed on the after side 3 times and the before side 0 times (the before side had 1 mass refusal instead). Over all after runs under gate load it is 7 stalled rounds in 40. I found no mechanism in the change that waits without the host stalling. The stalled windows show the stall in the shim's own exec→bind and in the user manager, outside the agent. But I have not proven the rate difference is chance. A follow-up should run interleaved before/after with a bind/fsync latency sampler beside it before this merges, if the orchestrator wants that settled. The remaining cause is dev-02 storage (J130's finding 3): a host fix, not an agent fix.

Also measured, not changed

  • inotify: dev-02 max_user_instances = 128 (compute01 8192). Exhausting it reproduces the same refusal (TooManyOpenFiles). But the failed gate opens had already spawned their shims, so inotify was not the cause. Peak use over 15 min of 4 busy gates on compute01 (root sampler, sees non-dumpable processes): 16.
  • Leak: a launch that fails after spawn still leaves its shim and shell running (13 from today's gates were alive on dev-02). Mine are killed. The other jobs' leaked shims are still there; I left them to the orchestrator.

scripts/verify fast, dev-02, verbatim (run alongside my own 20-round load run):

HEAD 485d2bf38166ad7bd6a46063db0f80687f2543f8
PT_INTERP: absent; size: 22481648 bytes; baseline 22418256; delta +63392 bytes (report only, D243 amended 2026-11-01)
PT_INTERP: absent; size: 20505112 bytes; baseline 20453280; delta +51832 bytes (report only, D243 amended 2026-11-01)
x86_64-unknown-linux-musl: +102400 bytes accepted with delta; zero/negative deltas reported; missing/wrong/dynamic/PT_INTERP/inspection defects rejected PASS
aarch64-unknown-linux-musl: +102400 bytes accepted with delta; zero/negative deltas reported; missing/wrong/dynamic/PT_INTERP/inspection defects rejected PASS
+---------------------------+--------------+
| Rust gate                 | Result       |
+---------------------------+--------------+
| fmt                       | PASS         |
| VT native fmt             | PASS         |
| CLI plugin compile-out    | PASS         |
| clippy                    | PASS         |
| SSH runtime test helper   | PASS         |
| real-binary fixture paths | PASS         |
| test                      | PASS         |
| SSH inode reuse fixture   | PASS         |
| shim process-per-user     | PASS         |
| cargo-deny                | PASS         |
| gate architecture         | PASS         |
| static builds             | PASS         |
+---------------------------+--------------+
2 tests failed:
(fail) text sidecars round-trip deterministically; compressed formats remain identity [6197.15ms]
  ^ this test timed out after 5000ms.
(fail) a close reason stays visible through a quick reconnect and clears after a stable attach [5122.02ms]
  ^ this test timed out after 5000ms.

 589 pass
 2 fail
 23765 expect() calls
Ran 591 tests across 93 files. [58.82s]
VERIFY_EXIT=1

The gate is red only on two tracer bun test cases that hit bun's 5 s per-test timeout. The branch changes no TypeScript (diff: crates/infraternal-agent/src/sessions.rs, its unit tests, two Python oracles). The same tracer test set, rerun at this head right after the gate, under load1 15: 591 pass, 0 fail. Both tests alone: 3/3. Not reproduced on unmodified dev (no slot/time left), so this is stated, not proven. Size: deltas are against the gate's stored baseline, not origin/dev; the agent change adds a small async wait function.

Real environment: the proof runs are the real binary as a rootless user on dev-02 (the path this changes). No root-only path. Not tried on the test server: that needs an orchestrator deploy.

Cleanup: dev-02 worktrees j157*, their targets and the scratch target are deleted, and my leaked shims are killed by PID. compute01 root sampler files are removed. Logs are kept in dev-02:~/j157-out/ (ab20/, gl20/, gl20-partial-launchfix-only/, after20/, glafter/, retire/, probe-*.txt, verify-fast.log).

**J157 finish.** Branch `agent-resource-j157`, head `485d2bf38166ad7bd6a46063db0f80687f2543f8`: 3 commits on origin/dev `b0eb5237c`. Not merged, not deployed. The `Manager::detect` change is parked on `agent-resource-j157-detect` (`7d03f6be2`) and needs a decision (finding 5). **Commits** - `412e7b383` test: the adopted-children oracle reads a job pid only once it is written (the j93c "pid-file race", a pure fixture race). - `6baf8fc29` agent: a terminal launch waits on its launcher, not the 2 s shim handshake (**product fix**). This is the root cause of the gates' `resource_unavailable` / "terminal unavailable": on a loaded host, process creation through the user manager + exec + shim startup took 3.5–5.1 s, and the 2 s `HANDSHAKE` refused healthy launches (and left their shells running). Real users on busy hosts hit the same refusal. - `485d2bf38` test: the retired-records oracle waits for retirement with a hang bound, not a 5 s race (fixture; retirement measured at up to 5.09 s under gate load). **Before / after: what the fix removes** | run (dev-02) | before (dev) | after | |---|---|---| | memory + fsync load, 20 interleaved rounds | 5/20 rounds with `resource_unavailable` | 0/20 | | gate-like load, 20 interleaved rounds | 1/20 (4 tests refused in 16 s, the gates' exact shape) | 0/20 | | gate-like load, final head, 20 rounds | (n/a) | 0/20 | | during my `verify fast`, final head, 20 rounds | (n/a) | 0/20; **20/20 suites clean** (load1 0.7–15) | Across 80 runs of the fixed builds, no open was refused with `resource_unavailable`. **The 20/20 target is NOT met under gate-like load.** Final head under gate-like load: **14/20 suites clean**. Every failure is a 20 s fixture `TimeoutError` (r1: 10 tests, r7: 10, r9: 9, r16: 9, r13: 2, r20: 1). They come from 20–45 s whole-host stalls on dev-02. In those windows the user manager starts no scope for 24–47 s, and one shim took **24 s from exec to binding its socket** (scope started 02:12:07, socket bound 02:12:30). The fsync probe with no tests running shows the same stalls (fsync max 48.5 s, `systemd-run --scope` max 11.0 s). In such a stall, the old build refuses at 2 s and the new one waits until the fixture's 20 s read times out, so both fail. A stall of 2–20 s now succeeds where it used to fail. **Open doubt, said plainly:** in the interleaved gate-load run, these mass timeouts landed on the `after` side 3 times and the `before` side 0 times (the before side had 1 mass refusal instead). Over all after runs under gate load it is 7 stalled rounds in 40. I found no mechanism in the change that waits without the host stalling. The stalled windows show the stall in the shim's own exec→bind and in the user manager, outside the agent. But I have not proven the rate difference is chance. A follow-up should run interleaved before/after with a bind/fsync latency sampler beside it before this merges, if the orchestrator wants that settled. The remaining cause is dev-02 storage (J130's finding 3): a host fix, not an agent fix. **Also measured, not changed** - inotify: dev-02 `max_user_instances` = 128 (compute01 8192). Exhausting it reproduces the same refusal (`TooManyOpenFiles`). But the failed gate opens had already spawned their shims, so inotify was not the cause. Peak use over 15 min of 4 busy gates on compute01 (root sampler, sees non-dumpable processes): 16. - Leak: a launch that fails after spawn still leaves its shim and shell running (13 from today's gates were alive on dev-02). Mine are killed. The other jobs' leaked shims are still there; I left them to the orchestrator. **`scripts/verify fast`, dev-02, verbatim** (run alongside my own 20-round load run): ``` HEAD 485d2bf38166ad7bd6a46063db0f80687f2543f8 PT_INTERP: absent; size: 22481648 bytes; baseline 22418256; delta +63392 bytes (report only, D243 amended 2026-11-01) PT_INTERP: absent; size: 20505112 bytes; baseline 20453280; delta +51832 bytes (report only, D243 amended 2026-11-01) x86_64-unknown-linux-musl: +102400 bytes accepted with delta; zero/negative deltas reported; missing/wrong/dynamic/PT_INTERP/inspection defects rejected PASS aarch64-unknown-linux-musl: +102400 bytes accepted with delta; zero/negative deltas reported; missing/wrong/dynamic/PT_INTERP/inspection defects rejected PASS +---------------------------+--------------+ | Rust gate | Result | +---------------------------+--------------+ | fmt | PASS | | VT native fmt | PASS | | CLI plugin compile-out | PASS | | clippy | PASS | | SSH runtime test helper | PASS | | real-binary fixture paths | PASS | | test | PASS | | SSH inode reuse fixture | PASS | | shim process-per-user | PASS | | cargo-deny | PASS | | gate architecture | PASS | | static builds | PASS | +---------------------------+--------------+ 2 tests failed: (fail) text sidecars round-trip deterministically; compressed formats remain identity [6197.15ms] ^ this test timed out after 5000ms. (fail) a close reason stays visible through a quick reconnect and clears after a stable attach [5122.02ms] ^ this test timed out after 5000ms. 589 pass 2 fail 23765 expect() calls Ran 591 tests across 93 files. [58.82s] VERIFY_EXIT=1 ``` The gate is red only on two tracer `bun test` cases that hit bun's 5 s per-test timeout. The branch changes no TypeScript (diff: `crates/infraternal-agent/src/sessions.rs`, its unit tests, two Python oracles). The same tracer test set, rerun at this head right after the gate, under load1 15: `591 pass, 0 fail`. Both tests alone: 3/3. Not reproduced on unmodified dev (no slot/time left), so this is stated, not proven. Size: deltas are against the gate's stored baseline, not origin/dev; the agent change adds a small async wait function. **Real environment:** the proof runs are the real binary as a rootless user on dev-02 (the path this changes). No root-only path. Not tried on the test server: that needs an orchestrator deploy. **Cleanup:** dev-02 worktrees `j157*`, their targets and the scratch target are deleted, and my leaked shims are killed by PID. compute01 root sampler files are removed. Logs are kept in `dev-02:~/j157-out/` (`ab20/`, `gl20/`, `gl20-partial-launchfix-only/`, `after20/`, `glafter/`, `retire/`, `probe-*.txt`, `verify-fast.log`).
kayg commented 2026-11-04 05:15:59 +00:00
Author
Owner
Copy link

J170 start. Branch launch-followups-j170, base origin/dev 81c432ef7. Worktree on code; red/green and the gate on dev-02-std through build-slot (compute01 queue was 7 deep).

Plan (~2 h):

  1. Manager::resolve: cache only an answered probe; a 2 s timeout means Detached for that launch alone, next launch probes again. Test fails first against cache-everything.
  2. abandon_launch: a launch that fails after spawn stops its own scope (systemctl [--user] stop infraternal-session-<id>.scope), asks the shim to close through its own authenticated rendezvous (session id in Hello), and kills the launcher child it owns and has not reaped. No name matching. Test fails first without the call.

Known limit I will report: a --detach shim that never answers Hello has no pid the launcher can know safely, so only the scope/Hello/launcher routes apply.

**J170 start.** Branch `launch-followups-j170`, base origin/dev `81c432ef7`. Worktree on `code`; red/green and the gate on dev-02-std through build-slot (compute01 queue was 7 deep). Plan (~2 h): 1. `Manager::resolve`: cache only an answered probe; a 2 s timeout means Detached for that launch alone, next launch probes again. Test fails first against cache-everything. 2. `abandon_launch`: a launch that fails after spawn stops its own scope (`systemctl [--user] stop infraternal-session-<id>.scope`), asks the shim to close through its own authenticated rendezvous (session id in Hello), and kills the launcher child it owns and has not reaped. No name matching. Test fails first without the call. Known limit I will report: a `--detach` shim that never answers Hello has no pid the launcher can know safely, so only the scope/Hello/launcher routes apply.
kayg commented 2026-11-04 09:01:30 +00:00
Author
Owner
Copy link

J170 progress. Branch launch-followups-j170, head b3201d81c068315dcfcebfb1afb8c05363f1095a (2 atomic commits on origin/dev 81c432ef7, pushed).

  • 664f12f54 agent: an unanswered init-manager probe stands for one launch and is not remembered. Manager::resolve caches only an answer; a 2 s timeout means Detached for that launch only, the next launch probes again.
  • b3201d81c agent: a failed terminal launch stops the terminal and shell it started (abandon_launch: Hello-authenticated CloseSession, its own scope via systemctl stop, its own unreaped launcher child; never by name).

Fails first (dev-02, at a head before squash, same code):

test sessions::tests::unanswered_manager_probe_is_not_remembered ... FAILED
a stall must not be cached
test sessions::tests::failed_launch_stops_the_process_it_started_and_no_other ... FAILED
the failed launch left its process 1222158 running

Green with the change: both pass, plus the 20 other sessions/rootless unit tests.

Gate, honestly: scripts/verify fast at b3201d81c is RED on the test row only (fmt, clippy, static builds, deny all PASS): two real-binary agent_daemon tests failed:

  • real_binary_keeps_background_backend_probe_off_the_control_path: TimeoutError in probe_listener.accept() (the signature J157 recorded on unmodified dev under load).
  • real_binary_rootless_revocation_after_restart_stops_every_owned_terminal: AssertionError: (347, 0) at agent_fixture.attach. New to me.
    dev-02 was at load ~37 during the run. I am running both tests interleaved on this head and on unmodified 81c432ef7, same host, to see whether the head is worse. Result follows; I'm not calling these flakes until that is in.
**J170 progress.** Branch `launch-followups-j170`, head `b3201d81c068315dcfcebfb1afb8c05363f1095a` (2 atomic commits on origin/dev `81c432ef7`, pushed). - `664f12f54` agent: an unanswered init-manager probe stands for one launch and is not remembered. `Manager::resolve` caches only an answer; a 2 s timeout means Detached for that launch only, the next launch probes again. - `b3201d81c` agent: a failed terminal launch stops the terminal and shell it started (`abandon_launch`: Hello-authenticated CloseSession, its own scope via `systemctl stop`, its own unreaped launcher child; never by name). **Fails first (dev-02, at a head before squash, same code):** ``` test sessions::tests::unanswered_manager_probe_is_not_remembered ... FAILED a stall must not be cached test sessions::tests::failed_launch_stops_the_process_it_started_and_no_other ... FAILED the failed launch left its process 1222158 running ``` Green with the change: both pass, plus the 20 other sessions/rootless unit tests. **Gate, honestly:** `scripts/verify fast` at `b3201d81c` is RED on the `test` row only (fmt, clippy, static builds, deny all PASS): two real-binary `agent_daemon` tests failed: - `real_binary_keeps_background_backend_probe_off_the_control_path`: `TimeoutError` in `probe_listener.accept()` (the signature J157 recorded on unmodified dev under load). - `real_binary_rootless_revocation_after_restart_stops_every_owned_terminal`: `AssertionError: (347, 0)` at `agent_fixture.attach`. New to me. dev-02 was at load ~37 during the run. I am running both tests interleaved on this head and on unmodified `81c432ef7`, same host, to see whether the head is worse. Result follows; I'm not calling these flakes until that is in.
kayg commented 2026-11-04 14:13:36 +00:00
Author
Owner
Copy link

J170 finish: NOT green. Branch launch-followups-j170, head 86d39249af081215ad478bd668f568694187d212 (3 atomic commits on origin/dev 81c432ef7, pushed; not merged, not deployed).

Commits

  • 664f12f54 an unanswered init-manager probe stands for one launch and is not remembered (Manager::resolve; the 2 s fallback is kept for that launch, the next launch asks again).
  • b3201d81c a failed terminal launch stops the terminal and shell it started (abandon_launch: Hello-authenticated CloseSession, its own scope systemctl [--user] stop infraternal-session-<id>.scope, its own unreaped launcher child; never by name, never another session).
  • 86d39249a a failed launch keeps closing a slow detached shim until it answers (J163's #334 question 2).

#334 question 2: same leak, one residual route. automation::create launches Manager::Detached; the shim is --detached, so its launcher exits and only the Hello-authenticated close can name it. A shim that bound but missed the 2 s Hello failed the launch ("operation failed") and lived on, unnamed. b3201d81c already covered launches that fail while the launcher lives; that route needed 86d39249a, which retries the close until 10 s (CLEANUP) instead of once.

Tests fail first (dev-02, verbatim)

test sessions::tests::unanswered_manager_probe_is_not_remembered ... FAILED
a stall must not be cached
test sessions::tests::failed_launch_stops_the_process_it_started_and_no_other ... FAILED
the failed launch left its process 1222158 running
test sessions::tests::slow_hello_failure_still_closes_the_detached_session_it_started ... FAILED
the failed launch left its detached session running: Elapsed(())

Green with the changes: all three pass; the 21 sessions/rootless unit tests pass. scripts/verify quick at 86d39249a: fmt PASS, clippy (workspace) PASS, unit tests (changed) PASS (142 passed).

scripts/verify fast at 86d39249a, dev-02, verbatim table (VERIFY_EXIT=1):

| fmt                       | PASS         |
| VT native fmt             | PASS         |
| CLI plugin compile-out    | PASS         |
| clippy                    | PASS         |
| SSH runtime test helper   | PASS         |
| real-binary fixture paths | PASS         |
| Traefik component manifest | PASS         |
| release component publication | PASS         |
| test                      | FAIL (101)   |
| SSH inode reuse fixture   | PASS         |
| shim process-per-user     | PASS         |
| cargo-deny                | PASS         |
| gate architecture         | PASS         |
| static builds             | PASS         |

Failing: remote_client::tests::unavailable_udp_does_not_add_seconds_before_tcp_admission (fallback_local_admission_us=2576793 against a 2 s bound), winning_quic_drops_the_unfinished_tls_dial_before_control_dispatch, supervisor::review_tests::durable_state::acknowledged_network_access_survives_superseded_worker_completion. All remote-client wall-clock tests, nothing in sessions/launch.

Honest reading of the gate. I ran the full gate four times; the test row failed every time, on a different unrelated set each time: (1) three restart::tests (strip test image exceeded 60s, "running for over 60 seconds", static builds 1208 s), (2) agent_daemon startup reads, (3) the remote_client tests above. Earlier run 1 also failed agent_daemon tests in Binary.start(). dev-02 load was 12 to 37 during most. Targeted comparisons, same host, interleaved, cargo test --workspace --locked --lib -- remote_client:: (57 tests): head 10/10 pass, unmodified 81c432ef7 5/5 pass at load 1 to 4; the two agent_daemon revocation/lazy-manager tests: head 8/8, base 4/4 at load ~5. So I found no sign the head is worse, but I did not get one full green gate and I have not shown the remote_client failures on unmodified dev under the full-suite load. A first A/B of mine ran 0 tests (feature filter); I discarded it.

Not done / limits

  • A --detach shim that never answers Hello within 10 s, and whose launcher already exited, still cannot be named safely (no pid the launcher knows; peer-cred pid can be reused). It is reported, not fixed.
  • The scope-stop route (systemctl stop) is code-reviewed but not exercised by a test or on a real user manager.
  • No root-only path, no test-server proof (needs an orchestrator deploy).
  • Size: static D243 deltas are against the gate's stored baseline (x86 +739072, arm +620384 bytes), which also includes unrelated merges since that baseline; I did not separate this branch's share.
  • dev-02-only: leftovers from other jobs there are untouched.

For kayg/orchestrator: want a gate rerun on a quiet dev-02 (or compute01) before merge?

**J170 finish: NOT green. Branch `launch-followups-j170`, head `86d39249af081215ad478bd668f568694187d212`** (3 atomic commits on origin/dev `81c432ef7`, pushed; not merged, not deployed). **Commits** - `664f12f54` an unanswered init-manager probe stands for one launch and is not remembered (`Manager::resolve`; the 2 s fallback is kept for that launch, the next launch asks again). - `b3201d81c` a failed terminal launch stops the terminal and shell it started (`abandon_launch`: Hello-authenticated CloseSession, its own scope `systemctl [--user] stop infraternal-session-<id>.scope`, its own unreaped launcher child; never by name, never another session). - `86d39249a` a failed launch keeps closing a slow detached shim until it answers (J163's #334 question 2). **#334 question 2: same leak, one residual route.** `automation::create` launches `Manager::Detached`; the shim is `--detach`ed, so its launcher exits and only the Hello-authenticated close can name it. A shim that bound but missed the 2 s Hello failed the launch ("operation failed") and lived on, unnamed. `b3201d81c` already covered launches that fail while the launcher lives; that route needed `86d39249a`, which retries the close until 10 s (`CLEANUP`) instead of once. **Tests fail first (dev-02, verbatim)** ``` test sessions::tests::unanswered_manager_probe_is_not_remembered ... FAILED a stall must not be cached test sessions::tests::failed_launch_stops_the_process_it_started_and_no_other ... FAILED the failed launch left its process 1222158 running test sessions::tests::slow_hello_failure_still_closes_the_detached_session_it_started ... FAILED the failed launch left its detached session running: Elapsed(()) ``` Green with the changes: all three pass; the 21 sessions/rootless unit tests pass. `scripts/verify quick` at `86d39249a`: fmt PASS, clippy (workspace) PASS, unit tests (changed) PASS (142 passed). **`scripts/verify fast` at `86d39249a`, dev-02, verbatim table (VERIFY_EXIT=1):** ``` | fmt | PASS | | VT native fmt | PASS | | CLI plugin compile-out | PASS | | clippy | PASS | | SSH runtime test helper | PASS | | real-binary fixture paths | PASS | | Traefik component manifest | PASS | | release component publication | PASS | | test | FAIL (101) | | SSH inode reuse fixture | PASS | | shim process-per-user | PASS | | cargo-deny | PASS | | gate architecture | PASS | | static builds | PASS | ``` Failing: `remote_client::tests::unavailable_udp_does_not_add_seconds_before_tcp_admission` (fallback_local_admission_us=2576793 against a 2 s bound), `winning_quic_drops_the_unfinished_tls_dial_before_control_dispatch`, `supervisor::review_tests::durable_state::acknowledged_network_access_survives_superseded_worker_completion`. All remote-client wall-clock tests, nothing in sessions/launch. **Honest reading of the gate.** I ran the full gate four times; the `test` row failed every time, on a different unrelated set each time: (1) three `restart::tests` (`strip test image exceeded 60s`, "running for over 60 seconds", static builds 1208 s), (2) `agent_daemon` startup reads, (3) the remote_client tests above. Earlier run 1 also failed `agent_daemon` tests in `Binary.start()`. dev-02 load was 12 to 37 during most. Targeted comparisons, same host, interleaved, `cargo test --workspace --locked --lib -- remote_client::` (57 tests): **head 10/10 pass, unmodified `81c432ef7` 5/5 pass** at load 1 to 4; the two `agent_daemon` revocation/lazy-manager tests: head 8/8, base 4/4 at load ~5. So I found no sign the head is worse, but I did **not** get one full green gate and I have not shown the remote_client failures on unmodified dev under the full-suite load. A first A/B of mine ran 0 tests (feature filter); I discarded it. **Not done / limits** - A `--detach` shim that never answers Hello within 10 s, and whose launcher already exited, still cannot be named safely (no pid the launcher knows; peer-cred pid can be reused). It is reported, not fixed. - The scope-stop route (`systemctl stop`) is code-reviewed but not exercised by a test or on a real user manager. - No root-only path, no test-server proof (needs an orchestrator deploy). - Size: static D243 deltas are against the gate's stored baseline (x86 +739072, arm +620384 bytes), which also includes unrelated merges since that baseline; I did not separate this branch's share. - dev-02-only: leftovers from other jobs there are untouched. **For kayg/orchestrator:** want a gate rerun on a quiet dev-02 (or compute01) before merge?
kayg commented 2026-11-04 16:15:25 +00:00
Author
Owner
Copy link

J200 review start. Reviewing launch-followups-j170 at 86d39249af against base 81c432ef7. No branch changes. Read J157/J170 reports and BRIEF-COMMON (foreground waits). Plan (~2 h): trace cleanup ownership, registration and manager probing; inspect and exercise scope stop; run interleaved base/head agent_daemon startup/revocation tests under load on a build VM; post APPROVE / REQUEST CHANGES with evidence. Checking both build queues before choosing the VM.

J200 review start. Reviewing launch-followups-j170 at 86d39249af081215ad478bd668f568694187d212 against base 81c432ef7. No branch changes. Read J157/J170 reports and BRIEF-COMMON (foreground waits). Plan (~2 h): trace cleanup ownership, registration and manager probing; inspect and exercise scope stop; run interleaved base/head agent_daemon startup/revocation tests under load on a build VM; post APPROVE / REQUEST CHANGES with evidence. Checking both build queues before choosing the VM.
kayg commented 2026-11-04 16:17:10 +00:00
Author
Owner
Copy link

J200 finding (P1): the new detached close retry is cancelled by its actual caller.

automation::execute wraps discovery + operations::run in tokio::time::timeout(Duration::from_secs(4), ...) (automation.rs:198). operations::create awaits launch_with_command directly (automation/operations.rs:333 onward). A missed launch Hello consumes 2 s; the first cleanup Hello consumes another 2 s. The 4 s outer timeout drops launch_configured and abandon_launch before the second cleanup attempt can complete. It also skips the launcher reap task below abandon_launch. Thus the new unit test (calling launch_configured with no caller timeout) does not prove the #334 automation route; a shim that answers on cleanup retry 2 still survives the real operation as an unnamed session.

The same cancellation boundary can happen before bind: the 30 s launch bound does not extend the automation caller's 4 s budget, and the async cleanup is reached only on returned errors, not future cancellation. Please make cleanup ownership survive caller cancellation and cover the actual automation deadline with a regression test. I am preparing a bounded reproduction alongside the requested base/head checks; no branch changes.

J200 finding (P1): the new detached close retry is cancelled by its actual caller. `automation::execute` wraps discovery + `operations::run` in `tokio::time::timeout(Duration::from_secs(4), ...)` (`automation.rs:198`). `operations::create` awaits `launch_with_command` directly (`automation/operations.rs:333` onward). A missed launch Hello consumes 2 s; the first cleanup Hello consumes another 2 s. The 4 s outer timeout drops `launch_configured` and `abandon_launch` before the second cleanup attempt can complete. It also skips the launcher reap task below `abandon_launch`. Thus the new unit test (calling launch_configured with no caller timeout) does not prove the #334 automation route; a shim that answers on cleanup retry 2 still survives the real operation as an unnamed session. The same cancellation boundary can happen before bind: the 30 s launch bound does not extend the automation caller's 4 s budget, and the async cleanup is reached only on returned errors, not future cancellation. Please make cleanup ownership survive caller cancellation and cover the actual automation deadline with a regression test. I am preparing a bounded reproduction alongside the requested base/head checks; no branch changes.
kayg commented 2026-11-04 16:23:13 +00:00
Author
Owner
Copy link

J200 progress: exact scope-stop argv exercised on compute01 using private UUID-named sleep scopes, both as claude (--user) and root (system manager): active before stop, inactive after; no remaining test scope. Logs ~/j200-out/{scope,system-scope}.log. The build is queued on compute01 (both VMs busy; compute01 has more slots and shorter queue). Evidence harness is outside the repository, and no reviewed branch files changed. Interleaved A/B will run the two exact Python oracles invoked by agent_daemon, 20 rounds per ref, alternating order under concurrent gate load.

J200 progress: exact scope-stop argv exercised on compute01 using private UUID-named sleep scopes, both as claude (--user) and root (system manager): active before stop, inactive after; no remaining test scope. Logs ~/j200-out/{scope,system-scope}.log. The build is queued on compute01 (both VMs busy; compute01 has more slots and shorter queue). Evidence harness is outside the repository, and no reviewed branch files changed. Interleaved A/B will run the two exact Python oracles invoked by agent_daemon, 20 rounds per ref, alternating order under concurrent gate load.
kayg commented 2026-11-04 16:30:52 +00:00
Author
Owner
Copy link

J200 finding (P2): late bind skips the entire advertised cleanup window.

At sessions.rs:938, while path.exists() && now < deadline exits immediately if the 30 s launch deadline arrives before the detached shim binds. The initial detaching launcher has already been successfully reaped by wait_for_hello, so launcher.start_kill() is a no-op. A shim that binds at 31 s (inside the intended 30+10 s budget) receives no close attempt and survives the refused launch. This is narrower than the disclosed limitation of a shim that never answers within the cleanup budget: this one can answer inside that budget, but cleanup never waits for it.

Use the existing rendezvous readiness/progress mechanism during cleanup until the absolute cleanup deadline, and test a launcher that exits successfully before a socket is published during that window. The new tests bind before the initial Hello attempt, so they miss this race. An independent public-API reproduction is ready outside the repository; execution is waiting for compute01's build slot. No branch changes.

J200 finding (P2): late bind skips the entire advertised cleanup window. At `sessions.rs:938`, `while path.exists() && now < deadline` exits immediately if the 30 s launch deadline arrives before the detached shim binds. The initial detaching launcher has already been successfully reaped by `wait_for_hello`, so `launcher.start_kill()` is a no-op. A shim that binds at 31 s (inside the intended 30+10 s budget) receives no close attempt and survives the refused launch. This is narrower than the disclosed limitation of a shim that never answers within the cleanup budget: this one can answer inside that budget, but cleanup never waits for it. Use the existing rendezvous readiness/progress mechanism during cleanup until the absolute cleanup deadline, and test a launcher that exits successfully before a socket is published during that window. The new tests bind before the initial Hello attempt, so they miss this race. An independent public-API reproduction is ready outside the repository; execution is waiting for compute01's build slot. No branch changes.
kayg commented 2026-11-04 16:44:15 +00:00
Author
Owner
Copy link

J200 progress (30 min): code review found P1 automation cancellation and P2 late-bind gap (separate comments above). Both base/head default-feature binaries built on compute01; distinct SHA-256s, and abandon_launch symbol present only in the head artifact. My runner update collided with its start and stopped the shell after both builds; corrected immutable continuation is queued through build-slot, reusing those binaries. No reviewed branch changes. Next: 20 interleaved rounds/ref of the exact two agent_daemon oracles, cleanup reproductions (external harness), fast gate. Additional registration check prepared: a killed pre-bind person launcher may leave its durable .terminal reservation and refuse later revocation. One target directory only: compute01:/worktrees/j200/target. Logs/scripts: compute01:~/j200-out/.

J200 progress (~30 min): code review found P1 automation cancellation and P2 late-bind gap (separate comments above). Both base/head default-feature binaries built on compute01; distinct SHA-256s, and abandon_launch symbol present only in the head artifact. My runner update collided with its start and stopped the shell after both builds; corrected immutable continuation is queued through build-slot, reusing those binaries. No reviewed branch changes. Next: 20 interleaved rounds/ref of the exact two agent_daemon oracles, cleanup reproductions (external harness), fast gate. Additional registration check prepared: a killed pre-bind person launcher may leave its durable .terminal reservation and refuse later revocation. One target directory only: compute01:~/worktrees/j200/target. Logs/scripts: compute01:~/j200-out/.
kayg commented 2026-11-04 17:15:12 +00:00
Author
Owner
Copy link

J200 finding (P2), reproduced through the real default-feature head binary: killing a pre-bind launcher leaves a permanent person-terminal reservation.

Reservation::spawned() clears the reservation's rollback handle (rootless_sessions.rs:21; called from sessions.rs:901), but abandon_launch never retires that attribution after stopping the launcher/scope. If the shim never bound, no shim exists to write .ended or call retire_attribution. Subsequent rootless revocation treats the record as requiring shell-cleanup proof and refuses forever.

Independent real-binary reproduction on compute01, head 86d39249a: an own-account person open uses a fixture-owned systemd-run wrapper that records its PID and exec sleep 300, never binding a socket. The launch reaches its real 30 s bound; cleanup ends exactly that launcher (pidfd readiness proves exit). Then revoke the same person. Output, verbatim:

{"open_refusal": {"error": "terminal unavailable", "reason": "resource_unavailable"}, "launcher_exit_proven": true, "record_count": 1, "rendezvous_count": 0, "revoke_refusal": {"error": "terminal unavailable", "reason": "resource_unavailable"}}
AssertionError: failed launch left a terminal reservation with no live shim or sealed exit; person revocation cannot complete

Script/log: compute01 ~/j200-out/reservation.py, ~/j200-out/reservation.log. No branch modifications. Please preserve the conservative rule that a missing socket alone is not exit proof, but retire the failed launch's attribution when its own cleanup can prove retirement; cover refused person-open followed by revoke.

Requested load A/B is also complete: 20 rounds of BOTH exact Python oracles for EACH ref, alternating ref order: base lazy 20/20, base expanded revoke 20/20, head lazy 20/20, head expanded revoke 20/20. Four owned CPU workers and both oracles pinned to two cores, nice 10, alongside other builds; recorded load1 14.47–16.82. No reported startup/replay signature reproduced, so this does not prove those signatures are unrelated under dev-02's storage stalls. Worker processes are stopped/reaped. Evidence ~/j200-out/ab-summary.json and ab-*.log.

J200 finding (P2), reproduced through the real default-feature head binary: killing a pre-bind launcher leaves a permanent person-terminal reservation. `Reservation::spawned()` clears the reservation's rollback handle (`rootless_sessions.rs:21`; called from `sessions.rs:901`), but `abandon_launch` never retires that attribution after stopping the launcher/scope. If the shim never bound, no shim exists to write `.ended` or call `retire_attribution`. Subsequent rootless revocation treats the record as requiring shell-cleanup proof and refuses forever. Independent real-binary reproduction on compute01, head `86d39249a`: an own-account person open uses a fixture-owned `systemd-run` wrapper that records its PID and `exec sleep 300`, never binding a socket. The launch reaches its real 30 s bound; cleanup ends exactly that launcher (pidfd readiness proves exit). Then revoke the same person. Output, verbatim: ``` {"open_refusal": {"error": "terminal unavailable", "reason": "resource_unavailable"}, "launcher_exit_proven": true, "record_count": 1, "rendezvous_count": 0, "revoke_refusal": {"error": "terminal unavailable", "reason": "resource_unavailable"}} AssertionError: failed launch left a terminal reservation with no live shim or sealed exit; person revocation cannot complete ``` Script/log: compute01 `~/j200-out/reservation.py`, `~/j200-out/reservation.log`. No branch modifications. Please preserve the conservative rule that a missing socket alone is not exit proof, but retire the failed launch's attribution when its own cleanup can prove retirement; cover refused person-open followed by revoke. Requested load A/B is also complete: 20 rounds of BOTH exact Python oracles for EACH ref, alternating ref order: base lazy 20/20, base expanded revoke 20/20, head lazy 20/20, head expanded revoke 20/20. Four owned CPU workers and both oracles pinned to two cores, nice 10, alongside other builds; recorded load1 14.47–16.82. No reported startup/replay signature reproduced, so this does not prove those signatures are unrelated under dev-02's storage stalls. Worker processes are stopped/reaped. Evidence `~/j200-out/ab-summary.json` and `ab-*.log`.
kayg commented 2026-11-04 17:19:35 +00:00
Author
Owner
Copy link

J200 evidence correction: ab.log records cpu_affinity=[8,9], load_workers=4, nice=19. The runner inherited nice 10 and its explicit adjustment produced nice 19. Both refs/oracles/workers used the same priority; the 80/80 pass counts and load range are unchanged. The initial quick run failed five unit fixtures with NotFound because my custom test-state parent did not yet exist; the parent is created before fast, so these setup failures are not branch findings. The external cleanup harness has the expected result (uncancelled control passes; cancellation and late-bind cases fail); I am also rechecking against the workspace-pinned dependency lock. Full fast is running.

J200 evidence correction: ab.log records cpu_affinity=[8,9], load_workers=4, nice=19. The runner inherited nice 10 and its explicit adjustment produced nice 19. Both refs/oracles/workers used the same priority; the 80/80 pass counts and load range are unchanged. The initial quick run failed five unit fixtures with NotFound because my custom test-state parent did not yet exist; the parent is created before fast, so these setup failures are not branch findings. The external cleanup harness has the expected result (uncancelled control passes; cancellation and late-bind cases fail); I am also rechecking against the workspace-pinned dependency lock. Full fast is running.
kayg commented 2026-11-04 17:39:26 +00:00
Author
Owner
Copy link

J200 progress: the pinned external harness confirms both cleanup failures (automation cancellation and late bind), with the unwrapped J170 control passing. The requested agent_daemon comparison completed: 20/20 per oracle on each ref, 80 invocations total, alternating base/head under load. Fresh fast passes agent_daemon 11/11 but fails two remote-client tests (UDP fallback admission and server-path capability advertisement); its static-build row is still compiling. After it releases the checkout I will compare four remote-client cases using matched base/head builds, then attach evidence and post REQUEST CHANGES. No branch changes. Foreground monitoring continues; one target: compute01:~/worktrees/j200/target. Logs: ~/j200-out/.

J200 progress: the pinned external harness confirms both cleanup failures (automation cancellation and late bind), with the unwrapped J170 control passing. The requested agent_daemon comparison completed: 20/20 per oracle on each ref, 80 invocations total, alternating base/head under load. Fresh fast passes agent_daemon 11/11 but fails two remote-client tests (UDP fallback admission and server-path capability advertisement); its static-build row is still compiling. After it releases the checkout I will compare four remote-client cases using matched base/head builds, then attach evidence and post REQUEST CHANGES. No branch changes. Foreground monitoring continues; one target: compute01:~/worktrees/j200/target. Logs: ~/j200-out/.
kayg commented 2026-11-04 17:57:30 +00:00
Author
Owner
Copy link

J200: full fast finished on exact head with VERIFY_EXIT=1. Only Rust test row failed (two remote-client timeouts); static builds, clippy, fmt and all remaining Rust rows PASS. Agent_daemon 11/11 PASS. Requested base/head load comparison completed 80/80 PASS. Additional four-case remote-client comparison is queued through build-slot on compute01 (both VMs busy; reusing the sole existing target). No branch modifications. Final REQUEST CHANGES follows the comparison and evidence cleanup; foreground waiting continues.

J200: full fast finished on exact head with VERIFY_EXIT=1. Only Rust test row failed (two remote-client timeouts); static builds, clippy, fmt and all remaining Rust rows PASS. Agent_daemon 11/11 PASS. Requested base/head load comparison completed 80/80 PASS. Additional four-case remote-client comparison is queued through build-slot on compute01 (both VMs busy; reusing the sole existing target). No branch modifications. Final REQUEST CHANGES follows the comparison and evidence cleanup; foreground waiting continues.
kayg commented 2026-11-04 18:09:12 +00:00
Author
Owner
Copy link

J200 final review: REQUEST CHANGES.

Reviewed launch-followups-j170, head 86d39249af081215ad478bd668f568694187d212, base 81c432ef7cd81f95d9e31bb73c92f353162d46ae. No branch changes, commits, pushes or deployment. Read J157/J170 reports, the project contract and BRIEF-COMMON.

  • P1: the actual automation caller cancels the new cleanup retry. automation.rs:198 wraps discovery and create in a 4 s timeout; automation/operations.rs:289 directly awaits launch. The initial unanswered Hello costs 2 s and the first cleanup handshake costs another 2 s. The caller then drops abandon_launch before its second cleanup attempt can close the shim, and before the launcher's reap task is installed. The regression fixture succeeds without that outer timeout and fails with the production 4 s budget. Cleanup must retain ownership independently of caller cancellation; test the automation deadline, not only unwrapped launch.
  • P2: late bind skips cleanup entirely. sessions.rs:938 requires the socket to exist before entering the cleanup loop. A detached launcher has already exited successfully; if its shim binds at 31 s, inside the nominal 30+10 s budget, no close connection is attempted. The independent public-API test reproduces this. Wait on the existing readiness mechanism through the cleanup deadline, including when the socket is initially absent.
  • P2: cleanup leaves person attribution without an owner able to retire it. Reservation::spawned disables rollback, but failed-launch cleanup does not retire the durable .terminal record. The real head binary, with an owned pre-bind launcher that cleanup stops, leaves one record, zero sockets, and subsequent person revocation refuses with resource_unavailable. PIDfd readiness proves the launcher exited. Keep the conservative no-socket-is-not-exit rule, but retire attribution after proven cleanup of that launch. See reservation.log for the full output.
  • P3: new helpers lack the required documentation comments. Manager::resolve_with (sessions.rs:84), the test alive helper and exported test bound lack documentation-style comments required by the current brief's D345 rule.

Other checks:

  • No cross-session or cross-user termination route found. Scope names contain a fresh UUID generated inside launch; only the matching manager/unit is addressed. Socket cleanup checks OS peer UID and exact Hello session ID before Attach/Close; it never signals the Hello PID or socket peer PID. The owned Tokio Child reserves its PID until reaping, and Tokio's completed-child state makes start_kill a no-op after wait. This avoids the peer-PID reuse hazard J170 rejected.
  • Scope argv is correct: launch systemd-run [--user] --quiet --scope --unit infraternal-session-<uuid> ..., stop systemctl [--user] stop infraternal-session-<uuid>.scope. Arguments are separate argv entries; the UUID needs no shell quoting. Both exact user-manager and root/system-manager commands were exercised on compute01 with private UUID-named scopes: active before, inactive after. This verifies the actual commands, rather than calling the private cleanup function under those manager variants.
  • A timed-out manager probe returns None, and get_or_try_init leaves the OnceCell empty. Warm-up, opens and revocation all use resolve. The next launch therefore probes again after a timeout; an answered result remains cached. The added sequential unit test covers that cache transition.
  • Cleanup uses async socket I/O and async child status, without a blocking sleep/process wait on the control path. The 10 s loop check is a nominal retry budget, not a timeout enclosing the final send; path.exists also remains synchronous metadata. Most importantly, the real automation caller cancels this nominal budget at 4 s.
  • Successful daemon registration occurs only after launch returns Ok. A late Hello does not itself insert an in-memory session. The failures above can still leave a live unnamed detached session or a durable person reservation, so the registration lifecycle is not complete.
  • The new fake-shim test proves receipt of CloseSession when launch is not cancelled. It does not prove the real automation deadline, late bind, person-record retirement, or shell exit after a close request. The bystander test covers a directly owned launcher, not a real double-forked shim.

Requested load comparison (compute01, real default-feature binaries, same unchanged Python oracles as the two agent_daemon tests): 20 alternating rounds/ref, both oracles per round. Four owned CPU workers, CPU affinity [8,9], nice 19 (the runner inherited nice 10), alongside other builds. All workers stopped/reaped.

base 81c432ef7: lazy-manager 20/20 PASS; expanded restart/revocation 20/20 PASS
head 86d39249a: lazy-manager 20/20 PASS; expanded restart/revocation 20/20 PASS
recorded load1: 14.47–16.82

Neither reported agent_daemon signature reproduced. This is evidence of no observed regression in this run, not proof about dev-02's storage stalls. This fresh gate also fails unavailable_udp_does_not_add_seconds_before_tcp_admission (fallback_local_admission_us=2004235) and server_path_capability_is_advertised_on_native_and_fallback_transports (Elapsed at remote_client_tests.rs:688); agent lib totals are 230 passed, 2 failed, 22 ignored. The admission/capability tests fail before terminal launch/cleanup, and the durable-state test uses a plugin callback rather than terminal creation. The restart strip timeout is in an unchanged external strip/copy fixture; no terminal launch is involved. I found no direct changed-code path explaining those signatures. I did not reproduce those failures on an unmodified full base gate, so I am not labelling all of J170's gate failures proven unrelated. A proposed additional four-case remote-client A/B sweep was cancelled before execution after queuing behind four other jobs; no results are claimed for that sweep. The requested agent_daemon A/B comparison above is complete.

Independent cleanup reproduction results: Tokio 1.53.3 and UUID 1.27.0, from the workspace lock; one test at a time with the warm target:

automation_budget_must_not_cancel_cleanup: FAILED (no CloseSession after caller cancellation)
late_bind_during_cleanup_window_must_be_closed: FAILED (no cleanup connection after late bind)
unwrapped_j170_fixture_closes: PASS

The real-binary reservation reproduction also fails its retirement invariant, as quoted above. These are external evidence tests, not modifications to the branch.

Fresh scripts/verify fast, compute01, exact head, verbatim summary:

+---------------------------+--------------+
| Rust gate                 | Result       |
+---------------------------+--------------+
| fmt                       | PASS         |
| VT native fmt             | PASS         |
| CLI plugin compile-out    | PASS         |
| clippy                    | PASS         |
| SSH runtime test helper   | PASS         |
| real-binary fixture paths | PASS         |
| Traefik component manifest | PASS         |
| release component publication | PASS         |
| test                      | FAIL (101)   |
| SSH inode reuse fixture   | PASS         |
| shim process-per-user     | PASS         |
| cargo-deny                | PASS         |
| gate architecture         | PASS         |
| static builds             | PASS         |
+---------------------------+--------------+
VERIFY_EXIT=1

The initial quick run's five NotFound failures were my missing custom test-state parent, not branch findings; the directory was created before fast. Fmt and workspace clippy passed in quick. The fast run passes all five of those fixtures; its real-binary agent_daemon suite also passes 11/11.

Static images: x86_64 23,170,672 bytes (gate baseline delta +752,416); arm64 21,073,920 bytes (gate baseline delta +620,640). Those are deltas against the gate's recorded baselines, not isolated measurements of these three commits. Both linkage/metadata checks pass.

Evidence: attached j200-evidence.tar.gz, harness-pinned.log, ab-summary.json, and gate-summary.txt. VM logs/scripts: compute01:~/j200-out/. All owned load workers/test processes ended; the optional queue entry was stopped before start. The sole VM target directory, copied executable artifacts and custom test-state directory were deleted. Both review checkouts are clean and detached at the exact reviewed head. Issues remain open.

J200 final review: **REQUEST CHANGES**. Reviewed `launch-followups-j170`, head `86d39249af081215ad478bd668f568694187d212`, base `81c432ef7cd81f95d9e31bb73c92f353162d46ae`. No branch changes, commits, pushes or deployment. Read J157/J170 reports, the project contract and BRIEF-COMMON. - **P1: the actual automation caller cancels the new cleanup retry.** `automation.rs:198` wraps discovery and create in a 4 s timeout; `automation/operations.rs:289` directly awaits launch. The initial unanswered Hello costs 2 s and the first cleanup handshake costs another 2 s. The caller then drops `abandon_launch` before its second cleanup attempt can close the shim, and before the launcher's reap task is installed. The regression fixture succeeds without that outer timeout and fails with the production 4 s budget. Cleanup must retain ownership independently of caller cancellation; test the automation deadline, not only unwrapped launch. - **P2: late bind skips cleanup entirely.** `sessions.rs:938` requires the socket to exist before entering the cleanup loop. A detached launcher has already exited successfully; if its shim binds at 31 s, inside the nominal 30+10 s budget, no close connection is attempted. The independent public-API test reproduces this. Wait on the existing readiness mechanism through the cleanup deadline, including when the socket is initially absent. - **P2: cleanup leaves person attribution without an owner able to retire it.** `Reservation::spawned` disables rollback, but failed-launch cleanup does not retire the durable `.terminal` record. The real head binary, with an owned pre-bind launcher that cleanup stops, leaves one record, zero sockets, and subsequent person revocation refuses with `resource_unavailable`. PIDfd readiness proves the launcher exited. Keep the conservative no-socket-is-not-exit rule, but retire attribution after proven cleanup of that launch. See `reservation.log` for the full output. - **P3: new helpers lack the required documentation comments.** `Manager::resolve_with` (`sessions.rs:84`), the test `alive` helper and exported test bound lack documentation-style comments required by the current brief's D345 rule. Other checks: - No cross-session or cross-user termination route found. Scope names contain a fresh UUID generated inside launch; only the matching manager/unit is addressed. Socket cleanup checks OS peer UID and exact Hello session ID before Attach/Close; it never signals the Hello PID or socket peer PID. The owned Tokio Child reserves its PID until reaping, and Tokio's completed-child state makes start_kill a no-op after wait. This avoids the peer-PID reuse hazard J170 rejected. - Scope argv is correct: launch `systemd-run [--user] --quiet --scope --unit infraternal-session-<uuid> ...`, stop `systemctl [--user] stop infraternal-session-<uuid>.scope`. Arguments are separate argv entries; the UUID needs no shell quoting. Both exact user-manager and root/system-manager commands were exercised on compute01 with private UUID-named scopes: active before, inactive after. This verifies the actual commands, rather than calling the private cleanup function under those manager variants. - A timed-out manager probe returns None, and get_or_try_init leaves the OnceCell empty. Warm-up, opens and revocation all use resolve. The next launch therefore probes again after a timeout; an answered result remains cached. The added sequential unit test covers that cache transition. - Cleanup uses async socket I/O and async child status, without a blocking sleep/process wait on the control path. The 10 s loop check is a nominal retry budget, not a timeout enclosing the final send; path.exists also remains synchronous metadata. Most importantly, the real automation caller cancels this nominal budget at 4 s. - Successful daemon registration occurs only after launch returns Ok. A late Hello does not itself insert an in-memory session. The failures above can still leave a live unnamed detached session or a durable person reservation, so the registration lifecycle is not complete. - The new fake-shim test proves receipt of CloseSession when launch is not cancelled. It does not prove the real automation deadline, late bind, person-record retirement, or shell exit after a close request. The bystander test covers a directly owned launcher, not a real double-forked shim. Requested load comparison (compute01, real default-feature binaries, same unchanged Python oracles as the two agent_daemon tests): 20 alternating rounds/ref, both oracles per round. Four owned CPU workers, CPU affinity [8,9], nice 19 (the runner inherited nice 10), alongside other builds. All workers stopped/reaped. ``` base 81c432ef7: lazy-manager 20/20 PASS; expanded restart/revocation 20/20 PASS head 86d39249a: lazy-manager 20/20 PASS; expanded restart/revocation 20/20 PASS recorded load1: 14.47–16.82 ``` Neither reported agent_daemon signature reproduced. This is evidence of no observed regression in this run, not proof about dev-02's storage stalls. This fresh gate also fails `unavailable_udp_does_not_add_seconds_before_tcp_admission` (fallback_local_admission_us=2004235) and `server_path_capability_is_advertised_on_native_and_fallback_transports` (Elapsed at remote_client_tests.rs:688); agent lib totals are 230 passed, 2 failed, 22 ignored. The admission/capability tests fail before terminal launch/cleanup, and the durable-state test uses a plugin callback rather than terminal creation. The restart strip timeout is in an unchanged external strip/copy fixture; no terminal launch is involved. I found no direct changed-code path explaining those signatures. I did not reproduce those failures on an unmodified full base gate, so I am not labelling all of J170's gate failures proven unrelated. A proposed additional four-case remote-client A/B sweep was cancelled before execution after queuing behind four other jobs; no results are claimed for that sweep. The requested agent_daemon A/B comparison above is complete. Independent cleanup reproduction results: Tokio 1.53.3 and UUID 1.27.0, from the workspace lock; one test at a time with the warm target: ``` automation_budget_must_not_cancel_cleanup: FAILED (no CloseSession after caller cancellation) late_bind_during_cleanup_window_must_be_closed: FAILED (no cleanup connection after late bind) unwrapped_j170_fixture_closes: PASS ``` The real-binary reservation reproduction also fails its retirement invariant, as quoted above. These are external evidence tests, not modifications to the branch. Fresh `scripts/verify fast`, compute01, exact head, verbatim summary: ```text +---------------------------+--------------+ | Rust gate | Result | +---------------------------+--------------+ | fmt | PASS | | VT native fmt | PASS | | CLI plugin compile-out | PASS | | clippy | PASS | | SSH runtime test helper | PASS | | real-binary fixture paths | PASS | | Traefik component manifest | PASS | | release component publication | PASS | | test | FAIL (101) | | SSH inode reuse fixture | PASS | | shim process-per-user | PASS | | cargo-deny | PASS | | gate architecture | PASS | | static builds | PASS | +---------------------------+--------------+ VERIFY_EXIT=1 ``` The initial quick run's five NotFound failures were my missing custom test-state parent, not branch findings; the directory was created before fast. Fmt and workspace clippy passed in quick. The fast run passes all five of those fixtures; its real-binary agent_daemon suite also passes 11/11. Static images: x86_64 23,170,672 bytes (gate baseline delta +752,416); arm64 21,073,920 bytes (gate baseline delta +620,640). Those are deltas against the gate's recorded baselines, not isolated measurements of these three commits. Both linkage/metadata checks pass. Evidence: attached `j200-evidence.tar.gz`, `harness-pinned.log`, `ab-summary.json`, and `gate-summary.txt`. VM logs/scripts: `compute01:~/j200-out/`. All owned load workers/test processes ended; the optional queue entry was stopped before start. The sole VM target directory, copied executable artifacts and custom test-state directory were deleted. Both review checkouts are clean and detached at the exact reviewed head. Issues remain open.
j200-evidence.tar.gz
83 KiB
harness-pinned.log
2.6 KiB
ab-summary.json
19 KiB
gate-summary.txt
829 B
kayg commented 2026-11-04 18:51:41 +00:00
Author
Owner
Copy link

J170 round 2 (J200 REQUEST CHANGES). Branch launch-followups-j170, head 68797535d86663524bb04925e483a7bddb88d590, pushed. origin/dev merged first (5e38e34ad; one conflict: dev's macOS gate on Manager::probe, kept).

Fix 68797535d (one commit, J200's four findings):

  • P1 cleanup is owned by a Launch guard moved onto its own task on any non-success exit, including the caller dropping the future; bounded per wait. The caller returns immediately.
  • P2 late bind cleanup waits on the directory watch until the shim binds, then closes it via its Hello session.
  • P2 person record retired only after proven exit (launcher reaped, or socket gone/sealed after close). No-socket alone still proves nothing.
  • P3 doc comments on every new item.

Fail first, compute01, on 5e38e34ad (old cleanup) with the new tests:

cancelling_the_launch_does_not_cancel_its_cleanup ... FAILED: cancelling the launch left its detached session running: Elapsed(())
a_shim_that_binds_after_the_launch_gave_up_is_still_closed ... FAILED: a late-binding shim was never closed: Elapsed(())
a_stopped_person_launch_retires_its_terminal_record ... FAILED: a stopped launch left its person record behind (left: 1)

verify quick and verify fast are queued on compute01 (position 5 of 4 slots) and will be posted verbatim.

**J170 round 2 (J200 REQUEST CHANGES).** Branch `launch-followups-j170`, head `68797535d86663524bb04925e483a7bddb88d590`, pushed. origin/dev merged first (`5e38e34ad`; one conflict: dev's macOS gate on `Manager::probe`, kept). Fix `68797535d` (one commit, J200's four findings): - **P1** cleanup is owned by a `Launch` guard moved onto its own task on any non-success exit, including the caller dropping the future; bounded per wait. The caller returns immediately. - **P2 late bind** cleanup waits on the directory watch until the shim binds, then closes it via its Hello session. - **P2 person record** retired only after proven exit (launcher reaped, or socket gone/sealed after close). No-socket alone still proves nothing. - **P3** doc comments on every new item. **Fail first, compute01, on `5e38e34ad` (old cleanup) with the new tests:** ``` cancelling_the_launch_does_not_cancel_its_cleanup ... FAILED: cancelling the launch left its detached session running: Elapsed(()) a_shim_that_binds_after_the_launch_gave_up_is_still_closed ... FAILED: a late-binding shim was never closed: Elapsed(()) a_stopped_person_launch_retires_its_terminal_record ... FAILED: a stopped launch left its person record behind (left: 1) ``` `verify quick` and `verify fast` are queued on compute01 (position 5 of 4 slots) and will be posted verbatim.
kayg commented 2026-11-04 20:04:59 +00:00
Author
Owner
Copy link

J170 round 2 finish: GREEN. Branch launch-followups-j170, head 7245bdbb67ffa7d0a96bb75ca5e7ef66bbb9f2d8, pushed, not merged, not deployed. J200's four findings fixed; origin/dev merged first.

Commits on top of dev 600ae5eb1: 664f12f54 probe, b3201d81c failed-launch cleanup, 86d39249a slow-Hello close, 5e38e34ad merge of origin/dev (one conflict: dev's macOS gate on the probe, kept), 68797535d cleanup owned by the launch, 7245bdbb6 clippy fixes.

J200 findings

  • P1 cleanup is owned by a Launch guard; any exit but success (error, or the caller dropping the future, as automation's 4 s deadline does) moves launcher child, watch, ids and record to abandon_launch on its own task, each wait bounded. The caller now returns at once. Test cancelling_the_launch_does_not_cancel_its_cleanup.
  • P2 late bind cleanup sleeps on the directory watch until the shim binds, then closes it through its Hello session. Test a_shim_that_binds_after_the_launch_gave_up_is_still_closed.
  • P2 person record retired only after proven exit: launcher killed and reaped, or socket gone/sealed after the close. A missing socket alone still proves nothing. Test a_stopped_person_launch_retires_its_terminal_record.
  • P3 doc comments on every new item; the doc coverage ratchet row passes.

Fail first (compute01, 5e38e34ad = merged old cleanup, new tests):

cancelling_the_launch_does_not_cancel_its_cleanup ... FAILED  cancelling the launch left its detached session running: Elapsed(())
a_shim_that_binds_after_the_launch_gave_up_is_still_closed ... FAILED  a late-binding shim was never closed: Elapsed(())
a_stopped_person_launch_retires_its_terminal_record ... FAILED  a stopped launch left its person record behind (left: 1)

Green at 7245bdbb6: all three pass (0.71 s, 1.52 s, 0.34 s), plus slow_hello_failure_… (4.02 s), failed_launch_stops_the_process_it_started_and_no_other (0.53 s), unanswered_manager_probe_is_not_remembered.

scripts/verify quick, compute01, verbatim:

| scope guard               | PASS         |
| fmt                       | PASS         |
| doc coverage ratchet      | PASS         |
| clippy (workspace)        | PASS         |
| unit tests (changed)      | PASS         |
QUICK pre-check passed.

(An earlier quick at 68797535d failed clippy: unneeded mut, a redundant closure and a missing backtick. Fixed in 7245bdbb6. I stopped that gate's leftover processes by PID.)

scripts/verify fast at 7245bdbb6, compute01, foreground, VERIFY_EXIT=0, verbatim:

| fmt                       | PASS         |
| VT native fmt             | PASS         |
| CLI plugin compile-out    | PASS         |
| clippy                    | PASS         |
| SSH runtime test helper   | PASS         |
| real-binary fixture paths | PASS         |
| Traefik component manifest | PASS         |
| release component publication | PASS         |
| test                      | PASS         |
| SSH inode reuse fixture   | PASS         |
| shim process-per-user     | PASS         |
| cargo-deny                | PASS         |
| gate architecture         | PASS         |
| doc coverage ratchet      | PASS         |
| static builds             | PASS         |

Sizes (report only): x86_64 23469584 bytes (+1051328 vs stored baseline), aarch64 21334400 (+881120). Those include all of dev's growth since the baseline, not just this branch.

Not done / limits

  • No root-only or test-server run, and the systemctl stop scope route is still not exercised by a test; J200 ran those exact commands on compute01 (active before, inactive after).
  • A --detach shim whose launcher already exited and which never binds or answers within the 10 s cleanup window is still not stopped (no safe pid). It is reported, not fixed.
  • Failing launches now return before cleanup finishes; a caller that wants to know cleanup's outcome cannot (it is logged nowhere). I judged that right for latency; say if you want a signal.
  • My earlier dev-02 gate flakes: not re-examined, per J200's 80/80.
**J170 round 2 finish: GREEN. Branch `launch-followups-j170`, head `7245bdbb67ffa7d0a96bb75ca5e7ef66bbb9f2d8`**, pushed, not merged, not deployed. J200's four findings fixed; origin/dev merged first. Commits on top of dev `600ae5eb1`: `664f12f54` probe, `b3201d81c` failed-launch cleanup, `86d39249a` slow-Hello close, `5e38e34ad` merge of origin/dev (one conflict: dev's macOS gate on the probe, kept), `68797535d` cleanup owned by the launch, `7245bdbb6` clippy fixes. **J200 findings** - **P1** cleanup is owned by a `Launch` guard; any exit but success (error, or the caller dropping the future, as automation's 4 s deadline does) moves launcher child, watch, ids and record to `abandon_launch` on its own task, each wait bounded. The caller now returns at once. Test `cancelling_the_launch_does_not_cancel_its_cleanup`. - **P2 late bind** cleanup sleeps on the directory watch until the shim binds, then closes it through its Hello session. Test `a_shim_that_binds_after_the_launch_gave_up_is_still_closed`. - **P2 person record** retired only after proven exit: launcher killed and reaped, or socket gone/sealed after the close. A missing socket alone still proves nothing. Test `a_stopped_person_launch_retires_its_terminal_record`. - **P3** doc comments on every new item; the doc coverage ratchet row passes. **Fail first (compute01, `5e38e34ad` = merged old cleanup, new tests):** ``` cancelling_the_launch_does_not_cancel_its_cleanup ... FAILED cancelling the launch left its detached session running: Elapsed(()) a_shim_that_binds_after_the_launch_gave_up_is_still_closed ... FAILED a late-binding shim was never closed: Elapsed(()) a_stopped_person_launch_retires_its_terminal_record ... FAILED a stopped launch left its person record behind (left: 1) ``` **Green at `7245bdbb6`:** all three pass (0.71 s, 1.52 s, 0.34 s), plus `slow_hello_failure_…` (4.02 s), `failed_launch_stops_the_process_it_started_and_no_other` (0.53 s), `unanswered_manager_probe_is_not_remembered`. **`scripts/verify quick`, compute01, verbatim:** ``` | scope guard | PASS | | fmt | PASS | | doc coverage ratchet | PASS | | clippy (workspace) | PASS | | unit tests (changed) | PASS | QUICK pre-check passed. ``` (An earlier quick at `68797535d` failed clippy: unneeded `mut`, a redundant closure and a missing backtick. Fixed in `7245bdbb6`. I stopped that gate's leftover processes by PID.) **`scripts/verify fast` at `7245bdbb6`, compute01, foreground, VERIFY_EXIT=0, verbatim:** ``` | fmt | PASS | | VT native fmt | PASS | | CLI plugin compile-out | PASS | | clippy | PASS | | SSH runtime test helper | PASS | | real-binary fixture paths | PASS | | Traefik component manifest | PASS | | release component publication | PASS | | test | PASS | | SSH inode reuse fixture | PASS | | shim process-per-user | PASS | | cargo-deny | PASS | | gate architecture | PASS | | doc coverage ratchet | PASS | | static builds | PASS | ``` Sizes (report only): x86_64 23469584 bytes (+1051328 vs stored baseline), aarch64 21334400 (+881120). Those include all of dev's growth since the baseline, not just this branch. **Not done / limits** - No root-only or test-server run, and the `systemctl stop` scope route is still not exercised by a test; J200 ran those exact commands on compute01 (active before, inactive after). - A `--detach` shim whose launcher already exited and which never binds or answers within the 10 s cleanup window is still not stopped (no safe pid). It is reported, not fixed. - Failing launches now return before cleanup finishes; a caller that wants to know cleanup's outcome cannot (it is logged nowhere). I judged that right for latency; say if you want a signal. - My earlier dev-02 gate flakes: not re-examined, per J200's 80/80.
kayg commented 2026-11-04 20:06:58 +00:00
Author
Owner
Copy link

J200 round 2 starts: re-reviewing only cleanup follow-up changes in 86d39249af081215ad478bd668f568694187d212..7245bdbb67ffa7d0a96bb75ca5e7ef66bbb9f2d8, branch launch-followups-j170. The range includes a merge of dev; review will isolate the cleanup commits and their interactions. No branch edits. Plan: inspect guard/task cancellation and attribution proof, run focused shutdown/bounds reproductions if needed, then post APPROVE or REQUEST CHANGES with evidence. Estimate 30 minutes; foreground polling only.

J200 round 2 starts: re-reviewing only cleanup follow-up changes in 86d39249af081215ad478bd668f568694187d212..7245bdbb67ffa7d0a96bb75ca5e7ef66bbb9f2d8, branch launch-followups-j170. The range includes a merge of dev; review will isolate the cleanup commits and their interactions. No branch edits. Plan: inspect guard/task cancellation and attribution proof, run focused shutdown/bounds reproductions if needed, then post APPROVE or REQUEST CHANGES with evidence. Estimate 30 minutes; foreground polling only.
kayg commented 2026-11-04 20:12:44 +00:00
Author
Owner
Copy link

J200 round 2 finding (P1): shutdown can discard cleanup without killing its owned launcher. sessions.rs:1007 discards the JoinHandle from runtime.spawn(abandon_launch(job)); neither Abandoned nor the Child has a kill-on-drop fallback. Handle::try_current succeeds while still inside a runtime, even when that runtime is about to shut down. If shutdown drops the spawned future before its first poll (or while it waits), it only drops Child, whose default policy leaves the process running. The no-current-runtime branch in Launch::drop does not cover this.

Focused reproduction on compute01, Tokio 1.53.3: copied Launch/Abandoned/drop/abandon_launch code unchanged from 7245bdbb6; enter a current-thread runtime, spawn an owned sleep, drop Launch, then drop the runtime without polling the cleanup. PIDfd evidence: runtime_dropped=true owned_launcher_alive=true; assertion fails, TEST_EXIT=101. Fixture teardown kills only that pinned process. Protocol/watch/scope stubs are not reached in this test. Source and logs: ~/j200-r2/harness/, ~/j200-r2/shutdown.log. No branch changes.

Cleanup needs a shutdown-safe ownership fallback and a tracked bounded shutdown drain. A spawned task alone does not guarantee the cleanup starts or completes. Add a test that shuts down the runtime with cleanup queued and another with cleanup suspended.

J200 round 2 finding (P1): shutdown can discard cleanup without killing its owned launcher. `sessions.rs:1007` discards the JoinHandle from runtime.spawn(abandon_launch(job)); neither Abandoned nor the Child has a kill-on-drop fallback. Handle::try_current succeeds while still inside a runtime, even when that runtime is about to shut down. If shutdown drops the spawned future before its first poll (or while it waits), it only drops Child, whose default policy leaves the process running. The no-current-runtime branch in Launch::drop does not cover this. Focused reproduction on compute01, Tokio 1.53.3: copied Launch/Abandoned/drop/abandon_launch code unchanged from 7245bdbb6; enter a current-thread runtime, spawn an owned sleep, drop Launch, then drop the runtime without polling the cleanup. PIDfd evidence: `runtime_dropped=true owned_launcher_alive=true`; assertion fails, TEST_EXIT=101. Fixture teardown kills only that pinned process. Protocol/watch/scope stubs are not reached in this test. Source and logs: ~/j200-r2/harness/, ~/j200-r2/shutdown.log. No branch changes. Cleanup needs a shutdown-safe ownership fallback and a tracked bounded shutdown drain. A spawned task alone does not guarantee the cleanup starts or completes. Add a test that shuts down the runtime with cleanup queued and another with cleanup suspended.
kayg commented 2026-11-04 20:13:48 +00:00
Author
Owner
Copy link

J200 round 2 finding (P1): a live detached descendant is treated as stopped, and its attribution/rendezvous are deleted. sessions.rs:1043-1054 assumes a running detaching launcher has not forked yet. That is false: the real detach implementation first forks, the original parent waits in waitpid, then the intermediate child forks the shim before exiting (pty.rs:879-895). Cancellation can catch the original/intermediate process still running while the grandchild already exists. Reaping only the launcher is not terminal-exit proof. The live-launcher branch never attempts an authenticated close, unlinks the socket, and then removes the person record at line 1066. A surviving terminal becomes undiscoverable to revocation.

Focused compute01 reproduction uses the unchanged new cleanup body, a two-fork fixture with the original waiting in waitpid and the intermediate paused after forking, plus a grandchild listening on the launch socket. Output: launcher_exit_proven=true detached_shim_alive=true socket_exists=false person_record_exists=false; TEST_EXIT=101. Both descendant identities were pinned with PIDfds before cleanup, and fixture teardown stopped them through those handles. The watch/protocol/scope stubs are not used on this branch. Evidence: ~/j200-r2/descendant.log and harness/. No branch changes.

Do not infer no descendants from a live parent. Preserve attribution/rendezvous until the detached shim or entire launch containment is proven stopped, and attempt late-bind/authenticated close even when its launcher was initially alive. The new retirement test uses only exec sleep (no fork), so cannot establish this invariant.

J200 round 2 finding (P1): a live detached descendant is treated as stopped, and its attribution/rendezvous are deleted. `sessions.rs:1043-1054` assumes a running detaching launcher has not forked yet. That is false: the real detach implementation first forks, the original parent waits in waitpid, then the intermediate child forks the shim before exiting (`pty.rs:879-895`). Cancellation can catch the original/intermediate process still running while the grandchild already exists. Reaping only the launcher is not terminal-exit proof. The live-launcher branch never attempts an authenticated close, unlinks the socket, and then removes the person record at line 1066. A surviving terminal becomes undiscoverable to revocation. Focused compute01 reproduction uses the unchanged new cleanup body, a two-fork fixture with the original waiting in waitpid and the intermediate paused after forking, plus a grandchild listening on the launch socket. Output: `launcher_exit_proven=true detached_shim_alive=true socket_exists=false person_record_exists=false`; TEST_EXIT=101. Both descendant identities were pinned with PIDfds before cleanup, and fixture teardown stopped them through those handles. The watch/protocol/scope stubs are not used on this branch. Evidence: ~/j200-r2/descendant.log and harness/. No branch changes. Do not infer no descendants from a live parent. Preserve attribution/rendezvous until the detached shim or entire launch containment is proven stopped, and attempt late-bind/authenticated close even when its launcher was initially alive. The new retirement test uses only exec sleep (no fork), so cannot establish this invariant.
kayg commented 2026-11-04 20:15:46 +00:00
Author
Owner
Copy link

J200 round 2: REQUEST CHANGES.

Reviewed the cleanup follow-up delta in 86d39249af081215ad478bd668f568694187d212..7245bdbb67ffa7d0a96bb75ca5e7ef66bbb9f2d8, branch launch-followups-j170, including fixes 68797535d and 7245bdbb6. Read J170's round-2 report and green gate. No branch edits, commits, pushes, merges or deployment.

  • P1: runtime shutdown can discard cleanup and leave its owned launcher running. At sessions.rs:987-1007, Launch::drop hands ownership to an untracked spawned task. Handle::try_current() can succeed immediately before runtime shutdown; that task may never be polled. Abandoned has no drop fallback and its Child retains the default kill-on-drop=false policy. Runtime shutdown therefore drops the child handle without killing the process. The no-current-runtime fallback does not cover this. Nor is there a cleanup task set drained when the infraternal agent stops inside a longer-lived runtime. Keep shutdown-safe ownership and track/drain cleanup with a bounded shutdown policy; test shutdown with cleanup queued and suspended.
  • P1: reaping a detaching launcher is not proof that its terminal stopped. At sessions.rs:1041-1054, a still-running launcher is assumed not to have forked. But the real pty.rs:879-895 double-fork permits the original launcher to wait in waitpid while the intermediate has already forked the shim. Cancellation can hit that scheduling window. Cleanup kills/reaps only the original launcher, skips authenticated detached-shim close, unlinks the rendezvous, then deletes the person record at line 1066. A live terminal loses both discovery and revocation attribution. Keep the socket/record until the detached descendant or whole launch containment is proven stopped; wait for/authenticate the shim even when its original launcher was initially alive. The new exec-sleep retirement fixture has no descendant and cannot prove this invariant.

Independent focused results on compute01, one quick unit case at a time, Tokio 1.53.3, UUID 1.27.0, libc 0.2.190:

runtime_shutdown_must_stop_the_owned_launcher:
  runtime_dropped=true owned_launcher_alive=true
  FAILED: runtime shutdown discarded cleanup and left its owned launcher running
  TEST_EXIT=101

live_detached_descendant_must_keep_its_attribution_and_socket:
  launcher_exit_proven=true detached_shim_alive=true socket_exists=false person_record_exists=false
  FAILED: launcher exit was treated as terminal exit: live detached shim lost both attribution and rendezvous
  TEST_EXIT=101

Evidence limits: this is a small source-extraction harness, not a full real-binary integration run. Launch, its Drop implementation, Abandoned and abandon_launch are copied unchanged from the reviewed head; the extraction hash is in source-receipt.txt. The watch type is a placeholder and scope/protocol functions are stubs, neither used by the tested branches. The shutdown test uses the actual Tokio runtime and Child; the descendant test uses real forked processes and a real Unix listener. PIDfds prove liveness/exit and restrict teardown to the fixture's own processes. The first build attempt could not find cc and executed no tests; the logged runs above use the available Nix compiler wrapper.

Other checks:

  • The guard now owns the launch before the first post-spawn await, so ordinary caller cancellation cannot drop that ownership in an unguarded gap. The new cancellation/late-bind tests cover useful happy cleanup paths, but not runtime shutdown or an already-forked live launcher.
  • The exited-launcher detached path now retains the directory watch and waits for a late bind. It still authenticates OS peer UID and exact Hello session UUID before CloseSession.
  • No process-name matching or new cross-session/user termination route found. Signals use the owned Child; systemctl addresses exactly the scope derived from this launch's fresh UUID and selected manager. No socket peer/Hello PID is signalled. The two failures leak or lose proof for the launch's own terminal.
  • Handshakes, directory readiness, scope stop, exit observation and child waits have finite timeouts. CLEANUP bounds the retry loop, rather than enclosing all subsequent work; the final handshake/exit-observation phases can extend beyond that window, and the small CloseSession send has no enclosing timeout. No additional unbounded-loop reproduction is claimed. These bounds do not repair the shutdown ownership loss above.
  • The previously missing documentation comments are present. Person-record retirement is now implemented, but the launcher's exit is insufficient proof in the fork race.

J170 reports VERIFY_EXIT=0 at the exact head, with every Rust row PASS. That gate does not cover the two failing invariants above. This is a review-only job with no repository code changes; the full gate was not rerun (no code changed, gate not applicable under BRIEF-COMMON's no-code job rule). The independent focused test output is attached verbatim in shutdown.log and descendant.log.

Attached: j200-r2-evidence.tar.gz, shutdown.log, descendant.log, source-receipt.txt. Small sources/logs remain at compute01:~/j200-r2/ and locally under ~/.cache/codex-scratch/j200-r2/evidence/. Both test runs finished; owned fixture processes were stopped through pinned handles, the process sweep is clear, and the sole VM target directory was deleted. The control review checkout is clean at 7245bdbb67ffa7d0a96bb75ca5e7ef66bbb9f2d8. Issue left open.

J200 round 2: **REQUEST CHANGES**. Reviewed the cleanup follow-up delta in `86d39249af081215ad478bd668f568694187d212..7245bdbb67ffa7d0a96bb75ca5e7ef66bbb9f2d8`, branch `launch-followups-j170`, including fixes `68797535d` and `7245bdbb6`. Read J170's [round-2 report and green gate](https://git.kayg.org/kayg/infraternal/issues/324#issuecomment-78893). No branch edits, commits, pushes, merges or deployment. - **P1: runtime shutdown can discard cleanup and leave its owned launcher running.** At `sessions.rs:987-1007`, `Launch::drop` hands ownership to an untracked spawned task. `Handle::try_current()` can succeed immediately before runtime shutdown; that task may never be polled. `Abandoned` has no drop fallback and its Child retains the default kill-on-drop=false policy. Runtime shutdown therefore drops the child handle without killing the process. The no-current-runtime fallback does not cover this. Nor is there a cleanup task set drained when the infraternal agent stops inside a longer-lived runtime. Keep shutdown-safe ownership and track/drain cleanup with a bounded shutdown policy; test shutdown with cleanup queued and suspended. - **P1: reaping a detaching launcher is not proof that its terminal stopped.** At `sessions.rs:1041-1054`, a still-running launcher is assumed not to have forked. But the real `pty.rs:879-895` double-fork permits the original launcher to wait in waitpid while the intermediate has already forked the shim. Cancellation can hit that scheduling window. Cleanup kills/reaps only the original launcher, skips authenticated detached-shim close, unlinks the rendezvous, then deletes the person record at line 1066. A live terminal loses both discovery and revocation attribution. Keep the socket/record until the detached descendant or whole launch containment is proven stopped; wait for/authenticate the shim even when its original launcher was initially alive. The new exec-sleep retirement fixture has no descendant and cannot prove this invariant. Independent focused results on compute01, one quick unit case at a time, Tokio **1.53.3**, UUID **1.27.0**, libc **0.2.190**: ```text runtime_shutdown_must_stop_the_owned_launcher: runtime_dropped=true owned_launcher_alive=true FAILED: runtime shutdown discarded cleanup and left its owned launcher running TEST_EXIT=101 live_detached_descendant_must_keep_its_attribution_and_socket: launcher_exit_proven=true detached_shim_alive=true socket_exists=false person_record_exists=false FAILED: launcher exit was treated as terminal exit: live detached shim lost both attribution and rendezvous TEST_EXIT=101 ``` Evidence limits: this is a small source-extraction harness, not a full real-binary integration run. `Launch`, its Drop implementation, `Abandoned` and `abandon_launch` are copied unchanged from the reviewed head; the extraction hash is in `source-receipt.txt`. The watch type is a placeholder and scope/protocol functions are stubs, neither used by the tested branches. The shutdown test uses the actual Tokio runtime and Child; the descendant test uses real forked processes and a real Unix listener. PIDfds prove liveness/exit and restrict teardown to the fixture's own processes. The first build attempt could not find cc and executed no tests; the logged runs above use the available Nix compiler wrapper. Other checks: - The guard now owns the launch before the first post-spawn await, so ordinary caller cancellation cannot drop that ownership in an unguarded gap. The new cancellation/late-bind tests cover useful happy cleanup paths, but not runtime shutdown or an already-forked live launcher. - The exited-launcher detached path now retains the directory watch and waits for a late bind. It still authenticates OS peer UID and exact Hello session UUID before CloseSession. - No process-name matching or new cross-session/user termination route found. Signals use the owned Child; systemctl addresses exactly the scope derived from this launch's fresh UUID and selected manager. No socket peer/Hello PID is signalled. The two failures leak or lose proof for the launch's own terminal. - Handshakes, directory readiness, scope stop, exit observation and child waits have finite timeouts. CLEANUP bounds the retry loop, rather than enclosing all subsequent work; the final handshake/exit-observation phases can extend beyond that window, and the small CloseSession send has no enclosing timeout. No additional unbounded-loop reproduction is claimed. These bounds do not repair the shutdown ownership loss above. - The previously missing documentation comments are present. Person-record retirement is now implemented, but the launcher's exit is insufficient proof in the fork race. J170 reports `VERIFY_EXIT=0` at the exact head, with every Rust row PASS. That gate does not cover the two failing invariants above. This is a review-only job with no repository code changes; the full gate was not rerun (no code changed, gate not applicable under BRIEF-COMMON's no-code job rule). The independent focused test output is attached verbatim in `shutdown.log` and `descendant.log`. Attached: `j200-r2-evidence.tar.gz`, `shutdown.log`, `descendant.log`, `source-receipt.txt`. Small sources/logs remain at `compute01:~/j200-r2/` and locally under `~/.cache/codex-scratch/j200-r2/evidence/`. Both test runs finished; owned fixture processes were stopped through pinned handles, the process sweep is clear, and the sole VM target directory was deleted. The control review checkout is clean at `7245bdbb67ffa7d0a96bb75ca5e7ef66bbb9f2d8`. Issue left open.
j200-r2-evidence.tar.gz
6.7 KiB
shutdown.log
1.2 KiB
descendant.log
936 B
source-receipt.txt
423 B
kayg commented 2026-11-04 20:19:07 +00:00
Author
Owner
Copy link

J170 design note (before patching J200 round 2). Both P1s come from one mistake: I treated "the launcher process I spawned is gone" as "the terminal is gone". For a detaching shim that is false: the launcher is the first of three processes, and the shim is a grandchild I hold no handle to.

1. Who owns what, and when

moment owner notes
before spawn Reservation removes the person record if the spawn fails
spawn → Hello accepted Launch guard owns the launcher Child (unreaped, so its pid cannot be reused), the ids, the rendezvous path and the record path. It does NOT own the shim: for --detach the shim is a grandchild
shim binds the shim the socket is the shim's; the guard only knows the path
success live session guard releases the child to a reaper; socket and record now belong to the terminal
failure or cancel Abandoned, in a tracked task moved out of the caller's future, so a cancelled caller cannot cancel it; counted in a process-wide tracker
shutdown tracker + drop fallback see 3

2. What proves a terminal is dead (and what does not)

  • NOT proof: the launcher's exit or reaping, if the launch detaches. Not proof either: a missing socket (the shim may bind late).
  • Proof A: a non-detaching launcher IS the shim (systemd-run execs it; a login shim is spawned directly). Killed and reaped through its own unreaped Child = shim dead. Only then may cleanup unlink that launch's own rendezvous leftovers.
  • Proof B: a detaching launcher that has never forked. Freeze it (SIGSTOP on its own unreaped child), read /proc/<pid>/task/*/children; if empty it cannot have started a shim, and it cannot fork while stopped. Then kill and reap. If the list is non-empty or unreadable: SIGCONT, do not kill, and fall to C.
  • Proof C: the shim's own exit. Authenticate its Hello (session id, peer uid), send CloseSession, then require the rendezvous gone or sealed by an exit record. Waits for a late bind on the directory watch, created before the first connect so no bind is missed.
  • Anything else is UNPROVEN: leave the socket and the record alone. The person record is deleted only on A, B or C. Cleanup never unlinks a detached shim's socket (it removes its own on exit).

3. Shutdown

  • Agent stop (run_channel returns): drain_cleanups waits for the tracker, bounded (HANDSHAKE x 2), so a longer-lived runtime finishes what it queued.
  • Runtime shutdown drops the spawned task: Abandoned::drop sees cleanup did not finish and runs the same cleanup to completion on a dedicated thread with its own current-thread runtime and a short budget. No runtime at all: same path.
  • Hard process exit with no unwind is not covered; surviving shims stay discoverable by their sockets.

Owner question (not blocking). If cleanup ends UNPROVEN (a live detached shim that never answered), a live unnamed terminal and its record stay, and revocation for that person refuses until it is closed. Recommendation: leave it (safe, visible to discovery, closable); no auto-kill without proof. I continue with that.

**J170 design note (before patching J200 round 2).** Both P1s come from one mistake: I treated "the launcher process I spawned is gone" as "the terminal is gone". For a detaching shim that is false: the launcher is the first of three processes, and the shim is a grandchild I hold no handle to. **1. Who owns what, and when** | moment | owner | notes | |---|---|---| | before spawn | `Reservation` | removes the person record if the spawn fails | | spawn → Hello accepted | `Launch` guard | owns the launcher `Child` (unreaped, so its pid cannot be reused), the ids, the rendezvous path and the record path. It does NOT own the shim: for `--detach` the shim is a grandchild | | shim binds | the shim | the socket is the shim's; the guard only knows the path | | success | live session | guard releases the child to a reaper; socket and record now belong to the terminal | | failure or cancel | `Abandoned`, in a tracked task | moved out of the caller's future, so a cancelled caller cannot cancel it; counted in a process-wide tracker | | shutdown | tracker + drop fallback | see 3 | **2. What proves a terminal is dead** (and what does not) - NOT proof: the launcher's exit or reaping, if the launch detaches. Not proof either: a missing socket (the shim may bind late). - Proof A: a non-detaching launcher IS the shim (systemd-run execs it; a login shim is spawned directly). Killed and reaped through its own unreaped `Child` = shim dead. Only then may cleanup unlink that launch's own rendezvous leftovers. - Proof B: a detaching launcher that has never forked. Freeze it (`SIGSTOP` on its own unreaped child), read `/proc/<pid>/task/*/children`; if empty it cannot have started a shim, and it cannot fork while stopped. Then kill and reap. If the list is non-empty or unreadable: `SIGCONT`, do not kill, and fall to C. - Proof C: the shim's own exit. Authenticate its Hello (session id, peer uid), send `CloseSession`, then require the rendezvous gone or sealed by an exit record. Waits for a late bind on the directory watch, created before the first connect so no bind is missed. - Anything else is UNPROVEN: leave the socket and the record alone. The person record is deleted only on A, B or C. Cleanup never unlinks a detached shim's socket (it removes its own on exit). **3. Shutdown** - Agent stop (`run_channel` returns): `drain_cleanups` waits for the tracker, bounded (`HANDSHAKE` x 2), so a longer-lived runtime finishes what it queued. - Runtime shutdown drops the spawned task: `Abandoned::drop` sees cleanup did not finish and runs the same cleanup to completion on a dedicated thread with its own current-thread runtime and a short budget. No runtime at all: same path. - Hard process exit with no unwind is not covered; surviving shims stay discoverable by their sockets. **Owner question (not blocking).** If cleanup ends UNPROVEN (a live detached shim that never answered), a live unnamed terminal and its record stay, and revocation for that person refuses until it is closed. Recommendation: leave it (safe, visible to discovery, closable); no auto-kill without proof. I continue with that.
kayg commented 2026-11-04 21:54:05 +00:00
Author
Owner
Copy link

J170 round 3 progress. Head 931f6dd9cc27d6b1a3c95c387de7a273e9731fec, pushed (design note is the comment above). Implemented per the note:

  • proof rules: non-detaching launcher killed+reaped (it is the shim); detaching launcher killed only after SIGSTOP + empty /proc/<pid>/task/*/children (never forked); otherwise only the shim's own authenticated exit counts. A live terminal's socket and record are never deleted on the launcher's exit.
  • shutdown: process-wide tracker drained (4 s) when the agent's channel loop ends; a cleanup dropped by runtime shutdown finishes on its own thread/runtime (2 s budget).

Fail first (compute01, ce40a62f6 = old cleanup, new tests):

runtime_shutdown_with_a_queued_cleanup_still_stops_the_owned_launcher ... FAILED: runtime shutdown discarded the cleanup and left launcher 1775536 running
a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... FAILED: the late shim was never asked to close: Elapsed(())
the_shims_own_observed_exit_retires_the_person_record ... ok   (positive half; passes before and after)

An earlier quick at 5f6003109 failed clippy on a doc-comment paragraph only (fixed in 931f6dd9c). quick, the named tests and verify fast are queued on compute01 (position 9 of 4 slots); verbatim results follow. No owner decision needed yet; the note's one non-blocking question (unproven cleanup leaves the live terminal in place) stands, safe option taken.

**J170 round 3 progress.** Head `931f6dd9cc27d6b1a3c95c387de7a273e9731fec`, pushed (design note is the comment above). Implemented per the note: - proof rules: non-detaching launcher killed+reaped (it is the shim); detaching launcher killed only after SIGSTOP + empty `/proc/<pid>/task/*/children` (never forked); otherwise only the shim's own authenticated exit counts. A live terminal's socket and record are never deleted on the launcher's exit. - shutdown: process-wide tracker drained (4 s) when the agent's channel loop ends; a cleanup dropped by runtime shutdown finishes on its own thread/runtime (2 s budget). **Fail first (compute01, `ce40a62f6` = old cleanup, new tests):** ``` runtime_shutdown_with_a_queued_cleanup_still_stops_the_owned_launcher ... FAILED: runtime shutdown discarded the cleanup and left launcher 1775536 running a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... FAILED: the late shim was never asked to close: Elapsed(()) the_shims_own_observed_exit_retires_the_person_record ... ok (positive half; passes before and after) ``` An earlier `quick` at `5f6003109` failed clippy on a doc-comment paragraph only (fixed in `931f6dd9c`). `quick`, the named tests and `verify fast` are queued on compute01 (position 9 of 4 slots); verbatim results follow. No owner decision needed yet; the note's one non-blocking question (unproven cleanup leaves the live terminal in place) stands, safe option taken.
kayg commented 2026-11-05 02:58:27 +00:00
Author
Owner
Copy link

J170 round 3 finish: GREEN. Branch launch-followups-j170, head d1bf9550a1ffb145c917cfaf454bb9f81edce35c, pushed, not merged, not deployed. Design note is the comment above; this implements it.

Both J200 P1s

  • P1 shutdown. A failed launch's cleanup is a tracked task (Launch -> Abandoned + process-wide counter). The channel loop drains it (bounded 4 s) when the agent stops. If the runtime shuts down first, Abandoned::drop runs the same cleanup to its end on a dedicated thread and runtime (2 s budget); children are reaped by polling try_wait, since the dead runtime's reaper is unusable. Test: runtime_shutdown_with_a_queued_cleanup_still_stops_the_owned_launcher (queued, never polled, runtime dropped).
  • P1 proof. The launcher's exit is no longer proof for a detaching shim. A non-detaching launcher IS the shim (killed and reaped). A detaching launcher is killed only after SIGSTOP, the kernel reporting it stopped, and empty /proc/<pid>/task/*/children (it never forked, so no shim can exist). Otherwise only the shim's own authenticated exit counts: Hello session id and uid checked, CloseSession sent, rendezvous gone or sealed. The directory watch is created before the first connect. A live terminal's socket and person record are never deleted without that proof, and cleanup never unlinks a detached shim's socket. Tests: a_forked_launcher_is_not_proof_the_live_terminal_is_dead (real forked descendant, live shim that answers the close but stays), and the positive half the_shims_own_observed_exit_retires_the_person_record.

Fail first (compute01, ce40a62f6 = old cleanup, new tests):

runtime_shutdown_with_a_queued_cleanup_still_stops_the_owned_launcher ... FAILED: runtime shutdown discarded the cleanup and left launcher 1775536 running
a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... FAILED: the late shim was never asked to close: Elapsed(())
the_shims_own_observed_exit_retires_the_person_record ... ok (positive half, passes before and after)

Green at the head: all nine launch tests pass (shutdown 0.34 s, forked 3.08 s, proven-exit 0.91 s, cancellation 0.72 s, late bind 1.52 s, person record 0.34 s, slow Hello 4.03 s, bystander 0.55 s, probe 0.00 s).

Commits since round 2: ce40a62f6 (merge of dev), 5f6003109 proof + shutdown, 931f6dd9c doc paragraph, defba7ccf and d1bf9550a future-size fixes. Cause of the last two: my drain wrapper stored the channel loop's arguments again and pushed a caller's future in infraternal remote tests past clippy's 16 KiB large_futures bound (dev was within bytes; I proved dev passes and my head failed with the same workspace clippy command). The loop is now boxed under the wrapper (one allocation per channel run, same bytes moved to the heap), and the drain adds nothing when no cleanup is pending.

scripts/verify quick, compute01, verbatim:

| scope guard               | PASS         |
| fmt                       | PASS         |
| doc coverage ratchet      | PASS         |
| clippy (workspace)        | PASS         |
| unit tests (changed)      | PASS         |

scripts/verify fast at d1bf9550a, compute01, foreground, VERIFY_EXIT=0, verbatim:

| fmt                       | PASS         |
| VT native fmt             | PASS         |
| CLI plugin compile-out    | PASS         |
| clippy                    | PASS         |
| SSH runtime test helper   | PASS         |
| real-binary fixture paths | PASS         |
| Traefik component manifest | PASS         |
| release component publication | PASS         |
| test                      | PASS         |
| SSH inode reuse fixture   | PASS         |
| shim process-per-user     | PASS         |
| cargo-deny                | PASS         |
| gate architecture         | PASS         |
| doc coverage ratchet      | PASS         |
| static builds             | PASS         |

Sizes (report only): x86_64 23491088 bytes (+1072832 vs stored baseline), aarch64 21353072 (+899792); these include all of dev's growth since the baseline.

Honest gate history at this head (three full runs, one green): run 1: static builds failed with No space left on device (compute01's disk filled by other jobs, 54 G free after) and one restart_deleted_path_review test failed ETXTBSY. Run 2: static builds PASS, test failed detached_shell_exit_reaps_shim_within_one_second and ..._with_a_live_background_descendant (1 s wall-clock tests of the shim, crates/infraternal/tests/shim.rs, no launch code). Run 3: all PASS. Those two shim tests then passed 12/12 at the head and 6/6 on the dev I merged, interleaved at load 17 to 22, so I treat them as load-sensitive; I did not reproduce the failures, so this is stated, not proven.

Limits / for kayg

  • Unproven cleanup (a live detached shim that never answers within the window, or a kernel without /proc/*/task/*/children) leaves the terminal, its socket and its person record in place; revocation for that person then refuses until the terminal is closed. Chosen as the safe option; say if you want a recovery path (for example the next agent start closing unclaimed unnamed launches).
  • Hard process exit with no unwinding is not covered; surviving shims stay discoverable by their sockets.
  • systemctl stop scope route: still no test of its own (J200 ran the exact commands).
  • No root-only or test-server run.
  • Leftovers: my launcher stand-ins from the red runs were killed by PID; the target dir is deleted.
**J170 round 3 finish: GREEN. Branch `launch-followups-j170`, head `d1bf9550a1ffb145c917cfaf454bb9f81edce35c`**, pushed, not merged, not deployed. Design note is the comment above; this implements it. **Both J200 P1s** - **P1 shutdown.** A failed launch's cleanup is a tracked task (`Launch` -> `Abandoned` + process-wide counter). The channel loop drains it (bounded 4 s) when the agent stops. If the runtime shuts down first, `Abandoned::drop` runs the same cleanup to its end on a dedicated thread and runtime (2 s budget); children are reaped by polling `try_wait`, since the dead runtime's reaper is unusable. Test: `runtime_shutdown_with_a_queued_cleanup_still_stops_the_owned_launcher` (queued, never polled, runtime dropped). - **P1 proof.** The launcher's exit is no longer proof for a detaching shim. A non-detaching launcher IS the shim (killed and reaped). A detaching launcher is killed only after `SIGSTOP`, the kernel reporting it stopped, and empty `/proc/<pid>/task/*/children` (it never forked, so no shim can exist). Otherwise only the shim's own authenticated exit counts: Hello session id and uid checked, `CloseSession` sent, rendezvous gone or sealed. The directory watch is created before the first connect. A live terminal's socket and person record are never deleted without that proof, and cleanup never unlinks a detached shim's socket. Tests: `a_forked_launcher_is_not_proof_the_live_terminal_is_dead` (real forked descendant, live shim that answers the close but stays), and the positive half `the_shims_own_observed_exit_retires_the_person_record`. **Fail first (compute01, `ce40a62f6` = old cleanup, new tests):** ``` runtime_shutdown_with_a_queued_cleanup_still_stops_the_owned_launcher ... FAILED: runtime shutdown discarded the cleanup and left launcher 1775536 running a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... FAILED: the late shim was never asked to close: Elapsed(()) the_shims_own_observed_exit_retires_the_person_record ... ok (positive half, passes before and after) ``` Green at the head: all nine launch tests pass (shutdown 0.34 s, forked 3.08 s, proven-exit 0.91 s, cancellation 0.72 s, late bind 1.52 s, person record 0.34 s, slow Hello 4.03 s, bystander 0.55 s, probe 0.00 s). **Commits since round 2:** `ce40a62f6` (merge of dev), `5f6003109` proof + shutdown, `931f6dd9c` doc paragraph, `defba7ccf` and `d1bf9550a` future-size fixes. Cause of the last two: my drain wrapper stored the channel loop's arguments again and pushed a caller's future in `infraternal` remote tests past clippy's 16 KiB `large_futures` bound (dev was within bytes; I proved dev passes and my head failed with the same workspace clippy command). The loop is now boxed under the wrapper (one allocation per channel run, same bytes moved to the heap), and the drain adds nothing when no cleanup is pending. **`scripts/verify quick`, compute01, verbatim:** ``` | scope guard | PASS | | fmt | PASS | | doc coverage ratchet | PASS | | clippy (workspace) | PASS | | unit tests (changed) | PASS | ``` **`scripts/verify fast` at `d1bf9550a`, compute01, foreground, VERIFY_EXIT=0, verbatim:** ``` | fmt | PASS | | VT native fmt | PASS | | CLI plugin compile-out | PASS | | clippy | PASS | | SSH runtime test helper | PASS | | real-binary fixture paths | PASS | | Traefik component manifest | PASS | | release component publication | PASS | | test | PASS | | SSH inode reuse fixture | PASS | | shim process-per-user | PASS | | cargo-deny | PASS | | gate architecture | PASS | | doc coverage ratchet | PASS | | static builds | PASS | ``` Sizes (report only): x86_64 23491088 bytes (+1072832 vs stored baseline), aarch64 21353072 (+899792); these include all of dev's growth since the baseline. **Honest gate history at this head (three full runs, one green):** run 1: `static builds` failed with `No space left on device` (compute01's disk filled by other jobs, 54 G free after) and one `restart_deleted_path_review` test failed `ETXTBSY`. Run 2: `static builds` PASS, `test` failed `detached_shell_exit_reaps_shim_within_one_second` and `..._with_a_live_background_descendant` (1 s wall-clock tests of the shim, `crates/infraternal/tests/shim.rs`, no launch code). Run 3: all PASS. Those two shim tests then passed 12/12 at the head and 6/6 on the dev I merged, interleaved at load 17 to 22, so I treat them as load-sensitive; I did not reproduce the failures, so this is stated, not proven. **Limits / for kayg** - Unproven cleanup (a live detached shim that never answers within the window, or a kernel without `/proc/*/task/*/children`) leaves the terminal, its socket and its person record in place; revocation for that person then refuses until the terminal is closed. Chosen as the safe option; say if you want a recovery path (for example the next agent start closing unclaimed unnamed launches). - Hard process exit with no unwinding is not covered; surviving shims stay discoverable by their sockets. - `systemctl stop` scope route: still no test of its own (J200 ran the exact commands). - No root-only or test-server run. - Leftovers: my launcher stand-ins from the red runs were killed by PID; the target dir is deleted.
kayg commented 2026-11-05 03:06:39 +00:00
Author
Owner
Copy link

J222 independent re-review start. Target branch launch-followups-j170, head d1bf9550a1ffb145c917cfaf454bb9f81edce35c; merge base with origin/dev e8e8d2ff79b328a71faa62e7f764d705a133eeb4 (current origin/dev 2a4f29de4ed888e203e2210780745f2070346d5a). Plan: inspect the cleanup/proof paths and D345 docs; rerun the three focused tests against the pre-fix and target refs on compute01; assess the boxed channel future and systemctl route; post evidence and verdict. Estimate: 2 h. Read-only review; no branch changes.

J222 independent re-review start. Target branch `launch-followups-j170`, head `d1bf9550a1ffb145c917cfaf454bb9f81edce35c`; merge base with origin/dev `e8e8d2ff79b328a71faa62e7f764d705a133eeb4` (current origin/dev `2a4f29de4ed888e203e2210780745f2070346d5a`). Plan: inspect the cleanup/proof paths and D345 docs; rerun the three focused tests against the pre-fix and target refs on compute01; assess the boxed channel future and systemctl route; post evidence and verdict. Estimate: 2 h. Read-only review; no branch changes.
kayg commented 2026-11-05 03:35:50 +00:00
Author
Owner
Copy link

Finding (P2) — crates/infraternal-agent/src/sessions.rs:1064-1079: cleanup can be discarded if the fallback thread cannot start. Builder::spawn is handled only on Ok; on Err, the closure and its captured Parts are dropped, including the Tokio Child, with no kill fallback. The launched child uses the default kill_on_drop = false. This leaves an owned failed-launch process running during runtime shutdown when the OS cannot create the cleanup thread (for example, thread/resource exhaustion). The tracker is then dropped too, so the drain no longer sees pending work. This is code-path evidence; I have not fault-injected thread creation failure.

**Finding (P2) — `crates/infraternal-agent/src/sessions.rs:1064-1079`: cleanup can be discarded if the fallback thread cannot start.** `Builder::spawn` is handled only on `Ok`; on `Err`, the closure and its captured `Parts` are dropped, including the Tokio `Child`, with no kill fallback. The launched child uses the default `kill_on_drop = false`. This leaves an owned failed-launch process running during runtime shutdown when the OS cannot create the cleanup thread (for example, thread/resource exhaustion). The tracker is then dropped too, so the drain no longer sees pending work. This is code-path evidence; I have not fault-injected thread creation failure.
kayg commented 2026-11-05 03:35:59 +00:00
Author
Owner
Copy link

Finding (P2) — crates/infraternal-agent/src/sessions.rs:1071,1079,1317-1363: the shutdown fallback can block past its 2 s budget. Abandoned::drop calls runtime.block_on(cleanup(..., SHUTDOWN_CLEANUP)) without an outer timeout, then synchronously joins the worker. In the detached path, the 2 s budget bounds retries only; after a successful connect, CloseSession gets a separate HANDSHAKE timeout and exit_observed gets another. In the non-detached path, stop_scope and reaped each have their own HANDSHAKE bound, while the passed cleanup budget is unused. Therefore a runtime drop can block beyond the claimed 2 s fallback budget; after the 4 s drain, it can add more shutdown time. This follows from the timeout scopes in code; I have not timed a forced slow-shim shutdown.

**Finding (P2) — `crates/infraternal-agent/src/sessions.rs:1071,1079,1317-1363`: the shutdown fallback can block past its 2 s budget.** `Abandoned::drop` calls `runtime.block_on(cleanup(..., SHUTDOWN_CLEANUP))` without an outer timeout, then synchronously joins the worker. In the detached path, the 2 s `budget` bounds retries only; after a successful connect, `CloseSession` gets a separate `HANDSHAKE` timeout and `exit_observed` gets another. In the non-detached path, `stop_scope` and `reaped` each have their own `HANDSHAKE` bound, while the passed cleanup budget is unused. Therefore a runtime drop can block beyond the claimed 2 s fallback budget; after the 4 s drain, it can add more shutdown time. This follows from the timeout scopes in code; I have not timed a forced slow-shim shutdown.
kayg commented 2026-11-05 04:11:24 +00:00
Author
Owner
Copy link

J222 finding (P1): an empty child list after the freeze does not prove the launcher never forked.

crates/infraternal-shim/src/pty.rs:879-895 double-forks: the original launch process waits for the intermediate child, which forks the detached shim and exits. There is a window after that waitpid reaps the intermediate and before the original process itself exits. If cleanup freezes the original in that window, /proc/<pid>/task/*/children is empty even though the detached grandchild is alive. sessions.rs:1238-1240 treats the empty current-child list as proof it never forked, kills/reaps the original and returns true; cleanup then removes the person record at 1200-1201 without authenticating or closing the shim. The detached socket is retained, but rootless_sessions.rs:98-106 sees its live, unsealed socket without an owner record and refuses revocation. The new fork test holds a direct child in wait, so it does not cover the post-reap window.

This follows from the target's real double-fork and cleanup control flow; I have not yet reproduced the precise scheduling window. Keep the person record unless the shim's authenticated exit is observed or a reliable no-fork proof is available.

J222 finding (P1): an empty child list after the freeze does not prove the launcher never forked. `crates/infraternal-shim/src/pty.rs:879-895` double-forks: the original launch process waits for the intermediate child, which forks the detached shim and exits. There is a window after that `waitpid` reaps the intermediate and before the original process itself exits. If cleanup freezes the original in that window, `/proc/<pid>/task/*/children` is empty even though the detached grandchild is alive. `sessions.rs:1238-1240` treats the empty current-child list as proof it never forked, kills/reaps the original and returns `true`; `cleanup` then removes the person record at `1200-1201` without authenticating or closing the shim. The detached socket is retained, but `rootless_sessions.rs:98-106` sees its live, unsealed socket without an owner record and refuses revocation. The new fork test holds a direct child in `wait`, so it does not cover the post-reap window. This follows from the target's real double-fork and cleanup control flow; I have not yet reproduced the precise scheduling window. Keep the person record unless the shim's authenticated exit is observed or a reliable no-fork proof is available.
kayg commented 2026-11-05 04:11:48 +00:00
Author
Owner
Copy link

J222 finding (P2): an unproven detached cleanup drops its owned launcher without stopping it.

If never_forked returns false and close_detached cannot observe the shim exit, cleanup returns false without signalling/reaping parts.child (sessions.rs:1205-1212). abandon_launch then sets job.parts = None (1165-1169); Tokio Child is not kill-on-drop here, so the launcher can continue running after its cleanup task is counted complete. This is reachable when the child list cannot be read and no authenticated socket appears, and also when a forked launcher remains live after an inconclusive close. The target test a_forked_launcher_is_not_proof_the_live_terminal_is_dead observes launcher_alive=true after cleanup and manually kills its launcher and descendant (tests/unit/sessions.rs:955-963).

Preserve the socket and person record when terminal exit is unproven, but still stop/reap the launcher process this launch owns. This is separate from the thread-creation failure in #79291, which loses cleanup before it runs.

J222 finding (P2): an unproven detached cleanup drops its owned launcher without stopping it. If `never_forked` returns false and `close_detached` cannot observe the shim exit, `cleanup` returns `false` without signalling/reaping `parts.child` (`sessions.rs:1205-1212`). `abandon_launch` then sets `job.parts = None` (`1165-1169`); Tokio `Child` is not kill-on-drop here, so the launcher can continue running after its cleanup task is counted complete. This is reachable when the child list cannot be read and no authenticated socket appears, and also when a forked launcher remains live after an inconclusive close. The target test `a_forked_launcher_is_not_proof_the_live_terminal_is_dead` observes `launcher_alive=true` after cleanup and manually kills its launcher and descendant (`tests/unit/sessions.rs:955-963`). Preserve the socket and person record when terminal exit is unproven, but still stop/reap the launcher process this launch owns. This is separate from the thread-creation failure in #79291, which loses cleanup before it runs.
kayg commented 2026-11-05 07:20:47 +00:00
Author
Owner
Copy link

J170 round 4 progress (J222's four findings). Head a11438729527ed1018da6a57e573c5482319a6f8, pushed, origin/dev merged (cdddfc8ce). Two atomic commits:

  • 21775b16e 79368 P1 + 79369 P2. The "never forked" proof now needs no children AND no reaped child (cminflt, cmajflt, cutime, cstime of the frozen launcher are all zero): a reaped child adds its faults and time to its parent, so the post-reap window of the real double fork (empty child list, grandchild alive) no longer reads as "never forked". The owned launcher is killed and reaped on every path; the socket and person record stay unless the shim's authenticated exit is observed.
  • a11438729 79291 + 79292 P2. The fallback's parts are shared with its thread, so a failed Builder::spawn still stops the launcher synchronously (stop_now). cleanup takes Budget { retry, total } with one outer deadline around everything (2 s at shutdown, 14 s otherwise); the non-detached path caps its waits by it.

Fail first (compute01, cdddfc8ce, new tests; thread-failure hook added to the old code):

a_launcher_that_has_reaped_its_child_is_not_proof_no_shim_exists ... FAILED: the live shim was never asked to close: Elapsed(())
a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... FAILED: an unproven cleanup left the launcher it owns running (J222 79369)
a_cleanup_thread_that_cannot_start_still_stops_the_owned_launcher ... FAILED: a failed cleanup thread left the owned launcher running
the_shutdown_cleanup_ends_within_its_budget_whatever_the_shim_does ... FAILED: the shutdown cleanup spent 3.5s against a 2s budget (paused clock, virtual time)

Green at the head: all twelve launch tests; verify quick PASS (scope guard, migration names, fmt, doc coverage, clippy workspace, unit tests). The first commit builds and passes its own tests on its own.

Gate, honestly. verify fast at the head failed twice on test, on two different unrelated tests, at load 8 to 25: (1) real_binary_rootless_revocation_after_restart_stops_every_owned_terminal (AssertionError (381, 0) in agent_fixture.attach; this exact signature also appeared on my first gate, at 7684c5042), then 32 interleaved runs of that oracle passed: head 16/16, dev 16/16; (2) restart::tests::restart_requests_use_the_servers_binary_and_reject_untrusted_paths (owner restart request deadline: Elapsed). Other restart:: tests failed in earlier rounds on ETXTBSY and strip test image exceeded 60s. Running the restart:: family head vs dev next, then verify fast again.

**J170 round 4 progress (J222's four findings).** Head `a11438729527ed1018da6a57e573c5482319a6f8`, pushed, origin/dev merged (`cdddfc8ce`). Two atomic commits: - `21775b16e` **79368 P1 + 79369 P2.** The "never forked" proof now needs no children AND no reaped child (`cminflt`, `cmajflt`, `cutime`, `cstime` of the frozen launcher are all zero): a reaped child adds its faults and time to its parent, so the post-reap window of the real double fork (empty child list, grandchild alive) no longer reads as "never forked". The owned launcher is killed and reaped on every path; the socket and person record stay unless the shim's authenticated exit is observed. - `a11438729` **79291 + 79292 P2.** The fallback's parts are shared with its thread, so a failed `Builder::spawn` still stops the launcher synchronously (`stop_now`). `cleanup` takes `Budget { retry, total }` with one outer deadline around everything (2 s at shutdown, 14 s otherwise); the non-detached path caps its waits by it. **Fail first (compute01, `cdddfc8ce`, new tests; thread-failure hook added to the old code):** ``` a_launcher_that_has_reaped_its_child_is_not_proof_no_shim_exists ... FAILED: the live shim was never asked to close: Elapsed(()) a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... FAILED: an unproven cleanup left the launcher it owns running (J222 79369) a_cleanup_thread_that_cannot_start_still_stops_the_owned_launcher ... FAILED: a failed cleanup thread left the owned launcher running the_shutdown_cleanup_ends_within_its_budget_whatever_the_shim_does ... FAILED: the shutdown cleanup spent 3.5s against a 2s budget (paused clock, virtual time) ``` Green at the head: all twelve launch tests; `verify quick` PASS (scope guard, migration names, fmt, doc coverage, clippy workspace, unit tests). The first commit builds and passes its own tests on its own. **Gate, honestly.** `verify fast` at the head failed twice on `test`, on two different unrelated tests, at load 8 to 25: (1) `real_binary_rootless_revocation_after_restart_stops_every_owned_terminal` (`AssertionError (381, 0)` in `agent_fixture.attach`; this exact signature also appeared on my first gate, at `7684c5042`), then 32 interleaved runs of that oracle passed: head 16/16, dev 16/16; (2) `restart::tests::restart_requests_use_the_servers_binary_and_reject_untrusted_paths` (`owner restart request deadline: Elapsed`). Other `restart::` tests failed in earlier rounds on `ETXTBSY` and `strip test image exceeded 60s`. Running the `restart::` family head vs dev next, then `verify fast` again.
kayg commented 2026-11-05 08:28:18 +00:00
Author
Owner
Copy link

J170 round 4 finish: GREEN. Branch launch-followups-j170, head a11438729527ed1018da6a57e573c5482319a6f8, pushed, not merged, not deployed. J222's four findings fixed in two atomic commits on top of the merge of origin/dev (cdddfc8ce).

Commits

  • 21775b16e (79368 P1, 79369 P2). An empty child list is no longer taken for "never forked". A frozen launcher is proven never to have forked only if it has no children AND has reaped none (cminflt, cmajflt, cutime, cstime in /proc/<pid>/stat all zero): a reaped child's faults and CPU time are added to its parent, and any forked process that ran took at least one copy-on-write fault, so the post-reap window of the real double fork (empty list, grandchild alive) reads as "forked". Anything else resumes the launcher and proves nothing; the person record and the socket stay unless the shim's authenticated exit is observed. The owned launcher is killed and reaped on every path (killing the original cannot touch the shim; its only remaining work is to exit).
  • a11438729 (79291, 79292, both P2). The shutdown fallback shares the launch's parts with its thread, so if Builder::spawn fails the launcher is still killed and reaped synchronously (stop_now, about 100 ms at most; socket and record untouched). cleanup takes Budget { retry, total } with one outer deadline around everything (2 s at shutdown, 14 s otherwise), and the non-detached path caps its waits by it; after a deadline the owned launcher is still stopped.

Fail first (compute01, cdddfc8ce; new tests, plus the thread-failure hook added to the old code):

a_launcher_that_has_reaped_its_child_is_not_proof_no_shim_exists ... FAILED: the live shim was never asked to close: Elapsed(())
a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... FAILED: an unproven cleanup left the launcher it owns running (J222 79369)
a_cleanup_thread_that_cannot_start_still_stops_the_owned_launcher ... FAILED: a failed cleanup thread left the owned launcher running
the_shutdown_cleanup_ends_within_its_budget_whatever_the_shim_does ... FAILED: the shutdown cleanup spent 3.5s against a 2s budget

The post-reap test cancels the launch at exactly the point where the launcher script has reaped a child (it touches ready after wait), a deterministic stand-in for the real window. The budget test runs on the runtime's paused clock, so it asserts the bound in virtual time, never wall-clock speed.

Green at the head, compute01: all twelve launch tests pass (post-reap 2.13 s, forked 3.12 s, thread-failure 0.32 s, budget 0.02 s, shutdown 0.35 s, proven-exit 0.92 s, cancel 0.73 s, late bind 1.51 s, person record 0.96 s, slow Hello 4.14 s, bystander 0.58 s, probe 0.00 s). Commit 21775b16e on its own: workspace builds, its four tests pass. (cargo clippy -p infraternal-agent --all-targets alone reports with_registry never used; that is the same dev warning as before and only appears without the workspace features. The gate's workspace clippy passes.)

scripts/verify quick, verbatim:

| scope guard               | PASS         |
| migration names           | PASS         |
| fmt                       | PASS         |
| doc coverage ratchet      | PASS         |
| clippy (workspace)        | PASS         |
| unit tests (changed)      | PASS         |

scripts/verify fast at a11438729, compute01, foreground, VERIFY_EXIT=0, verbatim:

| fmt                       | PASS         |
| migration names           | PASS         |
| migration tooling tests   | PASS         |
| VT native fmt             | PASS         |
| CLI plugin compile-out    | PASS         |
| clippy                    | PASS         |
| SSH runtime test helper   | PASS         |
| real-binary fixture paths | PASS         |
| Traefik component manifest | PASS         |
| release component publication | PASS         |
| test                      | PASS         |
| SSH inode reuse fixture   | PASS         |
| shim process-per-user     | PASS         |
| cargo-deny                | PASS         |
| gate architecture         | PASS         |
| doc coverage ratchet      | PASS         |
| static builds             | PASS         |

Sizes (report only): x86_64 23631440 bytes (+1213184 vs the stored baseline), aarch64 21479344 (+1026064); these include all of dev's growth since the baseline.

Honest gate history at this head: three full runs, one green. Run 1: real_binary_rootless_revocation_after_restart_stops_every_owned_terminal failed (AssertionError (381, 0) in agent_fixture.attach, load 25; this signature also appeared on my very first gate, at 7684c5042). Run 2: restart::tests::restart_requests_use_the_servers_binary_and_reject_untrusted_paths failed (owner restart request deadline: Elapsed, load 8). Run 3: all PASS. Interleaved comparisons, same host, at load 8 to 22: the revocation oracle passed 32/32 (head 16, dev 16); the whole restart:: family 19 tests x 12 (head 8, dev 4) all passed. Different unrelated load-sensitive tests failed each time and none reproduced, so I call them load-sensitive; I did not prove that against a failing dev.

Open points

  • The cminflt proof is kernel accounting, not a handshake. The alternative J222 named (the shim reports its pid, or holds a lifeline fd open, before the intermediate exits) is a shim startup change and a cleaner proof; I did not do it. If you want it, say so and I will scope it. Without proof, a hung launcher that never forked still leaves its person record in place, as chosen on #324.
  • Unproven cleanup leaves a live terminal, its socket and its person record; revocation for that person then refuses until the terminal is closed. Safe option taken, as before.
  • systemctl stop scope route: no test of its own; no root-only or test-server run.
  • Leftovers: my launcher stand-ins were killed by PID; I delete my target dir now.
**J170 round 4 finish: GREEN. Branch `launch-followups-j170`, head `a11438729527ed1018da6a57e573c5482319a6f8`**, pushed, not merged, not deployed. J222's four findings fixed in two atomic commits on top of the merge of origin/dev (`cdddfc8ce`). **Commits** - `21775b16e` (79368 P1, 79369 P2). An empty child list is no longer taken for "never forked". A frozen launcher is proven never to have forked only if it has no children AND has reaped none (`cminflt`, `cmajflt`, `cutime`, `cstime` in `/proc/<pid>/stat` all zero): a reaped child's faults and CPU time are added to its parent, and any forked process that ran took at least one copy-on-write fault, so the post-reap window of the real double fork (empty list, grandchild alive) reads as "forked". Anything else resumes the launcher and proves nothing; the person record and the socket stay unless the shim's authenticated exit is observed. The owned launcher is killed and reaped on every path (killing the original cannot touch the shim; its only remaining work is to exit). - `a11438729` (79291, 79292, both P2). The shutdown fallback shares the launch's parts with its thread, so if `Builder::spawn` fails the launcher is still killed and reaped synchronously (`stop_now`, about 100 ms at most; socket and record untouched). `cleanup` takes `Budget { retry, total }` with one outer deadline around everything (2 s at shutdown, 14 s otherwise), and the non-detached path caps its waits by it; after a deadline the owned launcher is still stopped. **Fail first (compute01, `cdddfc8ce`; new tests, plus the thread-failure hook added to the old code):** ``` a_launcher_that_has_reaped_its_child_is_not_proof_no_shim_exists ... FAILED: the live shim was never asked to close: Elapsed(()) a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... FAILED: an unproven cleanup left the launcher it owns running (J222 79369) a_cleanup_thread_that_cannot_start_still_stops_the_owned_launcher ... FAILED: a failed cleanup thread left the owned launcher running the_shutdown_cleanup_ends_within_its_budget_whatever_the_shim_does ... FAILED: the shutdown cleanup spent 3.5s against a 2s budget ``` The post-reap test cancels the launch at exactly the point where the launcher script has reaped a child (it touches `ready` after `wait`), a deterministic stand-in for the real window. The budget test runs on the runtime's paused clock, so it asserts the bound in virtual time, never wall-clock speed. **Green at the head, compute01:** all twelve launch tests pass (post-reap 2.13 s, forked 3.12 s, thread-failure 0.32 s, budget 0.02 s, shutdown 0.35 s, proven-exit 0.92 s, cancel 0.73 s, late bind 1.51 s, person record 0.96 s, slow Hello 4.14 s, bystander 0.58 s, probe 0.00 s). Commit `21775b16e` on its own: workspace builds, its four tests pass. (`cargo clippy -p infraternal-agent --all-targets` alone reports `with_registry never used`; that is the same dev warning as before and only appears without the workspace features. The gate's workspace clippy passes.) **`scripts/verify quick`, verbatim:** ``` | scope guard | PASS | | migration names | PASS | | fmt | PASS | | doc coverage ratchet | PASS | | clippy (workspace) | PASS | | unit tests (changed) | PASS | ``` **`scripts/verify fast` at `a11438729`, compute01, foreground, VERIFY_EXIT=0, verbatim:** ``` | fmt | PASS | | migration names | PASS | | migration tooling tests | PASS | | VT native fmt | PASS | | CLI plugin compile-out | PASS | | clippy | PASS | | SSH runtime test helper | PASS | | real-binary fixture paths | PASS | | Traefik component manifest | PASS | | release component publication | PASS | | test | PASS | | SSH inode reuse fixture | PASS | | shim process-per-user | PASS | | cargo-deny | PASS | | gate architecture | PASS | | doc coverage ratchet | PASS | | static builds | PASS | ``` Sizes (report only): x86_64 23631440 bytes (+1213184 vs the stored baseline), aarch64 21479344 (+1026064); these include all of dev's growth since the baseline. **Honest gate history at this head: three full runs, one green.** Run 1: `real_binary_rootless_revocation_after_restart_stops_every_owned_terminal` failed (`AssertionError (381, 0)` in `agent_fixture.attach`, load 25; this signature also appeared on my very first gate, at `7684c5042`). Run 2: `restart::tests::restart_requests_use_the_servers_binary_and_reject_untrusted_paths` failed (`owner restart request deadline: Elapsed`, load 8). Run 3: all PASS. Interleaved comparisons, same host, at load 8 to 22: the revocation oracle passed 32/32 (head 16, dev 16); the whole `restart::` family 19 tests x 12 (head 8, dev 4) all passed. Different unrelated load-sensitive tests failed each time and none reproduced, so I call them load-sensitive; I did not prove that against a failing dev. **Open points** - The cminflt proof is kernel accounting, not a handshake. The alternative J222 named (the shim reports its pid, or holds a lifeline fd open, before the intermediate exits) is a shim startup change and a cleaner proof; I did not do it. If you want it, say so and I will scope it. Without proof, a hung launcher that never forked still leaves its person record in place, as chosen on #324. - Unproven cleanup leaves a live terminal, its socket and its person record; revocation for that person then refuses until the terminal is closed. Safe option taken, as before. - `systemctl stop` scope route: no test of its own; no root-only or test-server run. - Leftovers: my launcher stand-ins were killed by PID; I delete my target dir now.
kayg commented 2026-11-05 09:36:10 +00:00
Author
Owner
Copy link

J230 re-review of launch-followups-j170 at a11438729527ed1018da6a57e573c5482319a6f8 (delta since d1bf9550a): REQUEST CHANGES (one P2, two P3).

Scope: 21775b16e and a11438729. In both commits only crates/infraternal-agent/src/sessions.rs and tests/unit/sessions.rs change. sessions.rs and its tests are byte-identical at d1bf9550a and the merge cdddfc8ce. Per the reviewer rule, I did not rerun verify fast/quick. I read J170's gate output (79639: fast green on run 3 of 3, with two load-sensitive failures in unrelated tests on runs 1 and 2; I did not re-verify those).

The four J222 findings

J222 Status Evidence
79368 P1, empty child list after the freeze Closed for the shipped spawn path, but the proof rests on an invariant nothing enforces. See P2-A below. The red/green pair works. My auto-reap probe defeats the proof.
79369 P2, unproven cleanup must still stop the launcher Closed stop_launch kills and reaps in the "may have forked" path (sessions.rs:1281-1284), and cleanup kills and reaps again after any outcome or timeout (sessions.rs:1253-1255). The socket and the record are kept unless proven.
79291 P2, Builder::spawn failure Closed Parts sit in an Arc<Mutex<Option<_>>>. On Err the closure drops only its Arc clone, and stop_now kills and reaps synchronously (sessions.rs:1107-1113, 1136). Poisoning is impossible: the lock is held only for take().
79292 P2, one outer deadline, budget used on the non-detached path Closed, with one leak past the deadline, see P3-B timeout(budget.total, stop_launch(..)) (sessions.rs:1250). The non-detached path caps stop_scope and reaped at HANDSHAKE.min(retry).

PID reuse: sound. never_forked signals and reads only child.id(), the agent's own unreaped child, so the PID cannot be recycled while it is read. The grandchild's PID is never used.

Red/green on compute01 (df -h /: 253G free; through build-slot; one scratch target)

Red is d1bf9550a with J170's test file and two minimal hooks overlaid: FAIL_CLEANUP_THREAD, which drops the parts as a failed Builder::spawn would, and const SHUTDOWN: Duration = SHUTDOWN_CLEANUP.

=== red d1bf9550a
test sessions::tests::the_shutdown_cleanup_ends_within_its_budget_whatever_the_shim_does ... FAILED   (the shutdown cleanup spent 3.5s against a 2s budget)
test sessions::tests::runtime_shutdown_with_a_queued_cleanup_still_stops_the_owned_launcher ... ok
test sessions::tests::a_cleanup_thread_that_cannot_start_still_stops_the_owned_launcher ... FAILED    (a failed cleanup thread left the owned launcher running)
test sessions::tests::a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... FAILED            (an unproven cleanup left the launcher it owns running (J222 79369))
test sessions::tests::the_shims_own_observed_exit_retires_the_person_record ... ok
test sessions::tests::a_launcher_that_has_reaped_its_child_is_not_proof_no_shim_exists ... FAILED    (the live shim was never asked to close: Elapsed(()))
test result: FAILED. 2 passed; 4 failed
=== green a11438729
test sessions::tests::the_shutdown_cleanup_ends_within_its_budget_whatever_the_shim_does ... ok
test sessions::tests::a_cleanup_thread_that_cannot_start_still_stops_the_owned_launcher ... ok
test sessions::tests::runtime_shutdown_with_a_queued_cleanup_still_stops_the_owned_launcher ... ok
test sessions::tests::a_launcher_that_has_reaped_its_child_is_not_proof_no_shim_exists ... ok
test sessions::tests::a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... ok
test sessions::tests::the_shims_own_observed_exit_retires_the_person_record ... ok
test result: ok. 6 passed; 0 failed; finished in 3.12s

(The first green run reused the red artifact, because cargo saw older mtimes in the second worktree. I touched the sources and rebuilt; the result above is from the rebuilt binary.)

The post-reap test does reach the post-reap state deterministically: the launcher reaps a real & subshell, then sits in exec sleep 300, so the list is empty and the counters are non-zero. It is a stand-in, not the real detach() double fork, but the kernel state it builds is the one that matters.

Findings

P2-A, crates/infraternal-agent/src/sessions.rs:1317,1330-1343 (proof) with crates/infraternal-shim/src/pty.rs:876-881 (launcher): the "has reaped none" proof holds only while the launcher's SIGCHLD disposition is the default. Nothing on the spawn path enforces that.

Kernel semantics of the four counters:

  • They are thread-group-wide (signal_struct), zero at fork, and kept across exec. They only grow. A non-zero value left from before exec only gives a false "forked", which is the safe direction.
  • They accrue only in wait_task_zombie, when the parent reaps a child through wait*(). A child that is auto-reaped (the parent's SIGCHLD is SIG_IGN, or it has SA_NOCLDWAIT) is released in exit_notify and adds nothing to the parent. waitpid then returns ECHILD.
  • Racing the freeze is sound. The accounting finishes inside the wait4 syscall, and a group stop is taken only on return to user space, so once /proc/<pid>/stat shows T, every reap is already counted. (This assumes a single-threaded launcher, which detach() requires.)
  • A child that ran and then forked always takes at least one copy-on-write fault: musl and glibc fork both write the child's TID/TLS. This is practical, not architectural, but I accept it. cutime/cstime are tick-rounded and contribute nothing on their own.

So all four counters stay at zero after a real fork in exactly one case: auto-reap. Measured on compute01 (Python, fork → grandchild → intermediate exits → parent waits):

dfl reaped children: [] {'cminflt': '94', 'cmajflt': '0', 'cutime': '0', 'cstime': '0'}
ign ECHILD (auto-reaped) children: [] {'cminflt': '0', 'cmajflt': '0', 'cutime': '0', 'cstime': '0'}

Against the real code: I copied J170's post-reap test with a launcher that ignores SIGCHLD before it forks (exec /usr/bin/python3 -c "...signal(SIGCHLD, SIG_IGN); fork; waitpid; touch ready; sleep"). It fails at the green head:

test sessions::tests::a_launcher_that_has_reaped_its_child_is_not_proof_no_shim_exists ... ok
test sessions::tests::j230_an_auto_reaping_launcher_is_not_proof_no_shim_exists ... FAILED
the live shim was never asked to close: Elapsed(())

In that run, never_forked returned true: frozen, no children, counters at zero. So cleanup counts the terminal as proven dead and cleanup removes the person record (sessions.rs:1256). That is the 79368 outcome. The probe is in ~claude/.cache/codex-scratch/j230/probe.diff on compute01; I did not commit it.

Reachability today: the launcher inherits its SIGCHLD disposition across exec. Neither isolated()/prepare_isolated_child (sessions.rs:638) nor detach() sets it. It is the default today only because the agent normally holds Tokio's SIGCHLD handler, which exec resets to the default, or was started with the default. I found no shipped path that ignores it, so I am not calling this P1. But it is a fail-open proof that depends on an invariant in another crate that nobody wrote down, and the brief wanted a proof. In the same state, detach() itself breaks: waitpid returns ECHILD, the launcher exits with an error, and the shim lives on.

Recommended fix, the design J170 deferred: a lifeline fd, preferred over a reported PID. The agent creates a pipe (O_CLOEXEC on its own read end) and passes the write end to the launcher as a known fd. Every process in the lineage inherits it through fork: the original, the intermediate, the shim. The shim keeps it open for its whole life and marks it CLOEXEC before it spawns the PTY child, so the user's shell does not pin it. The original and the intermediate drop it when they exit. Proof of death becomes EOF on the read end: no process descended from this launch holds the fd, so no shim exists, whether it never forked, died early or exited. With that:

  • the SIGSTOP freeze, /proc/*/children, the /proc/<pid>/stat counters, the SIGCHLD invariant and the SIGCONT race (P3-C) all go away;
  • PID reuse cannot matter, because no PID is used;
  • no EOF within the budget stays "unproven" and keeps the socket and record, as today;
  • it does not depend on the shim answering CloseSession, so a wedged shim that later dies is also handled.

A reported PID (the shim writes its PID before the intermediate exits) is weaker: a PID can be recycled once the shim dies, so it still needs a pidfd or a start-time check. If the lifeline is scheduled later, a two-line stopgap: detach() resets SIGCHLD to SIG_DFL before its first fork (reset_host_child_notifications in pty.rs already does exactly this), and a doc comment on has_reaped_children names the invariant.

P3-B, sessions.rs:1250-1255: the shutdown cleanup can run up to HANDSHAKE.min(total) past budget.total. After the outer timeout, cleanup kills and then reaps with a fresh bound of up to 2 s, so the worst case at shutdown is 4 s, not 2. This bites only if the killed launcher cannot be reaped at once (D state); a SIGKILLed, stopped or running launcher is reaped in milliseconds. the_shutdown_cleanup_ends_within_its_budget… passes an already-reaped true launcher (tests/unit/sessions.rs:1154), so it does not exercise this. Either fold the final reap into the deadline, or say in the Budget::total doc that the final kill-reap comes on top.

P3-C, sessions.rs:1308-1319: between seeing T and the SIGKILL, any process with the launcher's UID can SIGCONT it. It can then fork the intermediate after the counters were read. The window is microseconds, but every person on a rootless host shares that UID, and the payoff is a live terminal with no owner record (revocation then refuses). The lifeline fd closes this too.

Nothing new hangs shutdown or deletes a live terminal's state?

  • Hangs: Abandoned::drop still blocks on join, bounded by 2 s plus P3-B. stop_now is bounded at about 100 ms. Every new wait in stop_launch sits under the outer timeout, plus P3-B. I found no unbounded wait.
  • Deletion: the record is removed only if proven (sessions.rs:1256), and a timeout gives false. A cancel inside never_forked after SIGSTOP leaves a stopped launcher, which the final SIGKILL reaps. Killing the original launcher cannot reach the shim: there is a separate session after setsid, kill targets a single PID, and the original is not a session leader with a controlling terminal. The one way to delete live state is P2-A.

Housekeeping: my VM worktrees ~/worktrees/j230-{red,green} and the scratch target are removed after this comment. No leftover test processes (ps -u claude checked). Nothing pushed, merged or closed.

Verdict: REQUEST CHANGES. One P2: the zero-counter proof fails open under auto-reap. The recommended fix is the lifeline fd. Two P3s. J222's 79369, 79291 and 79292 are closed. 79368 is closed only while SIGCHLD keeps its default disposition.

**J230 re-review of `launch-followups-j170` at `a11438729527ed1018da6a57e573c5482319a6f8` (delta since `d1bf9550a`): REQUEST CHANGES (one P2, two P3).** Scope: `21775b16e` and `a11438729`. In both commits only `crates/infraternal-agent/src/sessions.rs` and `tests/unit/sessions.rs` change. `sessions.rs` and its tests are byte-identical at `d1bf9550a` and the merge `cdddfc8ce`. Per the reviewer rule, I did not rerun `verify fast`/`quick`. I read J170's gate output (79639: fast green on run 3 of 3, with two load-sensitive failures in unrelated tests on runs 1 and 2; I did not re-verify those). ## The four J222 findings | J222 | Status | Evidence | |---|---|---| | 79368 P1, empty child list after the freeze | **Closed for the shipped spawn path, but the proof rests on an invariant nothing enforces.** See P2-A below. | The red/green pair works. My auto-reap probe defeats the proof. | | 79369 P2, unproven cleanup must still stop the launcher | Closed | `stop_launch` kills and reaps in the "may have forked" path (sessions.rs:1281-1284), and `cleanup` kills and reaps again after any outcome or timeout (sessions.rs:1253-1255). The socket and the record are kept unless `proven`. | | 79291 P2, `Builder::spawn` failure | Closed | Parts sit in an `Arc<Mutex<Option<_>>>`. On `Err` the closure drops only its `Arc` clone, and `stop_now` kills and reaps synchronously (sessions.rs:1107-1113, 1136). Poisoning is impossible: the lock is held only for `take()`. | | 79292 P2, one outer deadline, budget used on the non-detached path | Closed, with one leak past the deadline, see P3-B | `timeout(budget.total, stop_launch(..))` (sessions.rs:1250). The non-detached path caps `stop_scope` and `reaped` at `HANDSHAKE.min(retry)`. | PID reuse: sound. `never_forked` signals and reads only `child.id()`, the agent's own unreaped child, so the PID cannot be recycled while it is read. The grandchild's PID is never used. ## Red/green on compute01 (`df -h /`: 253G free; through build-slot; one scratch target) Red is `d1bf9550a` with J170's test file and two minimal hooks overlaid: `FAIL_CLEANUP_THREAD`, which drops the parts as a failed `Builder::spawn` would, and `const SHUTDOWN: Duration = SHUTDOWN_CLEANUP`. ``` === red d1bf9550a test sessions::tests::the_shutdown_cleanup_ends_within_its_budget_whatever_the_shim_does ... FAILED (the shutdown cleanup spent 3.5s against a 2s budget) test sessions::tests::runtime_shutdown_with_a_queued_cleanup_still_stops_the_owned_launcher ... ok test sessions::tests::a_cleanup_thread_that_cannot_start_still_stops_the_owned_launcher ... FAILED (a failed cleanup thread left the owned launcher running) test sessions::tests::a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... FAILED (an unproven cleanup left the launcher it owns running (J222 79369)) test sessions::tests::the_shims_own_observed_exit_retires_the_person_record ... ok test sessions::tests::a_launcher_that_has_reaped_its_child_is_not_proof_no_shim_exists ... FAILED (the live shim was never asked to close: Elapsed(())) test result: FAILED. 2 passed; 4 failed === green a11438729 test sessions::tests::the_shutdown_cleanup_ends_within_its_budget_whatever_the_shim_does ... ok test sessions::tests::a_cleanup_thread_that_cannot_start_still_stops_the_owned_launcher ... ok test sessions::tests::runtime_shutdown_with_a_queued_cleanup_still_stops_the_owned_launcher ... ok test sessions::tests::a_launcher_that_has_reaped_its_child_is_not_proof_no_shim_exists ... ok test sessions::tests::a_forked_launcher_is_not_proof_the_live_terminal_is_dead ... ok test sessions::tests::the_shims_own_observed_exit_retires_the_person_record ... ok test result: ok. 6 passed; 0 failed; finished in 3.12s ``` (The first green run reused the red artifact, because cargo saw older mtimes in the second worktree. I touched the sources and rebuilt; the result above is from the rebuilt binary.) The post-reap test does reach the post-reap state deterministically: the launcher reaps a real `&` subshell, then sits in `exec sleep 300`, so the list is empty and the counters are non-zero. It is a stand-in, not the real `detach()` double fork, but the kernel state it builds is the one that matters. ## Findings **P2-A, `crates/infraternal-agent/src/sessions.rs:1317,1330-1343` (proof) with `crates/infraternal-shim/src/pty.rs:876-881` (launcher): the "has reaped none" proof holds only while the launcher's SIGCHLD disposition is the default. Nothing on the spawn path enforces that.** Kernel semantics of the four counters: - They are thread-group-wide (`signal_struct`), zero at fork, and kept across exec. They only grow. A non-zero value left from before exec only gives a false "forked", which is the safe direction. - They accrue only in `wait_task_zombie`, when the parent reaps a child through `wait*()`. A child that is **auto-reaped** (the parent's SIGCHLD is `SIG_IGN`, or it has `SA_NOCLDWAIT`) is released in `exit_notify` and adds nothing to the parent. `waitpid` then returns ECHILD. - Racing the freeze is sound. The accounting finishes inside the `wait4` syscall, and a group stop is taken only on return to user space, so once `/proc/<pid>/stat` shows `T`, every reap is already counted. (This assumes a single-threaded launcher, which `detach()` requires.) - A child that ran and then forked always takes at least one copy-on-write fault: musl and glibc `fork` both write the child's TID/TLS. This is practical, not architectural, but I accept it. `cutime`/`cstime` are tick-rounded and contribute nothing on their own. So all four counters stay at zero after a real fork in exactly one case: auto-reap. Measured on compute01 (Python, fork → grandchild → intermediate exits → parent waits): ``` dfl reaped children: [] {'cminflt': '94', 'cmajflt': '0', 'cutime': '0', 'cstime': '0'} ign ECHILD (auto-reaped) children: [] {'cminflt': '0', 'cmajflt': '0', 'cutime': '0', 'cstime': '0'} ``` Against the real code: I copied J170's post-reap test with a launcher that ignores SIGCHLD before it forks (`exec /usr/bin/python3 -c "...signal(SIGCHLD, SIG_IGN); fork; waitpid; touch ready; sleep"`). It fails at the green head: ``` test sessions::tests::a_launcher_that_has_reaped_its_child_is_not_proof_no_shim_exists ... ok test sessions::tests::j230_an_auto_reaping_launcher_is_not_proof_no_shim_exists ... FAILED the live shim was never asked to close: Elapsed(()) ``` In that run, `never_forked` returned `true`: frozen, no children, counters at zero. So cleanup counts the terminal as proven dead and `cleanup` removes the person record (sessions.rs:1256). That is the 79368 outcome. The probe is in `~claude/.cache/codex-scratch/j230/probe.diff` on compute01; I did not commit it. Reachability today: the launcher inherits its SIGCHLD disposition across exec. Neither `isolated()`/`prepare_isolated_child` (sessions.rs:638) nor `detach()` sets it. It is the default today only because the agent normally holds Tokio's SIGCHLD handler, which exec resets to the default, or was started with the default. I found no shipped path that ignores it, so I am not calling this P1. But it is a fail-open proof that depends on an invariant in another crate that nobody wrote down, and the brief wanted a proof. In the same state, `detach()` itself breaks: `waitpid` returns ECHILD, the launcher exits with an error, and the shim lives on. **Recommended fix, the design J170 deferred: a lifeline fd, preferred over a reported PID.** The agent creates a pipe (`O_CLOEXEC` on its own read end) and passes the write end to the launcher as a known fd. Every process in the lineage inherits it through `fork`: the original, the intermediate, the shim. The shim keeps it open for its whole life and marks it `CLOEXEC` before it spawns the PTY child, so the user's shell does not pin it. The original and the intermediate drop it when they exit. Proof of death becomes **EOF on the read end**: no process descended from this launch holds the fd, so no shim exists, whether it never forked, died early or exited. With that: - the SIGSTOP freeze, `/proc/*/children`, the `/proc/<pid>/stat` counters, the SIGCHLD invariant and the SIGCONT race (P3-C) all go away; - PID reuse cannot matter, because no PID is used; - no EOF within the budget stays "unproven" and keeps the socket and record, as today; - it does not depend on the shim answering `CloseSession`, so a wedged shim that later dies is also handled. A reported PID (the shim writes its PID before the intermediate exits) is weaker: a PID can be recycled once the shim dies, so it still needs a pidfd or a start-time check. If the lifeline is scheduled later, a two-line stopgap: `detach()` resets SIGCHLD to `SIG_DFL` before its first fork (`reset_host_child_notifications` in pty.rs already does exactly this), and a doc comment on `has_reaped_children` names the invariant. **P3-B, `sessions.rs:1250-1255`: the shutdown cleanup can run up to `HANDSHAKE.min(total)` past `budget.total`.** After the outer `timeout`, `cleanup` kills and then reaps with a fresh bound of up to 2 s, so the worst case at shutdown is 4 s, not 2. This bites only if the killed launcher cannot be reaped at once (D state); a SIGKILLed, stopped or running launcher is reaped in milliseconds. `the_shutdown_cleanup_ends_within_its_budget…` passes an already-reaped `true` launcher (tests/unit/sessions.rs:1154), so it does not exercise this. Either fold the final reap into the deadline, or say in the `Budget::total` doc that the final kill-reap comes on top. **P3-C, `sessions.rs:1308-1319`: between seeing `T` and the `SIGKILL`, any process with the launcher's UID can `SIGCONT` it.** It can then fork the intermediate after the counters were read. The window is microseconds, but every person on a rootless host shares that UID, and the payoff is a live terminal with no owner record (revocation then refuses). The lifeline fd closes this too. ## Nothing new hangs shutdown or deletes a live terminal's state? - Hangs: `Abandoned::drop` still blocks on `join`, bounded by 2 s plus P3-B. `stop_now` is bounded at about 100 ms. Every new wait in `stop_launch` sits under the outer timeout, plus P3-B. I found no unbounded wait. - Deletion: the record is removed only if `proven` (sessions.rs:1256), and a timeout gives `false`. A cancel inside `never_forked` after `SIGSTOP` leaves a stopped launcher, which the final `SIGKILL` reaps. Killing the original launcher cannot reach the shim: there is a separate session after `setsid`, `kill` targets a single PID, and the original is not a session leader with a controlling terminal. The one way to delete live state is P2-A. Housekeeping: my VM worktrees `~/worktrees/j230-{red,green}` and the scratch target are removed after this comment. No leftover test processes (`ps -u claude` checked). Nothing pushed, merged or closed. **Verdict: REQUEST CHANGES.** One P2: the zero-counter proof fails open under auto-reap. The recommended fix is the lifeline fd. Two P3s. J222's 79369, 79291 and 79292 are closed. 79368 is closed only while SIGCHLD keeps its default disposition.
kayg commented 2026-11-05 12:27:52 +00:00
Author
Owner
Copy link

J170 round 5 progress (J230's changes). Head 6fb1c65dcef6036a55f1e5c318b972a87b75eda9, pushed, origin/dev merged (69 commits, clean). The branch now has these commits on top of the previous head:

  • 024b0f2fc shim: a detached shim holds a launch lifeline (--lifeline-fd N): the write end of a pipe every process of the double fork inherits; the shim keeps it for its whole life, marks it close-on-exec before starting anything, and fails closed if N is not an open pipe. contracts/shim.md §2 states it.
  • 54422b7ab agent: cleanup is proven by end-of-file on the lifeline's read end (or the shim's own authenticated close and exit). The freeze, /proc counters, SIGCHLD dependency, pid handling and the SIGCONT window (P3-2) are gone. A closed lifeline also clears the dead shim's leftover socket and person record.
  • 030d0fe3e agent: the final kill-and-reap of the owned launcher is inside the single deadline (P3-1).

Fail first (compute01):

  • shim, on the merge before it (the shim rejected the option): lifeline_reaches_eof_when_the_shim_is_killed_and_not_before, lifeline_reaches_eof_when_the_shim_exits_normally, the_shell_and_its_children_never_hold_the_lifeline, a_missing_lifeline_descriptor_stops_the_shim_before_it_runs all FAILED.
  • agent, on the shim-only commit: an_auto_reaping_launcher_is_not_proof_no_shim_exists FAILED ("the live shim was never asked to close", J230's SIGCHLD-ignoring launcher, now a regression test that passes) and a_closed_lifeline_clears_the_dead_shims_socket_and_record FAILED ("a dead shim's socket outlived its closed lifeline").
  • P3-1, on the previous commit plus only the reap-stall hook: a_stalled_reap_of_the_launcher_stays_inside_the_single_deadline FAILED ("the cleanup spent 4s against a 2s budget", on the paused clock).
    Green: all 33 sessions::tests:: pass, the four shim tests pass, the CLI parity test passes. verify quick PASS (scope guard, migration names, fmt, doc coverage, clippy workspace, unit tests). verify fast is running on compute01; I will post it verbatim.

macOS: the shim flag, inherit_lifeline and hold_lifeline compile for both targets, but the agent's lifeline and cleanup stay Linux-only (dev's macOS launch tail is untouched). A cargo check on the Mac VM is queued behind the Mac lock, which another job (appwin-icons-j207, an interactive shell, 4.5 h) holds; I will not touch it. If it has not run by the end, I say so in the report.

**J170 round 5 progress (J230's changes).** Head `6fb1c65dcef6036a55f1e5c318b972a87b75eda9`, pushed, origin/dev merged (69 commits, clean). The branch now has these commits on top of the previous head: - `024b0f2fc` **shim**: a detached shim holds a launch **lifeline** (`--lifeline-fd N`): the write end of a pipe every process of the double fork inherits; the shim keeps it for its whole life, marks it close-on-exec before starting anything, and fails closed if N is not an open pipe. `contracts/shim.md` §2 states it. - `54422b7ab` **agent**: cleanup is proven by **end-of-file on the lifeline's read end** (or the shim's own authenticated close and exit). The freeze, `/proc` counters, SIGCHLD dependency, pid handling and the SIGCONT window (P3-2) are gone. A closed lifeline also clears the dead shim's leftover socket and person record. - `030d0fe3e` **agent**: the final kill-and-reap of the owned launcher is inside the single deadline (P3-1). **Fail first (compute01):** - shim, on the merge before it (the shim rejected the option): `lifeline_reaches_eof_when_the_shim_is_killed_and_not_before`, `lifeline_reaches_eof_when_the_shim_exits_normally`, `the_shell_and_its_children_never_hold_the_lifeline`, `a_missing_lifeline_descriptor_stops_the_shim_before_it_runs` all FAILED. - agent, on the shim-only commit: `an_auto_reaping_launcher_is_not_proof_no_shim_exists` FAILED ("the live shim was never asked to close", J230's SIGCHLD-ignoring launcher, now a regression test that passes) and `a_closed_lifeline_clears_the_dead_shims_socket_and_record` FAILED ("a dead shim's socket outlived its closed lifeline"). - P3-1, on the previous commit plus only the reap-stall hook: `a_stalled_reap_of_the_launcher_stays_inside_the_single_deadline` FAILED ("the cleanup spent 4s against a 2s budget", on the paused clock). Green: all 33 `sessions::tests::` pass, the four shim tests pass, the CLI parity test passes. `verify quick` PASS (scope guard, migration names, fmt, doc coverage, clippy workspace, unit tests). `verify fast` is running on compute01; I will post it verbatim. **macOS:** the shim flag, `inherit_lifeline` and `hold_lifeline` compile for both targets, but the agent's lifeline and cleanup stay Linux-only (dev's macOS launch tail is untouched). A `cargo check` on the Mac VM is queued behind the Mac lock, which another job (`appwin-icons-j207`, an interactive shell, 4.5 h) holds; I will not touch it. If it has not run by the end, I say so in the report.
kayg commented 2026-11-05 13:23:44 +00:00
Author
Owner
Copy link

J170 round 5 finish (J230's changes): GREEN on Linux. Branch launch-followups-j170, head 6fb1c65dcef6036a55f1e5c318b972a87b75eda9, pushed, not merged, not deployed. origin/dev was merged first (48632052d, then again at the head: 69 more commits, clean).

What changed (the decision on #324: replace the kernel-accounting proof with a lifeline fd)

  • 024b0f2fc shim: a detached shim gets --lifeline-fd N, the write end of a pipe. Every process of the double fork inherits it; the shim keeps it for its whole life, marks it close-on-exec before starting anything (so the shell never holds it), and fails closed if N is not an open pipe. contracts/shim.md §2 documents it. It is also in the hidden __shim action schema, Options::spawned (last two arguments only) and the CLI parity test.
  • 54422b7ab agent: after a failed detached launch the original launcher is killed and reaped (it cannot touch the shim), then cleanup waits for EOF on the lifeline's read end (proves no process of the launch is left: the shim never started, exited or was killed) or the shim's own authenticated close and exit. No proof keeps the socket and the person record and still stops the owned launcher. A closed lifeline also clears the dead shim's leftover socket files (a killed shim cannot unlink them, and a socket with no record makes person revocation refuse). Removed: the SIGSTOP freeze, the /proc child list and wait counters, the SIGCHLD dependency, pid handling, and P3-2's resume window.
  • 030d0fe3e agent (P3-1): the final kill-and-reap sits inside the single deadline; past it only the SIGKILL is sent (no wait).

Fail first (compute01)

shim, on the merge before (the shim rejected the option):
  lifeline_reaches_eof_when_the_shim_is_killed_and_not_before ... FAILED
  lifeline_reaches_eof_when_the_shim_exits_normally ... FAILED
  the_shell_and_its_children_never_hold_the_lifeline ... FAILED
  a_missing_lifeline_descriptor_stops_the_shim_before_it_runs ... FAILED (asserts "Bad file descriptor", not just any failure)
agent, on the shim-only commit:
  an_auto_reaping_launcher_is_not_proof_no_shim_exists ... FAILED: the live shim was never asked to close: Elapsed(())   (J230's SIGCHLD-ignoring launcher; now a passing regression test)
  a_closed_lifeline_clears_the_dead_shims_socket_and_record ... FAILED: a dead shim's socket outlived its closed lifeline
P3-1, on the previous commit plus only the reap-stall hook:
  a_stalled_reap_of_the_launcher_stays_inside_the_single_deadline ... FAILED: the cleanup spent 4s against a 2s budget   (paused clock)

Green at the head: all 33 sessions::tests:: pass; the four real-binary lifeline tests pass (they run the actual shim: EOF when killed and when it exits by itself, no EOF while it lives, the shell and a background child do not hold the fd, a missing fd refuses to run); the CLI parity test passes.

scripts/verify quick, verbatim:

| scope guard               | PASS         |
| migration names           | PASS         |
| fmt                       | PASS         |
| doc coverage ratchet      | PASS         |
| clippy (workspace)        | PASS         |
| unit tests (changed)      | PASS         |
| unit tests (changed, bin-only) | PASS         |

scripts/verify fast at 6fb1c65dc, compute01, foreground, VERIFY_EXIT=0, verbatim:

| fmt                       | PASS         |
| migration names           | PASS         |
| migration tooling tests   | PASS         |
| VT native fmt             | PASS         |
| CLI plugin compile-out    | PASS         |
| clippy                    | PASS         |
| SSH runtime test helper   | PASS         |
| real-binary fixture paths | PASS         |
| Traefik component manifest | PASS         |
| release component publication | PASS         |
| test                      | PASS         |
| SSH inode reuse fixture   | PASS         |
| shim process-per-user     | PASS         |
| cargo-deny                | PASS         |
| gate architecture         | PASS         |
| doc coverage ratchet      | PASS         |
| static builds             | PASS         |

Sizes (report only): x86_64 23935440 bytes (+1517184 vs the stored baseline), aarch64 21739776 (+1286496); these include all of dev's growth since the baseline, not just this branch.

Gate history at this head: two full runs, one green. Run 1: helper::startup_recovery_tests::delta_before_startup_keyframe_requests_repair_before_delivery in crates/plugins/desktop failed (startup delta must trigger a repair keyframe: Disconnected), a desktop-helper test in code this branch does not touch. Run 2 (same head): all PASS. My head-vs-dev comparison of that test ran 0 tests (wrong target selection) and proves nothing, so I discard it: the failure is unreproduced and unattributed, not proven unrelated.

Not verified / limits

  • macOS not compile-checked. The shim side (--lifeline-fd, inherit_lifeline, hold_lifeline) is shared code built for Linux and macOS, and the agent's lifeline and cleanup stay Linux-only (dev's macOS launch tail is unchanged). I queued a cargo check -p infraternal-shim -p infraternal-agent --all-targets on the Mac VM under the Mac lock, but other jobs held it for the whole 2 h (an interactive shell from appwin-icons-j207, then j243 builds), so I cancelled my own queued job. It needs one run before merge: lockf -k /tmp/mac-bench.lock nix develop -c cargo check --locked -p infraternal-shim -p infraternal-agent --all-targets.
  • A launch through systemd-run (User or System manager) is not detached and gets no lifeline: there the launcher is the shim, and killing and reaping it is the proof, as before. No root-only run and no test-server run.
  • The systemctl stop scope route still has no test of its own.
  • Unproven cleanup (a live shim that never answers within the window) still keeps its socket and person record, so person revocation refuses until the terminal is closed; that is the chosen safe option.
  • The target dir on compute01 is deleted; my processes were stopped by PID.
**J170 round 5 finish (J230's changes): GREEN on Linux. Branch `launch-followups-j170`, head `6fb1c65dcef6036a55f1e5c318b972a87b75eda9`**, pushed, not merged, not deployed. origin/dev was merged first (`48632052d`, then again at the head: 69 more commits, clean). **What changed** (the decision on #324: replace the kernel-accounting proof with a lifeline fd) - `024b0f2fc` shim: a detached shim gets `--lifeline-fd N`, the write end of a pipe. Every process of the double fork inherits it; the shim keeps it for its whole life, marks it close-on-exec before starting anything (so the shell never holds it), and **fails closed** if N is not an open pipe. `contracts/shim.md` §2 documents it. It is also in the hidden `__shim` action schema, `Options::spawned` (last two arguments only) and the CLI parity test. - `54422b7ab` agent: after a failed detached launch the original launcher is killed and reaped (it cannot touch the shim), then cleanup waits for **EOF on the lifeline's read end** (proves no process of the launch is left: the shim never started, exited or was killed) or the shim's own authenticated close and exit. No proof keeps the socket and the person record and still stops the owned launcher. A closed lifeline also clears the dead shim's leftover socket files (a killed shim cannot unlink them, and a socket with no record makes person revocation refuse). Removed: the SIGSTOP freeze, the `/proc` child list and wait counters, the SIGCHLD dependency, pid handling, and P3-2's resume window. - `030d0fe3e` agent (P3-1): the final kill-and-reap sits inside the single deadline; past it only the SIGKILL is sent (no wait). **Fail first (compute01)** ``` shim, on the merge before (the shim rejected the option): lifeline_reaches_eof_when_the_shim_is_killed_and_not_before ... FAILED lifeline_reaches_eof_when_the_shim_exits_normally ... FAILED the_shell_and_its_children_never_hold_the_lifeline ... FAILED a_missing_lifeline_descriptor_stops_the_shim_before_it_runs ... FAILED (asserts "Bad file descriptor", not just any failure) agent, on the shim-only commit: an_auto_reaping_launcher_is_not_proof_no_shim_exists ... FAILED: the live shim was never asked to close: Elapsed(()) (J230's SIGCHLD-ignoring launcher; now a passing regression test) a_closed_lifeline_clears_the_dead_shims_socket_and_record ... FAILED: a dead shim's socket outlived its closed lifeline P3-1, on the previous commit plus only the reap-stall hook: a_stalled_reap_of_the_launcher_stays_inside_the_single_deadline ... FAILED: the cleanup spent 4s against a 2s budget (paused clock) ``` Green at the head: all 33 `sessions::tests::` pass; the four real-binary lifeline tests pass (they run the actual shim: EOF when killed and when it exits by itself, no EOF while it lives, the shell and a background child do not hold the fd, a missing fd refuses to run); the CLI parity test passes. **`scripts/verify quick`, verbatim:** ``` | scope guard | PASS | | migration names | PASS | | fmt | PASS | | doc coverage ratchet | PASS | | clippy (workspace) | PASS | | unit tests (changed) | PASS | | unit tests (changed, bin-only) | PASS | ``` **`scripts/verify fast` at `6fb1c65dc`, compute01, foreground, VERIFY_EXIT=0, verbatim:** ``` | fmt | PASS | | migration names | PASS | | migration tooling tests | PASS | | VT native fmt | PASS | | CLI plugin compile-out | PASS | | clippy | PASS | | SSH runtime test helper | PASS | | real-binary fixture paths | PASS | | Traefik component manifest | PASS | | release component publication | PASS | | test | PASS | | SSH inode reuse fixture | PASS | | shim process-per-user | PASS | | cargo-deny | PASS | | gate architecture | PASS | | doc coverage ratchet | PASS | | static builds | PASS | ``` Sizes (report only): x86_64 23935440 bytes (+1517184 vs the stored baseline), aarch64 21739776 (+1286496); these include all of dev's growth since the baseline, not just this branch. **Gate history at this head: two full runs, one green.** Run 1: `helper::startup_recovery_tests::delta_before_startup_keyframe_requests_repair_before_delivery` in `crates/plugins/desktop` failed (`startup delta must trigger a repair keyframe: Disconnected`), a desktop-helper test in code this branch does not touch. Run 2 (same head): all PASS. My head-vs-dev comparison of that test ran 0 tests (wrong target selection) and proves nothing, so I discard it: the failure is unreproduced and unattributed, not proven unrelated. **Not verified / limits** - **macOS not compile-checked.** The shim side (`--lifeline-fd`, `inherit_lifeline`, `hold_lifeline`) is shared code built for Linux and macOS, and the agent's lifeline and cleanup stay Linux-only (dev's macOS launch tail is unchanged). I queued a `cargo check -p infraternal-shim -p infraternal-agent --all-targets` on the Mac VM under the Mac lock, but other jobs held it for the whole 2 h (an interactive shell from `appwin-icons-j207`, then `j243` builds), so I cancelled my own queued job. It needs one run before merge: `lockf -k /tmp/mac-bench.lock nix develop -c cargo check --locked -p infraternal-shim -p infraternal-agent --all-targets`. - A launch through `systemd-run` (User or System manager) is not detached and gets no lifeline: there the launcher is the shim, and killing and reaping it is the proof, as before. No root-only run and no test-server run. - The `systemctl stop` scope route still has no test of its own. - Unproven cleanup (a live shim that never answers within the window) still keeps its socket and person record, so person revocation refuses until the terminal is closed; that is the chosen safe option. - The target dir on compute01 is deleted; my processes were stopped by PID.
kayg commented 2026-11-05 15:32:35 +00:00
Author
Owner
Copy link

J246 re-review of launch-followups-j170 at 6fb1c65dcef6036a55f1e5c318b972a87b75eda9 (delta since a11438729: 024b0f2fc, 54422b7ab, 030d0fe3e; dev merges excluded): REQUEST CHANGES (one P1, four P3).

The lifeline design holds. I found no way for the shell to hold the descriptor, no way to get end-of-file while a shim lives, and no wait past the deadline. J230's P2-A, P3-B and P3-C are closed. The P1 is the required macOS compile check: infraternal-agent does not compile on macOS at this head. The cause predates this delta (b3201d81c), but it blocks the merge. Per the reviewer rule I did not run verify fast/quick; I read J170's gate output (79927).

P1

P1-1, crates/infraternal-agent/src/sessions.rs:80-81 (definition) with :1816 (caller): launch_bound() is #[cfg(target_os = "linux")], but await_launch calls it on every target. The macOS lib and lib test do not compile.
Mac VM (lume@infraternal-macos-01-std), under lockf -k /tmp/mac-bench.lock, in a snapshot of the exact tree (git archive 6fb1c65dc), nix develop -c cargo check --locked -p infraternal-shim -p infraternal-agent --all-targets:

error[E0425]: cannot find function `launch_bound` in this scope
    --> crates/infraternal-agent/src/sessions.rs:1816:9
1816 |         launch_bound(),
error: could not compile `infraternal-agent` (lib) due to 1 previous error
error: could not compile `infraternal-agent` (lib test) due to 1 previous error
  • First attempt (check.log): failed at the Zig step on a network fetch (zigimg … invalid HTTP response: HttpConnectionClosing).
  • Retry (check-retry.log): got past Zig, then hit the error above. The same error shows up with a no-op shim build.rs (check-stub.log). So this is a Rust error, not #366.
  • infraternal-shim (the --lifeline-fd flag, inherit_lifeline, hold_lifeline) builds on macOS. Only the agent fails.
  • git log -S"fn launch_bound" points to b3201d81c. a11438729 and d1bf9550a carry the same cfg, so earlier Linux-only reviews could not see it.

Fix, verified as a local probe that I did not commit:

  • drop the cfg on launch_bound;
  • narrow its test hook to #[cfg(all(test, target_os = "linux"))], because the tests module is Linux-only. Dropping the cfg alone still fails: E0433 … tests::LAUNCH_BOUND (check-probe.log).

With both changes the same command gives PROBE2_EXIT=0, Finished dev profile (check-probe2.log), plus one dead-code warning: Service::with_registry, plugin_control.rs:606, lib test, not in this delta. Logs are in ~/j246-evidence/ on the Mac VM.

Design questions from the brief

Can the shell or a child hold the descriptor? No.

  • Agent side. The write end reaches the launcher through inherit_descriptor: an F_DUPFD_CLOEXEC(8) copy whose close-on-exec flag is cleared only in that child's pre_exec. That hook is registered after inherit_only_standard_descriptors, so the order is right.
  • The agent's own copies. The pipe2 write end drops at the end of new_lifeline. The closure's copy drops with command (sessions.rs:1297). Other children the agent spawns at the same time see only CLOEXEC copies.
  • Before close-on-exec is set. Between exec and hold_lifeline (startup.rs:78), the shim only parses arguments and reads stdin. Nothing forks or execs before it.
  • Shell spawn. The shell spawn runs only_standard_descriptors itself (pty.rs:1804 Linux, 1855 macOS). The macOS __session-shell entry also closes everything above stderr. So the shell is protected twice; see P3-1.
  • Number collisions. N is 8 or higher. detach() duplicates only onto 0/1/2 and allocates its /dev/null source with the lowest-free F_DUPFD_CLOEXEC(3), so it cannot land on an open N. The dup2(…, 4) and dup2(…, 3) at pty.rs:727/1635/2296 run in children before exec, or in tests.

Can EOF arrive while a shim lives? No.

  • The shim never closes the descriptor. Its only fd sweeps run in children, and the macOS close_above_stderr runs in the separate exec'd entry.
  • The double fork has no gap. The intermediate is forked before the original exits, and the grandchild before the intermediate's exit(0). A failed fork exits with no shim, so EOF there is the truth.
  • Nothing in the shim re-execs itself: pty_handoff is not compiled into any module, and process-per-user is research-only. So no live terminal can drop its lifeline through exec.
  • Mutation M-close (the shim close()s the descriptor instead of setting CLOEXEC): lifeline_reaches_eof_when_the_shim_is_killed_and_not_before fails with "the lifeline reached EOF while the shim was alive", and the shell test's final check fails too.

Can a process that is not the shim fake liveness? Yes, which is safe.

  • A stray holder keeps the state and stops only the owned launcher. Examples: a concurrent fork in the agent before exec, or the no-runtime path of Launch::drop, which runs cleanup synchronously before command drops.

Can anything fake death? No.

  • Death needs every holder gone, and the only one left is the live shim. Same-uid ptrace is outside the threat model.
  • On the agent side, every error from try_clone, from_owned_fd, readable or try_read resolves to pending(), never to EOF.

Removing the socket on proof.

  • The path is {uuid_v4}.sock for this launch's own fresh id. It is removed only after EOF, when no process of the launch is alive.
  • A legitimate new shim never gets the same id. A same-uid process that plants a socket under that name could plant one anyway.
  • So removing it is safe.

One deadline (P3-1 fix).

  • stop_launch, then the final start_kill and reaped, all sit inside timeout(budget.total) (sessions.rs:1580). After the deadline only a signal is sent.
  • NORMAL fits exactly: reap 2 + retry 10 + final reap 2 = 14 = total.

Do the tests fail without their fix? (compute01, df -h /: 309G free, build-slot, one target, now deleted)

Green at head:

  • sessions::tests:: 33 passed, 1 ignored;
  • --test shim lifeline 4 passed;
  • CLI parity: spawned_shapes_reuse_full_shim_options_and_defaults and session_accepts_only_the_two_spawned_shapes pass.

Each mutation is a minimal patch, reverted afterwards. Script and log: ~/j246-evidence/{j246-run.sh,run.log} on compute01.

Mutation What fails
M-fake-death: lifeline_eof returns at once 7 tests: an_auto_reaping_launcher_is_not_proof_no_shim_exists (J230's SIGCHLD-ignoring launcher), a_launcher_that_has_reaped_its_child…, a_forked_launcher_is_not_proof…, a_shim_that_binds_after…, cancelling_the_launch…, the_shims_own_observed_exit…, slow_hello_failure…
M-keep-socket: no socket removal on lifeline proof a_closed_lifeline_clears_the_dead_shims_socket_and_record (tests/unit/sessions.rs:1265)
M-old-cleanup: 030d0fe3e's cleanup reverted, stall hook kept a_stalled_reap_of_the_launcher_stays_inside_the_single_deadline (:1377)
M-close: the shim closes the descriptor lifeline_reaches_eof_when_the_shim_is_killed_and_not_before, the_shell_and_its_children_never_hold_the_lifeline
M-noop: hold_lifeline returns Ok at once a_missing_lifeline_descriptor… "fails" only by hanging for 973.89 s, until I killed its shim by PID. The other three pass. See P3-2.
M-cloexec: validation kept, F_SETFD FD_CLOEXEC removed Nothing fails (4/4 pass). See P3-1.

So J230's SIGCHLD-ignoring launcher is a real regression test of the lifeline proof, and so are the socket-clear test and the deadline test. Of the four shim tests, two catch a shim that lets go of its lifeline, and one catches a missing-descriptor regression, but by hanging. None checks the shim's own close-on-exec.

P3

P3-1, crates/infraternal/tests/shim.rs:1489 with crates/infraternal-shim/src/pty.rs:101: the_shell_and_its_children_never_hold_the_lifeline does not test hold_lifeline's close-on-exec.

  • With the F_SETFD FD_CLOEXEC call removed, all four lifeline tests pass (M-cloexec). The shell spawn's own only_standard_descriptors sweep (pty.rs:1804/1855) already keeps the descriptor out of the shell.
  • Production is safe twice over. But 024b0f2fc's message presents this test as covering the shim's close-on-exec, and that step is untested.
  • Fix: add a unit test that F_GETFD shows FD_CLOEXEC after hold_lifeline. Or say in the test's doc that it covers the composed result, not the shim step.

P3-2, crates/infraternal/tests/shim.rs:1513-1528: a_missing_lifeline_descriptor_stops_the_shim_before_it_runs has no hang guard.

  • It calls .output() on a foreground __shim. If the fail-closed check regresses, the shim runs a shell forever, and the test hangs the gate instead of failing (973.89 s in M-noop).
  • Fix: spawn it, wait with a bound, then kill and assert.

P3-3, crates/infraternal/src/main.rs:1509-1511: on the full-CLI path, a --lifeline-fd outside i32 turns into None (i32::try_from(..).ok()), and the shim runs without validating it.

  • This contradicts contracts/shim.md §2 ("fails closed … never runs without its lifeline").
  • Today it fails safe: the real descriptor is still inherited and held, and the agent never sends such a value.
  • Fix: refuse it, as Options::spawned does through number(value)?.

P3-4, crates/infraternal-agent/src/sessions.rs:1574: the cleanup doc still says "A detached shim's socket is never unlinked here".

  • prove_detached_dead now removes it on lifeline proof (:1660-1662).
  • Fix: update the sentence; contracts/shim.md already describes the new rule.

Not done

  • Bonus systemctl stop scope route: not reviewed.
  • No root or test-server run. The changed paths are rootless detached launches, and the tests above run them for real.

Housekeeping:

  • My compute01 worktree and its 7.7 GB target are removed. The Mac snapshot and its 1.4 GB target are removed; its logs are kept.
  • The one hung mutation shim was killed by PID, and no j246 processes are left.
  • Nothing pushed, merged or closed.

Verdict: REQUEST CHANGES.

  • P1-1: macOS infraternal-agent does not compile at head (launch_bound cfg). It predates the delta, and the fix is two lines.
  • P3-1 to P3-4: test and doc gaps.

The lifeline design and the delta's three commits are otherwise approved.

**J246 re-review of `launch-followups-j170` at `6fb1c65dcef6036a55f1e5c318b972a87b75eda9` (delta since `a11438729`: `024b0f2fc`, `54422b7ab`, `030d0fe3e`; dev merges excluded): REQUEST CHANGES (one P1, four P3).** The lifeline design holds. I found no way for the shell to hold the descriptor, no way to get end-of-file while a shim lives, and no wait past the deadline. J230's P2-A, P3-B and P3-C are closed. The P1 is the required macOS compile check: `infraternal-agent` does not compile on macOS at this head. The cause predates this delta (`b3201d81c`), but it blocks the merge. Per the reviewer rule I did not run `verify fast`/`quick`; I read J170's gate output (79927). ## P1 **P1-1, `crates/infraternal-agent/src/sessions.rs:80-81` (definition) with `:1816` (caller): `launch_bound()` is `#[cfg(target_os = "linux")]`, but `await_launch` calls it on every target. The macOS lib and lib test do not compile.** Mac VM (`lume@infraternal-macos-01-std`), under `lockf -k /tmp/mac-bench.lock`, in a snapshot of the exact tree (`git archive 6fb1c65dc`), `nix develop -c cargo check --locked -p infraternal-shim -p infraternal-agent --all-targets`: ``` error[E0425]: cannot find function `launch_bound` in this scope --> crates/infraternal-agent/src/sessions.rs:1816:9 1816 | launch_bound(), error: could not compile `infraternal-agent` (lib) due to 1 previous error error: could not compile `infraternal-agent` (lib test) due to 1 previous error ``` - First attempt (`check.log`): failed at the Zig step on a network fetch (`zigimg … invalid HTTP response: HttpConnectionClosing`). - Retry (`check-retry.log`): got past Zig, then hit the error above. The same error shows up with a no-op shim `build.rs` (`check-stub.log`). So this is a Rust error, not #366. - `infraternal-shim` (the `--lifeline-fd` flag, `inherit_lifeline`, `hold_lifeline`) builds on macOS. Only the agent fails. - `git log -S"fn launch_bound"` points to `b3201d81c`. `a11438729` and `d1bf9550a` carry the same cfg, so earlier Linux-only reviews could not see it. Fix, verified as a local probe that I did not commit: - drop the `cfg` on `launch_bound`; - narrow its test hook to `#[cfg(all(test, target_os = "linux"))]`, because the `tests` module is Linux-only. Dropping the `cfg` alone still fails: `E0433 … tests::LAUNCH_BOUND` (`check-probe.log`). With both changes the same command gives `PROBE2_EXIT=0`, `Finished dev profile` (`check-probe2.log`), plus one dead-code warning: `Service::with_registry`, `plugin_control.rs:606`, `lib test`, not in this delta. Logs are in `~/j246-evidence/` on the Mac VM. ## Design questions from the brief **Can the shell or a child hold the descriptor? No.** - **Agent side.** The write end reaches the launcher through `inherit_descriptor`: an `F_DUPFD_CLOEXEC(8)` copy whose close-on-exec flag is cleared only in that child's `pre_exec`. That hook is registered after `inherit_only_standard_descriptors`, so the order is right. - **The agent's own copies.** The `pipe2` write end drops at the end of `new_lifeline`. The closure's copy drops with `command` (sessions.rs:1297). Other children the agent spawns at the same time see only CLOEXEC copies. - **Before close-on-exec is set.** Between exec and `hold_lifeline` (startup.rs:78), the shim only parses arguments and reads stdin. Nothing forks or execs before it. - **Shell spawn.** The shell spawn runs `only_standard_descriptors` itself (pty.rs:1804 Linux, 1855 macOS). The macOS `__session-shell` entry also closes everything above stderr. So the shell is protected twice; see P3-1. - **Number collisions.** N is 8 or higher. `detach()` duplicates only onto 0/1/2 and allocates its `/dev/null` source with the lowest-free `F_DUPFD_CLOEXEC(3)`, so it cannot land on an open N. The `dup2(…, 4)` and `dup2(…, 3)` at pty.rs:727/1635/2296 run in children before exec, or in tests. **Can EOF arrive while a shim lives? No.** - The shim never closes the descriptor. Its only fd sweeps run in children, and the macOS `close_above_stderr` runs in the separate exec'd entry. - The double fork has no gap. The intermediate is forked before the original exits, and the grandchild before the intermediate's `exit(0)`. A failed fork exits with no shim, so EOF there is the truth. - Nothing in the shim re-execs itself: `pty_handoff` is not compiled into any module, and `process-per-user` is research-only. So no live terminal can drop its lifeline through exec. - Mutation **M-close** (the shim `close()`s the descriptor instead of setting CLOEXEC): `lifeline_reaches_eof_when_the_shim_is_killed_and_not_before` fails with "the lifeline reached EOF while the shim was alive", and the shell test's final check fails too. **Can a process that is not the shim fake liveness? Yes, which is safe.** - A stray holder keeps the state and stops only the owned launcher. Examples: a concurrent fork in the agent before exec, or the no-runtime path of `Launch::drop`, which runs cleanup synchronously before `command` drops. **Can anything fake death? No.** - Death needs every holder gone, and the only one left is the live shim. Same-uid ptrace is outside the threat model. - On the agent side, every error from `try_clone`, `from_owned_fd`, `readable` or `try_read` resolves to `pending()`, never to EOF. **Removing the socket on proof.** - The path is `{uuid_v4}.sock` for this launch's own fresh id. It is removed only after EOF, when no process of the launch is alive. - A legitimate new shim never gets the same id. A same-uid process that plants a socket under that name could plant one anyway. - So removing it is safe. **One deadline (P3-1 fix).** - `stop_launch`, then the final `start_kill` and `reaped`, all sit inside `timeout(budget.total)` (sessions.rs:1580). After the deadline only a signal is sent. - NORMAL fits exactly: reap 2 + retry 10 + final reap 2 = 14 = `total`. ## Do the tests fail without their fix? (compute01, `df -h /`: 309G free, `build-slot`, one target, now deleted) Green at head: - `sessions::tests::` 33 passed, 1 ignored; - `--test shim lifeline` 4 passed; - CLI parity: `spawned_shapes_reuse_full_shim_options_and_defaults` and `session_accepts_only_the_two_spawned_shapes` pass. Each mutation is a minimal patch, reverted afterwards. Script and log: `~/j246-evidence/{j246-run.sh,run.log}` on compute01. | Mutation | What fails | |---|---| | **M-fake-death**: `lifeline_eof` returns at once | 7 tests: `an_auto_reaping_launcher_is_not_proof_no_shim_exists` (J230's SIGCHLD-ignoring launcher), `a_launcher_that_has_reaped_its_child…`, `a_forked_launcher_is_not_proof…`, `a_shim_that_binds_after…`, `cancelling_the_launch…`, `the_shims_own_observed_exit…`, `slow_hello_failure…` | | **M-keep-socket**: no socket removal on lifeline proof | `a_closed_lifeline_clears_the_dead_shims_socket_and_record` (tests/unit/sessions.rs:1265) | | **M-old-cleanup**: `030d0fe3e`'s `cleanup` reverted, stall hook kept | `a_stalled_reap_of_the_launcher_stays_inside_the_single_deadline` (:1377) | | **M-close**: the shim closes the descriptor | `lifeline_reaches_eof_when_the_shim_is_killed_and_not_before`, `the_shell_and_its_children_never_hold_the_lifeline` | | **M-noop**: `hold_lifeline` returns `Ok` at once | `a_missing_lifeline_descriptor…` "fails" only by **hanging for 973.89 s**, until I killed its shim by PID. The other three pass. See P3-2. | | **M-cloexec**: validation kept, `F_SETFD FD_CLOEXEC` removed | **Nothing fails** (4/4 pass). See P3-1. | So J230's SIGCHLD-ignoring launcher is a real regression test of the lifeline proof, and so are the socket-clear test and the deadline test. Of the four shim tests, two catch a shim that lets go of its lifeline, and one catches a missing-descriptor regression, but by hanging. None checks the shim's own close-on-exec. ## P3 **P3-1, `crates/infraternal/tests/shim.rs:1489` with `crates/infraternal-shim/src/pty.rs:101`: `the_shell_and_its_children_never_hold_the_lifeline` does not test `hold_lifeline`'s close-on-exec.** - With the `F_SETFD FD_CLOEXEC` call removed, all four lifeline tests pass (M-cloexec). The shell spawn's own `only_standard_descriptors` sweep (pty.rs:1804/1855) already keeps the descriptor out of the shell. - Production is safe twice over. But `024b0f2fc`'s message presents this test as covering the shim's close-on-exec, and that step is untested. - Fix: add a unit test that `F_GETFD` shows `FD_CLOEXEC` after `hold_lifeline`. Or say in the test's doc that it covers the composed result, not the shim step. **P3-2, `crates/infraternal/tests/shim.rs:1513-1528`: `a_missing_lifeline_descriptor_stops_the_shim_before_it_runs` has no hang guard.** - It calls `.output()` on a foreground `__shim`. If the fail-closed check regresses, the shim runs a shell forever, and the test hangs the gate instead of failing (973.89 s in M-noop). - Fix: spawn it, wait with a bound, then kill and assert. **P3-3, `crates/infraternal/src/main.rs:1509-1511`: on the full-CLI path, a `--lifeline-fd` outside `i32` turns into `None` (`i32::try_from(..).ok()`), and the shim runs without validating it.** - This contradicts contracts/shim.md §2 ("fails closed … never runs without its lifeline"). - Today it fails safe: the real descriptor is still inherited and held, and the agent never sends such a value. - Fix: refuse it, as `Options::spawned` does through `number(value)?`. **P3-4, `crates/infraternal-agent/src/sessions.rs:1574`: the `cleanup` doc still says "A detached shim's socket is never unlinked here".** - `prove_detached_dead` now removes it on lifeline proof (`:1660-1662`). - Fix: update the sentence; `contracts/shim.md` already describes the new rule. ## Not done - Bonus `systemctl stop` scope route: not reviewed. - No root or test-server run. The changed paths are rootless detached launches, and the tests above run them for real. Housekeeping: - My compute01 worktree and its 7.7 GB target are removed. The Mac snapshot and its 1.4 GB target are removed; its logs are kept. - The one hung mutation shim was killed by PID, and no `j246` processes are left. - Nothing pushed, merged or closed. **Verdict: REQUEST CHANGES.** - P1-1: macOS `infraternal-agent` does not compile at head (`launch_bound` cfg). It predates the delta, and the fix is two lines. - P3-1 to P3-4: test and doc gaps. The lifeline design and the delta's three commits are otherwise approved.
kayg commented 2026-11-05 17:26:16 +00:00
Author
Owner
Copy link

J170 round 6 finish (J246's changes): GREEN on Linux, and macOS compiles. Branch launch-followups-j170, head 1d95af2552cc4db683b654ec71c4a9fbe1e1557f, pushed, not merged, not deployed. origin/dev merged first (a09a2346a, f332ffed3's #366 Zig fix included), and its 53e41d1ca shape-lint fix merged again before the gate.

Commits since 6fb1c65dc (each fix its own commit)

  • 0837d1fc1 P1-1: launch_bound() is defined on every platform; only its test hook is cfg(all(test, linux)).
  • ad76da8f8 P3-4: the cleanup doc says a detached shim's socket is unlinked only on lifeline proof.
  • f89993804 P3-1: unit test holding_the_lifeline_marks_it_close_on_exec reads FD_CLOEXEC straight from the kernel, plus a_descriptor_that_is_not_an_open_pipe_is_refused.
  • 41af09924 P3-2: the missing-fd test spawns the shim under a 30 s hang guard (shim_exit_within) and still asserts "Bad file descriptor".
  • c939f8260 P3-3: an out-of-range --lifeline-fd is refused ("--lifeline-fd is out of range"), with a real-binary test under the same guard.
  • 1d95af255 D345 and D351 conformance for the new quick rows: doc comments on touched items and // sleep: <reason> on new fixed test sleeps. Comments only.

Fail first

  • P1-1, macOS (Mac VM, under its lock, cargo check --locked -p infraternal-shim -p infraternal-agent --all-targets):
a09a2346a (merge, before the fix):  error[E0425]: cannot find function `launch_bound` in this scope / could not compile `infraternal-agent`
0837d1fc1 (launch_bound fix):        Finished `dev` profile
c939f8260 and 1d95af255 (final):     Finished `dev` profile
  • P3-1, compute01: with hold_lifeline's F_SETFD FD_CLOEXEC step removed, holding_the_lifeline_marks_it_close_on_exec FAILED ("the held lifeline can still be inherited by an exec"); J246 showed all four earlier tests passing in that state.
  • P3-2: with hold_lifeline made a no-op, the new test FAILED after the 30 s guard ("a shim ran without its lifeline"); the old test hung 974 s.
  • P3-3: on the old main.rs the new test FAILED after the guard ("a shim ran with a lifeline descriptor it could not hold": the shim ran in the foreground).
  • P3-4 is a doc fix, no test.

Green at the head: sessions::tests:: 33 pass, pty::lifeline_tests 2, the real-binary lifeline tests 5 (EOF when killed, EOF on normal exit, shell and child never hold it, missing fd refused, out-of-range refused).

scripts/verify quick, verbatim:

| scope guard               | PASS         |
| migration names           | PASS         |
| fmt                       | PASS         |
| doc coverage ratchet      | PASS         |
| shape lints off (D351)    | PASS         |
| doc comments on diff      | PASS         |
| fixed test waits          | PASS         |
| clippy (workspace)        | PASS         |
| unit tests (changed)      | PASS         |
| unit tests (changed, bin-only) | PASS         |

(The first quick at c939f8260 failed the three new D351 rows; fixed in 1d95af255, and the shape-lint failure was a dev file fixed by 53e41d1ca.)

scripts/verify fast at 1d95af255, compute01, foreground, VERIFY_EXIT=0, verbatim:

| fmt                       | PASS         |
| migration names           | PASS         |
| migration tooling tests   | PASS         |
| VT native fmt             | PASS         |
| CLI plugin compile-out    | PASS         |
| clippy                    | PASS         |
| SSH runtime test helper   | PASS         |
| real-binary fixture paths | PASS         |
| Traefik component manifest | PASS         |
| release component publication | PASS         |
| test                      | PASS         |
| SSH inode reuse fixture   | PASS         |
| shim process-per-user     | PASS         |
| cargo-deny                | PASS         |
| gate architecture         | PASS         |
| doc coverage ratchet      | PASS         |
| shape lints off (D351)    | PASS         |
| doc comments on diff      | PASS         |
| fixed test waits          | PASS         |
| privileged integration    | PASS         |
| probe release exclusion   | PASS         |
| nightly fuzz reporter     | PASS         |
| static builds             | PASS         |

Sizes (report only): x86_64 24806896 bytes (+2388640 vs the stored baseline), aarch64 22500600 (+2047320); these include all of dev's growth since the baseline.

Not done / limits

  • macOS: compile only (cargo check --all-targets), no tests run there. The agent's lifeline and cleanup stay Linux-only; dev's macOS launch tail is unchanged.
  • A launch through systemd-run gets no lifeline (the launcher is the shim; reaping it is the proof). No root-only run, no test-server run, and no test of the systemctl stop scope route.
  • Unproven cleanup still keeps the socket and person record, so person revocation refuses until the terminal is closed.
  • Cleanup: my target dir on compute01 and the Mac checkout's scratch files are removed; I killed my stand-in processes by PID.
**J170 round 6 finish (J246's changes): GREEN on Linux, and macOS compiles. Branch `launch-followups-j170`, head `1d95af2552cc4db683b654ec71c4a9fbe1e1557f`**, pushed, not merged, not deployed. origin/dev merged first (`a09a2346a`, f332ffed3's #366 Zig fix included), and its `53e41d1ca` shape-lint fix merged again before the gate. **Commits since `6fb1c65dc` (each fix its own commit)** - `0837d1fc1` **P1-1**: `launch_bound()` is defined on every platform; only its test hook is `cfg(all(test, linux))`. - `ad76da8f8` **P3-4**: the `cleanup` doc says a detached shim's socket is unlinked only on lifeline proof. - `f89993804` **P3-1**: unit test `holding_the_lifeline_marks_it_close_on_exec` reads `FD_CLOEXEC` straight from the kernel, plus `a_descriptor_that_is_not_an_open_pipe_is_refused`. - `41af09924` **P3-2**: the missing-fd test spawns the shim under a 30 s hang guard (`shim_exit_within`) and still asserts "Bad file descriptor". - `c939f8260` **P3-3**: an out-of-range `--lifeline-fd` is refused ("--lifeline-fd is out of range"), with a real-binary test under the same guard. - `1d95af255` D345 and D351 conformance for the new quick rows: doc comments on touched items and `// sleep: <reason>` on new fixed test sleeps. Comments only. **Fail first** - **P1-1, macOS** (Mac VM, under its lock, `cargo check --locked -p infraternal-shim -p infraternal-agent --all-targets`): ``` a09a2346a (merge, before the fix): error[E0425]: cannot find function `launch_bound` in this scope / could not compile `infraternal-agent` 0837d1fc1 (launch_bound fix): Finished `dev` profile c939f8260 and 1d95af255 (final): Finished `dev` profile ``` - **P3-1**, compute01: with `hold_lifeline`'s `F_SETFD FD_CLOEXEC` step removed, `holding_the_lifeline_marks_it_close_on_exec` FAILED ("the held lifeline can still be inherited by an exec"); J246 showed all four earlier tests passing in that state. - **P3-2**: with `hold_lifeline` made a no-op, the new test FAILED after the 30 s guard ("a shim ran without its lifeline"); the old test hung 974 s. - **P3-3**: on the old `main.rs` the new test FAILED after the guard ("a shim ran with a lifeline descriptor it could not hold": the shim ran in the foreground). - P3-4 is a doc fix, no test. Green at the head: `sessions::tests::` 33 pass, `pty::lifeline_tests` 2, the real-binary lifeline tests 5 (EOF when killed, EOF on normal exit, shell and child never hold it, missing fd refused, out-of-range refused). **`scripts/verify quick`, verbatim:** ``` | scope guard | PASS | | migration names | PASS | | fmt | PASS | | doc coverage ratchet | PASS | | shape lints off (D351) | PASS | | doc comments on diff | PASS | | fixed test waits | PASS | | clippy (workspace) | PASS | | unit tests (changed) | PASS | | unit tests (changed, bin-only) | PASS | ``` (The first quick at `c939f8260` failed the three new D351 rows; fixed in `1d95af255`, and the shape-lint failure was a dev file fixed by `53e41d1ca`.) **`scripts/verify fast` at `1d95af255`, compute01, foreground, VERIFY_EXIT=0, verbatim:** ``` | fmt | PASS | | migration names | PASS | | migration tooling tests | PASS | | VT native fmt | PASS | | CLI plugin compile-out | PASS | | clippy | PASS | | SSH runtime test helper | PASS | | real-binary fixture paths | PASS | | Traefik component manifest | PASS | | release component publication | PASS | | test | PASS | | SSH inode reuse fixture | PASS | | shim process-per-user | PASS | | cargo-deny | PASS | | gate architecture | PASS | | doc coverage ratchet | PASS | | shape lints off (D351) | PASS | | doc comments on diff | PASS | | fixed test waits | PASS | | privileged integration | PASS | | probe release exclusion | PASS | | nightly fuzz reporter | PASS | | static builds | PASS | ``` Sizes (report only): x86_64 24806896 bytes (+2388640 vs the stored baseline), aarch64 22500600 (+2047320); these include all of dev's growth since the baseline. **Not done / limits** - macOS: compile only (`cargo check --all-targets`), no tests run there. The agent's lifeline and cleanup stay Linux-only; dev's macOS launch tail is unchanged. - A launch through `systemd-run` gets no lifeline (the launcher is the shim; reaping it is the proof). No root-only run, no test-server run, and no test of the `systemctl stop` scope route. - Unproven cleanup still keeps the socket and person record, so person revocation refuses until the terminal is closed. - Cleanup: my target dir on compute01 and the Mac checkout's scratch files are removed; I killed my stand-in processes by PID.
kayg commented 2026-11-05 17:56:49 +00:00
Author
Owner
Copy link

J253 re-review of launch-followups-j170 at 1d95af2552cc4db683b654ec71c4a9fbe1e1557f (delta since 6fb1c65dc, dev merges excluded: 0837d1fc1, ad76da8f8, f89993804, 41af09924, c939f8260, 1d95af255): APPROVE.

Per the reviewer rule I did not run verify fast or quick. I read J170's gate output (80230) and ran only the checks listed below.

J246's findings: all closed

P1-1, macOS compile: closed. crates/infraternal-agent/src/sessions.rs:80 has no platform cfg now, and its test hook is cfg(all(test, target_os = "linux")).
On lume@infraternal-macos-01-std, I snapshotted the exact tree (git archive 1d95af255) and ran it in the foreground under lockf -k /tmp/mac-bench.lock: nix develop -c cargo check --locked -p infraternal-shim -p infraternal-agent --all-targets

warning: associated function `with_registry` is never used
   --> crates/infraternal-agent/src/plugin_control.rs:606:25
warning: `infraternal-agent` (lib test) generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 17m 47s
CHECK_EXIT=0 secs=1072

No errors. The one warning is the dead-code warning J246 already saw. It predates this branch and is not in the delta. Log: ~/j253-evidence/check.log on the Mac VM.

The evidence for the remaining findings is on compute01, run through build-slot: ~/j253-evidence/{run.sh,run.log}. Every mutation was a one-line patch, reverted afterwards.

P3-1, the shim's own close-on-exec: closed. The new test is crates/infraternal-shim/src/pty.rs:2584 (holding_the_lifeline_marks_it_close_on_exec).

  • At head: pty::lifeline_tests 2 passed.
  • M-cloexec (the F_SETFD FD_CLOEXEC call at pty.rs:101 changed to if false): it fails, panicked at pty.rs:2591: the held lifeline can still be inherited by an exec.

P3-2, hang guard: closed. crates/infraternal/tests/shim.rs:1522-1560 adds REFUSAL_GUARD, 30 s, and shim_exit_within.

  • M-noop (hold_lifeline returns Ok at once): a_missing_lifeline_descriptor_stops_the_shim_before_it_runs fails with a shim ran without its lifeline, finished in 30.02s (56 s including the build). J246's run of the old test hung for 973.89 s.
  • The killed shim left no process behind.

P3-3, out-of-range --lifeline-fd: closed. The fix is at crates/infraternal/src/main.rs:1513-1517.

  • The real binary at head, full CLI path: __shim … --lifeline-fd 4294967296 prints shim startup/session failed: --lifeline-fd is out of range, CLI_EXIT=2. It exits at once and never forks.
  • M-oor (the old .and_then(|d| i32::try_from(*d).ok()) put back): an_out_of_range_lifeline_descriptor_stops_the_shim_before_it_runs fails, panicked at tests/shim.rs:1583: a shim ran with a lifeline descriptor it could not hold, after the 30 s guard.
  • Side note: --lifeline-fd -1 is refused by the parser (has no option -1, exit 2). Any negative value that does get through still hits descriptor < 3 in hold_lifeline.

P3-4, stale doc: closed. crates/infraternal-agent/src/sessions.rs:1571-1572 now says the socket is unlinked only on lifeline proof (prove_detached_dead). That matches contracts/shim.md.

Green at head (compute01): pty::lifeline_tests 2/2. The real-binary --test shim lifeline tests 5/5: EOF when killed, EOF on normal exit, the shell and its children never hold the lifeline, missing fd refused, out-of-range fd refused.

The lifeline design is unchanged

git diff 6fb1c65dc 1d95af255 on sessions.rs, pty.rs and startup.rs changes only:

  • the launch_bound cfg;
  • doc comments and // sleep: comments;
  • dev's removal of the #[allow(clippy::too_many_*)] attributes (D351);
  • the new lifeline_tests module.

hold_lifeline, inherit_descriptor, new_lifeline, lifeline_eof, prove_detached_dead and the single deadline in cleanup are byte-identical to what J246 approved. contracts/shim.md is not touched in the range. The main.rs change only tightens validation.

Findings

P3-a (nit, does not block), crates/infraternal-shim/src/pty.rs:2600,2611: the "regular file" case of a_descriptor_that_is_not_an_open_pipe_is_refused opens /dev/null, which is a character device, not a regular file. The test still proves the point (anything that is not a FIFO is refused). Only the label is wrong: rename it to "a character device", or open a real temp file.

No P1 or P2.

Not done

  • macOS: compile only, as asked. The agent's lifeline stays Linux-only.
  • No root run and no test-server run. The changed paths are rootless, and the tests above run them for real.

Housekeeping:

  • compute01: worktree and scratch target removed.
  • Mac: snapshot and target removed; check.log kept.
  • One stale __shim from j93fixture (66 858 s old) is running on compute01. It is not mine, and I left it alone.
  • Nothing pushed, merged or closed.

Verdict: APPROVE. J246's P1-1 and P3-1 to P3-4 are closed, with fail-first evidence for each. The lifeline design is unchanged. One P3 nit on a test label.

**J253 re-review of `launch-followups-j170` at `1d95af2552cc4db683b654ec71c4a9fbe1e1557f` (delta since `6fb1c65dc`, dev merges excluded: `0837d1fc1`, `ad76da8f8`, `f89993804`, `41af09924`, `c939f8260`, `1d95af255`): APPROVE.** Per the reviewer rule I did not run `verify fast` or `quick`. I read J170's gate output (80230) and ran only the checks listed below. ## J246's findings: all closed **P1-1, macOS compile: closed.** `crates/infraternal-agent/src/sessions.rs:80` has no platform cfg now, and its test hook is `cfg(all(test, target_os = "linux"))`. On `lume@infraternal-macos-01-std`, I snapshotted the exact tree (`git archive 1d95af255`) and ran it in the foreground under `lockf -k /tmp/mac-bench.lock`: `nix develop -c cargo check --locked -p infraternal-shim -p infraternal-agent --all-targets` ``` warning: associated function `with_registry` is never used --> crates/infraternal-agent/src/plugin_control.rs:606:25 warning: `infraternal-agent` (lib test) generated 1 warning Finished `dev` profile [unoptimized + debuginfo] target(s) in 17m 47s CHECK_EXIT=0 secs=1072 ``` No errors. The one warning is the dead-code warning J246 already saw. It predates this branch and is not in the delta. Log: `~/j253-evidence/check.log` on the Mac VM. The evidence for the remaining findings is on compute01, run through build-slot: `~/j253-evidence/{run.sh,run.log}`. Every mutation was a one-line patch, reverted afterwards. **P3-1, the shim's own close-on-exec: closed.** The new test is `crates/infraternal-shim/src/pty.rs:2584` (`holding_the_lifeline_marks_it_close_on_exec`). - At head: `pty::lifeline_tests` 2 passed. - **M-cloexec** (the `F_SETFD FD_CLOEXEC` call at pty.rs:101 changed to `if false`): it fails, `panicked at pty.rs:2591: the held lifeline can still be inherited by an exec`. **P3-2, hang guard: closed.** `crates/infraternal/tests/shim.rs:1522-1560` adds `REFUSAL_GUARD`, 30 s, and `shim_exit_within`. - **M-noop** (`hold_lifeline` returns `Ok` at once): `a_missing_lifeline_descriptor_stops_the_shim_before_it_runs` fails with `a shim ran without its lifeline`, `finished in 30.02s` (56 s including the build). J246's run of the old test hung for 973.89 s. - The killed shim left no process behind. **P3-3, out-of-range `--lifeline-fd`: closed.** The fix is at `crates/infraternal/src/main.rs:1513-1517`. - The real binary at head, full CLI path: `__shim … --lifeline-fd 4294967296` prints `shim startup/session failed: --lifeline-fd is out of range`, `CLI_EXIT=2`. It exits at once and never forks. - **M-oor** (the old `.and_then(|d| i32::try_from(*d).ok())` put back): `an_out_of_range_lifeline_descriptor_stops_the_shim_before_it_runs` fails, `panicked at tests/shim.rs:1583: a shim ran with a lifeline descriptor it could not hold`, after the 30 s guard. - Side note: `--lifeline-fd -1` is refused by the parser (`has no option -1`, exit 2). Any negative value that does get through still hits `descriptor < 3` in `hold_lifeline`. **P3-4, stale doc: closed.** `crates/infraternal-agent/src/sessions.rs:1571-1572` now says the socket is unlinked only on lifeline proof (`prove_detached_dead`). That matches contracts/shim.md. **Green at head** (compute01): `pty::lifeline_tests` 2/2. The real-binary `--test shim lifeline` tests 5/5: EOF when killed, EOF on normal exit, the shell and its children never hold the lifeline, missing fd refused, out-of-range fd refused. ## The lifeline design is unchanged `git diff 6fb1c65dc 1d95af255` on `sessions.rs`, `pty.rs` and `startup.rs` changes only: - the `launch_bound` cfg; - doc comments and `// sleep:` comments; - dev's removal of the `#[allow(clippy::too_many_*)]` attributes (D351); - the new `lifeline_tests` module. `hold_lifeline`, `inherit_descriptor`, `new_lifeline`, `lifeline_eof`, `prove_detached_dead` and the single deadline in `cleanup` are byte-identical to what J246 approved. `contracts/shim.md` is not touched in the range. The `main.rs` change only tightens validation. ## Findings **P3-a (nit, does not block), `crates/infraternal-shim/src/pty.rs:2600,2611`:** the "regular file" case of `a_descriptor_that_is_not_an_open_pipe_is_refused` opens `/dev/null`, which is a character device, not a regular file. The test still proves the point (anything that is not a FIFO is refused). Only the label is wrong: rename it to "a character device", or open a real temp file. No P1 or P2. ## Not done - macOS: compile only, as asked. The agent's lifeline stays Linux-only. - No root run and no test-server run. The changed paths are rootless, and the tests above run them for real. Housekeeping: - compute01: worktree and scratch target removed. - Mac: snapshot and target removed; `check.log` kept. - One stale `__shim` from j93fixture (66 858 s old) is running on compute01. It is not mine, and I left it alone. - Nothing pushed, merged or closed. **Verdict: APPROVE.** J246's P1-1 and P3-1 to P3-4 are closed, with fail-first evidence for each. The lifeline design is unchanged. One P3 nit on a test label.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
j470b-server-mesh-rejoin
mesh-compat-382
j444b-d381-key-wins-flood
j461b-macos-utun-reader-thread
j475b-remove-host-motion
fix-reconnect-new-link
j481-d380
dev
j479-mesh-soak
fix/j476-host-facts
j475-motion-pill-split
j470-mac-node-join
root-upgrade-d376
j474-review-merge
latency-targets-120hz
nightly-suite-shards
gotatun-pin-j469
macos-utun-reader-thread
fix-flood-echo-stall
wip/j416
motion-fast-ease-out
mesh-addhost-j259
takeover-close-terminals-d368
kbd-instant-followups
fix-trust-key-double-click
nightly-suite-partial-report
appwin-icons-j207
rusqlite-slice1
swipe-content-first-d374
mesh-mtu-381
addhost-instant-open
glass-contrast-d365-mock
glass-contrast-d365
signout-followups-442
fix-startup-toast-red
research-gotatun-matrix-j317
themes-sync-j304
fix-load-flakes
fix-nix-zig-patch
fix-compute01-browser-baseline
ratchet-size-baseline-419
predict-auto-d130
revoke-coverage-d367
nightly-suite-queue-timeout
nightly-browser-suite
fix-rustsec-2026-0459
fonts-ioskeley-obel
log-retention-d375
fix-kbd-no-anim-427
fix-dev-red-specs-2
takeover-proof-305-j205
wip/j410-server-pump
ui-consistency-d372
fix-osc52-clipboard
libghostty-vt-1.0
forget-retired-host-d369
j404-merge-426
fix-setup-lock-race
fix-touch-select-426
revoke-access-report-ui
fix-predictive-echo-specs
fix-selinux-agent-exec
wip/j395-checkpoint2
wip/j395
j391-review-scratch
ui-consistency-j195-deferred-rebased
fix-rustc-paths-421
decisions-wording-341
fix-install-suite-red
fix-dev-rtt-specs
fix-pending-removal-align
fix-privileged-skip
fix-install-suite-404
review-j368
mesh-dns-j254
revoke-access-report-d356b
revoke-privilege-round5
fix-removal-confirm
review-j360
gotatun-rotation-367
deps-refresh-340
review-j354
review-J351
review-j350
addhost-takeover-d359
revoke-report-items-347
revoke-end-sessions-d362
review-j348
size-mesh-ssh-346
size-trim-337
revoke-access-diff-d356
j259-pre-rebase-b06c5a6
j305-ended-at-start-spec
addhost-sudo-consent
fix-layout-real-specs-411
fix-shell-open-411
fix-console-hosts-373
fix-shell-exit
fuzz-host-name-353
fix-host-details-410
macos-filter-j197
research-gotatun-tricks
revoke-p3-j216l
stale-state-pill
revoke-followups-j216k
revoke-progress-j216e
backup/macos01-20261106/lume/j197
backup/macos01-20261106/lume/appwin-icons-j207-stash0
backup/macos01-20261106/claude/stash-0
backup/macos01-20261106/claude/stash-1
backup/macos01-20261106/claude/J80-macos-s1
backup/macos01-20261106/claude/J80
backup/macos01-20261106/claude/J80-slice1-20261102
backup/macos01-20261106/claude/J16b-appwin-on-dev
backup/macos01-20261106/claude/appwin-launch-idle-146
backup/macos01-20261106/claude/J80-s1-run
wip/j259
trust-285-p3c
runtime-lock-j216f
mesh-ssh-p3-j175
tun-proof-p3b
trust-285-p3
terminal-polish-j271
tun-proof-p3
ssh-install-flakes-j216i
fuzz-fixes-j264
trust-285-a
trust-285-b
restart-oversized-fixture-j198
backup/macos01-20261106/lume/appwin-icons-j207
restart-bound-j216h
up-preflight-p3-j198
backup/macos01-20261106/lume/j251
up-preflight-j198
wip/up-preflight-j198-j245-p2
wip/up-preflight-j198-j245-p1
agent-fsync-offloop-j196
wip/up-preflight-j198-j245
settings-offscreen-j231
wip/up-preflight-j198-j226-inspection
wip/j103f
themes-j206
wip/up-preflight-j198-j226
flakes-j216c
backup/macos01-20261106/claude/j197
flakes-j216b
backup/macos01-20261106/claude/j197-red
wip/up-preflight-j198-review
flakes-j216
backup/macos01-20261106/claude/appwin-icons-j207
engineering-j208b
appwin-mock-j209
toast-stack-j165
engineering-j208
decisions-d351
decisions-d350
decisions-d349
appearance-presets-mock-j129
decisions-d348
docs-audit-spec
doc-coverage-baseline-raise
decisions-d347
decisions-d346b
decisions-d346
j88h-profile
ui-consistency-j195-deferred
ui-consistency-j195
engineering-practices-j194
preferred-login-ui-j166
signin-pools-263-j174
macos-node-265-s1
decisions-d345
decisions-d344
proxy-cgroup-j164
verify-quick-fuzz-329
proxy-start-io-j168
timing-flakes-j163
up-263-s1
mcp-events-j147
verify-quick-fuzz-j158
proxy-p3-j159
agent-resource-j157-detect
wip/j103d-final
remote-cancel-hang-j155
proxy-traefik-281-j62d
j149-verify-quick
proxy-traefik-281-j62b
j150-drop-unnamed
wip/j79d-red-r4
wip/j103d-d332
rootless-toast-j142
wip/j103d-head
wip/j103d
j125c-mcp
macos-build-j136
backup/macos01-20261106/claude/j136
wip/j79d-red-cli
wip/j79d-red-server
j121-link-liveness
j118c-tracer-failures
wip/j79c-head
release-desktop-j131
j124c-signed-macos
wip/j79c-red-p3
wip/j79c-red-p23
wip/j79c-red-p22
merge-j128-check
perf-deferred-j128
sidebar-toggle-260
j127-shortcut-changed-dot
j125-mcp
j118-tracer-failures
j126-account-summary
settings-sections-mock-j92
j120-pinch-lock
sidebar-292
backup/macos01-20261106/claude/j120
taste-2026-11-03
row-actions-242-215
backup/macos01-20261106/claude/stash-0-untracked
backup/macos01-20261106/claude/J80-265-s1-run
backup/macos01-20261106/claude/J80-macos-s1-20261102
j112-canvas-commands
ui-no-overshoot-scroll
fix-recovery-restart
backup/macos01-20261106/claude/stash-1-untracked
taste-2026-11-02
size-baseline-ratchet
machine-cards-structure-j108
j109-boot-copy-mockups
fix-dev-red
wip/up-263-s1
j95b-settings-rows
proxy-traefik-281
j95-option-rows
docs-human-j91
docs-keys-palette
fix-flood-1013-270-j38d
mesh-ssh-s1
mesh-ssh-s0
addhost-d291-tunnel
test-stability-load
mesh-relay-220
repro-296
shim-thp-off-24
mesh-pins-258-v2
mesh-dns
signin-d311-merged
docs-pages
ui-size-283
fix-ui-batch-specs
wip/j12b-edge-update
gate-on-arm
fix-rename-sync-293
trust-285-j60b
fix-focus-ring-clip
integ-ui-1101
edge-update-v2
signin-d311
taste-2026-11-01
layout-per-tab-250
ui-approved-splitopt-224
ui-approved-splitopt-224-on-dev
icon-text-centre-282
ui-approved-189-212-216
research-reverse-proxy
transfer-budget-320
review-mesh-pins-258-r2
sidebar-pill-259
review-mesh-relay-220
wip/j12-edge-update-v2
nix-package
status-crash-loop-237
fix-dev-test-failures
fix-fixture-socket-path
wip/mesh-relay-220-pre-dev-advance-rebase
fix-canvas-side-clip
research-head-to-head
frosted-real-256
review-mesh-pins-258
wip/mesh-relay-220-pre-final-rebase
review-layout-per-tab-250-r2
broad-suite-triage
nix-devshell
review-status-crash-loop-237-r3
wip/nix-devshell-pre-p3
taste-frosted-real
wip/code-serve-demo-2026-10-30
wip/code-serve-glass-2026-10-30
wip/code-serve-clusters-2026-10-30
wip/code-merge-release94-dev-2026-10-30
wip/rproxy-delta-scratch-2026-10-30
wip/stale-note-257-staged-2026-10-30
wip/d241-size-staged-2026-10-30
review-broad-suite-207
wip/mesh-pins-258-pre-split
wip/mesh-relay-220-pre-rebase
mesh-measure-220
plan-211-update
review-edge-autoupdate-r3
perf24-reconnect
review-stale-note-r2
release-prod-live
d241-shell-marker
review-mesh-pins-server-258
review-revocation-r3
review-setup-token-r2
review-sync-journal-r3
wip/review-mesh-pins-258-prune-review-1995fee46
frosted-scroll-256
sync-journal-r3
revocation-per-account-wip
review-tokio-asyncfd-0386
review-sync-p3s
wip/stash-8by64-stash0-2026-10-30
mesh-drag-r2
setup-token-data-wip
release-9-4
auth-busy-1013
mesh-view-overlap
sync-p3s
review-auth-busy-wake
review-readd-r8-wip
mesh-drag-bounds
audit-auth-93
app-windows-stream-start
review-authority-cap-r3
app-windows-r7-measure
readd-minimal-r6
readd-minimal-r5
app-windows-r7-soak
readd-minimal-r4
app-windows-r7
app-windows-r6
readd-minimal-r3
readd-minimal-r2
readd-minimal
merge-release8-dev
review-busy-page-liveness-r2
review-busy-page-liveness
mesh-view-trim
review-authority-read-retry-r2
review-remove-uninstall-r2
release-retry-reattach
release-retry
review-remove-uninstall
review-authority-read-retry
app-windows-ship
mesh-autoswitch-p2
mesh-ssh
mesh-relay
mesh-macos-node
screen-and-control
session-labels
review-real-logins-r5
review-real-logins-r4
review-real-logins-r3
shim-memory-r2
review-real-logins-r2
shim-memory
shadow-addhost-instant
shadow-real-logins
shadow-same-session-flood
shadow-prod-perf-fix
hosts-from-server
app-windows-real
app-windows-transport
release-prod-6w
mesh-policy-verify
quiet-close-toast
dev-merge-r4
review-mesh-agent-transport
review-app-windows-transport
settings-roundtrip
review-release-pipeline-r4
mesh-agent-transport
ended-strip-fix
review-audit-ca-update-d244
settings-rows
phone-width
signout-clear
fix-restart-tests
dev-merge-rp
signer-hardening
review-audit-ca-update
terminal-fidelity
review-audit-ca-update-wip
mesh-integrate
review-mesh-agent-transport-515d3712
review-mesh-dataplane-feec1988
mesh-dataplane
signout
research-canvas-node
wip/review-renderer-cost
wip/review-terminal-fidelity
padlock-glyph
review-release-pipeline-2e95b689
phone-polish
sidebar-polish
inbox-generic
review-mesh-dataplane-378d4055
prod-batch
ui-inbox
ui-ca-unlock
ui-machine-sections
review-mesh-dataplane-a5f744fb
tls-session-prod
session-bounds-prod
fix-dead-session
session-bounds
tls-session
palette-polish
shortcuts-d222
research-appmeta
phone-gutter
ui-centre-button
wip/ui-centre-button
wip/tls-session
review-addhost-breakit
ui-platform
renderer-cost
remote-agents
mesh-api
ssh-bootstrap
prod-hotfix2
prod-fixes
review-remote-agents
review-mesh-dataplane
session-life
research-browser
review-mesh-api
research-capture
review-mesh-api-round3
review-mesh-api-round1
gate-stability
review-gate-stability
review-relay-direct
review-mesh-api-findings
proto-sessions
font-picker
proto-addhost
proto-windows
mesh-core
latency-gate
research-bsd
research-discovery
keytray-mods
mock-core
ui-tweaks
fix-motion-sel
fix-motion
integrate2
fix-robust
t-web
t-server
vt-ghostty
fix-snap-echo
t-web-auth
review-vt2
t-agent
lane-protocol
lane-shim
lane-auth
s1-base
s1-contracts
s1-workspace
No results found.
Labels
Clear labels   
launch-qa
Findings from launch QA.

No labels
launch-qa
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/infraternal#324
No description provided.