From ea0632412e07dfd428c2c5a5236da89831109b8e Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 17:39:27 -0400 Subject: [PATCH 01/10] docs: record dired Stage 1 (#165) as landed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #165's own commits could not update the handoff snapshot to name the merge that contains them, so the protocol obligation lands here. - `docs/agent-handoff.md`: absorb the dired lane into §1, replacing the placeholder that promised exactly this. The bullet carries Stage 1's durable substrate facts — why the tolerant `read_dir` had to be Rust, why exposing the core normalizer beat mirroring it in Lua, the fixed-width `_layout` contract Stage 3 reads offsets from, the ambient-action buffer guard, treating a failure as the answer instead of probing, the per-entry error cap, the first mode-scoped keymap and the pre-existing test it broke, and the dedication a descent does not carry. Refresh the head-of-`main` anchor and the last-updated line. - `docs/agent-handoff.md` §5: two ops lessons that cost real time. A fix must be committed before it is bitten, because `scripts/bite` restores by `git checkout --` and reverts to HEAD; a CONFLICTING PR runs no CI at all, because `pull_request` workflows build a merge ref GitHub does not create while the branch conflicts, and nothing reports the absence. - `docs/active-work.md`: remove the merged lane per update-protocol rule 4 and summarize it under "Closed since the last snapshot", keeping the two forward items Stage 2 needs (the rename rebind is first-match-only over a raw path, and Q#DR5's seam is the main-thread drain). Refresh the canonical base. Flag the two lane headers that still call a merged PR "IN REVIEW" — #161 and #166 — rather than editing lanes another thread owns. - `COHERENCE.md`: #165 is no longer a PR. Per §25 the audited claims this work changed were updated when it landed; this corrects their tense in seven places and the two prose lines that still asserted dired was in flight. - `docs/dired-framing.md`: status line to MERGED, and state plainly that Stages 2 and 3 each still need their own framing. Docs only; no code, no gate-relevant change. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0126d2sikA6jZpFin3rtLCSK --- COHERENCE.md | 21 +++-- docs/active-work.md | 201 ++++++++++-------------------------------- docs/agent-handoff.md | 113 +++++++++++++++++++++--- docs/dired-framing.md | 4 +- 4 files changed, 164 insertions(+), 175 deletions(-) diff --git a/COHERENCE.md b/COHERENCE.md index 95761b2..1cfba71 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -120,7 +120,7 @@ asymmetry**, and **per-arc coherence debt**. Coherence-shaped work already in flight at audit time: find-file / dired Stage 0 (`C-x C-f`, merged #162, `docs/dired-framing.md`) and its -Stage 1 directory view (PR #165), bottom panel Stage 1 (merged #155), +Stage 1 directory view (merged #165), bottom panel Stage 1 (merged #155), multi-root LSP affinity (merged #161), the config registry foundation (merged #127). @@ -195,7 +195,7 @@ working, unreachable capability: time; a complete 1,384-line dired existed only as a frozen test fixture (`tests/fixtures/pmacs-dired/init.lua`). **Fixed:** dired Stage 0 opens a path (`C-x C-f`, merged #162) and Stage 1 ships the - browsing view as a builtin (`C-x d` / `C-x C-j`, PR #165). The fixture + browsing view as a builtin (`C-x d` / `C-x C-j`, merged #165). The fixture stays frozen — its `install_local` + `require` routing *is* the M8 package-universality proof (Q#DR1) — and shrinking it is scheduled after Stage 3. @@ -363,11 +363,11 @@ Full verdict table: |---|---|---|---| | 1 | Install | **Partial** | Source build only: `cargo build --release --workspace --features pmacs/crdt` (`README.md`). No binaries, no packaging. Runtime deps (`/bin/sh`, git, tar, coreutils) documented, never checked at runtime | | 2 | Launch unconfigured | **Works** | `EditorState::new()` → empty `*scratch*`; missing config is not an error (`src/config.rs:7-9`); recentf/saveplace/autosave default-on | -| 3 | Open real project | **Missing at the CLI** | `pmacs .` still exits 1 (above): `load_file` does `File::open` (which succeeds on a directory) then `read_to_end` → EISDIR, which is not `NotFound`, so `resolve_target_buffer`'s create-a-`[new file]` arm never fires. Dired Stage 1 (PR #165) supplies the buffer a directory should resolve *to*; routing `pmacs .` into it is Journey Stage 1's work, which must not invent a second directory surface | +| 3 | Open real project | **Missing at the CLI** | `pmacs .` still exits 1 (above): `load_file` does `File::open` (which succeeds on a directory) then `read_to_end` → EISDIR, which is not `NotFound`, so `resolve_target_buffer`'s create-a-`[new file]` arm never fires. Dired Stage 1 (merged #165) supplies the buffer a directory should resolve *to*; routing `pmacs .` into it is Journey Stage 1's work, which must not invent a second directory surface | | 4 | Understand interface | **Partial** | Mode line gives name/modified/L:C/scroll + mode/LSP/terminal segments; but no welcome text (`EditorCore::new` sets `status: String::new()`), no cheat sheet, and `C-h` deletes a word (§18) | | 5 | Edit | **Works** | Full CUA + Emacs keymap in 161 lines (`builtin/keymaps/default.lua`); isearch, query-replace, kill ring, undo/redo, auto-indent/pair/comment, atomic save. Genuinely excellent zero-config | | 6 | Language intelligence | **Partial** | Rust grammar bundled and auto-attaches; rust-analyzer preconfigured (`builtin/runtime/lsp.lua:44-52`) — but a missing binary fails silently (§1.2) and highlighting masks it. No LSP status command exists to diagnose | -| 7 | Find symbol / file | **File: fixed (open by path merged #162; browsing PR #165). Symbol: works but undiscoverable** | No find-file/dired/picker existed at audit. Now `C-x C-f` opens a known path and `C-x d` / `C-x C-j` browse (flat listing, `dired` mode keymap); `M-.`/`M-?`/`C-c o` still bound but advertised nowhere and server-gated; no workspace-symbol command; `pmacs.index.*` has no UI | +| 7 | Find symbol / file | **File: fixed (open by path merged #162; browsing #165). Symbol: works but undiscoverable** | No find-file/dired/picker existed at audit. Now `C-x C-f` opens a known path and `C-x d` / `C-x C-j` browse (flat listing, `dired` mode keymap); `M-.`/`M-?`/`C-c o` still bound but advertised nowhere and server-gated; no workspace-symbol command; `pmacs.index.*` has no UI | | 8 | Open terminal | **Works but undiscoverable** | Full PTY with scrollback + modeline segment — reachable only as `M-x terminal`, no keybinding. *Was broken outright on the GPU frontend until the double terminal-layout sync was fixed: the child took a `SIGWINCH` storm at tick cadence, so typing into it was impossible while output still flowed.* | | 9 | Build / test | **Partial** | `M-x compile.run` works, defaults cwd to detected project root, parses Rust `-->` errors — but no keybinding, an **empty first prompt** (`initial = last and last.cmdline or ""`, `builtin/runtime/compile.lua:1134-1138`), and no `cargo build`/`cargo test` suggestion despite `ProjectKind::Cargo` existing (`src/project.rs:77`) | | 10 | Inspect error | **Partial (good once reached)** | `E:n W:n` modeline counts, underlines, `M-g n/p` + ``C-x ` `` walking a unified compile/grep/diag source, message echo, `RET` visits. Gated entirely on step 6 or 9 succeeding first | @@ -460,7 +460,7 @@ level is the one missing. Audited level-by-level: **Beginner** (should see: files, buffers, search, diagnostics, terminal, build actions, menus, missing-tool guidance): -- files ✓ since #162 / PR #165 (`C-x C-f` opens a path, `C-x d` browses; +- files ✓ since #162 / #165 (`C-x C-f` opens a path, `C-x d` browses; neither is advertised anywhere but the keymap) · buffers ✓ (`C-x b`, `*buffer-list*`) · search ✓ (`C-s`/`C-r`/`C-M-s`; project.search is M-x-only) · diagnostics ✓ once a server runs · terminal ✓ but @@ -1183,7 +1183,7 @@ Primitive-by-primitive against the list above: hierarchy, package dependency graph, worker trees, git status) will each need it; building it once *before* dired's directory view and the workers tree harden their own conventions is exactly this - section's point. Dired Stage 1 (PR #165) landed **without** inventing + section's point. Dired Stage 1 (merged #165) landed **without** inventing one: its listing is flat (Emacs parity), and the recursive in-buffer case — `i` insert-subdirectory — is a named deferral in `docs/dired-framing.md` §13, which is where a shared tree primitive @@ -1340,7 +1340,8 @@ greets a new user says nothing (`EditorCore::new` sets an empty status). Note the dependency: five of the ten onboarding steps above currently -lead somewhere broken or invisible (find a file — in flight; inspect a +lead somewhere broken or invisible (find a file — the mechanism is fixed +since #162/#165 but is advertised nowhere except the keymap; inspect a diagnostic — silent-failure risk; view workers — undiscoverable; setting provenance — unanswerable). Onboarding is correctly sequenced *after* the P1/P4 fixes, but the cheap floor — a welcome buffer in @@ -1404,7 +1405,7 @@ Establish the end-to-end workflow; treat regressions as release blockers. **State: broken at step 3 (§2). Mostly wiring, and unusually cheap:** directory-argument handling (the remaining half of step 3 — dired Stage 1 landed the buffer it should resolve to); a find-file -surface (**done**: #162 open-by-path, PR #165 browsing); surfacing the +surface (**done**: #162 open-by-path, #165 browsing); surfacing the LSP spawn failure with guidance (§1.2); a compile keybinding + `cargo build`/`test` default from the existing `ProjectKind::Cargo`; a terminal keybinding; a welcome buffer. The @@ -1474,7 +1475,9 @@ implementation — this list is direction, not commitment): 1. **Journey Stage 1** (P1): directory open + compile defaults + LSP-failure surfacing + bindings + welcome buffer + the first - journey acceptance suite. Rides alongside the in-flight dired arc. + journey acceptance suite. Dired Stage 1 has landed (#165), so the + buffer a directory resolves *to* already exists; this arc routes + `pmacs .` into it rather than growing a second directory surface. 2. **Discovery surface** (P4): the describe/list/where-is command family, M-x rich rows, help unification, help prefix. 3. **Transient keymap layer** (§6): the overlay scope + lifetime diff --git a/docs/active-work.md b/docs/active-work.md index c934c30..1266e40 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -5,6 +5,14 @@ landed on `main`. Read it after `docs/agent-handoff.md`. Remove completed entries when their PR merges; do not let this become a second permanent backlog. +**Two lane headers below are stale on purpose**, pending the docs updates +their own lanes owe: multi-root LSP affinity **#161 has merged** (the +Lean 4 lane still says IN REVIEW; its continuation is PR #167) and GPU +terminal input **#166 has merged** (its lane still says IN REVIEW; PR +#168 records it). Trust the canonical-base line below over a lane header: +if a PR number appears in `git log --first-parent githubsucks/main`, it +has landed regardless of what its lane says. + ## Repository authority - Canonical development URL: @@ -14,11 +22,14 @@ backlog. machine-local: `origin` may name this canonical URL, a release mirror, or something else, and therefore has no authority by name alone. - Canonical base at this snapshot: - `githubsucks/main` @ `8c86d34` (the dired framing #164 atop find-file - #162, COHERENCE.md #163, Lean 4 Stage 1 #160, the minimap blank-slab fix - #159, bottom-panel Stage 1 #155, the inline-math re-scout #154, the vterm - PTY-flake fix #153, and the GPU initial-target doc refresh #152; protocol - v20). + `githubsucks/main` @ `c8ec8f3` (dired Stage 1 #165 atop GPU terminal + input #166, multi-root LSP affinity #161, the dired framing #164, + find-file #162, COHERENCE.md #163, Lean 4 Stage 1 #160, the minimap + blank-slab fix #159, bottom-panel Stage 1 #155, the inline-math re-scout + #154, the vterm PTY-flake fix #153, and the GPU initial-target doc + refresh #152; protocol v20). **Lanes below that name an older base have + not been re-based; derive their integration surface from + `git diff ..main`.** - On the transfer source, `origin/main` named a release mirror at `d3fa632` and lagged badly. On the current destination, `origin` names the canonical URL. This difference is why all recovery begins by @@ -189,154 +200,6 @@ If it does not, stop and repair the remote/fetch configuration. suites**; `git diff --check` clean. The sweep needs an isolated `XDG_CONFIG_HOME` and `-- --skip basedpyright`. -## Dired lane — Stage 0 MERGED; Stage 1 IN REVIEW (PR #165) - -- Approved framing: `docs/dired-framing.md` **revision 6** — rev 5 is the - approved text (merged as its own docs PR #164), rev 6 adds §0's Stage 1 - implementation notes (S1-1…S1-9). Stages 2 (marks and operations) and 3 - (wdired) each get their own detailed framing after the prior stage lands. -- **Stage 0 (`C-x C-f` find-file) MERGED as #162** (`main` @ `2af1ab3`, - 2026-07-25, one review round, 12/12 CI green). Durable facts moved to - `docs/agent-handoff.md` §1 per rule 3 below. -- **Stage 1 branch: `githubsucks/dired-stage1`**, worktree - `../pmacs-dired-stage1`, based on `githubsucks/main` @ `8c86d34` (the - framing merge #164). **A fresh cut, not a rebase:** the older `dired` - branch (`ffdd642`, worktree `../pmacs-dired-arc`) was based on the - superseded `0827dd1` and carried only the framing content #164 already - put on `main`, so merging it would have reconciled two histories of one - document. It is left untouched and carries nothing unmerged. -- **Stage 1 implemented; no wire change (protocol stays v20).** What - landed on the branch: - - `builtin/runtime/dired.lua`: one buffer per directory named - `*dired:*` with the handle-table ownership check; - read-only intercept + `set_round_trip_input`; the `dired` major mode - and its mode-scoped keymap (`RET`/`f`, `^`, `n`/`p`, `g`, `q`, `s`); - basename cursor re-seating across every wholesale repaint; - `display_file` for file visits and same-window reuse for directory - descent; `C-x d` / `C-x C-j`; the `dired.kill-when-opening` setting. - Loaded after `window.lua`. - - `src/fs.rs`: `ReadDirTolerance`, `FsDirEntryError`, `FsDirListing`, - and one walk that either fails on a per-entry condition or records it - (Q#DR6). `src/async_runtime.rs` carries the listing in - `ReplyKind::ReadDir` / `JobResult::ReadDir`; `src/lua_bindings/mod.rs` - keys the Lua result **shape** on `errors.is_some()`, so the bare array - the frozen M8.2 fixture consumes with `ipairs` is untouched; - `builtin/runtime/fs.lua` validates read-op opts and **rejects unknown - keys** (a typo'd `tolerant` used to degrade silently to fatal). - - `src/editor_core.rs` + `src/lua_bindings/mod.rs`: - `normalize_buffer_path` is `pub` and exposed as - `pmacs.path.canonicalize` — Q#DR2's preferred end state, so no Lua - mirror exists and Stage 2 owes no mirror removal. This makes B2 - ("tolerant `read_dir` is the only Rust change") false by one small - binding, deliberately. - - `tests/dired_acceptance.rs`: 22 tests over framing items 1–16, - dispatch-driven; item 17 is the m8_1/m8_2/m8_3 additivity gate. -- **The framing claim the substrate falsified (S1-2):** R2-3 expected a - dedicated dired panel to carry its dedication across a descent. - `display_buffer` never replaces the buffer in a slot dedicated to - another one — it discards every side-specific parameter and falls back - to the document window (Q#BP3 2.iii), and the exact-window arm errors. - Dired does not unpin the user's panel; both arms are pinned. -- **The vacuity the bites found (S1-3):** acceptance 3c cannot pin the - descent *routing*. Dired holds focus in its own panel, so a raw - `switch_buffer` lands in the same window and every 3c assertion holds - either way. Dedication is the only discriminator, so the - dedicated-panel test is the real pin — and the vacuity is documented at - the assertion rather than relabelled. -- **The pre-existing test dired's first mode-scoped binding broke - (S1-4):** `describe_key_identifies_every_default_binding` asserted every - binding in the stack resolves through `describe.key` context-free, which - held only while the modes table was empty. It now sets the effective - context per binding and explicitly *clears* the mode for global ones, - because a leaked mode legitimately shadows a global chord of the same - name (dired's `RET` shadows `edit.newline-and-indent`). -- Durable substrate facts, independent of this arc: - - `pmacs.buffer.kill` (not `remove`) redirects windows off a doomed - buffer before removal, so `kill-when-opening` kills **after** the - replacement is displayed. - - Interactive origin does **not** survive an await: work resumed in - `tick_async` sees no `InteractiveCommandOrigin`, so `pmacs.window.*` - acts for the *ambient* active frontend (S1-9). - - Kinds are lstat-based in both `read_dir` and `stat`, so nothing in an - entry says whether a symlink points at a directory; `RET` probes by - trying to list it (S1-8). - - A path-backed buffer's *name* is its full path, not its basename — - worth knowing before writing any name assertion. - - `C-x d` takes **no** completion source on purpose (S1-5): with one, - RET on an empty field opens whatever sorts first, and - RET-on-where-you-are is the gesture the binding exists for. The field - is prefilled instead. -- **Bite verification:** 15 claims, each mutated in place and required to - fail the test that names it. `dired.lua` is new, so `scripts/bite`'s - file swap does not apply; every mutation was applied and reverted with - `git checkout --`. One came back VACUOUS and is recorded above. -- **Review round 1 addressed** (framing rev 7, S1-10…S1-12). Three - behavioral fixes, each bite-verified: `dired.revert`'s re-seat is - guarded on the active buffer (an ambient `move_to_line` after an await - moved an unrelated buffer's cursor — the buffer-level instance of - S1-9); `fmt_size` keeps the column width past ten digits, because - `_layout` is a contract Stage 3 is planned against; and the symlink - descent dropped its probe, since `open_directory`'s - changed-nothing-on-failure invariant *is* the probe (it was listing the - target directory twice). Plus a consecutive-`readdir`-error cap, because - **nothing cancels a dired listing** — it carries no supersede key, so - cancellation was never the backstop the tolerant loop implicitly relied - on. Naming/comment findings taken as-is. - - Durable process lesson, hit twice now: a mutation-bite helper restores - with `git checkout --`, which reverts to **HEAD** — so a fix must be - committed *before* it is bitten. Round 1's fixes were briefly wiped by - exactly that. -- **Canonical main integrated twice** — at `46a1b8f` (multi-root LSP - affinity #161) and again at `b889873` (GPU terminal input #166), both - merged rather than rebased per the #135/#137 precedent so the review - anchors stay addressable. Each conflict was a single doc hunk resolved - as the union: this lane owns COHERENCE's journey step 7 file half, #161 - owns the in-flight list, #166 owns step 8's GPU-terminal addendum. - Three things worth carrying: - - **A conflicting PR silently stops running CI.** GitHub builds - `pull_request` runs against the merge ref, which does not exist while - the PR conflicts, so no run is created and nothing reports a - failure — the checks list simply stays as it was. Three pushes to - this branch produced no CI at all before the cause was found. Watch - `mergeable` on a long-lived lane, not just the check list. - - #161's own COHERENCE finding **falsified a claim in this lane's - module doc**: `pmacs.error` is never defined in production, so an - uncaught raise inside a `pmacs.async` coroutine does not reach - `*errors*` as the comment said. It reaches a bare `error()` inside - `pmacs._async.tick()`, whose result `tick_async` discards with - `let _ =` — i.e. nowhere. That makes dired's per-coroutine `pcall` + - `set_status` load-bearing rather than tidy, and the comment now says - so. - - **A lane in review against a fast-moving `main` needs its gates rerun - per integration, not per push.** Main advanced twice inside this - review round, and the second time landed while the first - integration's sweep was still running. The numbers below describe the - twice-merged tree. -- Verification on the twice-merged tree (`main` @ `b889873`): - `cargo fmt --check` clean; strict workspace Clippy clean; **1,832 - default + 2,009 CRDT** library tests; dired acceptance **25 default + - 25 CRDT**; m8_1 10 / m8_2 15 / m8_3 32 unchanged; multi-root 13 and - vterm Stage 3 5 (both suites main added, green under this lane's - `mod.rs` and `editor.rs` changes); M4 121; required GPU 155; - **isolated-`XDG_CONFIG_HOME` workspace sweep 3,205 passed across 93 - suites, zero failures**; `git diff --check` clean. The sweep needs the - isolated config for the reason recorded in the bottom-panel lane - below. -- Coherence (framing §0.5, required since #163): serves `COHERENCE.md` §20 - Priority 1, which names this work explicitly; journey step 7's file half - goes from no surface to a surface; **adds no interaction island** — keys - are a mode-scoped keymap, and wdired will be a mode swap; adopts - `pmacs.config` for `dired.kill-when-opening`; inherits §9's - worker-attribution gap for its `read_dir` jobs without worsening it. The - audited claims this changes are updated in `COHERENCE.md` itself, per its - §25. -- **Boundary with the Journey Stage 1 arc** (`COHERENCE.md` §20 arc-cut - 1): CLI directory-argument handling (`pmacs .` exits 1) belongs there, - not here — Stage 1 does **not** fix it. The two meet at - `resolve_target_buffer`; dired supplies the buffer a directory should - resolve *to*, and `pmacs .` should route into it rather than growing a - second directory surface. - ## GPU terminal input lane — IN REVIEW - Portable branch: `githubsucks/gpu-terminal-input`, worktree @@ -593,6 +456,38 @@ git worktree add --track \ ## Closed since the last snapshot +- **Dired Stage 1 (the directory view) — MERGED as #165** (`main` @ + `c8ec8f3`, 2026-07-25, after one review round). pmacs has a directory + surface: `C-x d` / `C-x C-j`, one read-only buffer per directory named + `*dired:*`, a `dired` major mode carrying + `RET`/`f`, `^`, `n`/`p`, `g`, `q`, `s`. No wire change (v20). The Rust is + two things — a per-entry-tolerant `read_dir` (Q#DR6), which had to be + Rust because `read_dir_blocking` fails a whole listing on any of five + per-entry conditions and a tolerant wrapper cannot be written in Lua at + all, and `normalize_buffer_path` going `pub` as + `pmacs.path.canonicalize` (Q#DR2's preferred end state, so no Lua mirror + exists and Stage 2 owes no mirror removal). The frozen m8_1/m8_2/m8_3 + counts are unchanged, which is the additivity gate. 15 claims + bite-verified; one came back VACUOUS (acceptance 3c cannot pin descent + routing — dired holds focus in its own panel, so dedication is the only + discriminator) and is documented at the assertion rather than + relabelled. Its branch (`dired-stage1`) and worktree + (`../pmacs-dired-stage1`) are done; the abandoned `dired` branch + (`ffdd642`, `../pmacs-dired-arc`) was superseded by a fresh cut and + carries nothing unmerged. **Stage 2 (marks and operations) and Stage 3 + (wdired) each still need their own framing**, and the frozen fixture + shrinks after Stage 3. Durable substrate facts and both new ops lessons + live in `docs/agent-handoff.md` §§1/5; the implementation notes are + `docs/dired-framing.md` §0, S1-1…S1-12. Two named forward items for + Stage 2: `apply_resource_op`'s rename rebind is exact-PathBuf-equality, + first-match-only, looked up with the raw path while stored paths are + normalized — so a directory rename strands every buffer under it, and + `pmacs.fs.rename` has zero production callers, so it can be fixed at + the primitive; and Q#DR5's seam is the main-thread drain + `AsyncRuntime::tick`, not `_take_result`, where rename settles as an + undifferentiated `ReplyKind::FsUnit` and so must be keyed on + `JobKind::FsRename`. + - **GPU initial target — MERGED as #148** (`main` @ `0dd16a5`, 2026-07-24, after two review rounds). `pmacs --gpu [--socket …] FILE` opens a target before the GPU window appears. Protocol bumped 19 → 20: a semantic-session diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index bb5677a..f6900cc 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -1,7 +1,9 @@ # Agent handoff — cross-machine continuity -**Last updated: 2026-07-25, after find-file (#162) landed — the dired -arc's Stage 0 — following COHERENCE.md (#163), Lean 4 Stage 1 (#160), the +**Last updated: 2026-07-25, after dired Stage 1 (#165) landed — the +directory view — following GPU terminal input (#166), multi-root LSP +affinity (#161), the dired framing (#164), find-file (#162) — the dired +arc's Stage 0 — COHERENCE.md (#163), Lean 4 Stage 1 (#160), the minimap blank-slab fix (#159), bottom-panel Stage 1 (#155), the inline-math re-scout (#154), the vterm PTY-flake fix (#153), and the GPU initial-target doc refresh (#152); and before that GPU @@ -26,11 +28,13 @@ commands, read `docs/active-work.md` immediately after this file. ## 1. Where the project stands (2026-07-25) -- `main` @ `2af1ab3` (find-file #162 atop COHERENCE.md #163, Lean 4 Stage 1 - #160, minimap blank-slab #159, bottom-panel Stage 1 #155, inline-math - re-scout #154, vterm PTY-flake #153, and doc refresh #152). Protocol - unchanged at **v20**. The bullets below describe the arcs in their own - terms; this line is the head-of-`main` anchor. +- `main` @ `c8ec8f3` (dired Stage 1 #165 atop GPU terminal input #166, + multi-root LSP affinity #161, the dired framing #164, find-file #162, + COHERENCE.md #163, Lean 4 Stage 1 #160, minimap blank-slab #159, + bottom-panel Stage 1 #155, inline-math re-scout #154, vterm PTY-flake + #153, and doc refresh #152). Protocol unchanged at **v20**. The bullets + below describe the arcs in their own terms; this line is the + head-of-`main` anchor. - **`COHERENCE.md` is now required reading and a required framing input — #163.** It carries the product-coherence thesis, an audited scorecard, per-concern gaps, and §20's priority order, and it is the @@ -70,11 +74,78 @@ commands, read `docs/active-work.md` immediately after this file. against an open buffer yet fails to load one that is not open — find-file expands the tilde Lua-side. Loading through the normalized path is a named deferral. - - **Stage 1 (the directory view) is IN REVIEW as PR #165** — the - builtin `dired.lua`, the per-entry-tolerant `read_dir` opt, and - `pmacs.path.canonicalize`. Its branch state, substrate facts, and - verification live in `docs/active-work.md`; this section absorbs them - when it merges. +- **dired Stage 1 — the directory view — LANDED — #165** + (`docs/dired-framing.md` §0, S1-1…S1-12; merge `c8ec8f3`; one review + round). pmacs now has a directory surface: `C-x d` / `C-x C-j` open a + read-only listing, one buffer per directory named + `*dired:*`, with a `dired` major mode whose + mode-scoped keymap carries `RET`/`f`, `^`, `n`/`p`, `g`, `q`, `s`. + Protocol unchanged at **v20**. **Stage 2 (marks and operations) and + Stage 3 (wdired) each still need their own framing**; the frozen + fixture shrinks after Stage 3. + - **The Rust is confined to two things**: a per-entry-tolerant + `read_dir` (`ReadDirTolerance {Fatal, PerEntry}` → + `FsDirListing {entries, errors}`), because `read_dir_blocking` fails + a whole listing on any of five per-entry conditions and the tolerant + wrapper its own module doc delegates to package authors **cannot be + written in Lua** (one error value, no partial vec); and + `editor_core::normalize_buffer_path` becoming `pub`, exposed as + `pmacs.path.canonicalize`. Only non-UTF-8 **names** stay fatal — + byte-preserving paths would be needed. The Lua result **shape** keys + on `errors.is_some()`, so the bare array the frozen M8.2 fixture + consumes with `ipairs` is untouched. + - **Exposing a core normalizer beat mirroring it in Lua.** A Lua mirror + would have been a second canonical form — the same class of bug as + the five tab-width constants (#137). Applies to any future Lua-side + path reckoning. + - **A fixed-width column must be fixed-width for every input.** The + exported `pmacs.dired._layout` (MARK 0, KIND 2, PERMS 3–12, SIZE 13, + MTIME 24, NAME 41) is the contract Stage 3 reads offsets from, and + `%10d` overflows at ≥10 GB, silently shifting every column right of + it. Sizes now fall back to a width-clamped magnitude (K/M/G/T/P/E). + - **An ambient action must be gated on the buffer it assumes.** A + revert's cursor re-seat settles a tick or more later, by which time + the user may have switched buffers; the paint names its buffer and is + safe, but seating is ambient. This is the buffer-level instance of + the rule below that interactive origin does not survive an await. + - **A failure IS an answer — don't probe first.** Kinds are lstat-based + in both `read_dir` and `stat`, so nothing in an entry says whether a + symlink points at a directory. `RET` tries to list it and treats the + failure as the answer; an explicit probe was a second full + `read_dir`, so a descent listed twice. + - **Unbounded per-entry error collection needs a cap when nothing + cancels the work.** A dired listing carries no supersede key, so + cancellation was never the backstop the tolerant loop implicitly + relied on (`READDIR_MAX_CONSECUTIVE_ENTRY_ERRORS = 1024`). + - **This is the first builtin with mode-scoped keys** (#129's first + non-detection consumer), which broke the pre-existing + `describe_key_identifies_every_default_binding`: it asserted every + binding resolves through `describe.key` context-free, which held only + while the modes table was empty. It now sets the effective context + per binding and explicitly **clears** the mode for global ones, + because a leaked mode legitimately shadows a global chord of the same + name (dired's `RET` shadows `edit.newline-and-indent`), plus a floor + assertion that at least one mode-scoped binding exists. + - **A dedicated panel does not carry its dedication across a descent** + — the framing expected it to. `display_buffer` never replaces the + buffer in a slot dedicated to another one; it discards every + side-specific parameter and falls back to the document window (Q#BP3 + 2.iii), and the exact-window arm errors. Dired does not unpin the + user's panel; both arms are pinned. + - Smaller facts worth knowing before touching this code: a path-backed + buffer's **name is its full path**, not its basename, which matters + for any name assertion; `pmacs.buffer.kill` (not `remove`) redirects + windows off a doomed buffer first, so `dired.kill-when-opening` kills + **after** the replacement is displayed; ownership is checked against + the handle table only, never the buffer name; and `C-x d` takes **no** + completion source on purpose (with one, `RET` on an empty field opens + whatever sorts first, and RET-where-you-are is the gesture the binding + exists for — the field is prefilled instead). + - Verification at merge: 1,832 default + 2,009 CRDT library tests; + dired acceptance 25 + 25 CRDT; the frozen m8_1 10 / m8_2 15 / m8_3 32 + unchanged, which is the additivity gate for the `read_dir` change; M4 + 121; required GPU 155; isolated-`XDG_CONFIG_HOME` workspace sweep + 3,205 across 93 suites. 15 claims bite-verified. - **GPU initial target LANDED — #148** (`docs/gpu-initial-target-framing.md` rev 3; merge `0dd16a5`; two review rounds). `pmacs --gpu [--socket NAME|PATH] FILE` transports exact Unix path @@ -760,6 +831,24 @@ final variant — its own round-trip cannot detect a discriminant shift. trap-guarded one-file swap over read-only `git show`, with an inverted verdict (exit 0 iff the tests FAIL against the old version), making bite-verification machine-checkable. +- **A fix must be COMMITTED before it is bitten.** `scripts/bite` + restores by `git checkout --`, which reverts the file to **HEAD**, not + to the state it found — so any uncommitted work in a bitten file is + destroyed. A whole review round's fixes were wiped this way during + #165. Corollary for a NEW file: the swap-over-`git show` mode does not + apply at all, so its claims must be bitten by hand-editing, which makes + the commit-first rule load-bearing rather than hygienic. +- **A CONFLICTING PR silently runs no CI at all.** GitHub builds + `pull_request` workflow runs against the PR's **merge ref**, which it + does not create while the branch conflicts with its base. So pushes + land, the branch updates, no run is ever queued, and **nothing reports + the absence** — the checks list simply keeps showing the last + successful run, which reads as current. Three pushes to #165 produced + zero CI before the cause was found, and `gh pr checks` returns nothing + usable here. On any lane that lives through a moving `main`, check + `gh pr view --json mergeable,mergeStateStatus,headRefOid` and + confirm a run exists **for the current head sha**, not merely that a + recent run was green. - **Stacked PRs**: retarget the child to main BEFORE merging the parent — GitHub auto-closes a PR whose base branch is deleted and cannot reopen it (#104 → re-opened as #105). diff --git a/docs/dired-framing.md b/docs/dired-framing.md index ada853e..83e6f1a 100644 --- a/docs/dired-framing.md +++ b/docs/dired-framing.md @@ -1,7 +1,9 @@ # Dired — framing **Revision 7 — 2026-07-25. Status: APPROVED; Stage 0 MERGED as #162; -Stage 1 IN REVIEW as PR #165, review round 1 addressed.** +Stage 1 MERGED as #165 (`main` @ `c8ec8f3`, one review round). Stage 2 +(marks and operations) and Stage 3 (wdired) each still need their own +framing before implementation; the frozen fixture shrinks after Stage 3.** Rev 1 passed a ground-truth review; rev 2 fixed round 1's seven findings; rev 3 fixed round 2's six and was approved; rev 4 recorded what Stage 0's implementation falsified in the approved text (§0); rev 5 adds the From dea1d20e4eaea101020af3997e0016668de159c6 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 17:58:00 -0400 Subject: [PATCH 02/10] docs: refresh the canonical base for #158 The inline-math slice landed while this PR was open. Its own merge removed its ledger lane, so the stale-header note above still names exactly two; only the base anchor needed moving. --- docs/active-work.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index 1266e40..d7a8c9d 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -22,8 +22,9 @@ has landed regardless of what its lane says. machine-local: `origin` may name this canonical URL, a release mirror, or something else, and therefore has no authority by name alone. - Canonical base at this snapshot: - `githubsucks/main` @ `c8ec8f3` (dired Stage 1 #165 atop GPU terminal - input #166, multi-root LSP affinity #161, the dired framing #164, + `githubsucks/main` @ `5aa9044` (the inline-math slice #158 atop dired + Stage 1 #165, GPU terminal input #166, multi-root LSP affinity #161, + the dired framing #164, find-file #162, COHERENCE.md #163, Lean 4 Stage 1 #160, the minimap blank-slab fix #159, bottom-panel Stage 1 #155, the inline-math re-scout #154, the vterm PTY-flake fix #153, and the GPU initial-target doc From 2b42204693a0df3099e5a4cad0bd43951387abb9 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 21:10:16 -0400 Subject: [PATCH 03/10] docs: integrate #175 and align the recovery threshold with the base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #175 (bottom-panel Stage 2 framing) landed after this branch's last head and touches both shared docs, so the previous green run did not cover the combination. Merged cleanly this time — no conflict. Also fixes an inconsistency this PR introduced: the recovery check still accepted `d152120` while the canonical-base line above declared a newer commit. A threshold looser than the base it guards passes on a tree the rest of the file does not describe, so the two now move together and the text says why. --- docs/active-work.md | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index bfacee3..1ad3bdc 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -23,14 +23,14 @@ here too until #172 removed it — that is the update those two owe.) machine-local: `origin` may name this canonical URL, a release mirror, or something else, and therefore has no authority by name alone. - Canonical base at this snapshot: - `githubsucks/main` @ `ccf29e3` (the CRDT undo repro #157 atop the - inline-math landed-doc refresh #172, the bottom-panel landed-doc - refresh #156, the inline-math slice #158, dired Stage 1 #165, the GPU - terminal input fix #166, Lean 4 Stage 2 #161, the dired framing #164, - COHERENCE.md #163, find-file #162, Lean 4 Stage 1 #160, and the minimap - blank-slab fix #159; protocol v20). **Lanes below that name an older - base have not been re-based; derive their integration surface from - `git diff ..main`.** + `githubsucks/main` @ `c93f9ee` (the bottom-panel Stage 2 framing #175 + atop the CRDT undo repro #157, the inline-math landed-doc refresh #172, + the bottom-panel landed-doc refresh #156, the inline-math slice #158, + dired Stage 1 #165, the GPU terminal input fix #166, Lean 4 Stage 2 + #161, the dired framing #164, COHERENCE.md #163, find-file #162, Lean 4 + Stage 1 #160, and the minimap blank-slab fix #159; protocol v20). + **Lanes below that name an older base have not been re-based; derive + their integration surface from `git diff ..main`.** - On the transfer source, `origin/main` named a release mirror at `d3fa632` and lagged badly. On the current destination, `origin` names the canonical URL. This difference is why all recovery begins by @@ -64,7 +64,11 @@ git worktree list git status --short --branch ``` -The `git log` command must expose `d152120` or a newer intentional main. +The `git log` command must expose `c93f9ee` — the base named above — or a +newer intentional main. Keep this threshold and the canonical-base line in +step: a recovery check that accepts an older commit than the base it +declares canonical will pass on a tree the rest of this file does not +describe. If it does not, stop and repair the remote/fetch configuration. ## Lean 4 lane (Arc 8) — Stage 1 MERGED; Stage 2 IN REVIEW (PR #161) From dd581cd90ce1cc72c5d9a5f09caa3ca852b853b9 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 21:12:51 -0400 Subject: [PATCH 04/10] docs: frame the PTY terminate diagnostic (revision 4) A docs-only PR (#172) failed Test (macos-latest / luajit) on acc28_child_input_and_the_c_c_escape_work_unchanged_in_a_panel with "kill: EPERM: Operation not permitted" raised out of terminate. A docs diff cannot cause that, main was green at the PR's exact base, and three other PRs passed the same job. This framing reaches revision 4 after three review rounds, and what it proposes is much smaller than what it started with. Revisions 1 to 3 each proposed a tolerance rule -- treat some errno as success -- and each was unsound in the same way: they concluded something about a process from something that was not about that process. Revision 1 concluded from an errno alone, which says only that a syscall failed. Revision 2 concluded from the spawned leader while a PTY signal targets the tty's foreground process group, which diverges from the leader exactly when job control is in use. Revision 3 corrected EPERM but kept group-directed ESRCH, which proves only that the selected foreground group vanished, not that the leader exited. So no tolerance rule lands. The disposition is preserved exactly: every failing call still fails, with no state transition and no ledger arming. What lands is that the failure explains itself, recording the target source and value, the spawn-time pgid or leader pid, the errno, and the leader's real try_wait state as five separate facts. Every candidate fix is decidable from those together and none is decidable from the errno alone. Two claims are stated more narrowly than earlier revisions had them. Consulting try_wait reaps an exited child and caches its status, so this is not "strictly additive" -- it is "no disposition change", with an event-count test pinning that poll_one still emits exactly one exit event. And the test seam injects the kill attempt's result only, never the observation, so the real ChildHandle::try_wait runs against the real child; a stubbed observation would bypass the path under test. Parked with their reasons: all tolerance rules, terminate becoming idempotent for an already-reaped process (an independent fix answering a different failure), and signal_target's read-then-kill of tcgetpgrp, which is the most likely real fix site. The lane closes when this lands rather than waiting for the flake to recur; the next occurrence carries its own evidence under whoever's PR. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk --- docs/process-signal-tolerance-framing.md | 258 +++++++++++++++++++++++ 1 file changed, 258 insertions(+) create mode 100644 docs/process-signal-tolerance-framing.md diff --git a/docs/process-signal-tolerance-framing.md b/docs/process-signal-tolerance-framing.md new file mode 100644 index 0000000..1d3c45d --- /dev/null +++ b/docs/process-signal-tolerance-framing.md @@ -0,0 +1,258 @@ +# Framing — make the PTY terminate failure self-describing (diagnostic only) + +**Revision 4.** Status: awaiting review round 4. Lane: +`pty-terminate-eperm`, worktree `../pmacs-math-slice`, based on +`githubsucks/main` @ `ccf29e3`. + +**Diagnostic only. No disposition changes, no tolerance rules, no +behavioural fix.** Every rule this document proposed across revisions 1 +to 3 is parked (§5). The lane's entire deliverable is that the next +occurrence of the failure explains itself. + +## Revision history + +**Revision 3 → 4**, after review round 3 (two blocking, one major) and +its scope call. All accepted. + +- **Group-directed ESRCH was also unsafe**, for the same reason EPERM + was: it proves the selected *foreground group* vanished, not that the + leader exited. A job-control race — foreground job exits after + `tcgetpgrp` and before `kill`, shell alive and not yet reclaiming the + terminal — would have been reported as success with the leader never + signalled. Rev 3's acceptance 7 pinned that unsafe behaviour. **All + tolerance is parked** (§5). +- **Rev 3's Stage A implemented Stage B.** It declared itself + diagnostic-only, then listed tolerance and bookkeeping acceptances. + Removed. +- **Q#PS6 (already-reaped `terminate` is `Ok`) is parked separately.** + It is an independent behavioural fix answering a different failure; + under one-feature/one-PR it does not ride with instrumentation. +- **"Strictly additive / cannot regress behaviour" was overstated** and + is narrowed (Q#PD3). +- The injected-kill seam is restored as an explicit decision (Q#PD4). + +**Rounds 1–3, for the record.** Rev 1 classified on errno alone and +claimed a live owned child cannot yield EPERM — false. Rev 2 gated on +`try_wait`, which observes the leader while a PTY signal targets the +foreground group — unsound whenever those diverge, and it could not be +shown to fix the observed failure at all. Rev 3 corrected EPERM but left +ESRCH unsafe and mixed the stages. **Three consecutive designs were +wrong in the same direction: each tried to conclude something about a +process from something that was not about that process.** + + +## 0. Coherence impact (COHERENCE §20) + +- **Journey step 8, "Open a terminal"** (§2), teardown half. **No grade + change and no behavioural change** — this lane only improves what a + failure reports. +- **Serves §9 (worker model), failure attribution**, in its most literal + sense: an error that names only an errno cannot be attributed. +- **Interaction islands: none. Config registry: not adopted. + Background-work attribution: unchanged.** +- **No audited claim in COHERENCE.md changes**, so under §25 no + COHERENCE edit rides this PR. + + +## 1. Ground truth (scouted @ `ccf29e3`, re-verified each revision) + +### 1.1 The failure reports an errno and nothing else + +`ProcessSupervisor::signal` (`src/process.rs:921`) maps the `kill` +failure to `format!("kill: {e}")` (`:931`). That string is everything a +reader gets. + +### 1.2 The signal target is not the observation target + +- **Signal target** — `signal_target` (`:687`) returns `-pgrp` for a + PTY, where `pgrp = master.process_group_leader()`: the tty's + **current foreground process group**, read at signal time. +- **Observation target** — `ChildHandle::try_wait` (`:668`) observes the + **spawned leader**. + +They coincide only while the leader owns the terminal. Job control is +precisely the mechanism that makes them diverge, and the PTY path is +**always group-directed by design** — spawn rejects `group = true` for +PTY mode with the rationale that "PTY children already lead their own +session and are signaled group-wide" (`:1428-1429`). + +**This is why every tolerance rule across rev 1–3 failed review**, and +why the diagnostic must record the target and the leader state as +*separate* facts. + +### 1.3 The reap ledger is disjoint from this path + +`tick_reap_ledger` (`:1075`) treats any probe error as "nothing left we +can reach" for **bounded growth**, asserting EPERM "cannot happen for our +own children". It is armed only for `proc.spec.group`, which PTY mode +cannot set. Rev 1's "asymmetry" argument was a misreading; withdrawn. + +### 1.4 The observed failure, and the limits of the evidence + +macOS CI, PR #172 (**docs-only** diff), `Test (macos-latest / luajit)`, +`acc28_child_input_and_the_c_c_escape_work_unchanged_in_a_panel` +([attempt 1](https://github.com/levineuwirth/pmacs/actions/runs/30177276839/attempts/1)): + +``` +in function 'terminate' +cause: ExternalError(Process("kill: EPERM: Operation not permitted")) +``` + +**Established:** the errno, and the call path +(`pmacs.terminal.terminate` → `session.rs:566` → `signal`). + +**Not established:** that the child had exited (the probe's last source +statement is a file write at +`tests/bottom_panel_stage1_acceptance.rs:2239`; CPython teardown follows +and does not synchronise with it); that any pgid was recycled; or what +the signal target actually was. + +**This is the whole reason the lane is diagnostic.** Every candidate fix +needs at least one of those three facts, and none is available. + +### 1.5 Caller inventory + +| Caller | Disposition | +|---|---| +| `src/lsp.rs:1364`, `:2427` | discards (`let _ =`) | +| `src/mcp.rs:1229`, `:1239`, `:1915` | discards (`let _ =`) | +| `src/terminal/session.rs:319`, `:607`, `:635` | discards (`let _ =`) | +| **`src/terminal/session.rs:566`** (propagating at `:577`) | **propagates** as `TerminalError::Process` | +| supervisor-internal `shutdown` path | discards | +| `src/lua_bindings/mod.rs:8150`, `:8164` | propagates to Lua | +| `src/lua_bindings/mod.rs:8717` | propagates (via `session.rs:566`) | +| `src/daemon.rs:4162` | **test-only** `.expect`, not production | + +No test in the repository asserts either error string, so widening the +message breaks nothing. + +### 1.6 `portable-pty` caches the exit status on Unix + +Pinned `portable-pty 0.9.0`: `spawn_command` returns +`std::process::Child` (`unix.rs:228`), and `impl Child for +std::process::Child::try_wait` delegates to +`std::process::Child::try_wait` (`lib.rs:271-277`), which caches into +`self.status`. Both `ChildHandle` variants therefore cache. + + +## 2. Decisions + +### Q#PD1 — what the widened error records + +On a `kill` failure in `signal`, the error carries: + +| Field | Why | +|---|---| +| **target source** — `tcgetpgrp` vs `group` vs `leader-pid` fallback | which branch of `signal_target` (`:687`) ran | +| **target kind and value** — `-pgid` or `pid`, with the number | the entity actually signalled | +| **spawn-time pgid / leader pid** | a divergence from the target is the job-control hypothesis, visible only by comparison | +| **errno** | as today | +| **leader `try_wait` state** — `exited(status)` / `live` / `unobservable(e)` | separates "the leader is gone" from "the group we signalled is gone" — the distinction all three failed designs collapsed | + +Every candidate Stage B rule is decidable from these five together, and +none is decidable from the errno alone. + +### Q#PD2 — the disposition is preserved exactly + +The call still fails, with the same `Err`, in every case. No state +transition changes, no ledger arming changes, no tolerance. A reader +diffing behaviour should find none. + +### Q#PD3 — the honest claim is "no disposition change", not "strictly additive" + +Rev 3 said the diagnostic was only an error-string change and could not +regress behaviour. **That overstated it.** `try_wait` on an exited child +**reaps it and caches the status**, so consulting it in the failure path +is an internal state change: the child may be reaped earlier than it +otherwise would be. + +Observably safe, because both variants cache (§1.6) and `poll_one` +(`:1133`) will still see `Ok(Some(_))` and emit its event. But safe by +argument is not safe by assertion, so the terminate-failure-then-tick +event pin is retained (acceptance 5). + +### Q#PD4 — the injected-kill seam injects the KILL, never the observation + +Acceptance 5 needs a forced `kill` failure while the **real** +`ChildHandle::try_wait` runs against the **real** child. A stubbed +observation would bypass exactly the code path in question. + +So the seam is a test-only override of the *kill attempt's result*, +consumed once by the signal path; everything downstream — target +selection, the observation, the error construction — runs for real. This +also makes the diagnostic's own fields testable without racing the +kernel. + +### Q#PD5 — nothing else lands here + +No tolerance rule, no idempotence change, no `signal_target` change. See +§5. + + +## 3. Bets (falsifiable) + +- **B1 — The five fields are sufficient to discriminate the §1.4 + hypotheses.** Falsified if a recurrence carries all five and still + leaves the cause ambiguous — which would itself be a finding worth + having. +- **B2 — Widening the message breaks no caller.** Evidence: §1.5, and no + test asserts the string. + +*Retracted across revisions and not reinstated:* rev 1's "a live owned +child cannot yield EPERM"; rev 2's "exit observation suffices"; rev 2's +"this removes the failure class"; rev 3's "group ESRCH is safe to +tolerate". + + +## 4. Acceptance + +1. A group-directed `kill` failure produces an error carrying all five + Q#PD1 fields, with the target rendered as `-pgid` and the leader + state distinct from it. +2. A leader-directed `kill` failure does the same, with the target + rendered as `pid` and the target source recorded as the fallback + branch. +3. The leader state renders each of `exited(status)`, `live`, and + `unobservable(e)` correctly. +4. **The disposition is unchanged**: every injected failure still + returns `Err`, with no state transition and no ledger arming + (Q#PD2). Falsified by revert — flipping any arm to `Ok` fails this. +5. **Forced injected kill failure against the real PTY child + observation**, then tick: exactly one exit event, with the correct + status (Q#PD3/Q#PD4). A fully stubbed observation does not satisfy + this and is rejected as vacuous. +6. The existing suites stay green, pinning "no behavioural change" from + the outside. + + +## 5. Parked (not deferred-and-forgotten — each needs its own evidence) + +- **All tolerance rules.** Group-directed EPERM *and* ESRCH both fail on + the §1.2 entity split; leader-directed tolerance is plausible but + unmotivated until evidence shows the fallback branch is ever taken. + Needs Stage A evidence first. +- **Q#PS6, `terminate` on an already-reaped process returning `Ok`.** + Independent behavioural fix, different failure (§1.6 of rev 3), its + own lane under one-feature/one-PR. +- **`signal_target`'s read-then-kill of `tcgetpgrp`** — still the most + likely real fix site, still unframed. +- `terminate` cancelling pending restarts; PTYs in + `pmacs.process.list`; any change to `C-c` delivery. + + +## 6. Gates + +Full suite per `CLAUDE.md`. Touched suites: +`bottom_panel_stage1_acceptance`, the vterm stages, and +`compile_mode_acceptance`. Sweep with `-- --skip basedpyright`. + + +## 7. Branch plan + +`pty-terminate-eperm`, one PR, diagnostic only. This framing is its first +commit; the instrumentation and its tests are the second. + +**The lane then closes.** It does not wait for the flake to recur: the +next occurrence — whenever it happens, under whoever's PR — carries its +own evidence, and Stage B is framed then. Math work proceeds immediately +after this lands. From 62316a9ced3e8e36d9771fb3bd365640cc757052 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 21:25:58 -0400 Subject: [PATCH 05/10] fix(process): make a failing kill describe itself (Q#PD1) A failing kill in ProcessSupervisor::signal reported an errno and nothing else, which is not enough to diagnose the macOS CI failure that prompted this lane: three different hypotheses about that EPERM produce the same message, and the fix each one implies is different. The error now carries five facts as separate fields: the target source (which branch of signal_target ran), the target kind and value, the spawn-time group for a group-directed signal, the errno, and the spawned leader's real try_wait state. Keeping the target and the leader apart is the whole point. For a PTY the signal goes to the terminal's foreground process group, read from the tty at signal time, while the leader is the child that was spawned. Those are different entities whenever job control has moved the terminal, and three rejected designs for this code were unsound precisely because they concluded something about one from the other. The report states both and concludes nothing. The disposition is unchanged. Every call that failed before still fails, with no state transition and no reap-ledger arming. That is asserted directly rather than assumed, because it is what separates this from the tolerance rules review rejected. Q#PD3, stated narrowly: this is not a pure message change. Consulting try_wait reaps an exited child and caches its status, so the child may be reaped earlier than it otherwise would be. That is observably safe because portable-pty 0.9.0 returns a std::process::Child on Unix and delegates try_wait straight to it, so the status is cached and poll_one still sees it -- but safe by argument is not safe by assertion, so a test forces a kill failure against the real PTY child and then checks that exactly one terminal event survives. Q#PD4: the test seam injects the kill attempt's result only, never the observation. Target selection, the real ChildHandle::try_wait against the real child, and the error construction all run unmodified; a stubbed observation would bypass the code path under test. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk --- src/process.rs | 385 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 378 insertions(+), 7 deletions(-) diff --git a/src/process.rs b/src/process.rs index 2b53bf3..88fb42e 100644 --- a/src/process.rs +++ b/src/process.rs @@ -470,6 +470,13 @@ pub struct ProcessSupervisor { /// TERM→KILL window used when arming the ledger. Constant /// [`GROUP_TERM_GRACE`] in production; overridable in tests. group_term_grace: Duration, + /// Q#PD4 test seam: forces the next `kill(2)` attempt in + /// [`Self::signal`] to fail with this errno, consumed once. + /// Always `None` in production — there is no way to set it outside + /// `cfg(test)`. It replaces the *kill result only*, so the leader + /// observation still runs against the real child handle; a stubbed + /// observation would bypass the code path under test. + forced_kill_errno: Option, } /// One armed group in the reap ledger. @@ -684,7 +691,50 @@ impl ChildHandle { } } -fn signal_target(proc: &ManagedProcess, pid: u32) -> Result { +/// Which branch of [`signal_target`] chose the target (Q#PD1). +/// +/// Recorded on failure because the branches differ in what a failing +/// `kill` can possibly mean: only [`Self::LeaderPid`] aims at the +/// spawned child itself. The other two aim at a *group*, which for a +/// PTY is read from the terminal and can belong to something the +/// supervisor never spawned. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum TargetSource { + /// The tty's current foreground process group, read at signal + /// time. Diverges from the leader exactly when job control has + /// moved the terminal. + ForegroundGroup, + /// A `group = true` pipe child leading its own process group. + SpawnGroup, + /// The child's own pid. + LeaderPid, +} + +impl TargetSource { + fn as_str(self) -> &'static str { + match self { + Self::ForegroundGroup => "tcgetpgrp", + Self::SpawnGroup => "group", + Self::LeaderPid => "leader-pid", + } + } + + /// Whether the target is a process group rather than one process. + fn is_group(self) -> bool { + matches!(self, Self::ForegroundGroup | Self::SpawnGroup) + } +} + +/// The entity a signal was actually aimed at, plus the branch that +/// chose it. Carried so a failure can report the target as a fact +/// separate from the leader's state (Q#PD1). +#[derive(Debug, Clone, Copy)] +struct SignalTarget { + pid: Pid, + source: TargetSource, +} + +fn signal_target(proc: &ManagedProcess, pid: u32) -> Result { if let Some(runtime) = proc.runtime.as_ref() && let ChildHandle::Pty { _master: master, .. @@ -692,7 +742,10 @@ fn signal_target(proc: &ManagedProcess, pid: u32) -> Result { && let Some(pgrp) = master.process_group_leader() && pgrp > 0 { - return Ok(Pid::from_raw(-pgrp)); + return Ok(SignalTarget { + pid: Pid::from_raw(-pgrp), + source: TargetSource::ForegroundGroup, + }); } // `group = true` pipe children lead a fresh process group // (`process_group(0)` at spawn ⇒ pgid == pid), so fatal signals @@ -700,11 +753,80 @@ fn signal_target(proc: &ManagedProcess, pid: u32) -> Result { // (Q#CM3). if proc.spec.group { let pgid = i32::try_from(pid).map_err(|e| e.to_string())?; - return Ok(Pid::from_raw(-pgid)); + return Ok(SignalTarget { + pid: Pid::from_raw(-pgid), + source: TargetSource::SpawnGroup, + }); } - Ok(Pid::from_raw( - i32::try_from(pid).map_err(|e| e.to_string())?, - )) + Ok(SignalTarget { + pid: Pid::from_raw(i32::try_from(pid).map_err(|e| e.to_string())?), + source: TargetSource::LeaderPid, + }) +} + +/// The spawned leader's state at the moment a `kill` failed (Q#PD1). +/// +/// Deliberately reported *beside* the target rather than folded into a +/// verdict: for a PTY the two are different entities whenever job +/// control has moved the terminal, and three successive designs for +/// this code were unsound precisely because they collapsed them. +enum LeaderObservation { + Exited(TermStatus), + Live, + Unobservable(String), + NoRuntime, +} + +impl LeaderObservation { + fn render(&self) -> String { + match self { + Self::Exited(TermStatus::Exited(code)) => format!("exited(code {code})"), + Self::Exited(TermStatus::Signaled(sig)) => format!("exited(signal {sig})"), + Self::Live => "live".to_owned(), + Self::Unobservable(e) => format!("unobservable({e})"), + Self::NoRuntime => "no-runtime".to_owned(), + } + } +} + +/// Observe the spawned leader. Note this *reaps* an exited child and +/// caches its status; that is why Q#PD3 claims "no disposition change" +/// rather than "strictly additive", and why an event-count test pins +/// that `poll_one` still emits exactly one exit event afterwards. +fn observe_leader(proc: &mut ManagedProcess) -> LeaderObservation { + let Some(runtime) = proc.runtime.as_mut() else { + return LeaderObservation::NoRuntime; + }; + match runtime.child.try_wait() { + Ok(Some(status)) => LeaderObservation::Exited(status), + Ok(None) => LeaderObservation::Live, + Err(e) => LeaderObservation::Unobservable(e), + } +} + +/// Render a failing `kill` as the five facts of Q#PD1. The disposition +/// is unchanged (Q#PD2) — this only replaces a message that said +/// nothing but the errno. +fn signal_failure_report( + target: SignalTarget, + leader_pid: u32, + errno: &nix::errno::Errno, + leader: &LeaderObservation, +) -> String { + let expected = if target.source.is_group() { + match i32::try_from(leader_pid) { + Ok(p) => format!(", expected_group=-{p}"), + Err(_) => String::new(), + } + } else { + String::new() + }; + format!( + "kill: {errno} (target={} via {}, leader_pid={leader_pid}{expected}, leader={})", + target.pid.as_raw(), + target.source.as_str(), + leader.render(), + ) } /// Termination status of one generation. Internal --- the supervisor @@ -807,9 +929,20 @@ impl ProcessSupervisor { shut_down: false, reap_ledger: HashMap::new(), group_term_grace: GROUP_TERM_GRACE, + forced_kill_errno: None, } } + /// Q#PD4 test seam: make the next `kill(2)` attempt in + /// [`Self::signal`] report `errno` instead of calling the kernel. + /// Consumed by that one attempt. Everything downstream — target + /// selection, the leader observation against the real child, and + /// the error construction — runs unmodified. + #[cfg(test)] + fn force_next_kill_errno(&mut self, errno: nix::errno::Errno) { + self.forced_kill_errno = Some(errno); + } + /// Override the SIGTERM-to-SIGKILL grace window. Test helper. pub fn set_grace_period(&mut self, d: Duration) { self.grace_period = d; @@ -928,7 +1061,21 @@ impl ProcessSupervisor { return Err(format!("process {id} is not running")); }; let target = signal_target(proc, pid)?; - nix::sys::signal::kill(target, Some(signal)).map_err(|e| format!("kill: {e}"))?; + // Q#PD4: the seam injects the KILL attempt's result only — + // never the observation below — so target selection, the real + // `ChildHandle::try_wait` against the real child, and the error + // construction all run for real. Consumed once. + let kill_result = match self.forced_kill_errno.take() { + Some(errno) => Err(errno), + None => nix::sys::signal::kill(target.pid, Some(signal)), + }; + if let Err(errno) = kill_result { + // Q#PD1/Q#PD2: the failure describes itself; the + // disposition is unchanged — this still returns `Err`, + // with no state transition and no ledger arming. + let leader = observe_leader(proc); + return Err(signal_failure_report(target, pid, &errno, &leader)); + } if matches!(signal, Signal::SIGTERM | Signal::SIGKILL | Signal::SIGHUP) { proc.state = ProcessState::Exiting { pid, @@ -2131,6 +2278,230 @@ mod tests { ); } + /// Spawn a PTY child that stays alive until terminated, and wait + /// for its `Started` event so a pid and a foreground group exist. + fn spawn_live_pty(sup: &mut ProcessSupervisor, name: &str) -> ProcessId { + let mut spec = ProcessSpec::new(name, "/bin/sh"); + spec.args = vec!["-c".into(), "sleep 30".into()]; + spec.mode = ProcessMode::Pty { + rows: 24, + cols: 80, + mode: TerminalMode::Canonical, + }; + let id = sup.spawn(spec).expect("spawn"); + let _ = drain_until(sup, id, Duration::from_secs(5), |evs| { + evs.iter() + .any(|e| matches!(e.kind, ProcessEventKind::Started { .. })) + }); + id + } + + /// Q#PD1 acceptance 1 — a group-directed failure names the target, + /// the branch that chose it, the expected group, the errno, and the + /// leader's own state, as five separate facts. + /// + /// The leader field is the one that matters: for a PTY the signal + /// goes to the terminal's foreground group, which is a different + /// entity from the spawned child whenever job control has moved + /// the terminal. Three rejected designs for this code collapsed + /// the two; the report keeps them apart. + #[test] + fn a_group_directed_kill_failure_reports_target_and_leader_separately() { + let mut sup = ProcessSupervisor::new(); + let id = spawn_live_pty(&mut sup, "diag-group"); + + sup.force_next_kill_errno(nix::errno::Errno::EPERM); + let err = sup.terminate(id).expect_err("injected EPERM must fail"); + + assert!(err.contains("EPERM"), "errno is reported: {err}"); + assert!( + err.contains("via tcgetpgrp"), + "the target SOURCE distinguishes a tty-read group from a spawn group: {err}" + ); + assert!( + err.contains("target=-"), + "a group target renders negative: {err}" + ); + assert!( + err.contains("expected_group=-"), + "the spawn-time group is shown so a divergence is visible: {err}" + ); + assert!( + err.contains("leader=live"), + "the leader is observed independently of the group: {err}" + ); + // Non-vacuity: the two numbers are actually rendered, not empty. + assert!( + err.contains("leader_pid=") && !err.contains("leader_pid=0,"), + "a real leader pid is reported: {err}" + ); + } + + /// Q#PD1 acceptance 2 — a leader-directed failure records the + /// fallback branch and a positive target, and omits the group + /// field that would be meaningless for it. + #[test] + fn a_leader_directed_kill_failure_reports_the_fallback_branch() { + let mut sup = ProcessSupervisor::new(); + let mut spec = ProcessSpec::new("diag-leader", "/bin/sh"); + spec.args = vec!["-c".into(), "sleep 30".into()]; + let id = sup.spawn(spec).expect("spawn"); + let _ = drain_until(&mut sup, id, Duration::from_secs(5), |evs| { + evs.iter() + .any(|e| matches!(e.kind, ProcessEventKind::Started { .. })) + }); + + sup.force_next_kill_errno(nix::errno::Errno::ESRCH); + let err = sup.terminate(id).expect_err("injected ESRCH must fail"); + + assert!(err.contains("ESRCH"), "errno is reported: {err}"); + assert!( + err.contains("via leader-pid"), + "a non-group pipe child targets its own pid: {err}" + ); + assert!( + !err.contains("target=-"), + "a leader target renders positive: {err}" + ); + assert!( + !err.contains("expected_group="), + "the group field is omitted where it has no meaning: {err}" + ); + let _ = sup.signal(id, Signal::SIGKILL); + } + + /// Q#PD1 acceptance 3 — every leader state renders distinctly. The + /// `Unobservable` and `NoRuntime` arms cannot be produced by a + /// real child on demand, so they are pinned directly; `live` and + /// `exited` are pinned through the real path by the tests around + /// this one. + #[test] + fn every_leader_observation_renders_distinctly() { + assert_eq!( + LeaderObservation::Exited(TermStatus::Exited(0)).render(), + "exited(code 0)" + ); + assert_eq!( + LeaderObservation::Exited(TermStatus::Signaled("SIGTERM".into())).render(), + "exited(signal SIGTERM)" + ); + assert_eq!(LeaderObservation::Live.render(), "live"); + assert_eq!( + LeaderObservation::Unobservable("try_wait: boom".into()).render(), + "unobservable(try_wait: boom)" + ); + assert_eq!(LeaderObservation::NoRuntime.render(), "no-runtime"); + } + + /// Q#PD1 acceptance 3, exited arm through the REAL path — the + /// leader has genuinely exited and the report says so. + #[test] + fn a_failure_after_the_child_exits_reports_the_leader_as_exited() { + let mut sup = ProcessSupervisor::new(); + let mut spec = ProcessSpec::new("diag-exited", "/bin/sh"); + spec.args = vec!["-c".into(), "exit 3".into()]; + let id = sup.spawn(spec).expect("spawn"); + // Wait for the child to actually be gone, but do NOT tick past + // the point where the record leaves Running — `signal` needs a + // live record to reach the kill at all. + std::thread::sleep(Duration::from_millis(300)); + + sup.force_next_kill_errno(nix::errno::Errno::EPERM); + let err = sup.terminate(id).expect_err("injected EPERM must fail"); + + assert!( + err.contains("leader=exited("), + "an exited leader is observed as exited, not guessed from the errno: {err}" + ); + } + + /// Q#PD2 acceptance 4 — **the disposition is unchanged.** An + /// injected failure still fails, and neither the state transition + /// nor the reap-ledger arming runs. This is the assertion that + /// separates a diagnostic from the tolerance rules three review + /// rounds rejected; flipping any arm to `Ok` fails it. + #[test] + fn an_injected_failure_changes_no_state_and_arms_no_ledger() { + let mut sup = ProcessSupervisor::new(); + let mut spec = ProcessSpec::new("diag-disposition", "/bin/sh"); + spec.args = vec!["-c".into(), "sleep 30".into()]; + spec.group = true; + let id = sup.spawn(spec).expect("spawn"); + let _ = drain_until(&mut sup, id, Duration::from_secs(5), |evs| { + evs.iter() + .any(|e| matches!(e.kind, ProcessEventKind::Started { .. })) + }); + assert!( + sup.reap_ledger.is_empty(), + "precondition: nothing armed before the attempt" + ); + + sup.force_next_kill_errno(nix::errno::Errno::EPERM); + let err = sup.terminate(id).expect_err("injected EPERM must fail"); + assert!(err.contains("via group"), "a group=true pipe child: {err}"); + + assert!( + matches!( + sup.processes.get(&id).expect("record").state, + ProcessState::Running { .. } + ), + "a failed kill must not transition the record to Exiting" + ); + assert!( + sup.reap_ledger.is_empty(), + "a failed kill must not arm the reap ledger" + ); + + let _ = sup.signal(id, Signal::SIGKILL); + } + + /// Q#PD3/Q#PD4 acceptance 5 — the diagnostic consults the REAL + /// `ChildHandle::try_wait` on the REAL child, which reaps it and + /// caches the status. `poll_one` must still emit exactly one exit + /// event afterwards. + /// + /// A stubbed observation would bypass the double-`try_wait` path + /// entirely and pin nothing, so the injection replaces the kill + /// result only. + #[test] + fn observing_the_leader_does_not_consume_the_exit_event() { + let mut sup = ProcessSupervisor::new(); + let mut spec = ProcessSpec::new("diag-one-event", "/bin/sh"); + spec.args = vec!["-c".into(), "exit 7".into()]; + spec.mode = ProcessMode::Pty { + rows: 24, + cols: 80, + mode: TerminalMode::Canonical, + }; + let id = sup.spawn(spec).expect("spawn"); + std::thread::sleep(Duration::from_millis(300)); + + // The forced failure drives `observe_leader`, which try_waits + // the real PTY child for the first time. + sup.force_next_kill_errno(nix::errno::Errno::EPERM); + let err = sup.terminate(id).expect_err("injected EPERM must fail"); + assert!( + err.contains("leader=exited("), + "the real handle was consulted: {err}" + ); + + // Now the supervisor's own try_wait must still see the status. + let evs = drain_until(&mut sup, id, Duration::from_secs(5), has_exited); + let terminal = evs + .iter() + .filter(|e| { + matches!( + e.kind, + ProcessEventKind::Exited { .. } | ProcessEventKind::Signaled { .. } + ) + }) + .count(); + assert_eq!( + terminal, 1, + "exactly one terminal event survives the diagnostic's try_wait" + ); + } + #[test] fn signal_terminates_a_running_child() { let mut sup = ProcessSupervisor::new(); From 52731ba1216055ab1c1380ee4f3cc63b42bfae26 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 21:30:23 -0400 Subject: [PATCH 06/10] style: pass Errno by value (clippy pedantic) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk --- src/process.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/process.rs b/src/process.rs index 88fb42e..1453e10 100644 --- a/src/process.rs +++ b/src/process.rs @@ -810,7 +810,7 @@ fn observe_leader(proc: &mut ManagedProcess) -> LeaderObservation { fn signal_failure_report( target: SignalTarget, leader_pid: u32, - errno: &nix::errno::Errno, + errno: nix::errno::Errno, leader: &LeaderObservation, ) -> String { let expected = if target.source.is_group() { @@ -1074,7 +1074,7 @@ impl ProcessSupervisor { // disposition is unchanged — this still returns `Err`, // with no state transition and no ledger arming. let leader = observe_leader(proc); - return Err(signal_failure_report(target, pid, &errno, &leader)); + return Err(signal_failure_report(target, pid, errno, &leader)); } if matches!(signal, Signal::SIGTERM | Signal::SIGKILL | Signal::SIGHUP) { proc.state = ProcessState::Exiting { From 40f7f8169019204ec2df2a85366f2673326d604b Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 22:16:56 -0400 Subject: [PATCH 07/10] test(process): pin exact diagnostic values and drop the timing-dependent sleeps Round-1 review found both test weaknesses. The exited-child tests used a fixed 300 ms sleep as proof the child had exited, which on a loaded runner can be false and would turn them into spurious failures. nix's waitid is unavailable on macOS and libc::waitid would need unsafe, which the crate forbids, so the tests now synchronise on the observation under test: a bounded loop that drives the production diagnostic until it reports the leader as exited. Each failing attempt leaves the record untouched because the failure path returns before any bookkeeping, so the loop is side-effect free, and it is strictly stronger than a sleep because it observes the actual state rather than assuming it. The assertions were substring checks -- target=-, expected_group=-, leader=exited( -- which a hardcoded target or a wrong exit code would satisfy. They are now exact message equality built from the pid the kernel actually assigned and the errno's own Display, and the one-event test asserts the surviving event carries exit code 7 rather than any terminal event. The group test also spawns /bin/sleep directly rather than through a shell, since a shell may place the command in a different foreground process group than the one being asserted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk --- src/process.rs | 213 ++++++++++++++++++++++++++++--------------------- 1 file changed, 122 insertions(+), 91 deletions(-) diff --git a/src/process.rs b/src/process.rs index 1453e10..a944de4 100644 --- a/src/process.rs +++ b/src/process.rs @@ -2278,103 +2278,131 @@ mod tests { ); } - /// Spawn a PTY child that stays alive until terminated, and wait - /// for its `Started` event so a pid and a foreground group exist. - fn spawn_live_pty(sup: &mut ProcessSupervisor, name: &str) -> ProcessId { - let mut spec = ProcessSpec::new(name, "/bin/sh"); - spec.args = vec!["-c".into(), "sleep 30".into()]; + /// Spawn a PTY child that leads its own session and stays alive + /// until terminated, returning its id and OS pid. + /// + /// `/bin/sleep` directly rather than through a shell: a shell may + /// place the command in a different foreground process group, and + /// these tests assert the exact target the tty reports. + fn spawn_live_pty(sup: &mut ProcessSupervisor, name: &str) -> (ProcessId, u32) { + let mut spec = ProcessSpec::new(name, "/bin/sleep"); + spec.args = vec!["30".into()]; spec.mode = ProcessMode::Pty { rows: 24, cols: 80, mode: TerminalMode::Canonical, }; let id = sup.spawn(spec).expect("spawn"); - let _ = drain_until(sup, id, Duration::from_secs(5), |evs| { + (id, spawn_started_pid(sup, id)) + } + + /// Drain until `Started` and return the OS pid it carries. + fn spawn_started_pid(sup: &mut ProcessSupervisor, id: ProcessId) -> u32 { + let evs = drain_until(sup, id, Duration::from_secs(5), |evs| { evs.iter() .any(|e| matches!(e.kind, ProcessEventKind::Started { .. })) }); - id + evs.iter() + .find_map(|e| match e.kind { + ProcessEventKind::Started { pid } => Some(pid), + _ => None, + }) + .expect("Started carries a pid") + } + + /// Drive the production diagnostic until it observes the leader as + /// exited, bounded by `timeout`. + /// + /// A fixed sleep is NOT proof of exit — on a loaded runner the child + /// can still be live, which would turn these tests into false + /// failures. This synchronises on the very observation under test. + /// Each failing attempt leaves the record untouched, because the + /// failure path returns before any bookkeeping (Q#PD2), so looping + /// is side-effect free. + fn terminate_until_leader_exited( + sup: &mut ProcessSupervisor, + id: ProcessId, + timeout: Duration, + ) -> String { + let deadline = Instant::now() + timeout; + loop { + sup.force_next_kill_errno(nix::errno::Errno::EPERM); + let err = sup.terminate(id).expect_err("injected EPERM must fail"); + if err.contains("leader=exited(") { + return err; + } + assert!( + Instant::now() < deadline, + "leader never observed as exited within {timeout:?}: {err}" + ); + std::thread::sleep(Duration::from_millis(10)); + } } /// Q#PD1 acceptance 1 — a group-directed failure names the target, /// the branch that chose it, the expected group, the errno, and the /// leader's own state, as five separate facts. /// - /// The leader field is the one that matters: for a PTY the signal - /// goes to the terminal's foreground group, which is a different - /// entity from the spawned child whenever job control has moved - /// the terminal. Three rejected designs for this code collapsed - /// the two; the report keeps them apart. + /// Asserted as an exact message against the pid the kernel actually + /// assigned, so a hardcoded target could not satisfy it. The leader + /// field is the one that matters: for a PTY the signal goes to the + /// terminal's foreground group, a different entity from the spawned + /// child whenever job control has moved the terminal. Three rejected + /// designs for this code collapsed the two; the report keeps them + /// apart, and here they are asserted to agree only because nothing + /// has moved the terminal. #[test] fn a_group_directed_kill_failure_reports_target_and_leader_separately() { let mut sup = ProcessSupervisor::new(); - let id = spawn_live_pty(&mut sup, "diag-group"); + let (id, pid) = spawn_live_pty(&mut sup, "diag-group"); sup.force_next_kill_errno(nix::errno::Errno::EPERM); let err = sup.terminate(id).expect_err("injected EPERM must fail"); - assert!(err.contains("EPERM"), "errno is reported: {err}"); - assert!( - err.contains("via tcgetpgrp"), - "the target SOURCE distinguishes a tty-read group from a spawn group: {err}" + let expected = format!( + "kill: {} (target=-{pid} via tcgetpgrp, leader_pid={pid}, expected_group=-{pid}, leader=live)", + nix::errno::Errno::EPERM ); - assert!( - err.contains("target=-"), - "a group target renders negative: {err}" - ); - assert!( - err.contains("expected_group=-"), - "the spawn-time group is shown so a divergence is visible: {err}" - ); - assert!( - err.contains("leader=live"), - "the leader is observed independently of the group: {err}" - ); - // Non-vacuity: the two numbers are actually rendered, not empty. - assert!( - err.contains("leader_pid=") && !err.contains("leader_pid=0,"), - "a real leader pid is reported: {err}" + assert_eq!( + err, expected, + "the report names the exact target the tty reported, the exact \ + leader pid, and observes the leader as live" ); + + let _ = sup.signal(id, Signal::SIGKILL); } /// Q#PD1 acceptance 2 — a leader-directed failure records the - /// fallback branch and a positive target, and omits the group - /// field that would be meaningless for it. + /// fallback branch and a positive target, and omits the group field + /// that would be meaningless for it. Exact message again. #[test] fn a_leader_directed_kill_failure_reports_the_fallback_branch() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("diag-leader", "/bin/sh"); - spec.args = vec!["-c".into(), "sleep 30".into()]; + let mut spec = ProcessSpec::new("diag-leader", "/bin/sleep"); + spec.args = vec!["30".into()]; let id = sup.spawn(spec).expect("spawn"); - let _ = drain_until(&mut sup, id, Duration::from_secs(5), |evs| { - evs.iter() - .any(|e| matches!(e.kind, ProcessEventKind::Started { .. })) - }); + let pid = spawn_started_pid(&mut sup, id); sup.force_next_kill_errno(nix::errno::Errno::ESRCH); let err = sup.terminate(id).expect_err("injected ESRCH must fail"); - assert!(err.contains("ESRCH"), "errno is reported: {err}"); - assert!( - err.contains("via leader-pid"), - "a non-group pipe child targets its own pid: {err}" + let expected = format!( + "kill: {} (target={pid} via leader-pid, leader_pid={pid}, leader=live)", + nix::errno::Errno::ESRCH ); - assert!( - !err.contains("target=-"), - "a leader target renders positive: {err}" - ); - assert!( - !err.contains("expected_group="), - "the group field is omitted where it has no meaning: {err}" + assert_eq!( + err, expected, + "a non-group pipe child targets its own pid, and the group \ + field is omitted where it has no meaning" ); + let _ = sup.signal(id, Signal::SIGKILL); } /// Q#PD1 acceptance 3 — every leader state renders distinctly. The - /// `Unobservable` and `NoRuntime` arms cannot be produced by a - /// real child on demand, so they are pinned directly; `live` and - /// `exited` are pinned through the real path by the tests around - /// this one. + /// `Unobservable` and `NoRuntime` arms cannot be produced by a real + /// child on demand, so they are pinned directly; `live` and `exited` + /// are pinned through the real path by the tests around this one. #[test] fn every_leader_observation_renders_distinctly() { assert_eq!( @@ -2393,25 +2421,27 @@ mod tests { assert_eq!(LeaderObservation::NoRuntime.render(), "no-runtime"); } - /// Q#PD1 acceptance 3, exited arm through the REAL path — the - /// leader has genuinely exited and the report says so. + /// Q#PD1 acceptance 3, exited arm through the REAL path — the leader + /// has genuinely exited and the report carries its exact code, not + /// merely "some exit". #[test] fn a_failure_after_the_child_exits_reports_the_leader_as_exited() { let mut sup = ProcessSupervisor::new(); let mut spec = ProcessSpec::new("diag-exited", "/bin/sh"); spec.args = vec!["-c".into(), "exit 3".into()]; let id = sup.spawn(spec).expect("spawn"); - // Wait for the child to actually be gone, but do NOT tick past - // the point where the record leaves Running — `signal` needs a - // live record to reach the kill at all. - std::thread::sleep(Duration::from_millis(300)); + let pid = spawn_started_pid(&mut sup, id); - sup.force_next_kill_errno(nix::errno::Errno::EPERM); - let err = sup.terminate(id).expect_err("injected EPERM must fail"); + let err = terminate_until_leader_exited(&mut sup, id, Duration::from_secs(10)); - assert!( - err.contains("leader=exited("), - "an exited leader is observed as exited, not guessed from the errno: {err}" + let expected = format!( + "kill: {} (target={pid} via leader-pid, leader_pid={pid}, leader=exited(code 3))", + nix::errno::Errno::EPERM + ); + assert_eq!( + err, expected, + "the exact exit code is observed from the real child, not \ + inferred from the errno" ); } @@ -2427,10 +2457,7 @@ mod tests { spec.args = vec!["-c".into(), "sleep 30".into()]; spec.group = true; let id = sup.spawn(spec).expect("spawn"); - let _ = drain_until(&mut sup, id, Duration::from_secs(5), |evs| { - evs.iter() - .any(|e| matches!(e.kind, ProcessEventKind::Started { .. })) - }); + let pid = spawn_started_pid(&mut sup, id); assert!( sup.reap_ledger.is_empty(), "precondition: nothing armed before the attempt" @@ -2438,7 +2465,12 @@ mod tests { sup.force_next_kill_errno(nix::errno::Errno::EPERM); let err = sup.terminate(id).expect_err("injected EPERM must fail"); - assert!(err.contains("via group"), "a group=true pipe child: {err}"); + + let expected = format!( + "kill: {} (target=-{pid} via group, leader_pid={pid}, expected_group=-{pid}, leader=live)", + nix::errno::Errno::EPERM + ); + assert_eq!(err, expected, "a group=true pipe child reports via group"); assert!( matches!( @@ -2458,7 +2490,7 @@ mod tests { /// Q#PD3/Q#PD4 acceptance 5 — the diagnostic consults the REAL /// `ChildHandle::try_wait` on the REAL child, which reaps it and /// caches the status. `poll_one` must still emit exactly one exit - /// event afterwards. + /// event, carrying the exact code. /// /// A stubbed observation would bypass the double-`try_wait` path /// entirely and pin nothing, so the injection replaces the kill @@ -2474,34 +2506,33 @@ mod tests { mode: TerminalMode::Canonical, }; let id = sup.spawn(spec).expect("spawn"); - std::thread::sleep(Duration::from_millis(300)); + let _ = spawn_started_pid(&mut sup, id); - // The forced failure drives `observe_leader`, which try_waits - // the real PTY child for the first time. - sup.force_next_kill_errno(nix::errno::Errno::EPERM); - let err = sup.terminate(id).expect_err("injected EPERM must fail"); + // Drives `observe_leader`, which try_waits the real PTY child + // for the first time and reaps it. + let err = terminate_until_leader_exited(&mut sup, id, Duration::from_secs(10)); assert!( - err.contains("leader=exited("), - "the real handle was consulted: {err}" + err.contains("leader=exited(code 7)"), + "the real handle was consulted and carries the exact code: {err}" ); - // Now the supervisor's own try_wait must still see the status. + // The supervisor's own try_wait must still see that status. let evs = drain_until(&mut sup, id, Duration::from_secs(5), has_exited); - let terminal = evs + let terminal: Vec = evs .iter() - .filter(|e| { - matches!( - e.kind, - ProcessEventKind::Exited { .. } | ProcessEventKind::Signaled { .. } - ) + .filter_map(|e| match e.kind { + ProcessEventKind::Exited { code, .. } => Some(code), + ProcessEventKind::Signaled { .. } => Some(-1), + _ => None, }) - .count(); + .collect(); assert_eq!( - terminal, 1, - "exactly one terminal event survives the diagnostic's try_wait" + terminal, + vec![7], + "exactly one terminal event survives the diagnostic's try_wait, \ + carrying the child's real exit code" ); } - #[test] fn signal_terminates_a_running_child() { let mut sup = ProcessSupervisor::new(); From 18d481b046a5237a5b955dea45e9b13043cf20df Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 22:19:19 -0400 Subject: [PATCH 08/10] docs: record the PTY terminate diagnostic lane in the ledger The ledger's own update protocol requires a lane for volatile work, and PR #176 had none: branch, worktree, review state, and verification were all missing. Records why the lane ships a diagnostic rather than a fix -- three rejected tolerance designs, the two facts that killed the original argument (group=true is rejected for PTY mode so the reap ledger never applies to that path, and the ledger comment asserts EPERM cannot happen rather than ruling that it means dead), and that the CI evidence never established the child had exited. Also records the round-1 test fixes and the four verified bites, so a reader can tell which assertions are load-bearing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk --- docs/active-work.md | 69 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/docs/active-work.md b/docs/active-work.md index b60d62c..ffd9b1e 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -55,6 +55,75 @@ git status --short --branch The `git log` command must expose `d152120` or a newer intentional main. If it does not, stop and repair the remote/fetch configuration. +## PTY terminate diagnostic lane — IN REVIEW (PR #176) + +- Portable branch: `githubsucks/pty-terminate-eperm`; worktree + `../pmacs-math-slice`. **PR #176**, base `main`, based on `ccf29e3` + with `c93f9ee` (#175) merged in. +- Approved framing: `docs/process-signal-tolerance-framing.md` + **revision 4**, after three review rounds. +- **Diagnostic only. No disposition change.** Every call that failed + before still fails, with no state transition and no reap-ledger + arming. `src/process.rs` is the only source file touched. +- **Why nothing is fixed:** revisions 1–3 each proposed a *tolerance* + rule and all three were rejected as unsound in the same way — each + concluded something about a process from something that was not about + that process. Rev 1 from an errno alone (EPERM means the caller lacks + permission, not that the id was recycled); rev 2 from `try_wait`, + which observes the spawned **leader** while a PTY signal targets + `-tcgetpgrp(...)`, entities that diverge exactly when job control has + moved the terminal; rev 3 from group-directed **ESRCH**, which proves + only that the selected foreground group vanished. +- **Two facts that killed the original argument.** `group = true` is + *rejected* for PTY mode at spawn (`src/process.rs:1428-1429`), so the + reap ledger never applies to the PTY path at all; and the ledger + comment (`:1075`) says EPERM "cannot happen for our own children" and + drops the entry for **bounded growth** — not a ruling that EPERM means + dead. +- **The CI evidence never established the child had exited.** The probe's + last source statement is a file write and CPython teardown does not + synchronise with it, so no tolerance rule could even be shown to fix + the symptom. That is the whole reason the lane is diagnostic. +- What ships: a failing `kill` now reports five separate facts — target + source, target kind/value, spawn-time group, errno, and the leader's + real `try_wait` state. The test seam injects the **kill result only**, + never the observation, so the real `ChildHandle::try_wait` runs against + the real child. +- **Not "strictly additive".** `try_wait` reaps and caches, so an exited + child may be reaped earlier than otherwise. Safe because + `portable-pty` 0.9.0 returns a `std::process::Child` on Unix and + delegates `try_wait` to it, so `poll_one` still sees the cached + status — pinned by an exactly-one-terminal-event test rather than + assumed. +- Round-1 review fixes: the exited-child tests no longer use a fixed + sleep as proof of exit (nix's `waitid` is unavailable on macOS and + `libc::waitid` needs `unsafe`, which the crate forbids), instead + driving the production diagnostic in a bounded loop until it observes + the exit; and every assertion is now exact message equality built from + the kernel-assigned pid, since the substring forms would have accepted + a hardcoded target or a wrong exit code. +- Bites, all verified rather than assumed: tolerating the failure fails + the disposition test; stubbing the leader observation fails three + tests including the one-event pin; a hardcoded target fails four; a + wrong exit code fails two. +- Verification: fmt, `git diff --check`, strict workspace clippy clean; + lib 1,838 + CRDT 2,015 (both +6, exactly the new tests); GPU 202; M4 + 121; bottom-panel 46; compile-mode 67; vterm 9/6/5; sweep 3,256 across + 93 suites with two load-contention flakes that pass 3/3 isolated + (`read_dir_supersede_cancels_in_flight_predecessor`, known + pre-existing, and + `headless_snapshot_round_trip_summary_restores_the_minimap`). The + second is structurally unreachable from this diff: `pmacs-gpu` depends + on `pmacs-protocol`, never on `pmacs`. +- **Parked, each with its reason:** all tolerance rules (need the + evidence this PR produces); `terminate` idempotence for an + already-reaped process (independent fix, different failure, one + feature per PR); and `signal_target`'s read-then-kill of `tcgetpgrp` + — still the most likely real fix site. +- **The lane closes when this merges.** It does not wait for the flake + to recur; the next occurrence carries its own evidence under whoever's + PR, and a Stage B framing follows then. + ## Lean 4 lane (Arc 8) — Stage 1 MERGED; Stage 2 IN REVIEW (PR #161) - Stage 1 **merged as #160** (`main` @ `0827dd1`, 2026-07-25, one review From 00cc615db557d8f8cf67a574df3e4f0e5a65aef8 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 22:25:05 -0400 Subject: [PATCH 09/10] test(process): read the pid without ticking in the fast-exit tests The parallel workspace sweep failed observing_the_leader_does_not_consume_the_exit_event with "process ProcessId(26) is not running". A real defect in the test, not a flake. The helper that fetched the pid drained for the Started event, and draining ticks. A tick can observe an immediately-exiting child and transition the record out of Running, after which signal returns "is not running" and never reaches the diagnostic -- so the loop spun to its 10 s bound and panicked. It passed standalone because the drain returned on Started before poll_one saw the exit; only the sweep's load shifted the timing enough to lose that race. Fast-exiting children now read the pid straight from the supervisor record, which does not tick. The bounded loop also fails fast when the record has left Running, so a future recurrence is diagnosed in one line rather than surfacing as a timeout. Verified under matched load: 15/15 green with all 16 cores saturated. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk --- src/process.rs | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/src/process.rs b/src/process.rs index a944de4..9e02629 100644 --- a/src/process.rs +++ b/src/process.rs @@ -2296,7 +2296,27 @@ mod tests { (id, spawn_started_pid(sup, id)) } - /// Drain until `Started` and return the OS pid it carries. + /// The OS pid straight from the supervisor's own record, WITHOUT + /// ticking. + /// + /// `drain_until` ticks, and a tick can observe a fast child's exit + /// and transition the record out of `Running` — after which + /// `signal` returns "is not running" and never reaches the + /// diagnostic at all. Any test whose child exits promptly must read + /// the pid this way. (Found by the parallel workspace sweep: the + /// drain-based helper raced only under load.) + fn record_pid(sup: &ProcessSupervisor, id: ProcessId) -> u32 { + match sup.processes.get(&id).expect("record").state { + ProcessState::Running { pid, .. } | ProcessState::Exiting { pid, .. } => pid, + ProcessState::Starting => panic!("spawn has not reported a pid yet"), + ProcessState::Terminated(_) => { + panic!("the record already left Running; the pid is unavailable") + } + } + } + + /// Drain until `Started` and return the OS pid it carries. Safe + /// only for children that outlive the drain; see [`record_pid`]. fn spawn_started_pid(sup: &mut ProcessSupervisor, id: ProcessId) -> u32 { let evs = drain_until(sup, id, Duration::from_secs(5), |evs| { evs.iter() @@ -2331,6 +2351,11 @@ mod tests { if err.contains("leader=exited(") { return err; } + assert!( + !err.contains("is not running"), + "the record left Running before the diagnostic could run, so \ + this test never exercised it: {err}" + ); assert!( Instant::now() < deadline, "leader never observed as exited within {timeout:?}: {err}" @@ -2430,7 +2455,9 @@ mod tests { let mut spec = ProcessSpec::new("diag-exited", "/bin/sh"); spec.args = vec!["-c".into(), "exit 3".into()]; let id = sup.spawn(spec).expect("spawn"); - let pid = spawn_started_pid(&mut sup, id); + // NOT `spawn_started_pid`: draining ticks, and this child exits + // immediately. + let pid = record_pid(&sup, id); let err = terminate_until_leader_exited(&mut sup, id, Duration::from_secs(10)); @@ -2506,7 +2533,9 @@ mod tests { mode: TerminalMode::Canonical, }; let id = sup.spawn(spec).expect("spawn"); - let _ = spawn_started_pid(&mut sup, id); + // NOT `spawn_started_pid`: draining ticks, and a tick can reap + // this immediately-exiting child before the diagnostic runs. + let _ = record_pid(&sup, id); // Drives `observe_leader`, which try_waits the real PTY child // for the first time and reaps it. From 3b7cc67197924474544d9d73e6539c9446f1ac75 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Sat, 25 Jul 2026 22:33:22 -0400 Subject: [PATCH 10/10] docs: record the sweep-found test race in the lane The parallel sweep failed one of the new tests for a real reason, not a flake: drain_until ticks, and a tick can reap an immediately-exiting child before the diagnostic runs. Recorded with the matched-load measurement that shows the fix is load-bearing (0/15 fixed vs 1/10 unfixed under full saturation), and the final sweep numbers. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk --- docs/active-work.md | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index ffd9b1e..497dd63 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -106,15 +106,32 @@ If it does not, stop and repair the remote/fetch configuration. the disposition test; stubbing the leader observation fails three tests including the one-event pin; a hardcoded target fails four; a wrong exit code fails two. +- **The sweep found a real defect in these tests, not a flake.** + `observing_the_leader_does_not_consume_the_exit_event` failed with + "process ProcessId(26) is not running": the pid helper drained for + `Started`, and **`drain_until` ticks**. A tick can observe an + immediately-exiting child and move the record out of `Running`, after + which `signal` never reaches the diagnostic at all, so the bounded + loop spun to its limit. It passed standalone because the drain + returned on `Started` before `poll_one` saw the exit; only load lost + the race. Fast-exiting children now read the pid straight from the + supervisor record (no tick), and the loop fails fast if the record + left `Running`. **Verified under matched load: 0/15 with all 16 cores + saturated, while the old ticking helper fails 1/10 — the fix is + load-bearing.** - Verification: fmt, `git diff --check`, strict workspace clippy clean; lib 1,838 + CRDT 2,015 (both +6, exactly the new tests); GPU 202; M4 - 121; bottom-panel 46; compile-mode 67; vterm 9/6/5; sweep 3,256 across - 93 suites with two load-contention flakes that pass 3/3 isolated - (`read_dir_supersede_cancels_in_flight_predecessor`, known - pre-existing, and - `headless_snapshot_round_trip_summary_restores_the_minimap`). The - second is structurally unreachable from this diff: `pmacs-gpu` depends - on `pmacs-protocol`, never on `pmacs`. + 121; bottom-panel 46; compile-mode 67; vterm 9/6/5; **isolated-config + `--no-fail-fast` sweep 3,258 across 93 suites, zero failures**. + Earlier sweeps on this branch showed two failures and then one; the + totals reconcile (3,256/2 → 3,257/1 → 3,258/0, same test count). The + two that were genuinely unrelated — + `read_dir_supersede_cancels_in_flight_predecessor` (known + pre-existing) and + `headless_snapshot_round_trip_summary_restores_the_minimap` — are + load-contention flakes; the second is structurally unreachable from + this diff, since `pmacs-gpu` depends on `pmacs-protocol` and never on + `pmacs`. - **Parked, each with its reason:** all tolerance rules (need the evidence this PR produces); `terminate` idempotence for an already-reaped process (independent fix, different failure, one