Skip to content

Stop the ledger reporting every session as dead - #92

Merged
ndr-ds merged 4 commits into
mainfrom
ndr-ds/agentctl-ledger-compaction-race
Aug 31, 2026
Merged

ndr-ds merged 4 commits into
mainfrom
ndr-ds/agentctl-ledger-compaction-race

Conversation

@ndr-ds

@ndr-ds ndr-ds commented Aug 31, 2026 •

Copy link
Copy Markdown

The ledger's liveness oracle could report every session as dead, and drained is permanent, so the ledger silently stopped ingesting. Two independent ways in; both are fixed here because they are the same oracle and the same loss.

Why "everything is dead" is catastrophic

scan_and_append_locked gates drained on writer liveness, and drained never reverses:

if !writer_alive && prev.drained { continue; }   // never re-reads the file

dead_writer_is_drained_once_then_skipped documents why that is correct — "a real dead writer cannot append". That justification holds only when we have positively determined the writer is gone. Both bugs below break that premise while still producing false.

Bug 1 — an unreadable directory read as "nobody is alive"

let Ok(entries) = fs::read_dir(sessions_dir) else {
    return out;   // empty set
};

One scan taken while ~/.claude/sessions is unreadable marked every transcript drained, and every later append was lost for good.

Bug 2 — reading a directory a sandbox never writes to

scan_and_append hardcoded ~/.claude/sessions, while process::sidecar_candidate_dirs deliberately probes reaper::sandbox_sessions_dir() and the home path. Inside a sandbox the pointers are in the former and the latter is empty, so the directory is perfectly readable and simply contains nothing — the oracle answers "all dead" with full confidence.

Measured on this box:

directory pointer files
/var/lib/sandbox-sessions 92
~/.claude/sessions 0

This is the steady state in a sandbox, not a transient — so it is the dominant trigger, and Bug 1's Option guard does not catch it because nothing failed.

The change

  • read_live_session_ids takes a slice of directories and returns Option<HashSet<String>>, unioning their pointers. Reading a directory is three-valued, because two of the outcomes are not the same answer:
read_dir meaning effect
Ok answered contributes its ids
Err(NotFound) absent — e.g. a host has no sandbox dir contributes nothing, but is still an answer
Err(_) unknown — exists and will not read forces None; nothing is drained

None therefore means "liveness is unknown", either because a directory that exists could not be read or because none answered at all. Collapsing absent into unknown would return None on every host and disable the drained-skip fast path for 99% of files; collapsing unknown into absent is the original bug.

  • Unknown liveness counts as may still be writing: let writer_may_be_live = live.as_ref().is_none_or(|live| live.contains(&session_id));
  • scan_and_append passes session_pointer_dirs() — sandbox first, then home, matching sidecar_candidate_dirs' order.
  • writer_alive is renamed to writer_may_be_live, because the old name is what made the conflation invisible: !writer_alive reads as "the writer is dead" when it also meant "we did not look, or looked in the wrong place".

Errors now fail in the safe direction — an unresolvable oracle re-stats some files it could have skipped, instead of permanently losing their rows.

What that costs, measured. The liveness-gated skip avoids the stat; the mtime/size skip below it is not gated, so an unknown oracle still skips the open+read for unchanged files. The worst case — every pointer directory absent, so None on every scan — is therefore one stat per transcript. On this box's tree that is 4708 files in 5–6 ms, against a background scan interval of tens of seconds. Silent permanent loss is not worth trading for that.

Tests

Both mutation-checked: revert the guard and the matching test fails.

  • an_unreadable_sessions_dir_must_not_permanently_drain_a_live_writer — identical file operations twice, differing only in whether the directory is readable. Before: rows_appended 1 then 0. After: 1 then 1.
  • a_pointer_in_either_directory_keeps_a_writer_live — the pointer exists only in the second directory. Restricting the union to the first (dirs.iter().take(1)) fails it.
  • an_unreadable_sibling_directory_makes_liveness_unknown — one directory readable and empty, a sibling that exists and will not read. Mapping Err(_) to absent fails it. It uses a regular file rather than a permission bit, because the tests run as uid 0 and root lists a 0o000 directory anyway.
  • an_absent_directory_still_lets_a_dead_writer_drain — pins the other direction: mapping NotFound to unknown fails it. Without this the fast path could be disabled on every host with CI still green.
  • dead_writer_is_drained_once_then_skipped still passes, so the drained-skip fast path — documented as 99%+ of files — is unchanged.

cargo clippy --all-targets -- -D warnings clean; 797 tests pass.

Not claimed

I have not observed either bug in production telemetry. What is established: agentctl never creates ~/.claude/sessions (every create_dir_all for it is test code), ~/.claude is a virtiofs mount that has wedged before, and the sandbox pointer counts above are from this machine.

Bug 2 was found by the adversarial review — as the residual of a finding I rejected. The finding itself claimed Some(∅) proved measurable loss from 294 offsets whose on-disk size exceeds last_byte; that evidence does not hold, because last_byte is assigned current_size, the stat size at scan time, so any file appended to since its last scan shows that gap. The reviewer who refuted it noticed the hardcoded path on the way past.

Related

concurrent_scans_do_not_lose_rows failed macOS CI twice on unrelated PRs. Its writer is a thread with no pointer in any directory, so it legitimately sees an empty live set and exercises this same permanent-drain path. I have not changed that test, and whether these fixes fully explain its failures still needs confirmation — it should not be loosened until it does.

@ndr-ds ndr-ds changed the title Do not read an unreadable sessions dir as every writer being dead Stop the ledger reporting every session as dead Aug 31, 2026
@ndr-ds
ndr-ds marked this pull request as ready for review August 31, 2026 22:23
@ndr-ds
ndr-ds merged commit 2b6aa32 into main Aug 31, 2026
5 checks passed
@ndr-ds
ndr-ds deleted the ndr-ds/agentctl-ledger-compaction-race branch August 31, 2026 22:23
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