pmacs/docs/process-teardown-stdin-dead...

562 lines
26 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Framing — close child stdin before joining readers (process teardown deadlock)
A pipe-mode child that exits on stdin EOF can deadlock the supervisor's
teardown forever. `RuntimeHandles::drop` joins its reader threads in the
`Drop` body, which runs **before** the `stdin` field drops, so the child
never receives the EOF that would make it close the very pipe write ends
those readers are blocked on. The fix is a two-line reorder that reuses a
mechanism already present in this file.
This is the diagnosed root cause of
`m4_5_basedpyright_initializes_and_negotiates_encoding` hanging
indefinitely — the hazard that has parked `cargo test --workspace` runs
(once for 2h26m) and forced `-- --skip basedpyright` into every gate
recipe.
**Scope: `src/process.rs` only. No protocol change. No Lua surface. No
new primitive.**
---
## Revision history
- **rev 1** — initial framing. Root cause established by live diagnosis
(gdb stacks + `/proc` fd forensics on a wedged process), reproduced
5/5 deterministically at `e003b81`.
- **rev 2** — review round 1. rev 1's synthetic child was **itself
vacuous** (the third in this lane): POSIX assigns `/dev/null` to a
background job's stdin when job control is off, so `sh -c 'cat &
exit 0'` EOFs instantly and exits against the *unfixed* tree.
Q#TD6 now uses the explicit-redirect form and criterion 2 gains a
positive control. Also: Q#TD3's bound widened to cover a blocked
stdin writer (a child that read stdin but stopped draining it),
criterion 5 extended to `docs/agent-handoff.md` §3, Bet 4 marked as
lane-stopping.
- **rev 4** — review round 2. §1.7's causal account was **wrong**: the
basedpyright wrapper uses `subprocess.run` and *waits*; the orphan is
created by pmacs SIGTERMing the wrapper at shutdown, not by the wrapper
exiting at spawn. Corrected here, in the handoff and in the ledger, and
§5's P2 restated — the follow-up is "stop orphaning them", not "tolerate
self-orphaning". Also: the `setsid` dependency is now skip-unless-armed
rather than a hard assert, since it is util-linux and the standard
`--lib` gate must not fail on an undeclared tool.
- **rev 3** — CI falsified rev 2's repair. `<&0` is defeated on `dash`
(the rule applies *before* explicit redirections, so `<&0` duplicates
`/dev/null` onto itself); it passed locally only because `/bin/sh` here
is `bash`. **Control 2 caught it in CI and named its own cause** — the
fourth vacuity in this lane, and the first one a control found instead
of a reviewer. The reproduction now uses `setsid --fork cat`, removing
the shell entirely. Bet 2's falsifier list also gained
`bottom_panel_stage1_acceptance`, which holds PTY-in-panel tests and
was a genuine gap in rev 1's list.
---
## 0. Coherence impact (COHERENCE §20)
This is a defect fix, not coherence work, and it should not claim
otherwise.
- **Journey steps touched:** none directly. It protects the steps that
depend on a live language server (§2 step 6 onward) from an unbounded
teardown, but it adds no journey surface.
- **Interaction islands added:** none.
- **Config registry:** no new options.
- **Background-work attribution:** unchanged. The supervisor's process
model is untouched; only the order of two teardown operations moves.
- **Protocol:** unchanged.
The one genuine coherence connection is indirect and worth stating
plainly: the hang parks `cargo test --workspace`, which is the ratchet
every COHERENCE priority is verified against (§19, §25). A gate that can
hang forever degrades every other lane's evidence. That is the argument
for doing this now rather than parking it — not a claim that it advances
a priority.
---
## 1. Ground truth (scouted @ `e003b81`)
Line numbers are hints; symbols are authoritative.
### 1.1 The reproduction is deterministic, not intermittent
`docs/agent-handoff.md` and the test-improvement audit both describe this
hang as intermittent. On a machine where `basedpyright-langserver`
resolves to a uv-installed shim it is **completely reliable**: 5 runs, 5
hangs, via
```
cargo test --test m4_acceptance -- --exact \
m4_5_basedpyright_initializes_and_negotiates_encoding
```
§1.7 explains why it looks intermittent across machines. The practical
consequence: this defect is directly testable, and any fix has a
revert-bite.
### 1.2 The cycle, in five links
Observed stack of the wedged test thread (gdb, `sudo` required —
`ptrace_scope=1`):
```
tests/m4_acceptance.rs:1374 Rc<RefCell<ProcessSupervisor>> dropped
→ ProcessSupervisor::drop src/process.rs:1545
→ ProcessSupervisor::shutdown src/process.rs:1463
→ ProcessSupervisor::tick src/process.rs:1188
→ ProcessSupervisor::poll_one src/process.rs:1268 (drop site: :1337)
→ RuntimeHandles::drop src/process.rs:631
→ JoinHandle::join ← blocked, indefinitely
```
The links:
1. **`RuntimeHandles::drop` (`:631`)** sets `cancel`, then joins every
handle in `self.readers`.
2. **Rust runs a type's `Drop::drop` body before dropping its fields.**
`stdin: Option<StdinWriter>` is a *field* (`:505`), so it cannot drop
until the body returns. The body never returns.
3. **`StdinWriter` (`:556`)** holds the `Sender`; `StdinWriter::spawn`
(`:568`) moves the `ChildStdin` sink into its thread, which drops the
sink only once `rx.recv()` errors. Sender alive ⇒ sink alive ⇒ **the
child's stdin write end never closes.**
4. **The child therefore never sees EOF**, stays alive, and keeps the
stdout/stderr **write** ends it inherited.
5. **The readers are blocked in `read()`** at `:1886` inside
`spawn_reader` (`:1874`). `cancel` is consulted only at the loop top
(`:1883`) and around `send_timeout` — **never while `read` is
blocked.**
Verified on the live process: the test held fd 4 (child stdin, WRONLY)
and fds 5 and 7 (stdout/stderr, RDONLY); the server process held the
matching opposite ends on fds 0, 1, 2. Two reader threads sat in
`anon_pipe_read`, and the stdin-writer thread sat parked in
`Receiver::recv` at `:577` — alive, still owning the sink.
Confirmation from the other direction: when the wedged test process was
killed, its fd 4 closed, the server immediately saw stdin EOF and
exited. The cycle's load-bearing link is exactly the one the fix cuts.
### 1.3 The existing comment names the false premise
`RuntimeHandles::drop` documents its own reasoning:
> Wake any reader thread blocked in a bounded `send` — dropping the
> master closes the kernel pipe and unblocks `read`, but does nothing
> for a reader stuck on a full channel […]
The premise is true for a **PTY master** and false for **pipe mode**,
where `read` unblocks only when *every* write end closes. `cancel` was
introduced for the full-channel case and is correct for it; the comment
mistakenly treats the `read` case as already handled.
### 1.4 `shutdown()`'s SIGKILL phase is unreachable on this path
`shutdown()` (`:1463`) sends SIGTERM to all ids, then runs a bounded
grace loop (`deadline` at `:1476`) that calls `tick()`, and *then*
escalates to SIGKILL. The stack shows the deadlock occurs **inside that
grace loop's `tick()`**, because `poll_one` drops `RuntimeHandles` the
moment it observes the recorded pid exited. The SIGKILL phase is never
reached.
So "shutdown force-kills everything first" is not true of this path.
(An earlier working assumption of mine said it did; the stack refutes
it.) Even if reached, SIGKILL targets the *recorded* pid, which per
§1.7 is not the surviving process.
### 1.5 Only pipe-mode, non-group spawns are affected
`spawn_pipes` (~`:1712`) chooses per stream:
| `spec.group` | reader | cancellable mid-`read`? |
| --- | --- | --- |
| `true` | `spawn_group_reader` (`:1941`) — `O_NONBLOCK` + `poll` | **yes** |
| `false` | `spawn_reader` (`:1874`) — blocking `read` | **no** |
PTY mode (~`:1724`) also uses `spawn_reader`, but there §1.3's premise
holds: dropping the master genuinely ends the read. The `spawn_ansi_parser`
reader also lives in `readers`, and reads a channel rather than an fd, so
it is unaffected.
Non-group pipe consumers are, per `spawn_reader`'s own doc comment, the
**REPL and LSP** paths. This defect is therefore reachable by every LSP
server and every REPL — not by terminals.
### 1.6 The fix mechanism already exists in this file
`close_stdin` (`:1611`) already does precisely what is needed, and
already documents the semantics and the idempotence:
```rust
// Dropping the writer closes the pipe at the kernel
// level. `take()` is idempotent — second call sees None.
let _ = runtime.stdin.take();
```
The fix is applying an existing, already-reviewed mechanism at the one
site that is missing it. It introduces no new concept.
### 1.7 Why basedpyright wedges and clangd/gopls do not
`basedpyright-langserver` is a uv-installed **Python console script**:
```python
from basedpyright.langserver import main
sys.exit(main())
```
`main()` reaches `run_node.run`, which calls `nodejs_wheel`'s `node(...)`
— and that is **`subprocess.run`** (`nodejs_wheel/executable.py:50`). It
**waits**. Verified in the installed 1.39.6 source, not assumed.
So the wrapper does *not* exit at spawn time, and **pmacs creates the
orphan itself**:
1. The wrapper runs `node …/langserver.index.js --stdio` and blocks. Node
is a genuine grandchild; the initialize handshake completes normally.
2. At teardown, `shutdown()` sends **SIGTERM to the recorded pid** — the
Python wrapper — before entering its grace loop.
3. The wrapper dies on the default disposition and **does not forward the
signal**. Node is reparented to `PPid: 1`, still holding the inherited
pipes, idle in `ep_poll`.
4. `poll_one` then observes the recorded pid terminated, drops
`RuntimeHandles`, and enters the deadlock.
**rev 13 of this doc said the wrapper "spawns node and exits".** That was
wrong, and the evidence against it was already in hand: the test's
assertions all pass *before* teardown, so the handshake succeeded — which
is impossible if the wrapper had exited at spawn. The observation that
generated the claim (`PPid: 1`, wrapper gone) was taken **after**
`shutdown()` had already killed it.
This matters for the parked work, not for the fix. The follow-up is not
"tolerate servers that self-orphan" — it is **stop orphaning them**:
signal the process group rather than a wrapper pid that swallows the
signal. P2 in §5 is restated accordingly.
`clangd` and `gopls` are real binaries: genuine children, reaped
normally, write ends closed, blocking `read` returns `Ok(0)` cleanly. The
"intermittency" in the handoff is not timing — it is *which server binary
is installed how*.
### 1.8 Limits of the evidence
- The deterministic reproduction is **one machine, one server**. The
causal chain is verified there link by link; its generality to other
shim-launched servers is reasoned, not measured.
- The gdb capture is a single sample of a state that was stable across a
four-minute window and identical across two independent runs. That is
strong for a deadlock and would be weak for a race.
- Nothing here establishes how often the hang has fired in CI. CI never
installs basedpyright (`PMACS_REQUIRE_PYRIGHT` is deliberately never
set, #194), so in CI this test skips and the defect is **dark**. Every
observation is local.
---
## 2. Decisions
### Q#TD1 — the fix is a reorder inside `Drop`, not a new primitive
```rust
impl Drop for RuntimeHandles {
fn drop(&mut self) {
self.cancel.store(true, Ordering::Relaxed);
// Close the child's stdin BEFORE joining. A stdio child exits
// on EOF and closes its stdout/stderr write ends, and that —
// not `cancel` — is what unblocks a reader parked in `read`
// (`cancel` is only observed between reads and around `send`).
// The sink lives in the `stdin` field, which cannot drop until
// this body returns, so joining first deadlocks against it.
let _ = self.stdin.take();
for h in std::mem::take(&mut self.readers) {
let _ = h.join();
}
}
}
```
Rejected alternative: reordering the struct's *fields*. Field order does
not help — the explicit `Drop::drop` body runs before **all** fields
regardless of their declaration order. This is the trap that makes the
bug non-obvious, and it belongs in the comment.
### Q#TD2 — the reorder is unconditional across modes
Applying it only to pipe+non-group would require `RuntimeHandles::drop`
to learn which mode it is in, which it currently does not need to know.
Closing stdin before teardown is correct in both modes, so the reorder is
unconditional.
This is a uniformity change, and uniformity changes in this repo have
made total functions partial before. It is therefore carried as a **bet
with a named falsifier** (§3, Bet 2), not as an assumption: PTY-mode
`stdin` is the pty *writer*, and dropping it while `pair.master` and the
cloned reader still exist must not end the read early.
### Q#TD3 — the fix assumes the child drains stdin to EOF, and covers nothing outside that
Stated up front because it bounds the claim: the fix works by making the
child exit. A child that never reads stdin — or reads it and ignores EOF
— keeps its write ends open and still wedges the join.
There is a third member of that family, and it is not covered by the
wording above because such a child *did* read stdin: **the EOF is only
delivered if the writer thread reaches the end of its queue.** Its body
is a blocking `sink.write_all(&bytes)` (`:578`), so a child that has
stopped draining stdin while queued bytes remain blocks the writer
indefinitely — the sink never drops, EOF never arrives, and the join
re-wedges. This needs only a full stdin pipe buffer at teardown time, not
a misbehaving child. For LSP teardown the queue is near-empty and the
practical risk is nil, but the bound belongs in the claim: **the fix
assumes the child keeps draining stdin until EOF.** A full stdin pipe
with a non-draining child is P1's case as well.
Covering *that* case requires making the blocking `read` itself
cancellable, i.e. moving non-group readers onto `spawn_group_reader`'s
`O_NONBLOCK` + `poll` mechanism. `spawn_reader`'s doc comment already
names this as a deferral from the compile-mode framing. It stays parked
(§5, P1) rather than riding this PR, because it is a behavioural change
to every REPL and LSP ingest path and deserves its own review.
The honest claim for this PR is therefore: **it fixes the observed
deadlock for stdio children that honour EOF, which is what LSP servers
are, and narrows — not eliminates — the class.**
### Q#TD4 — queued stdin writes are not lost, and the writer is not joined
`crossbeam`'s `Receiver::recv` drains buffered items before reporting
disconnection, so dropping the `Sender` still lets the writer thread
write everything already queued. The writer thread is **not** joined
here, so there remains no guarantee the final flush completes before the
process is signalled. That is pre-existing, unchanged by this PR, and
noted rather than fixed (P3, §5).
Draining is also the mechanism by which the fix can fail to deliver EOF
at all when the child has stopped reading — see Q#TD3's third case.
### Q#TD5 — the leaked orphan server is not fixed here
After the fix, the wedge is gone but a shim-launched server is still an
orphaned grandchild that teardown's recorded pid cannot signal. It exits
here only because it honours stdin EOF — by cooperation, not by
enforcement. A server that ignores EOF leaks. Parked (§5, P2).
### Q#TD6 — the synthetic reproduction must model EOF-honouring, not sleeping, and needs an explicit stdin redirect
Two distinct traps here, and this lane has now walked into **three**
vacuous reproductions, so the reasoning is recorded rather than the
conclusion alone.
**Trap 1 — a sleeping child models the wrong defect.**
`sh -c 'sleep 300 & exit 0'` orphans a grandchild that holds the write
ends but **never reads stdin**, so closing stdin does not free it. That
reproduces a hang this fix does *not* address; it belongs to P1 (§5), not
here.
**Trap 2 — a background job does not inherit stdin.** POSIX XCU §2.9.3:
> If job control is disabled, the standard input of an asynchronous
> list, before any explicit redirections, shall be assigned to
> `/dev/null`.
Job control is off in every non-interactive `sh`, so in
`sh -c 'cat & exit 0'` the background `cat` gets **`/dev/null`**, not the
inherited pipe. It EOFs immediately and exits **against the unfixed
tree** — the test would pass either way and Bet 3's revert-bite would
report VACUOUS.
Measured on this machine (`/bin/sh` → `bash`), stdin attached to a
held-open fifo, checking the orphan's `/proc/<pid>/fd/0`:
| form | grandchild | fd 0 |
| --- | --- | --- |
| `sh -c 'cat & exit 0'` | **gone** | — (EOF'd from `/dev/null`) |
| `sh -c 'cat <&0 & exit 0'` | alive | the real pipe |
**Trap 3 — `<&0` does not repair it, and the obvious fix is wrong.** rev 2
proposed `sh -c 'cat <&0 & exit 0'`, verified on this machine. **CI
falsified it.** Re-read the rule: `/dev/null` is assigned *before any
explicit redirections*, so by the time `<&0` runs, fd 0 already **is**
`/dev/null`, and the redirect faithfully duplicates it onto itself.
`bash` happens to skip the default when a stdin redirect is present;
`dash` — Ubuntu's `/bin/sh`, and CI's — does not. Measured:
| shell | form | grandchild | fd 0 |
| --- | --- | --- | --- |
| bash | `cat & exit 0` | gone | — |
| bash | `cat <&0 & exit 0` | alive | real pipe |
| dash | `cat <&0 & exit 0` | **gone** | — (CI: control 2 failed) |
The local probe could not have caught this: `/bin/sh` here is `bash`.
**The model is therefore `setsid --fork cat`, with no shell at all.**
`setsid --fork` forks, the parent exits, and the child inherits
stdin/stdout/stderr untouched — no asynchronous list, no `/dev/null`
rule, no implementation variance. The recorded pid (`setsid`) terminates
promptly so `poll_one` reaches the teardown path, while `cat` survives
holding the inherited pipes and exits on EOF exactly as a stdio language
server does. Unfixed, this deadlocks; fixed, teardown completes.
`setsid(1)` is util-linux, which the Linux gate already assumes.
Presence is **asserted, not skipped** — a skip would reintroduce the
silent-green shape lane 2 removed.
The controls are what make this recoverable rather than a silent
regression: control 2 failed loudly in CI and named its own cause. That
is #192's lesson one level down — the bite needs a control, and so does
the reproduction.
---
## 3. Bets (falsifiable)
1. **The reorder resolves the observed hang.** Falsified if
`m4_5_basedpyright_initializes_and_negotiates_encoding` still fails to
terminate after the change.
2. **The reorder is safe for PTY mode.** Falsified by any regression in
`vterm_stage1/2/3_acceptance`, `terminal_config_acceptance`,
`terminal_copy_mode_acceptance`, `m6_4/m6_5_repl_acceptance`,
`m6_7_scrollback_acceptance`, `m6_8_multi_repl_acceptance`,
`worker_shutdown_acceptance`, or **`bottom_panel_stage1_acceptance`**
— added in rev 3: it holds PTY-in-panel tests (`acc28` drives real
child input and the `C-c` escape) and its absence from rev 1's list
was a real gap, not a judgement call.
3. **The synthetic test bites.** Falsified if the new test passes with
`let _ = self.stdin.take();` removed. This must be checked by actual
revert, per the standing rule that a new pin needs its own bite.
4. **The basedpyright test passes rather than merely terminating.** The
hang is at teardown (`m4_acceptance.rs:1374`), *after* the body's
assertions, so it should now pass outright. Falsified if it terminates
with a failure — which would mean a second, independent defect.
**If falsified, stop the lane and frame that defect separately.** Do
not paper over it: "terminates" was never the goal, and a failing
assertion here is new information, not a loose end.
---
## 4. Acceptance
1. `RuntimeHandles::drop` takes `stdin` before joining readers, with a
comment naming the drop-body-before-fields trap.
2. New unit test in `src/process.rs` (so it runs under the standard
`cargo test --lib` gate, not only an acceptance suite):
`teardown_closes_stdin_before_joining_readers`.
- Spawns `setsid --fork cat` as a **non-group pipe** process. The
choice of `setsid` over a shell background job is load-bearing
(Q#TD6) and gets a comment saying so. `setsid` presence is
**asserted, not skipped.**
- **Two positive controls, before teardown starts:** (1) the recorded
child has actually exited — while it lives it holds the output pipe
itself, so control 2 would pass for the wrong reason; (2) both
readers are still blocked in `read`, which is only true while
something still holds the write ends. Without these the test
silently degrades into modelling the wrong thing and reports green
while doing it — which is exactly what happened on `dash`, and
control 2 is what caught it.
- `#[cfg(target_os = "linux")]`: the controls read `/proc`, and
`setsid(1)` is util-linux (absent on macOS). Gate it explicitly and
say why, rather than letting it be incidentally Linux-only. (Same
reasoning as the APFS gate — `cfg(unix)` would be wrong here.)
- Performs the full reap-and-drop sequence on a helper thread and
asserts completion via `recv_timeout`, so a regression **fails**
within a bounded window instead of hanging. A test that hangs on
regression would reproduce the exact hazard this PR removes.
- Bound: 10s (default `grace_period` is 2s, `:927`).
- On the failure path the helper thread stays wedged and the `cat`
survives until the harness's fds close at process exit. That is
bounded and acceptable — but the test comment must **say so**, or a
future reviewer correctly flags a leaked thread as a defect.
3. The bite is demonstrated by revert, and the result recorded in the PR
body — pass/fail both ways, per Bet 3.
4. `cargo test --test m4_acceptance` runs **without**
`-- --skip basedpyright` and completes, locally, on the machine where
it currently hangs 5/5.
5. Docs, in **both** places the superseded cause lives — replacing it,
not appending to it:
- `docs/agent-handoff.md` §5 gains the drop-body-before-fields lesson
and the corrected cause, replacing "no timeout on the initialize
handshake".
- `docs/agent-handoff.md` §3's machine caveat currently says the
desktop's **local binary is broken and hangs**. §1.7 shows the
binary was never broken: the shim architecture plus this defect
was. Left alone, §3 keeps steering readers toward a false model —
and toward keeping the skip forever.
**Deliberately not a criterion:** removing `-- --skip basedpyright` from
`CLAUDE.md`'s standing gate list. It is a separate call that is the
user's to make, and it changes only *local* behaviour — CI skips the test
regardless (§1.8). I will propose it with evidence after the fix has been
green repeatedly, rather than fold a process change into a defect fix.
When that proposal comes it owes two things beyond the green runs: the
`docs/agent-handoff.md` §3 caveat updated (criterion 5 covers it here,
but the *skip* rationale lives with it), and an explicit note that
`PMACS_REQUIRE_PYRIGHT` stays **unarmed** in CI until the per-test
timeout lane (3a) merges — the ordering #194 established, where presence
of the variable decides execution and arming without a timeout would give
CI the same unbounded hang this PR removes locally.
---
## 5. Parked (each needs its own evidence)
- **P1 — cancellable non-group `read`.** Move `spawn_reader` onto
`spawn_group_reader`'s `O_NONBLOCK` + `poll` mechanism so `cancel` is
observed within `READER_SEND_POLL_INTERVAL` (`:421`, 50ms) even
mid-`read`. Bounds teardown unconditionally, including for children
that ignore EOF (Q#TD3) — **and** the blocked-writer case, where EOF is
never delivered because `write_all` is stuck on a full pipe. Already
named as a deferral by `spawn_reader`'s own doc comment. Tests: the
`sleep 300` shape from Q#TD6 (child never reads stdin), plus a
fill-the-pipe-then-stop-reading shape for the writer case.
- **P2 — stop orphaning wrapper-launched servers (Q#TD5).** Restated in
rev 4, because the corrected §1.7 changes the target: the orphan is not
self-inflicted by the server, it is created by **us** SIGTERMing a
wrapper that does not forward the signal. Spawn stdio servers in their
own process group and signal the group, reusing the machinery the group
path and `reap_ledger` already have. Fixes a real leak: every
basedpyright-backed session currently leaves a `node` process behind.
Note the ordering consequence — a group-directed SIGTERM would reach
node directly, so this also removes the condition the present fix works
around, rather than merely tolerating it.
- **P3 — join the stdin writer thread** so the final flush is ordered
against child termination (Q#TD4).
- **P4 — re-audit the "intermittent" label** in `docs/agent-handoff.md`
and the audit now that §1.7 explains it. Rides this PR's doc update
only insofar as criterion 5 requires; a broader sweep is separate.
---
## 6. Gates
Per `CLAUDE.md`, each as its own step with a real exit status checked
(never `cmd | tail` — a pipe returns the tail's status and has masked a
real failure here before):
- `cargo fmt --check`
- `cargo clippy --workspace --all-targets -- -D warnings`
- `cargo test --lib`
- `cargo test --lib --features crdt`
- `cargo test --test m4_acceptance`**without** the basedpyright skip
- The PTY/REPL suites named in Bet 2
- `cargo test --test worker_shutdown_acceptance`
- `PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu`
- `git diff --check`
Commit before gating, so the results describe the pushed tree.
---
## 7. Branch plan
One branch, one PR: `process-teardown-stdin-deadlock`, from `main` @
`e003b81` or later. Worktree `pmacs-hang` (already clean at that SHA).
Small diff — the reorder, one unit test, one comment, the handoff
update. P1P4 do not ride it.
`docs/active-work.md` is integrated **late**, immediately before pushing,
to avoid the ledger-contention treadmill with the other open lanes.