Skip to content

Read a legacy daemon that exited unreaped as gone in explicit controls - #210

Merged
myobie merged 1 commit into
mainfrom
agent/legacy-control-zombie-is-gone
Nov 4, 2026
Merged

myobie merged 1 commit into
mainfrom
agent/legacy-control-zombie-is-gone

Conversation

@myobie

@myobie myobie commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

A follow-up to #207 and #209, found in a post-merge read of #209.

Cause. #207 made check_daemon_identity read a zombie as Gone. check_control_identity sends modern records through that function, but its pre-boot-ID branch (no daemonBootId) checked only the thread group and the start token, and a zombie keeps both. So a legacy record over a killed, unreaped daemon read as LegacyVerified. Since #209, attach is one of that verifier's callers, through read_control_signal_target_with.

What it affected. Nothing I found acted on it wrongly:

  • attach, kill and rm check the inventory identity first. For that zombie it's Gone, so they never reach the legacy branch. Measured with the CLI (below).
  • rm's exit wait already counted a zombie as exited, through has_process_exited_for_reap.
  • Child signals (pty_client::signal, library only) resolve their target with the same function and have no inventory check. A legacy zombie therefore came back as the target. The next step asks a dead daemon for STATUS, which can only fail, so the caller got that error instead of NotRunning. This is from reading the code; I didn't run it.

Fix. The legacy branch now reads the zombie state the way check_daemon_identity does:

  • On Linux: State: from the same /proc/<pid>/status read that gives Tgid: (linux_leader_status).
  • On macOS: the SZOMB row from the process table.

Tests.

  • New: registry::list::control_identity_tests::a_legacy_daemon_that_exited_unreaped_is_gone_not_verified.
    • A live child under a legacy record is LegacyVerified, and it's the attach and signal target.
    • Killed and not waited for, it keeps its start token (asserted on Linux). It's then Gone, with no target.
    • After it's reaped, it's still Gone.
  • Control. I reverted the code change and kept the test. It fails with left: LegacyVerified, right: Gone, on Linux and on macOS.
  • macOS. With the fix, the new test and the other legacy and zombie identity tests pass: 9 passed. They ran on a Mac outside the build sandbox, where /bin/ps exists, so they ran and did not skip. The release build's macOS check phase has no ps, so there the test skips.
  • Linux, sandboxed root.
    • cargo test -p pty-core -p pty-client: 658 passed.
    • -p pty with cli_attach_guard, client_library, cli_rm, cli_e2e, cli_binary_upgrade, cli_up_down and daemon_lifecycle: 71 passed, 1 ignored.
  • CLI fixture. A legacy record shaped like the ones 063ff7f wrote (daemonPid, daemonStartToken, a PID sidecar, no daemonBootId), over a sleep that is killed and never waited for. Output is the same on d618d49 (before fix: attach to token-verified legacy daemons #209), 74e4dbb (fix: attach to token-verified legacy daemons #209 merged) and this branch:
pty list --json pty attach --no-restart z pty kill z
status=vanished identity=gone exit 1, Session "z" is not running (status: vanished). exit 1, Session "z" is not running. Use "pty rm z" to remove it.

Not changed. No tier-1 files, no JSON shape, no status values. Modern records with boot evidence take the same path as before.

check_daemon_identity has read a zombie as Gone since #207, but the
pre-boot-ID branch of check_control_identity still looked only at the
thread group and the start token, and a zombie keeps both. A legacy
record over a killed, unreaped daemon was LegacyVerified there, and that
is the verifier attach also uses since #209.

Nothing acted on it wrongly. Attach, kill and rm read the inventory
identity first, which is Gone, and rm's exit wait counts a zombie as
exited. But the target resolution that attach and child signals share
returned the zombie's PID, so a signal went on to ask a dead daemon for
STATUS instead of reporting the session not running.

Read the zombie state in the same /proc/<pid>/status read on Linux, and
from the process table row on macOS, as check_daemon_identity does.
@myobie

myobie commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Independent read GO at exact head 5a3a79c46518c06e27b3e4e467e2fb90a1f532b4. All 9 checks are SUCCESS on this head and the PR is CLEAN. The Nix build job failed once on an unrelated test and passed on one rerun (see Risks).

Read:

  • check_control_identity has two policies: the full identity check for records with boot evidence, and a pre-boot-ID policy for legacy records. The earlier zombie fix covered only the first. The legacy policy checked the thread group and the start token, and a zombie keeps both, so a killed, unreaped legacy daemon read as verified.
  • The change makes the legacy policy read a zombie as Gone as well: Z/X in /proc/<pid>/status on Linux through the existing one-read status helper, and the zombie row on macOS. It mirrors the earlier change in check_daemon_identity.
  • No wrong action happens today, because attach, kill and rm check the inventory identity first. This closes a gap in the library path that signals a child by PID, where a dead daemon's STATUS call failed instead of reporting not running.
  • The new unit test creates a legacy record over a killed, unreaped process and checks both the identity and the control target before and after the reap; the author reports it fails on the reverted code on Linux and on a Mac.
  • One file (registry/list.rs), no wire-format change, no tier-1 file (events.jsonl, <id>.json, docs/disk-layout.md).

Risks (low):

  • The Nix build failed on its first attempt in registry::keyed::tests::a_record_naming_no_pid_is_probed_but_never_turned_into_a_loss, a socket-probe test for a record with no PID. It never reaches the changed function and passed on one rerun, so it looks like an existing flaky test; it is worth a loop run on main.
  • If the new test fails midway it leaves its temp directory behind.

@myobie
myobie merged commit d997dd3 into main Nov 4, 2026
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant