Row 47, the S5 follow-up. flows.test gains a test for a hold set inside held input, which kills mutant Mr. claim W5 and W13 reap their own force-stopped scopes, K19 kills its scope if its kill or release fails, and the harness gains liveShims, killShims and shimsGone with a final sweep in reap. claim, cohort and races each end by asserting no shim outlives the file. Author: Dewey. Approved round 1 by Filbert (comment 27041) and Darkwing (comment 27043) on candidate 0587e393. The src side, where a force stop never sends release, is row 50 (#1536). Co-Authored-By: Claude Opus 5.5 <[email protected]>
128 lines
6.2 KiB
Markdown
128 lines
6.2 KiB
Markdown
# Row 47 (#1533): Mr flows test and the leftover shim
|
|
|
|
Dewey, 2026-10-10. Brief: `docs/plans/2026-10-10_s5-follow-up-and-hygiene.md`
|
|
§ "S5 follow-up". Base 21e0f1a3. The candidate touches five test files in
|
|
`packages/conversation/tests/` and this file. No `src/` file and no README
|
|
changes.
|
|
|
|
## Mr
|
|
|
|
`flows.test.mjs` gains "terminal: a hold inside held input holds again, and
|
|
once it is drained later input and Ctrl-C still reach the terminal (Darkwing
|
|
note 1 on #1522)". It is the draft in `~/dewey-scratch/s5/mr-test.patch`
|
|
unchanged. Under Mr (`holds = this.held !== null;` after the re-parse in
|
|
`#run` becomes `holds = false;`) it is the only failing test, 164/165.
|
|
|
|
## Why the shim outlived the suite
|
|
|
|
- `src/shim.mjs` ignores SIGTERM and SIGHUP (line 188) and exits only on the
|
|
`release` op once `engine` reports `populated 0` (line 160).
|
|
- No `src/` code sends `release`. `forceStopCohort` (`src/cohort.mjs:150`)
|
|
runs term, freeze, members and kill, then returns its proof. Controller
|
|
close doesn't release either. So after a force stop the scope stays up
|
|
with its shim, reparented to the systemd user manager, until something
|
|
SIGKILLs the unit.
|
|
- Two claim tests force-stop a scope and leave it: W5's probe fixture
|
|
(the traced release run) and W13. Both relied on the file's `after`
|
|
hook, whose `reap` sends `systemctl --user kill` and `stop` with a 5 s
|
|
timeout and checks neither result.
|
|
- On 7ab2f92d with logging added (scratch only,
|
|
`~/dewey-scratch/s5/probe-instrumentation.diff`), the W5 probe unit was
|
|
`active` from probe close (03:29:14.783Z) to the start of `after`
|
|
(03:29:43.671Z), and `inactive` after `reap` (03:29:44.211Z).
|
|
- In Filbert's row 41 round 2 gate the shim left running was
|
|
`chat03-nWFPcN/f55`, unit `mosaic-chat-cd9f5111af2b95a6b6110b9a1`. That is
|
|
the W5 probe: f55 is the probe fixture in claim.test, and the `afterleak`
|
|
mutant below leaks the same fixture. The user journal shows it started at
|
|
20:43:51.093 local and never killed.
|
|
|
|
Limit of the evidence: that journal has no `mosaic-chat` lines after
|
|
20:43:56 local, though W5 ran 33.7 s and passed
|
|
(`~/filbert-scratch/s6-logs/gate-r2/node-conversation.txt`). The system
|
|
journal isn't readable to me. So I can show the probe was left for `after`
|
|
and that `after`'s kill didn't land in that run, but not why the
|
|
`systemctl` call missed.
|
|
|
|
## The change
|
|
|
|
- `harness.mjs`: `liveShims(root)` lists running shims whose socket is
|
|
under `root`, from `/proc` cmdline and cgroup. `killShims(root)` SIGKILLs
|
|
each one's scope and then the shim by PID. `shimsGone(root, ms)` waits up
|
|
to 5 s and returns what is left. `reap(fx)` ends with
|
|
`killShims(fx.base)`, so a shim the records don't name, or one the
|
|
unchecked `systemctl` calls missed, is still killed.
|
|
- `claim.test.mjs`: W5 reaps its probe and W13 reaps its fixture as soon
|
|
as they close, not in `after`.
|
|
- `cohort.test.mjs` K19: if its own kill or release fails, the `finally`
|
|
SIGKILLs the scope before asserting, so a failure doesn't leave the
|
|
shim for `after`.
|
|
- claim, cohort and races (the files that launch scopes): `after` reaps,
|
|
sweeps with `killShims()`, waits with `shimsGone()`, and then asserts
|
|
that no shim is left ("no shim outlives this file (#1533)"). A failing
|
|
`after` hook fails the file in node:test, with exit 1. Each file also
|
|
ends with a test "no shim from this file's tests is left running for the
|
|
after hook", which names the test's leak before `after` cleans it up.
|
|
`tmp` is per process, so each file checks only its own shims.
|
|
|
|
The shim keeps ignoring SIGTERM. That is deliberate (README, force stop:
|
|
"so that a stray TERM never drops the scope's anchor"), so the fix is in the
|
|
tests that spawn it.
|
|
|
|
## Finding for Sage (out of scope, not fixed)
|
|
|
|
A force-stopped scope never ends on its own: nothing in `src/` sends
|
|
`release`, so its shim and the empty scope stay up until something kills the
|
|
unit. In tests that is now handled. In a running controller each force stop
|
|
leaves one `mosaic-chat-*.scope` with an idle node shim. This is in
|
|
`cohort.mjs`/`controller.mjs`, which the brief puts out of scope, so I
|
|
haven't touched it. I recommend a follow-up row: after a proven force stop
|
|
(and on close of a stopped binding), send `release`, with a test that the
|
|
unit is gone afterwards.
|
|
|
|
## Mutation check
|
|
|
|
Scratch copies of the working tree, the full conversation suite per mutant
|
|
(`~/dewey-scratch/s5/mut-tools/run.sh`). After each run the runner lists
|
|
any shim left under the mutant's TMPDIR, then SIGKILLs its scope and PID.
|
|
|
|
| Mutant | Change | Result | Shims left after the suite | Killed by |
|
|
|---|---|---|---|---|
|
|
| base | none | 165/165 | none | (baseline) |
|
|
| Mr | `holds = false;` after re-parsing held text | killed, 164/165 | none | terminal: a hold inside held input holds again … |
|
|
| noprobe | W5 doesn't reap its probe | killed, 164/165 | none | claim: no shim from this file's tests is left running for the after hook |
|
|
| now13 | W13 doesn't reap its fixture | killed, 164/165 | none | the same test |
|
|
| afterleak | noprobe, and `after` doesn't reap or sweep | killed, 164 pass, 2 fail | 1 (`mosaic-chat-c719df56e9490f21f5919fe8e`, killed by the runner) | the same test, and the claim file's `after` hook |
|
|
|
|
The `afterleak` hook failure:
|
|
|
|
```
|
|
✖ .../packages/conversation/tests/claim.test.mjs (5101.703413ms)
|
|
AssertionError [ERR_ASSERTION]: no shim outlives this file (#1533)
|
|
+ [
|
|
+ {
|
|
+ pid: 3106581,
|
|
+ socket: '/home/jwoltje/dewey-scratch/s5/tmp/chat03-S6WdOu/f55/sock/shim-c719df56e949.sock',
|
|
+ unit: 'mosaic-chat-c719df56e9490f21f5919fe8e'
|
|
+ }
|
|
+ ]
|
|
- []
|
|
```
|
|
|
|
That run is the round 2 leak reproduced on purpose: the probe's shim
|
|
outlives the suite. With the candidate's `after` hook it can't.
|
|
|
|
## Gate
|
|
|
|
Sequential, on a detached worktree of 21e0f1a3 with the candidate's test
|
|
changes applied (patch sha256 `f57759e0…41df9d`), 03:42:59Z to 03:47:11Z,
|
|
output in `~/dewey-scratch/s5/r47-gate/out/`:
|
|
|
|
- conversation 165/0, webui 22/0
|
|
- test-auth 15/0, test-conductor 17/0, test-config 24/0, test-discord 66/0,
|
|
test-extension-package 18/0, test-foundation 44/0, test-release 14/0,
|
|
test-task 98/0
|
|
- test-queue 27/0 (node 148/0). Its canonical-root check skips in a
|
|
worktree, as it does in every worktree gate.
|
|
|
|
No shim was running after the gate.
|