diff --git a/builtin/packages/repl/init.lua b/builtin/packages/repl/init.lua index 3b336f6..df8d5ca 100644 --- a/builtin/packages/repl/init.lua +++ b/builtin/packages/repl/init.lua @@ -259,6 +259,10 @@ function repl.spawn(opts) local spec = { label = name, + -- Worker identity Stage 1: the label is the REPL's session name, + -- which distinguishes two REPLs from each other and says nothing + -- about what is running. The purpose names the interpreter. + purpose = "interactive " .. h._display_name .. " session", command = argv[1], args = args, pty = { rows = rows, cols = cols, mode = "raw" }, diff --git a/builtin/runtime/async.lua b/builtin/runtime/async.lua index af74cc1..eb813da 100644 --- a/builtin/runtime/async.lua +++ b/builtin/runtime/async.lua @@ -88,6 +88,28 @@ function Handle:await() error("await: cannot await inside pmacs.window.commit_to; " .. "await first, then commit") end + -- Worker identity Stage 1 (Q#W-2 rule 1): `pmacs.workers.dispatch` + -- pushes the registered handler's name for the dynamic extent of the + -- handler call, so that jobs allocated inside it are attributable to + -- the third party that asked for them. Parking here would leave the + -- name pushed while this coroutine is suspended, and every job + -- allocated in the meantime --- in any coroutine, on any later tick + -- --- would inherit it. Same hazard, same shape, same remedy as the + -- commit-scope refusal above. + -- + -- Two properties this placement buys, both load-bearing: + -- + -- * it rejects BEFORE parking (ahead of the `_is_complete` check and + -- the `coroutine.yield`), because a guard consulted after the yield + -- has already happened guards nothing; + -- * it rejects UNCONDITIONALLY, not only when a yield would really + -- occur. A guard that fires only for an incomplete handle would + -- pass or fail depending on whether the job happened to settle + -- first --- green under test, intermittent in production. + if async_mod._in_dispatch_name_scope() then + error("await: cannot await inside pmacs.workers.dispatch; " .. + "await first, then dispatch") + end if not async_mod._is_complete(self._id) then -- Yield self so pmacs.async's step() can park us. R46 carve-out: -- this `coroutine.yield` is runtime code; package code uses @@ -240,7 +262,28 @@ setmetatable(async_public, { end, }) +-- The SECOND supported yield API. `Handle:await()` is the first; any +-- rule about a non-yieldable dynamic extent has to cover both, or the +-- extent stays open through a second door. +-- +-- Both refusals below are that rule. The commit-scope one is a +-- **pre-existing gap being closed** (worker identity framing Q#W-7): +-- Journey Stage 1a's Q#JR14b invariant was enforced on `:await()` only, +-- so a coroutine inside `pmacs.window.commit_to` could park through here +-- and produce exactly the misrouting that guard exists to prevent. +-- +-- Placement is the whole point: both fire *before* the `coroutine.yield` +-- below, and both fire unconditionally. A refusal sited after the yield +-- would never run in the case it exists for. function async_public.yield_to_next_tick() + if async_mod._in_commit_scope() then + error("yield_to_next_tick: cannot yield inside pmacs.window.commit_to; " .. + "yield first, then commit") + end + if async_mod._in_dispatch_name_scope() then + error("yield_to_next_tick: cannot yield inside pmacs.workers.dispatch; " .. + "yield first, then dispatch") + end coroutine.yield({ _is_pmacs_next_tick = true }) end @@ -366,14 +409,85 @@ local handlers = { end, } +-- Worker identity Stage 1 (Q#W-2): `name` used to die here. +-- +-- The audit's "every third-party job renders under a builtin's label" is +-- exact, and the reason is this function: the handler is arbitrary Lua, +-- nothing below it takes a name, and a handler that reaches straight for +-- `pmacs._async._dispatch_*` bypasses the wrapper layer entirely. So the +-- name is pushed onto a runtime-owned stack for the dynamic extent of +-- the handler call and read at `allocate`, the single funnel every job +-- passes through. Seven rules govern it; five are visible here: +-- +-- 1. The extent is NON-YIELDABLE, and that is enforced rather than +-- assumed --- see the refusals in `Handle:await` and +-- `pmacs.async.yield_to_next_tick`. +-- 3. Nesting is a stack; innermost wins. +-- 4. Fan-out shares the name: five jobs dispatched by one handler are +-- five jobs named alike. They *were* all dispatched under it. +-- 5. UNWIND-SAFE, and this is the one that makes a naive version worse +-- than none. A handler that raises must still pop --- otherwise one +-- failure poisons every subsequent dispatch in the session with a +-- stale name, and the feature starts lying silently instead of +-- failing loudly. Hence pcall, pop, rethrow. +-- 7. Outside any extent nothing changes: a builtin invoked directly +-- records its own purpose. +-- +-- Rule 2 (work dispatched later, from an `on_complete` callback or a +-- resumed coroutine, is deliberately NOT covered) and rule 6 +-- (composition, `": "`) live on the Rust side. +-- +-- The pop/rethrow half, hoisted so it is written once and allocates +-- nothing per dispatch. +-- +-- Varargs across a function boundary, NOT `local ok, result = pcall(…)`: +-- this function used to be `return handler(args, opts)`, which +-- propagates EVERY return value, and bracketing it must not silently +-- truncate a handler that returns more than one. `table.pack` / +-- `table.unpack` would say the same thing but are Lua 5.2 surface, and +-- LuaJIT is this project's default backend (`Cargo.toml`: +-- `default = ["luajit"]`). +local function finish_dispatch(ok, ...) + async_mod._pop_dispatch_name() + if not ok then + -- Level 0: the handler's error travels unchanged. R45's structured + -- errors are tables, and a re-raise that appended position info + -- would corrupt a plain-string error and be silently ignored for a + -- table one --- so neither shape is served by the default level. + error((...), 0) + end + return ... +end + function pmacs.workers.dispatch(name, args, opts) local handler = handlers[name] if handler == nil then error("pmacs.workers.dispatch: unknown handler '" .. tostring(name) .. "'") end - return handler(args, opts) + async_mod._push_dispatch_name(name) + return finish_dispatch(pcall(handler, args, opts)) end +-- Worker identity Stage 1: the name registered here is DISPLAY TEXT. +-- +-- It used to be type-checked and nothing more, which was defensible +-- while it died inside `dispatch`. It no longer dies there: the ambient +-- carries it into every job the handler allocates, and it is composed +-- into `purpose` as `": "`, which the `*workers*` table +-- and the modeline indicator both render. So it gets the same +-- meaningful-value standard `purpose` already gets in +-- `required_purpose` (`src/lua_bindings/mod.rs`) --- and one rule +-- `purpose` deliberately does NOT get. +-- +-- The asymmetry is the point. A purpose may legitimately contain a +-- newline: a filesystem path can, and `pmacs-magit`'s spawn purpose is a +-- whole argv --- so its one-line constraint is enforced by ESCAPING at +-- the surfaces that have one row (`purpose_for_one_row`), following the +-- `#228` decision on `Command.description`. A registered handler NAME +-- has no such case. It is an identifier a package chooses for itself and +-- passes back to `dispatch`, so a control character in it is a mistake +-- or an attempt at one, and refusing at the source costs nobody +-- anything. function pmacs.workers.register(name, handler) -- Allows future Rust-side modules (or test harnesses) to register -- additional dispatchable names. v0.1 has no plugin loader but the @@ -381,6 +495,20 @@ function pmacs.workers.register(name, handler) if type(name) ~= "string" then error("pmacs.workers.register: name must be a string") end + -- Empty and whitespace-only satisfy the type and say nothing --- the + -- exact pair `required_purpose` rejects, and the exact pair R42 + -- rejects for config descriptions. + if name:match("^%s*$") ~= nil then + error("pmacs.workers.register: name must not be empty or whitespace-only") + end + -- `%c` is the C control class: NUL, the C0 range, DEL. A newline + -- forges a row in `*workers*`, a CR rewrites one on a terminal and an + -- ESC starts a sequence in one. Checked AFTER the whitespace rule so + -- a name that is only "\n" reports the emptier problem, which is the + -- one the caller can act on. + if name:find("%c") ~= nil then + error("pmacs.workers.register: name must not contain control characters") + end if type(handler) ~= "function" then error("pmacs.workers.register: handler must be a function") end @@ -581,6 +709,60 @@ function pmacs._async.tick() end end +-- --------------------------------------------------------------------------- +-- Statusline activity indicator (worker identity Stage 1, Q#W-3/Q#W-6). +-- --------------------------------------------------------------------------- +-- +-- `COHERENCE.md` §9 records that no progress indicator exists anywhere +-- --- no spinner, no busy count --- which makes §3's promise of "visible +-- asynchronous work" false unless the user knows to run +-- `M-x editor.list-workers`. This is the fourth `pmacs.statusline.register` +-- adopter (after `mode`, `terminal` and `lsp`) and the first thing that +-- makes background work visible without a command. +-- +-- No wire change: `pmacs.statusline.register` rides the existing +-- `StatuslineSegments` vector, so a fourth provider adds an ELEMENT, not +-- a variant. That is what lets this lane run beside the two holding the +-- protocol-bump slot. + +-- A visibility toggle, and only that (Q#W-6). A permanently-visible +-- statusline element is different in kind from an internal behaviour: it +-- costs modeline width on every frame, and "I do not want this in my +-- modeline" is a preference someone genuinely holds on day one. There is +-- deliberately NO setting for purpose capture itself --- that is +-- substrate, not preference. +pmacs.config.define { + name = "ui.activity-indicator", + description = "Show a modeline count of in-flight background jobs, with the oldest job's purpose. Absent entirely when nothing is running.", + type = "boolean", + default = true, + mutability = "live", +} + +pmacs.statusline.register { + name = "activity", + side = "right", + -- Above `terminal` (10) and `lsp` (0): when the modeline is too narrow + -- for everything, "the editor is busy, on this" is the segment worth + -- keeping. Right-side display order is priority-ascending, so it also + -- lands nearest the protected cursor/scroll group. + priority = 20, + face = "ui.modeline.activity", + fn = function(_ctx) + if pmacs.config.get("ui.activity-indicator") ~= true then return nil end + -- `_activity_summary` rather than `pmacs.workers.snapshot()`: this + -- runs once per visible window per frame, and a snapshot would clone + -- the whole 64-entry completed ring that the indicator never reads. + local summary = async_mod._activity_summary() + -- nil, not "" and not "0 jobs": the evaluator treats an empty string + -- as "no segment" too, but a zero-count string would be a segment + -- that costs width forever to say nothing is happening. Absence is + -- the design (Q#W-3), so absence is what this returns. + if summary == nil then return nil end + return "⋯" .. tostring(summary.in_flight) .. " " .. summary.purpose + end, +} + -- Diagnostic / test helpers: number of parked coroutines, number of -- pending Rust-side jobs. Used by Rust integration tests to drive the -- runtime to quiescence. diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 22fc643..7645416 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -875,6 +875,11 @@ local function start_run(slot, cmdline, opts) -- stdin, own process group, TERM=dumb. local spec = { label = slot.label, + -- Worker identity Stage 1: the label distinguishes one compile slot + -- from another; the purpose is the command the user actually asked + -- for, which is what they want to see when they wonder why the + -- editor is busy. + purpose = "compiling: " .. cmdline, command = "/bin/sh", args = { "-c", "exec 2>&1; " .. cmdline }, env = { TERM = "dumb" }, diff --git a/builtin/runtime/lean.lua b/builtin/runtime/lean.lua index 09ad280..6ccb9dd 100644 --- a/builtin/runtime/lean.lua +++ b/builtin/runtime/lean.lua @@ -494,10 +494,14 @@ local function start_probe(root) -- "lake": a user pointing `command` at an absolute path to lake should -- have THAT probed, not whatever `lake` resolves to on PATH. local spec = { - -- COHERENCE §9: `ProcessSpec.label` is the only identity a process - -- carries, and it is what `pmacs.process.list` renders. A user - -- wondering why their editor touched `lake` finds an owner here. + -- COHERENCE §9: `ProcessSpec.label` identifies the process, and it + -- is what `pmacs.process.list` renders alongside the purpose. A user + -- wondering why their editor touched `lake` finds it here. label = "lean:lake-version-probe", + -- Worker identity Stage 1: the label was carrying both jobs — the + -- identity AND the explanation — which is the conflation the purpose + -- field exists to undo. The label stays a key; this is the sentence. + purpose = "checking the Lean toolchain version before starting a server", command = cfg.command, args = { "--version" }, stdin = "null", diff --git a/docs/active-work.md b/docs/active-work.md index ad3851a..7262ab5 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -265,6 +265,283 @@ also removed: this branch's "R8 NEEDS A LANE" investigation block, and durable facts are in the retired registry row and the handoff §6 census. +## Worker identity Stage 1 (§9) — IMPLEMENTED, no PR yet + +**Written with the lane's first commit**, per the standing correction +from #171 and #215. + +**Branch `worker-identity-stage1`**, base `githubsucks/main` @ +`4bc55e8` (the #225 merge). **`githubsucks/worker-identity-stage1` is +the authoritative tip** — the ref, not a SHA. Recover with +`git fetch githubsucks && git checkout worker-identity-stage1`. + +- **Framing `docs/worker-identity-framing.md`, revision 4, APPROVED + 2026-08-09** after four review rounds. + Scope: `COHERENCE.md` §9's "mechanism without identity", and journey + step 11 — the last of Priority 1's own work, sitting in another + section's arc. +- **Revision 2 took two blockers.** `owner` is **removed entirely**: + populated from static per-subsystem constants it is an origin, not an + owner, and would misattribute third-party work at the exact point §9 + wants attribution. It is not retained under a safer name either — + `origin`/`subsystem` would be adopted as ownership by use and would + squat on the slot P3 must fill. And the handler-name recovery was + **respecified as a mechanism**: revision 1 claimed the name was "in + hand at the one place that throws it away", which was wrong about the + call chain (`dispatch` → arbitrary handler → Lua wrapper → Rust + binding, with the wrapper layer documented as bypassable). +- **Revision 3 took a third blocker: the ambient's extent is not + synchronous.** A handler may `Handle:await()` and park with the name + still pushed, leaking attribution to unrelated later work. Rule 1 now + **enforces** non-yieldability, modelled on the existing + `_in_commit_scope()` refusal in `Handle:await` + (`builtin/runtime/async.lua:87-90`) — rejecting before the park, + unconditionally rather than only when a yield would occur, and + covering **both** yield points. +- **Q#W-7 — a pre-existing defect found while scouting that guard, and + APPROVED for repair in this lane.** `pmacs.async.yield_to_next_tick()` + (`async.lua:243-245`) is public, yields, and carries **no** + `_in_commit_scope` refusal — so Journey Stage 1a's Q#JR14b invariant + has a second entrance. Same helper, same invariant, same edit family, + so splitting it would have preserved a known hole without reducing + integration risk. **Reachability by a real caller is UNPROVEN** — the + defect was found by reading, and the tests pin the guard rather than + reproducing a user-visible bug. That belongs in the commit message so + nobody later cites this as an observed failure. +- **Revision 4 also scoped rule 1's claim to what it enforces.** + Revision 3 said "all yield points"; it covers **the two supported + pmacs yield APIs**. Raw `coroutine.yield` stays reachable — R46 is a + convention, and the scheduler diagnoses a non-Handle yield only after + the coroutine has suspended (`async.lua:197` resumes, `:212` + inspects), so no refusal in a yield helper can intercept it. Recorded + as a residual, and explicitly **not** covered by a test that would + imply otherwise. +- **NO WIRE CHANGE**, which is what lets this run beside the two lanes + already in flight. The statusline activity indicator is a **fourth** + `pmacs.statusline.register` provider (terminal/syntax/lsp are the + three existing adopters), evaluated per frame inside `paint_frame` + (`src/editor.rs:4560`) and riding the existing `StatuslineSegments` + vector. No variant, no bump. +- **Scope:** a **required** `purpose` on `PendingJob` and `ProcessSpec` + through the single allocation funnel (`src/async_runtime.rs:746`, + which every dispatcher and `register_external` passes through), a + runtime-owned dispatch-name ambient recovering the handler name that + `pmacs.workers.dispatch` currently discards, the `*workers*` + rendering, and the indicator. Non-optional so the **compiler**, not a + test, proves every caller supplied one. +- **Two scouting findings that shaped the design**, both verified: + `PendingJob` carries **eight** fields, not the audit's seven, and the + eighth's doc comment **cites §9 by name** as the reason identity + belongs on the job rather than in a side map — so this extends a + merged decision. And **`pmacs.process.list` filters to + `LineOriented`** (`src/lua_bindings/mod.rs:8980`), with **three + acceptance suites using `#pmacs.process.list()` as a leak detector**, + so making terminal PTYs visible is deferred to Stage 2 with a + separate accessor rather than by widening this one. +- **Deliberate deviation from the audit, flagged for review:** §9 names + owner/**purpose**/parent together as the prerequisite; Stage 1 takes + **only `purpose`** — one of the three, not two. `owner` was removed in + revision 2: nothing in the runtime knows which package asked for a + job, so an `owner` field could only have been filled with the same + handler name `purpose` already carries, and an empty one reads as + "unowned" rather than "not tracked". `parent` is out for the matching + reason — it needs an ambient "currently-running job" context, and an + unpopulated `parent` reads as "no parent" rather than "not tracked" + (Q#W-5). The package-ownership slot stays **deliberately empty** until + P3 can fill it with a real signal (framing §3, §7). +- **Gates:** `scripts/gate --acceptance worker_identity_acceptance + --acceptance journey_acceptance --acceptance + statusline_segments_acceptance --acceptance compile_mode_acceptance + --acceptance m8_6_acceptance`. No `--protocol` — no wire change. + `compile_mode` and `m8_6` joined at review round 1, which moved their + spawn call sites; `m8_6` covers the `pmacs-magit` fixture, and a newly + required field is exactly the kind of change that breaks a package + fixture quietly. +- **IMPLEMENTED at `1aca0ee`**, with review round 1's blocker fixed at + `2162737` and review round 2's three findings at `6661125`. + `tests/worker_identity_acceptance.rs` is the new suite: **24 tests**, + plus one consumer-side witness beside the private renderer in + `pmacs-gpu`. +- **`journey_acceptance` passed UNTOUCHED (47/47)** — the stop signal + did not fire. Q#W-7 edits the `commit_to` guard family, so any of its + established pins needing an edit would have meant this altered Journey + Stage 1a's semantics rather than closing a gap in them. Its diff + versus `main` is empty, and so is the diff for all three + `#pmacs.process.list()` leak-detector suites + (`m6_8_multi_repl_acceptance`, `compile_mode_acceptance`, + `lean4_stage1_acceptance`) — Q#W-4's preservation claim, checked the + way the framing asked. +- **One pre-existing assertion did change, and it is an inventory + rather than a contract**: `statusline_segments_acceptance`'s builtin + provider list becomes `["activity", "mode", "terminal", "lsp"]`. + `activity` sorts first because `async.lua` is loaded before + `syntax.lua`, `terminal.lua` and `lsp.lua`. That assertion exists to + grow when a builtin provider is added; it is listed here so the change + is not mistaken for an accommodation. +- **23 mutation checks, each test falsified by removing its own fix.** + The ones worth naming: siting the `await` guard *inside* the + `_is_complete` branch (the already-complete case then slips through — + which is the whole reason the guard is unconditional); replacing + `pcall`/pop/rethrow with a bare handler call (a raising handler leaves + the name pushed and the *next* dispatch inherits it); composing + `""` instead of `": "` and vice versa (each half + passes the other's test); `first()` instead of `last()` on the name + stack; oldest→newest in `activity_summary`; and, on the GPU side, + painting an unthemed modeline face as the band colour, which would + have made the indicator invisible without failing anything else. + One of the twenty is a **preservation** check rather than a new + claim: bracketing `pmacs.workers.dispatch` with + `local ok, result = pcall(...)` truncates a handler that returns more + than one value, which every other test in the suite tolerates. Round + 1 added three more against the spawn refusal: restoring the + label fallback, accepting an empty/whitespace-only purpose, and + reading the field non-raw so a metatable can smuggle one in. +- **Two residuals, stated rather than tested around.** Raw + `coroutine.yield` inside either dynamic scope still leaks the scope — + loudly, through `pmacs.error`, but it leaks; no refusal sited in a + yield helper can intercept it (framing §2). And Q#W-7's reachability + by a real caller stays **unproven**: the commit message says so, and + the test pins the guard rather than reproducing a fault. +- **Review round 1 blocker — `pmacs.process.spawn` now REQUIRES + `purpose`.** The first implementation made it optional at the Lua + surface, falling back to `label`. That preserved compatibility and + delivered nothing: §9's complaint about `ProcessSpec` is exactly that + `label` is "caller-supplied, unvalidated convention", so a purpose + defaulting to it hands every caller back the convention the lane exists + to replace. Refused on five shapes — absent, empty, whitespace-only, + wrong type, metatable-provided — each asserting the process list is + unchanged, since a validation that rejects after spawning has already + done the thing it rejected. +- **That is a BREAKING CHANGE to a public Lua API, taken now on + purpose.** §10 grades extension trust "missing (one class)" and P7 + package lifecycle has not started, so the third-party population is + ~zero and the cost only rises later. Checked for a reason that would be + wrong and found none: `pmacs.process.spawn` has no API-reference + documentation and no stability promise in `docs/` (the package-author + guide's only mentions are an audit-rule classification and a pointer to + the bundled REPL; its semver language governs packages' own versioning, + not pmacs's Lua surface), and `lua_to_spec` has exactly one caller. + **Eleven executable call sites updated**, each with a real description + rather than the label copied across: `repl/init.lua`, `compile.lua`, + `lean.lua`, the `pmacs-magit` fixture, and seven in tests. The two + `pmacs.process.spawn("ls")` occurrences in `src/audit/mod.rs` and + `tests/m7_9_acceptance.rs` are **audit fixture source text** — lexed, + never executed — and are deliberately untouched. +- **Review round 2 — the display-text boundary, fixed at `6661125`.** + Three findings, and the fix is deliberately different in each place + because the constraint is. + - **P2a: invalid UTF-8 bypassed the `purpose` diagnostic.** + `required_purpose` read the field with `value.to_str()?`; Lua strings + are BYTE strings, so `purpose = string.char(255)` surfaced mlua's + generic conversion error before this lane's own message existed. It + refused before spawning, so nothing leaked — the defect was the + message. **Third occurrence of this class in the project** (the + destination-capture lane corrected the same shape two rounds ago), so + the whole diff was audited for it: exactly one more, + `_push_dispatch_name` taking `name: String`, now `mlua::String` with + an owned diagnostic. Those two are the only Lua-string reads this + lane added; every other binding it adds takes `()`. The remaining + `pmacs.process.spawn` fields (`label`, `command`, `args`, `env`, + `cwd`) still convert generically — **pre-existing, untouched, and + named here rather than silently inherited.** + - **P2b, half one: handler names are refused at the source.** + `pmacs.workers.register` type-checked and nothing more, which was + fine while the name died inside `dispatch`. It no longer dies there, + so the name now gets `purpose`'s meaningful-value standard plus + control characters. + - **P2b, half two: purposes are ESCAPED at presentation, not rejected + at the registry — consistent with the `#228` decision.** A purpose + may legitimately contain a newline (a path can; `pmacs-magit`'s spawn + purpose is an argv), so the one-line constraint belongs to the + surface that has one row. `purpose_for_one_row` states the property + it exists for — **a row must not be able to forge another row** — + escapes the Unicode `Cc` class (so ESC cannot open a terminal + sequence either), borrows unchanged when there is nothing to escape + (byte-identity is structural, not asserted), and does **not** escape + backslashes: no number of them makes a second row, and doubling them + would cost byte-identity for ordinary text. Two callers: the + `*workers*` rows and `ActivitySummary`, which exists for one consumer + with exactly one row. `pmacs.workers.snapshot()` is the + `describe-command` of this lane and stays raw — asserted, so a clip + that deleted the text everywhere would fail rather than pass. + - **P3: two stale recovery summaries**, both fixed section-locally — + the framing doc's "Implementation may proceed", and this file's claim + that Stage 1 took the "first two" of owner/purpose/parent. It takes + **one**: `owner` was removed in revision 2, and the claim that + argument overturned was still standing here. + - **Seven more mutation checks, each failing its own test and no + other** (30 for the lane): the two UTF-8 diagnostics, the two + register guards, the two escaping call sites, and + `purpose_for_one_row` neutered to the identity — which fails both + surfaces' tests and nothing else, since it is the shared helper. + - **All 13 gate steps green at `6661125`** (log + `20260809T173314Z-1552101`): lib 1920, lib-crdt 2105, + worker_identity 24, journey **47/47 UNTOUCHED**, statusline 7, + compile_mode 73, m8_6 12, m4 151, gpu 242. The three + `#pmacs.process.list()` leak detectors and `journey_acceptance` are + **byte-identical to `main`** in round 2 — the stop signals did not + fire, and round 2 edited no test outside its own suite. **The + preceding run of the same command was red on three tests and none of + them was this diff's** — R7 for the third time plus two wall-clock + budget tests; recorded in `docs/ci-red-signatures.md` rather than + re-run away silently. +- **Review round 3 — a diagnostic that named the wrong surface, fixed + at `b2e8efd`.** `required_purpose`'s invalid-UTF-8 refusal told the + caller their process purpose "is displayed to the user in `*workers*` + and in the modeline". **Neither is a process surface.** Stage 1 + deliberately keeps processes out of both (Q#W-4, framing §3) — a + process's purpose is exposed through `pmacs.process.list` and nothing + else — so the message sent the reader looking for their process in two + places it will never appear. The refusal itself is correct and stays: + a purpose with no display form anywhere is still refused. + - **The two UTF-8 refusals now name different surfaces, because they + reach different ones.** The job-side twin (`_push_dispatch_name`) + legitimately names `*workers*` and the modeline — a handler name is + composed into a job's purpose, and a job does render in both — so it + was made to say so explicitly rather than left at the vaguer "as + part of every job's purpose", which named no surface at all and + would have made the divergence unassertable. + - **A new test asserts both directions, positive and negative** + (`the_two_utf8_refusals_each_name_the_surface_their_own_text_reaches`, + 25 in the suite — 24 before this round, plus this one; an earlier + revision of this bullet said 26): the process message contains + `pmacs.process.list` + and **not** `*workers*`/`modeline`; the job message contains both of + those and **not** `pmacs.process.list`. The existing row-table + assertion in `spawning_without_a_real_purpose_is_refused_and_starts_nothing` + now runs as far as the surface name too. Without the negative half a + later "unify the wording" edit reintroduces exactly one wrong + sentence and passes everything else. + - **Three mutation checks, each red on its own claim:** restoring the + old process wording fails both content assertions; collapsing the + job message onto the process wording fails only the new test (which + is the point — the old job test asserted the prefix alone); and + restoring the job message's original vague wording fails it too. + - **The rustdoc carried the same defect risk and was fixed with it** — + `required_purpose` now states which surface it names and why not the + other two, and the `_push_dispatch_name` comment states the + converse. A string literal corrected while its doc comment still + argues the other way is one refactor from reverting itself. + - **Gate: all 13 steps green at `cb7730d`** (log + `20260809T200907Z-2672209`). **The two preceding runs of the same + command were red on step `12-sweep`, on a DIFFERENT wall-clock + render-budget test each time** (`20260809T195332Z-2113672`, + `20260809T200120Z-2427128`; load average 12.9/23.9 with sibling + lanes building). All three pass in isolated reruns, none reds twice, + and the diff is two string literals, their doc comments and one + test — no render path is touched. Recorded as **U7** in + `docs/ci-red-signatures.md` rather than re-run away silently. + `journey_acceptance` **47/47 UNTOUCHED** and the three + `#pmacs.process.list()` leak detectors unedited — the stop signals + did not fire. +- **Surfaces that changed shape, for anyone rebasing onto this:** + `AsyncRuntime::allocate`/`allocate_with_resource` collapsed into one + private `JobSpec`-taking funnel; `register_external` grew a third + parameter; `ProcessSpec::new` grew a third parameter (~40 call sites, + nearly all tests); `ActiveJobInfo`/`CompletedJobInfo`/`ProcessSpec` + each grew a required `purpose` field, and `pmacs.process.spawn` + requires `purpose` in its spec table. + ## Discovery Stage 2 — PR #228 OPEN, **MERGE-BLOCKED** **PR #228** — https://github.com/levineuwirth/pmacs/pull/228. Opened @@ -726,6 +1003,7 @@ authoritative tip** — the ref, not a SHA. Recover with sweep step each fail the suite. ||||||| parent of 72bbb96 (docs: LSP LaTeX coverage framing revision 2, on a branch at last) ||||||| parent of 312ec7a (docs: frame Discovery Stage 2 (revision 2) — M-x rows) +||||||| parent of 8f86908 (docs: frame worker identity Stage 1 (revision 1)) ## QoL arc retirement — PR #224 OPEN (docs only) diff --git a/docs/ci-red-signatures.md b/docs/ci-red-signatures.md index 0e4e6a9..8ef65c7 100644 --- a/docs/ci-red-signatures.md +++ b/docs/ci-red-signatures.md @@ -496,30 +496,108 @@ Stage 4; the lane touches no `pmacs-gpu` code at all. | **selector** | `-p pmacs-gpu attach::tests::managed_retry_survives_transients_and_uses_the_successful_stream` | | **job / flavor** | local (Linux), `cargo test --workspace --features crdt --no-fail-fast`, i.e. under full-sweep load | | **required fragments** | `transient sequence must attach` + `Handshake(Io(` + `BrokenPipe` (or `code: 32`) | -| **status** | **new incident, unreproduced — causal status UNRESOLVED** | -| **what IS established** | one occurrence at `pmacs-gpu/src/attach.rs:1680`; the test drives a scripted transient-then-success sequence over a real socket pair | +| **status** | **THIRD OCCURRENCE 2026-08-09 — causal status still UNRESOLVED, but one candidate mechanism is now EXCLUDED** | +| **what IS established** | **three** occurrences at `pmacs-gpu/src/attach.rs:1680`, the second and third with all three fragments **verified** rather than inferred; the test drives a scripted transient-then-success sequence over a real socket pair. **The added GPU test is not the mechanism** — see the third-occurrence control below | | **what is NOT** | whether the broken pipe is the *fixture's* writer closing early or a real retry-path defect. **This row is not a claim that it is harmless** | -| **rerun evidence** | 6 isolated runs green, plus a full `--workspace --features crdt` sweep green (113 targets). Per the rerun rule this establishes **intermittence only** | +| **rerun evidence** | occurrence 1: 6 isolated runs green, plus a full `--workspace --features crdt` sweep green (113 targets). Occurrence 2: **30 green on the observing branch** (15 isolated selector, 15 full `-p pmacs-gpu`) **plus a 15-run merge-base control, also green**. Occurrence 3: 5 isolated selector runs green, 10 full `-p pmacs-gpu` runs green **with** the added test, and **1 failure in 10 with the added test `#[ignore]`d** — the first rerun in this row's history that reproduced anything. Per the rerun rule the green runs establish intermittence only; the red control run is what carries the exclusion | | **retirement** | hardening that removes the named mechanism plus a discriminating witness — or a diagnosis showing the fixture, not the code, closes the pipe | -**Not attributed to this lane**, and the reasoning is not merely "my -diff looks unrelated": Stage 4 adds no wire surface, no protocol -version change, and touches no file in `pmacs-gpu`. A merge-base -control would settle it if this recurs. +**Not attributed to the observing lane**, and in neither case is the +reasoning merely "my diff looks unrelated": long-lines Stage 4 added no +wire surface, no protocol version change, and touched no file in +`pmacs-gpu`. -### U2 — `m6_1_pty_raw_mode_disables_kernel_echo`, one local occurrence +**Second occurrence — worker identity Stage 1, 2026-08-09, local +(Linux).** Recorded at the `scripts/gate` **`gpu` step** +(`PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu`), which is a **third +flavor**: not the `--features crdt` sweep of occurrence 1, and not U3's +default-features workspace sweep. Two things make it a match rather than +a `U` note: -Has a selector, which U1 lacks — but still no fragments, so it cannot -be matched either. Recorded so a recurrence is recognisable. +* **The fragments were captured this time.** `transient sequence must + attach: Attach(Handshake(Io(Os { code: 32, kind: BrokenPipe, message: + "Broken pipe" })))` — all three of the row's required fragments, + verified against the durable gate log rather than a filtered live + stream. **That is what U2 and U3 both lost**, and it is why U3 could + not be judged a recurrence. Reading the gate's own `NN-gpu.log` is the + mechanical fix U3 prescribed, and it worked. +* **The merge-base control R7 asked for was run** — 15 runs at `4bc55e8`, + green. It is **non-discriminating**, not exculpatory: the observing + branch was equally green over 30 runs, so neither side reproduced and + the control separates nothing. Recorded as a null result rather than + as evidence. + +**One causal path is NOT excluded and is named here rather than +dismissed.** The observing lane added a test to `pmacs-gpu`'s test module +(`main.rs`) — a GPU-heavy `render_offscreen` case. It touches no +`attach.rs`, no protocol, and no wire, but it does add a concurrent test +to the same binary, and the failing test is a socket handshake with a +one-second deadline. Contention is a plausible mechanism for a +`BrokenPipe`, and 30 green runs do not rule it out. If a third occurrence +lands, **run the control with the added test removed** rather than at the +merge base — that is the discriminating comparison this one was not. + +**Third occurrence — worker identity Stage 1 review round 2, +2026-08-09, local (Linux). Same selector, same `gpu`-step flavor, all +three fragments verified** against the durable gate log +(`20260809T172606Z-1387979/11-gpu.log`): `transient sequence must +attach: Attach(Handshake(Io(Os { code: 32, kind: BrokenPipe, message: +"Broken pipe" })))`. A match on this file's own rule, not a `U` note. + +**The control the second-occurrence note prescribed was run, and this +time it discriminated — against the hypothesis.** Ten full +`PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu` runs with the added +`render_offscreen` test present: **10/10 green**. Ten more with that +test `#[ignore]`d, changing nothing else: **1 failure in 10**, carrying +all three required fragments +(`without/run-6.log`, `pmacs-gpu/src/attach.rs:1680`). + +So the concurrent-GPU-test path named above is **excluded**: removing +the suspect made the failure *more* frequent, not less, which no +contention story from that test survives. What the run does establish is +that **the failure reproduces on demand at roughly 1-in-10 under +ordinary `-p pmacs-gpu` load** — the first time any rerun in this row's +history has reproduced it at all. That is a materially better starting +point than three isolated sightings, and it is the fact a diagnosis +should be built on: the rate makes a bisect of `attach.rs`'s handshake +path affordable, where before it was not. + +**It is still not attributed to the observing lane**, and now for a +measured reason rather than an argument from diff shape: the arm without +the lane's only `pmacs-gpu` addition is the arm that went red. + +**What would retire it is unchanged** — the mechanism, not the rate. +The next agent to touch this row should reproduce at 1-in-10 and +instrument which side closes the pipe, rather than re-running for green. + +### U2 — `m6_1_pty_raw_mode_disables_kernel_echo`, THIRD known occurrence + +**Corrected 2026-08-09 after review.** A previous edit of this row +called the 2026-08-09 failure the *second* occurrence and claimed it +captured the fragment for the first time. **Both were wrong**, and the +evidence was already in this repository: +`docs/active-work.md` records a **2026-08-06** loaded `--features crdt` +run failing this selector *and* `m6_1_pty_canonical_mode_keeps_kernel_echo` +with the same `stty -a output was: ""`, and it already proposed a +mechanism family — **read-before-write on the child's output**, the +shape of **R4** (readiness predicate satisfied by an empty file) and +**R6** (readiness file never published). + +So the fragment was captured before, under another feature flavor, and +this row's earlier "no mechanism has been proposed" was false of the +tree it was written in. | field | value | |---|---| | **selector** | `--lib process::tests::m6_1_pty_raw_mode_disables_kernel_echo` | | **job / flavor** | local (Linux), during `cargo test --tests --no-fail-fast` — the lib target alongside a full PTY-heavy corpus | -| **required fragments** | **none captured** — output was filtered to the `FAILED` line | -| **status** | **new incident, unreproduced** | -| **what IS established** | it failed once (`1916 passed; 1 failed`), in no registry row, under a full-corpus run | -| **what is NOT** | any mechanism. Not reproduced in a later full `--tests --no-fail-fast` sweep (108 targets, exit 0) nor in 3 isolated `--lib` runs (1917/0 each) | +| **required fragments** | `panicked at src/process.rs:3953` · `raw mode should disable echo; stty -a output was: ""` | +| **status** | **at least three occurrences, load-correlated; the diff is EXCLUDED on the 2026-08-09 one** | +| **what IS established** | **Three occurrences.** **(1)** the original: failed once (`1916 passed; 1 failed`) under a full-corpus `--tests --no-fail-fast` run, fragments not captured. **(2) 2026-08-06**, loaded `--features crdt`: this selector **and** `m6_1_pty_canonical_mode_keeps_kernel_echo` both failed with the same `stty -a output was: ""` — the first capture, and the occurrence that proposed the read-before-write family. **(3) 2026-08-09**, worker-identity tip: `1919 passed; 1 failed` in `scripts/gate` step `03-lib` at load ~21, and **the tree contained ZERO code change since a 13/13 green run on the same lane** — the only delta was three lines of `docs/active-work.md`. A markdown edit cannot break a PTY test, so the change under test is ruled out as a cause rather than merely doubted. Passes isolated (`1 passed`, 0.01s). **Occurrence 2 is the one that matters most**: it shows the failure is not confined to one feature flavor and can take both selectors at once | +| **what the fragment ACTUALLY shows** | **The supervisor collected empty stdout** — `drain_until` then `collect_stdout(&evs)` (`src/process.rs:3948-3951`); the assertion inspects that string. It does **NOT** establish that `stty` emitted nothing: the bytes could have been lost in PTY delivery or in event collection. An earlier edit of this row said "`stty` produced no output at all", which asserts a mechanism the test cannot see. What is true is narrower and still useful: this is not a *termios* failure — nothing shows echo being configured wrongly — but which of {child never wrote, PTY dropped it, collection missed it} is open. The assertion's message invites the wrong reading, since it prints an empty string as though it were `stty`'s answer | +| **what is NOT** | **No mechanism is ESTABLISHED** — one is *proposed*: read-before-write on the child's output, the R4/R6 readiness family (occurrence 2). Proposed is not confirmed, and nothing here discriminates it from PTY delivery or event-collection loss. Not reproduced in a later full sweep (108 targets, exit 0), nor in 3 isolated `--lib` runs (1917/0 each), nor in the isolated rerun after occurrence 3. **Three occurrences establish intermittence and a load correlation; none establishes cause** | +| **discriminating control for the next occurrence** | capture the **full process event stream and the child's exit disposition**, not only the collected string — that is what separates "child never wrote" from "delivery or collection lost it", and the collected string cannot distinguish them however many times it is sampled. Cross-check against R4/R6's readiness family, which `docs/active-work.md`'s 2026-08-06 entry already implicates | +| **cross-reference** | `docs/active-work.md` — 2026-08-06 occurrence, `--features crdt`, **both** the raw and canonical selectors, same fragment, read-before-write hypothesis | | **rival explanation not excluded** | leaked `pmacs --daemon` processes, which the handoff names as a standing confound for any load-sensitive local red | ### U3 — the R7 selector again, fragments lost the same way U2's were @@ -552,6 +630,54 @@ it again here by piping a sweep through `grep`. The fix is mechanical: stream. A signature that is cheap to capture and impossible to reconstruct should never be traded for terminal brevity. +*(Renumbered from U4/U5 to **U6/U7** on the rebase onto `0857bf4`: `gate-protocol-build` landed its own U4/U5 in #229, and git merged both files **without a conflict**, producing duplicate ids across four sites. The pre-rebase warning is retired here because it has been carried out.)* + +### U6 — two wall-clock budget tests fail together in one `lib-crdt` step + +Recorded during worker identity Stage 1 review round 2, 2026-08-09, in +the same gate run that produced R7's third occurrence. **Fragments were +captured**, so unlike U1–U3 this one is matchable — it is a `U` row +because it has one occurrence and no mechanism, not because the evidence +was lost. + +| field | value | +|---|---| +| **selector** | `--lib --features crdt optimistic::tests::criterion_1_end_of_line_typing_completes_sub_frame_per_keystroke` **and** `editor::tests::composition_overhead_under_ten_percent`, failing in the same run | +| **job / flavor** | local (Linux), `scripts/gate` step `04-lib-crdt`, with sibling worktrees building concurrently | +| **required fragments** | `criterion 1: per-keystroke orchestrator time` + `exceeds 1ms`; and `composition machinery added more than 10% overhead` | +| **status** | **new incident, one occurrence, not reproduced** | +| **what IS established** | both are **wall-clock budget assertions** — 1.264ms against a 1ms budget, and 1.297× against a 1.10× budget — so both are load-sensitive by construction. Both green in an isolated rerun of exactly those two selectors, and both green in the next full gate run of the same command (2105 passed) | +| **what is NOT** | whether the machine's concurrent load caused it. The confound is real (this machine runs one shared `CARGO_TARGET_DIR` and several worktrees) but **was not measured**, so it is a rival explanation, not a finding | +| **rival explanation not excluded** | a genuine regression in either path. Nothing in the observing diff touches the optimistic-echo orchestrator or the composition pipeline, but "my diff looks unrelated" is not evidence, and this row does not treat it as such | + +**Two budget tests failing in one run and neither in the next is the +signature worth matching**, more than either name alone: a real +regression in two unrelated subsystems at once is far less likely than +one loaded machine. If a future run reds **one** of these without the +other, that is a different incident and should be judged as one. + +### U7 — a *different* wall-clock render-budget test reds each sweep + +Recorded during worker identity Stage 1 review round 3, 2026-08-09. +**Two consecutive `scripts/gate` runs of the same command, on the same +tree, red on step `12-sweep` with a different test each time** — which +is the signature, and it is a stronger one than any single selector. + +| field | value | +|---|---| +| **selector** | run 1: `--test m8_2_acceptance dired_open_renders_10k_entries_under_200ms` **and** `--test m8_9_acceptance outline_5_level_100_entry_renders_within_100ms`; run 2: `--test dired_acceptance dired_renders_10k_entries_within_200ms` | +| **job / flavor** | local (Linux), `scripts/gate` step `12-sweep` (`cargo test --workspace --no-fail-fast`), **load average 12.9 / 23.9** with sibling worktrees building concurrently | +| **required fragments** | `must render within 200ms; took ` / `open() (parse + render) took ` + `spec budget is 100ms` | +| **status** | **new incident, three selectors, none reproduced** | +| **what IS established** | all three are **wall-clock render-budget assertions** (224ms and 258ms against a 200ms budget; 114ms against a 100ms budget), so all three are load-sensitive by construction. Each was green in an isolated rerun of its own selector, no selector reds twice, and **the third run of the same command on the same tree was green on all 13 steps** (log `20260809T200907Z-2672209`). The observing diff is **two string literals, their doc comments and one test** — it touches no render path at all, and cannot | +| **what is NOT** | that load caused it. The one-shared-`CARGO_TARGET_DIR` confound is real and again **unmeasured**, so it stays a rival explanation rather than a finding | +| **relation to U6** | same shape, different step and different tests: U6 is two budget tests in `04-lib-crdt` failing **together**; this is three render-budget tests in `12-sweep` failing **one per run**. Kept separate rather than merged, because merging would assert a shared mechanism nothing here shows | + +**The rotating selector is the thing to match.** A regression that +moved between three unrelated render paths on an unchanged tree is far +less likely than one loaded machine; a future run that reds the *same* +one of these twice is a different incident and should be judged as one. + **The retirements are not occurrences and do not close the log.** R1 and R3 stay live, and each retired row keeps its signature so a later red matching one reopens it. diff --git a/docs/worker-identity-framing.md b/docs/worker-identity-framing.md new file mode 100644 index 0000000..aa165e0 --- /dev/null +++ b/docs/worker-identity-framing.md @@ -0,0 +1,717 @@ +# Worker identity — Stage 1: what is running, and what it is doing + +*(Revision 1 was subtitled "and who asked for it". With `owner` +removed that title overclaimed the lane: it answers **what**, and — +under `pmacs.workers.dispatch` — **under which registered handler**. +Neither is who owns it.)* + +**Status: revision 4, APPROVED 2026-08-09. IMPLEMENTED — see +`docs/active-work.md` for the commits, the gate outcome and the review +rounds.** + +**Revision 4 scopes rule 1's claim to what it can actually enforce, and +takes Q#W-7 into this lane.** Revision 3 said the rule covered "all +yield points"; it covers **the two supported pmacs yield APIs**. Raw +`coroutine.yield` stays reachable — R46 is a convention, and the +scheduler diagnoses a non-Handle yield only *after* the coroutine has +suspended (`async.lua:197` resumes, `:212` inspects), so no refusal +sited in a yield helper can intercept it. The residual is named in §2 +rather than papered over. + +**Revision 3 closes a hole in revision 2's ambient: the extent it +called "synchronous" is not.** A registered handler is arbitrary Lua +and may `Handle:await()`, parking the coroutine with the name still +pushed so that unrelated later work inherits it. Rule 1 now **enforces** +non-yieldability rather than assuming it, following the guard this file +already carries for `pmacs.window.commit_to`. Scouting that guard +turned up a second supported yield API it does not cover — Q#W-7, a +pre-existing defect in another lane's invariant. Revision 3 reported it +rather than patching it in silence; **revision 4 fixes it here, on +approval**, since it is the same helper, the same invariant and the +same edit family. + +**Revision 2 removes `owner` and respecifies the handler-name path, +after review found the first dishonest and the second unbuildable as +described.** `owner` populated from static per-subsystem constants is +an *origin*, not an owner, and would misattribute third-party work at +exactly the point §9 wants attribution. And "the name is in hand at the +one place that throws it away" was **wrong about the call chain** — it +is thrown away across three layers, one of which callers are documented +to bypass. Both re-scouted in the tree. + +--- + +## 1. Why this, and why now + +`COHERENCE.md` §9 grades the worker model **mechanism without +identity**, and §0 names **step 11 (background-work ownership)** as one +of the two remaining thin ends of the golden journey. §20 Priority 1 is +blunt about where that leaves things: + +> **The remaining thin end is no longer inside this priority.** Step 1 +> is install, which is **P8**; step 11 is background-work ownership, +> which is §9. + +So this is the last of Priority 1's own journey, sitting in another +section's arc. Everything else P1 named has landed. + +**The felt gap is smaller and sharper than the arc.** §9's audit ends +with a claim that is checkable, and I checked it: + +> **No progress indicator exists anywhere** — no statusline spinner, no +> busy count. + +`grep -c -i "spinner\|progress\|busy" src/statusline.rs` returns **0**. +So §3's promise of "visible asynchronous work" is **false today** unless +the user knows to run `M-x editor.list-workers`. Every build, LSP index, +grep, parse and — as of the lane merging beside this one — every `git +status` runs with no indication that anything is happening at all. + +**And the git Stage 1 lane in flight right now makes it worse, by its +own admission.** `docs/git-integration-framing.md` Q#G-5 states it +plainly: git runs as a spawned process, spawned processes do not appear +in `*workers*`, and the lane therefore "adds a fifth thing that runs in +the background and is not attributable from one place". It accepted that +cost because these are short-lived reads. This lane is the one that +repays it. + +## 2. Ground truth + +Scouted in the tree, not recalled from the audit — and the audit has +drifted in one place, recorded below. + +- **The audit's `PendingJob` field list is stale, and the drift is + informative.** §9 lists seven fields; the struct + (`src/async_runtime.rs:367-411`) carries **eight**. The addition is + `resource: Option`, from dired Stage 2a — and **its doc + comment cites `COHERENCE.md` §9 by name** as the reason it is a field + on the job rather than a side map: + + > `COHERENCE.md` §9 is why this is a field on the job and not a side + > map — the parse job→buffer link already lives in a side map and §9 + > names that as the defect. + + So the precedent for putting identity **on the job** is already set, + already argued, and already merged. This lane extends a decision + rather than introducing one. + +- **There is a SINGLE allocation funnel, and that is what makes this + tractable.** Every job in the system is born in `allocate` + (`src/async_runtime.rs:746`), which delegates to + `allocate_with_resource` (`:757`). The ten `dispatch_*` methods + (`:803`–`:980`) and `register_external` (`:1011`, used by MCP and LSP) + all pass through it. An identity field added there reaches every job + by construction — there is no second birth site to miss. + +- **The two-function split is itself a warning.** `allocate_with_resource` + exists only because one prior lane needed one extra parameter. A + second lane doing the same produces + `allocate_with_resource_and_identity`, and a third produces something + worse. This is the point to collapse it (Q#W-1). + +- **`JobKind` is still a closed 12-variant enum** + (`src/async_runtime.rs:305-343`) — Sleep, ComputeSum, EmitN, Grep, + Parse, FsReadDir, FsStat, FsRename, FsChmod, FsRemove, McpRequest, + LspRequest. Confirmed unchanged since the audit. + +- **A third-party job's own name is retained nowhere, and recovering it + is NOT cheap. Revision 1 said it was, and was wrong about the call + chain.** The full path, read rather than assumed: + + ``` + pmacs.workers.dispatch(name, args, opts) -- async.lua:369 + → handlers[name](args, opts) -- arbitrary Lua + → dispatch_grep(spec, opts) -- Lua wrapper, :312 + → async_mod._dispatch_grep(spec, supersede_key(opts), max_batch) + → the Rust binding → allocate() + ``` + + **`name` is not a parameter of any layer below the first.** The Rust + dispatchers accept job arguments, a supersede key and stream data — + nothing else. So revision 1's "change the allocation funnel and the + name is recovered" is false: changing `allocate` gives the name + nowhere to arrive *from*. + + **And the wrapper layer cannot be the capture point either.** + `async.lua:337-345` deliberately exposes `pmacs.workers._new_handle` / + `_new_stream` so that "other builtin runtime files (`pmacs.fs` in + M8.1, future siblings) can construct handles for ids dispatched + through **their own raw `_dispatch_*` primitives**". A handler that + goes straight to `async_mod._dispatch_*` bypasses `dispatch_grep` and + friends entirely — and those are precisely the callers doing + non-standard work, i.e. the ones attribution is for. + + The audit's "every third-party job renders under a builtin's label" + is exact. The mechanism that fixes it is Q#W-2, and it is a real + mechanism, not a parameter. + +- **`ProcessSpec` has one identity field and it is a convention** + (`src/process.rs:193-235`): `label: String`, documented as + "human-readable ... surfaced in events and the `pmacs.process.list` + output". No owner, no purpose, no parent. Callers spell it however + they like (`lsp:{name}`, a terminal buffer name). + +- **A dynamic scope that must not be yielded out of ALREADY EXISTS + here, guard and rationale included.** `Handle:await()` refuses to run + inside `pmacs.window.commit_to` (`builtin/runtime/async.lua:87-90`), + raising *"await: cannot await inside pmacs.window.commit_to; await + first, then commit"*. Its comment states the hazard in general terms: + yielding out of the extent "would restore the scope while this + coroutine is still parked, so the rest of the commit would resume + ambient". `commit_to` itself is "an RAII guard on the Rust stack" — + the same shape this lane needs. + +- **There are TWO SUPPORTED yield APIs, not one.** `Handle:await()` + yields at `async.lua:95`; **`pmacs.async.yield_to_next_tick()` yields + at `async.lua:244`** and is public (`pmacs.async` is `async_public`, + `:247`). Any rule about a non-yieldable extent has to cover both. The + `commit_to` guard covers only the first — see Q#W-7. + +- **Raw `coroutine.yield` remains reachable, and NO guard of this shape + can cover it.** R46 is a convention — *"package code uses `:await()` + rather than `coroutine.yield`"* (`async.lua:26-27`) — not an + enforcement. The scheduler does diagnose a non-Handle yield + (`async.lua:217-223`, *"use Handle:await() per R46"*), **but only + after the fact**: `step` calls `coroutine.resume(co)` at `:197` and + inspects what came back at `:212`, by which point the coroutine has + already suspended. A refusal placed in a yield helper is never + consulted, and the enclosing `pmacs.workers.dispatch` never returns + to run its pop. + + So the honest bound is: a package that violates R46 *inside* a + dispatch-name scope can leak the name. It is not silent — the + scheduler raises it through `pmacs.error` into `*errors*` — but the + scope is not restored, and this framing does not claim otherwise. + +And the two findings that actually shape the design: + +- **A statusline provider API already exists, with three Lua adopters.** + `pmacs.statusline.register` is live in `terminal.lua:477`, + `syntax.lua:551` and `lsp.lua:1145`, taking + `{ name, side, priority, face, fn(ctx) }` and returning a string or + `nil`. An activity indicator is a **fourth registration**, not a new + mechanism. + + **The three are named by FILE above and by NAME in the registry, and + the two do not line up.** `syntax.lua` registers its provider as + **`"mode"`** (it projects the major mode, `syntax.lua:552`), so the + registry inventory reads `["mode", "terminal", "lsp"]` — which is what + `tests/statusline_segments_acceptance.rs` asserts. Recorded because it + is genuinely surprising: a reader looking for the syntax adopter by + name does not find one. A fourth registration therefore changes that + assertion, and where the new name sorts depends on **load order**, not + on the name: `async.lua` is evaluated before `syntax.lua`, + `terminal.lua` and `lsp.lua` (`src/editor.rs`), so a provider + registered there lands first. + + **And it is evaluated per frame**: `evaluate_statusline` is called + inside `paint_frame` (`src/editor.rs:4560`), before the long mutable + core borrow. So an indicator updates while work is in flight without + any new tick machinery — and, decisively for scheduling, **without + touching the wire**. `EvaluatedStatuslineSegment` is already + `Vec`-valued on an existing message; a fourth provider adds an element, + not a variant. + +- **`pmacs.process.list` deliberately hides terminal PTYs, and + un-hiding them is NOT free.** The binding filters to + `AnsiParserProfile::LineOriented` + (`src/lua_bindings/mod.rs:8980-8984`). `git log -S` dates that filter + to `bbc1f33 feat(vterm): add Stage 1 terminal core` — terminals were + excluded on purpose. + + **Three acceptance suites use `#pmacs.process.list()` as a leak + detector**: `tests/m6_8_multi_repl_acceptance.rs:385`/`:459` ("size + must not grow across cycles"), `tests/compile_mode_acceptance.rs:133`/ + `:458` ("process list returns to baseline"), and + `tests/lean4_stage1_acceptance.rs:327`/`:349`. **Removing the filter + would inflate every one of those baselines by each open terminal.** + + This is why §9's "a terminal PTY appears in no user-visible activity + view" is a real defect with a **non-obvious fix**, and why this lane + does not casually widen the existing accessor (Q#W-4). + +## 3. The staging, and why the line falls where it does + +§9's full statement wants owner, workspace, buffer, parent, children, +latency class, cancellation scope, resource budget, execution location, +progress, and failure attribution. **Two of those cannot be built at +all right now**: `Workspace` is §7, graded *missing*, and `Location` is +§8, graded *missing (architecture ready)*. A lane that added +`workspace: Option` would be adding a field typed on a +thing that does not exist. + +**Stage 1 (this lane): a required `purpose` on the job and the process, +and the first indicator. NO WIRE CHANGE. NO `owner`.** + +- **`purpose`, non-optional**, on `PendingJob`, carried through the + single allocation funnel, and on `ProcessSpec` alongside the existing + `label`. +- **A dispatch-identity ambient** so `pmacs.workers.dispatch` stops + discarding the registered handler name (Q#W-2). +- `*workers*` renders `purpose`. +- **A statusline activity indicator** — the fourth provider + registration, and the part a user feels on day one. + +**`owner` is deliberately absent, and revision 1 was wrong to include +it.** The proposal was `owner = "lsp"` populated from a static +per-subsystem constant at each dispatcher. But a generic dispatcher has +no trustworthy knowledge of who invoked it, and `pmacs.process.spawn` +is callable by any package — so a static subsystem label is an +**origin or category, not an owner**, and it would confidently +misattribute third-party work to a builtin at exactly the point §9 +wants attribution. A field that asserts a falsehood is worse than an +absent one: `*workers*` would *look* attributed while naming the wrong +party. + +**Nor is it retained under a safer name.** Calling it `origin` or +`subsystem` would be honest, but a second string field sitting beside +`purpose` and grouping the view would be *adopted* as ownership by the +next reader regardless of its name — and it would squat on the slot +P3's real package signal has to fill. Stage 2 needs a grouping key; it +should get a real one, not a placeholder promoted by use. + +**Stage 2 (separate lane): join the planes.** One activity view over +jobs, processes, LSP servers and terminals. This is what Stage 1's +identity is *for* — the audit's own conclusion is that "the four views +exist precisely because there is no common key to merge them on". It +also owns the terminal-visibility decision (Q#W-4), because that is a +question about the unified view, not about the accessor. + +**Stage 3 (unscheduled): the tree and scoped cancellation.** +`parent`/`children`, and cancel-by-owner / by-buffer / by-subtree. This +needs an ambient "currently-running job" context so a child dispatched +inside a job can find its parent without every call site threading it — +a real mechanism with its own failure modes, and the reason parent is +**not** in Stage 1 (Q#W-5). + +**Workspace and location are never this arc's**, at any stage. They +arrive from §7 and §8 and this arc consumes them. + +**The line falls at the wire on purpose, and it is again a scheduling +decision.** The discovery Stage 2 lane holds the v22→v23 bump slot, and +git Stage 2 is already queued behind it. `PROTOCOL_VERSION` is a strict +serialization point. Stage 1 here touching no wire is what lets it run +beside both. + +## 4. Coherence impact (§20) + +- **§9 worker ownership — the direct target**, and specifically the + audit's named prerequisite: *"Owner/purpose/parent fields on the job + and process specs are the prerequisite; the unified view and the + ownership tree fall out of them."* **Stage 1 takes ONE of the three + — `purpose`.** `owner` waits for P3 to supply a package signal worth + recording (§3); `parent` waits for Stage 3 (Q#W-5). Taking one of + three named prerequisites is a deviation from the audit, and it is + stated here rather than left to be noticed. +- **Journey step 11 — the direct target.** §0 names background-work + ownership as one of two remaining thin ends. This does not close the + step (Stage 2's unified view is most of that) but it is the first + thing that makes work *visible*, which is what step 11 is about. +- **§3 zero-configuration state:** repairs a claim that is currently + false. "Visible asynchronous work" becomes true by default, with no + configuration and no command to know about. +- **Interaction islands (§6): none added.** The indicator is a + statusline provider; it intercepts no keys and adds no precedence + rung. +- **§14 workbench primitives: untouched.** `*workers*` already exists; + this changes what it renders, not what renders it. +- **Config registry:** one setting at most, and my vote is a *visibility* + toggle only (Q#W-6). +- **The debt this repays is named and dated.** `git-integration-framing.md` + Q#G-5 recorded a deliberate negative §9 impact. This lane does not + fully discharge it — a labelled process is still not in `*workers*` + until Stage 2 — but it makes the process state *what it is doing* in + a required field rather than a caller-spelled convention. +- **No P3 alignment is claimed.** Revision 1 argued this lane aligned + with P3's ownership arc. With `owner` removed, it does not: P3 stays + entirely ahead of it, and this lane deliberately leaves that slot + empty rather than filling it with something P3 would have to displace. + +## 5. Open questions + +### Q#W-1 — how is identity supplied at the allocation funnel? + +The existing shape is `allocate(kind, supersede, stream)` delegating to +`allocate_with_resource(kind, supersede, stream, resource)`. Adding two +more positional parameters gives a five-argument function and a +six-argument variant, and the next lane adds a seventh. + +*My vote: **collapse the pair into one funnel taking a struct***, e.g. +`allocate(JobSpec { kind, supersede, stream, resource, purpose })`, so +the ten dispatchers read as named-field literals rather than positional +soup. Ten call sites plus `register_external` is a bounded, mechanical +edit, and it removes the `_with_resource` wart rather than adding +beside it. + +**`JobSpec` is private, and `purpose` is non-optional.** Private +because the public dispatcher APIs should not grow a parameter every +time this arc adds a field; non-optional because that is what makes the +compiler, rather than a test, the thing that proves every caller +supplied one (§6). A `Default` impl would defeat exactly that, so +`purpose` is not defaulted even if other fields are. + +**The counter-argument, which is real:** this touches every dispatcher +in a lane whose subject is identity, which is scope the reviewer did not +ask for. **If review prefers the minimal edit**, the alternative is one +more parameter on the existing pair, and the collapse becomes its own +small lane. I would rather be told than assume. + +### Q#W-2 — the dispatch identity path **(rewritten in rev 2, rule 1 added in rev 3)** + +Revision 1 treated this as a parameter-passing detail. §2 shows it is +not: `name` dies at `pmacs.workers.dispatch` and nothing below it takes +a name, so the value must be carried *out of band* across an arbitrary +handler. + +**Revision 2 then called the extent "synchronous" and assumed it. +Review found that it is not.** A registered handler is arbitrary Lua +running inside `pmacs.async`, and it may call `Handle:await()` — a +legal, yieldable path that the existing tests already exercise inside +`pcall`. While a handler is parked, its pushed name **stays on the +stack**, and every tick callback and every other coroutine that +allocates a job in the meantime inherits it. That is not a corner case; +it is the ordinary shape of a handler that awaits. + +So rule 1 below is no longer an observation about how handlers happen +to behave. It is an **enforced** property, and the enforcement already +has a precedent in this exact file (§2a). + +**The capture point is Rust, not Lua**, and the reason is the bypass in +§2. If the ambient lived in the Lua wrapper layer, a handler calling +`async_mod._dispatch_*` directly — the documented pattern for runtime +files with their own primitives — would produce an unattributed job, +and those are the callers attribution exists for. Putting it in the +runtime means it is read at `allocate`, **the same single funnel Q#W-1 +is already collapsing**. One mechanism, one site, no path around it. + +*My vote: **a dispatch-name stack owned by the async runtime***, with +`pmacs.workers.dispatch` bracketing its handler call through two +runtime-internal bindings (`_push_dispatch_name` / `_pop_dispatch_name`). + +**The contract, in full:** + +1. **THE EXTENT IS NON-YIELDABLE, AND THAT IS ENFORCED, NOT ASSUMED.** + Awaiting inside a dispatch-name scope is **refused**, because + yielding would park the coroutine with the name still pushed and + hand it to whatever allocates next. + + The guard is modelled on the one already in the file (§2): + `_in_dispatch_name_scope()` joins `_in_commit_scope()` as a refusal + in the same place, with the same shape of message and the same + remedy — **await first, then dispatch**. + + Three details that decide whether the guard actually holds: + + - **It rejects BEFORE parking.** The `commit_to` guard is the first + thing in `await`, ahead of the `_is_complete` check and the + `coroutine.yield`. The new one sits beside it, for the same + reason: a guard that fires after the yield has already happened + guards nothing. + - **It rejects UNCONDITIONALLY, not only when the handle is + incomplete.** A guard that fires only when a yield would really + occur has behaviour depending on whether the job happened to + finish first — it would pass under test and fail in production, + intermittently. `commit_to`'s guard is unconditional and this one + matches it. + - **It covers BOTH SUPPORTED YIELD APIs — and that is the exact + extent of the claim.** `pmacs.async.yield_to_next_tick()` + (`async.lua:243-245`) yields too, and is public, so it gets the + same refusal; guarding only `await` would leave the hole open + through a second door (and Q#W-7 is the proof that this happens, + because `commit_to` has exactly that gap today). + + **What rule 1 does NOT cover is raw `coroutine.yield`** (§2). + R46 forbids it to package code by convention only, and the + scheduler's diagnostic fires *after* suspension, so no refusal + sited in a yield helper can intercept it. Revision 3 said "all + yield points" and was overclaiming. The property is: **the + supported ways to yield are refused inside the scope; an R46 + violation can still leak the name, loudly.** +2. **Work dispatched later is NOT covered, deliberately.** A job + dispatched from an `on_complete` callback or a resumed coroutine + runs ticks later, outside the extent, and carries only its own + `purpose`. Pretending otherwise would need the asynchronous + lifetime mechanism this lane defers (Q#W-5). +3. **Nesting is a stack; innermost wins.** Handler `a` calling + `pmacs.workers.dispatch("b", …)` gives jobs allocated inside `b` the + name `b`, and restores `a` on return. +4. **Fan-out shares the name.** A handler dispatching five jobs + produces five jobs named alike. They *were* all dispatched under it; + that is the fact being recorded, not a collision. +5. **Unwind-safe, and this is the one that makes a naive version worse + than none.** A handler that errors must still pop — otherwise one + failure poisons every subsequent dispatch in the session with a + stale name, and the feature silently starts lying. `pmacs.workers. + dispatch` runs the handler under `pcall`, pops, and rethrows. +6. **Precedence over a caller-supplied purpose: COMPOSE, do not + replace.** Where the dispatch site supplied its own purpose, the + recorded value is `": "`; where it did not, the + recorded value is `""`. Replacing would recreate blocker 1 in + a new place — `dispatch_grep` supplies `"grep: …"`, and letting that + win would lose the third party again, while letting the name win + would discard the only description of the actual work. Composition + is capped at the innermost name by rule 3, so no unbounded chain. +7. **Outside any extent, nothing changes.** A builtin invoked directly + records its own `purpose`. + +**A known and accepted property, stated rather than discovered later:** +the ambient captures *causal* extent, not *intent*. If a handler +triggers unrelated work within its extent — an edit that schedules a +parse — that job takes the name. Because rule 1 refuses both supported +yield APIs, that window is bounded by a single un-parked call for any +caller obeying R46, and within such a window I think "this ran because +that handler ran" is the honest reading. (A caller violating R46 is +outside this property, and outside rule 1 — §2.) It is also the only definition enforceable at a single +funnel. **If review disagrees, the alternative is +capture-at-the-Lua-wrapper**, which is narrower and misses the raw +`_dispatch_*` callers — a trade of false positives for false negatives, +and I would rather over-attribute inside a bounded call than silently +drop the third-party case. + +**Why this ambient is admissible while Q#W-5's is not.** They are not +the same mechanism — **and revision 2 was entitled to that claim only +after rule 1 made it true.** As written in revision 2 the extent could +be parked by any awaiting handler, which is most of the way to the +asynchronous lifetime I used as the reason for deferring `parent`. +With rule 1 the difference is real and enforced: this is a +single-threaded dynamic extent that **cannot** be suspended, with a +deterministic pop on both the normal and the error path. A `parent` +ambient must span a job's asynchronous lifetime by design — across +ticks, through callbacks that run after the parent settled — and cannot +be fixed by refusing to yield, because yielding is the whole point. The +first is a stack; the second is a lifetime model. + +### Q#W-3 — what does the indicator actually show? + +*My vote: **a count plus the oldest in-flight job's `purpose`, and +nothing when idle*** — e.g. `⋯2 lsp: indexing`, absent entirely at +zero. With `owner` gone (§3) `purpose` is the only identity there is, +which is also why it is required rather than optional. + +**Oldest, not newest or "busiest".** Revision 1 said "busiest", which +is not a defined quantity — jobs carry no cost estimate. Oldest is +computable from `dispatched_at`, which `PendingJob` already has, and it +answers the question a user actually asks of a stuck editor: *what is +taking so long?* + +- **Absent at zero, not `0 jobs`.** A statusline segment that is always + present costs width forever to say "nothing is happening". The + existing providers already return `nil` to render nothing + (`lsp.lua:1156`), so this is the established idiom. +- **A count, not a spinner.** A spinner needs an animation frame clock + and says only "something"; a count says how much. Per-frame evaluation + makes either possible, so this is a product choice, not a constraint. +- **Not names plural.** One purpose keeps it to a bounded width; the + full list is what `*workers*` is for. + +### Q#W-4 — do terminal PTYs become visible in Stage 1? + +**No — and the reason is evidence, not caution.** `pmacs.process.list` +filters to `LineOriented`, and three acceptance suites assert on +`#pmacs.process.list()` as a leak baseline (§2). Widening that accessor +would inflate all three with every open terminal, and "fix the tests" +is the wrong response to a test that is correctly detecting a semantic +change. + +*My vote: **leave the accessor alone in Stage 1**, and let Stage 2's +unified view introduce a **separate** enumeration that includes PTYs.* +The leak detectors keep asserting what they were written to assert; the +new surface answers the new question. Two accessors with different +contracts is better than one accessor whose meaning silently changed +under its existing callers. + +### Q#W-5 — does `parent` belong in Stage 1? + +*My vote: **no.*** The audit names owner/purpose/**parent** together as +the prerequisite, and after revision 2 this lane takes only `purpose` — +so both omissions need justifying, not just this one. `owner`'s is in +§3; `parent`'s is here. + +`purpose` is a **value the dispatcher already knows** at the call site. +A parent is not — it is whatever job is *currently running* when a +child is dispatched. A `parent` field that nothing populates is worse +than no field: it renders as `None` everywhere and reads as "this job +has no parent" rather than "this system does not track parents". + +**And the objection this has to answer, since the lane now builds an +ambient of its own (Q#W-2):** why is one admissible and not the other? +Because Q#W-2's extent **cannot be suspended** — rule 1 refuses both +yield points, so it is bounded by one un-parked call with a +deterministic pop on the normal and the error path. + +**That distinction is only load-bearing because rule 1 exists.** +Revision 2 asserted this same paragraph while its ambient *could* be +parked by any awaiting handler, which made the two mechanisms far more +alike than the argument admitted. The honest version: a `parent` +ambient must identify the running job *across ticks* — a job dispatched +from an `on_complete` callback should name the job whose completion +fired it, and that callback runs after the parent settled, outside any +dispatch call. Refusing to yield cannot rescue it, because yielding is +the mechanism it needs. That is a lifetime model, not a stack, and it +is Stage 3's subject rather than a field this lane can add cheaply. + +Stage 3 builds the lifetime model and the field together, where the +field can be tested by a populated case. + +### Q#W-7 — the same hole exists in `commit_to` today — **RESOLVED, fixed here (rev 4)** + +Found while scouting rule 1, and reported rather than quietly patched. + +`Handle:await()` refuses to run inside `pmacs.window.commit_to` +(`async.lua:87-90`) precisely so a coroutine cannot park with the +frontend scope pushed. **But `pmacs.async.yield_to_next_tick()` +(`async.lua:243-245`) also yields, is public, and carries no such +refusal.** A coroutine inside `commit_to` can therefore park through +that door and produce exactly the misrouting the `await` guard exists +to prevent. Journey Stage 1a's Q#JR14b invariant has a second entrance. + +I have **not** verified that a real caller does this — the reachability +of the bug is unproven, and I would rather say so than dress a +code-reading up as a repro. + +**RESOLVED — approved for this lane.** It is the same supported yield +helper, the same invariant, and the same `async.lua` edit family; +splitting it would preserve a known hole without reducing integration +risk. So `yield_to_next_tick` gains **both** refusals — the new +`_in_dispatch_name_scope()` and the missing `_in_commit_scope()` — and +the `commit_to` gap closes in the same commit as rule 1. + +**Its witnesses are the same pair as rule 1's, not a smoke test:** the +refusal fires, **and** the commit scope is restored afterwards. A guard +that raises while leaving the scope pushed converts a silent misrouting +into a noisy one and fixes nothing. + +Reachability by a real caller stays **unproven** — this is a defect +found by reading, and the tests pin the guard rather than reproducing a +user-visible bug. That distinction belongs in the commit message too, +so nobody later cites this as evidence the bug was observed. + +### Q#W-6 — is any of this configurable? + +*My vote: **one boolean, `ui.activity-indicator` (default `true`), +through `pmacs.config.define`.*** §11 grades the registry "partial +(foundation only)" and this document's sibling framings have both +resisted speculative settings — but a permanently-visible statusline +element is different in kind from an internal behaviour: it costs width +on every frame, and "I do not want this in my modeline" is a +preference someone will genuinely hold on day one rather than a +hypothetical. `git.enabled` and `ui.line-wrap` are the precedent shape. + +No setting for `purpose` capture itself — that is substrate, not +preference. + +## 6. Verification + +- **Presence is enforced by the COMPILER, not by a test.** `purpose` is + non-optional in `JobSpec`, so a dispatcher that supplies none does not + build. Revision 1 claimed a single funnel assertion proved "every job + carries an identity"; **it does not** — a funnel test proves the + funnel stores what it was handed, and says nothing about whether + fourteen callers handed it anything meaningful. Presence is a type + obligation; the tests below are for *semantics*. +- **Representative entry paths assert the semantics**, one per distinct + shape rather than one per dispatcher: a pool dispatcher, an + `register_external` job (MCP/LSP bypass the worker pool entirely and + are the likeliest to be missed), and a spawned process. +- **A `pmacs.workers.dispatch("name", …)` job reports `"name"`**, and + the witness is **a handler registered from Lua that calls a real + dispatcher** — not a synthetic funnel test. A test that pushes the + ambient by hand proves the stack works and leaves the actual defect + (`name` dying in an arbitrary handler) unwitnessed. +- **Awaiting inside a handler is REFUSED, and the scope restores after + the refusal** (Q#W-2 rule 1). Two assertions, and the second is the + load-bearing one: a guard that raises but leaves the name pushed has + converted a silent misattribution into a silent misattribution plus + an error. The witness dispatches again after the rejection and + asserts the new job carries **no** stale name. +- **`pmacs.async.yield_to_next_tick()` inside a handler is refused + too**, with the same restore-after assertion. Guarding one supported + yield API and not the other leaves the hole open through a second + door (§2). +- **`yield_to_next_tick` inside `pmacs.window.commit_to` is refused, + and the commit scope restores after the refusal** (Q#W-7) — the + pre-existing gap, closed here. Both halves asserted, for the same + reason as rule 1's: a refusal that leaves the scope pushed has + swapped a silent fault for a loud one. +- **NOT asserted, and deliberately: that a raw `coroutine.yield` + inside either scope is prevented.** It is not (§2). Writing a test + that "proves" coverage this design does not have would be worse than + the gap, and the gap is recorded instead. +- **The refusal fires even when the awaited handle is already + complete** (rule 1) — the case that separates an unconditional guard + from one whose behaviour depends on a race. +- **The ambient survives a failing handler** (Q#W-2 rule 5): a handler + that errors, then a subsequent unrelated dispatch, asserting the + second job does **not** carry the first's name. This is the + regression that would otherwise appear as intermittent + misattribution long after the lane lands. +- **Nesting and fan-out** (rules 3–4): a handler dispatching two jobs + gives both its name; a handler dispatching through another registered + handler gives the inner jobs the inner name and restores the outer. +- **Composition, not replacement** (rule 6): a handler calling a + dispatcher that supplies its own purpose yields `": "` + — asserted for both halves, since a test on the prefix alone passes + when the description is dropped. +- **Work dispatched from an `on_complete` callback carries no handler + name** (rule 2) — the boundary of the extent, asserted deliberately + so it reads as designed rather than broken. +- **The statusline shows nothing at idle**, asserted as *absent + segment*, not as empty string — a zero-width segment still consumes a + separator. +- **The statusline shows a count while work is in flight**, witnessed + through the real per-frame evaluation path (`paint_frame`), not by + calling the provider function directly. A provider that works in + isolation and never gets evaluated is the failure this must exclude. +- **The indicator honours `ui.activity-indicator = false`** (Q#W-6), + witnessed as an absent segment with work genuinely in flight — the + case that separates "disabled" from "idle". +- **`#pmacs.process.list()` is UNCHANGED for every existing caller** + (Q#W-4). The three leak-detector suites + (`m6_8_multi_repl_acceptance`, `compile_mode_acceptance`, + `lean4_stage1_acceptance`) are the assertion, and they must pass + untouched. **If any of them needs editing, the design is wrong**, and + that is the signal to stop rather than to adjust a baseline. +- **A spawned process carries a required `purpose` alongside its + existing `label`**, and **`label`'s current callers keep working + unchanged** — `lsp:{name}` and terminal buffer names are live + conventions with existing consumers. +- **Both frontends render the segment**, since it rides the existing + `StatuslineSegments` path — asserted for the grid TUI and + `pmacs-gpu`, because "it is on an existing message" is a claim about + the producer and says nothing about whether a consumer draws it. + +**What this will NOT prove:** that background work is attributable from +one place (that is Stage 2's unified view — this lane makes it +*possible*, not *done*), that a terminal PTY is visible anywhere +(Q#W-4), that cancellation can range over an owner (Stage 3), or **that +any job is attributed to the PACKAGE responsible for it** — `purpose` +records what work is being done and, under `pmacs.workers.dispatch`, +which registered handler it ran under. Neither is package ownership, +which waits for P3 (§3). + +Gates via `scripts/gate --acceptance `. **No +`--protocol`**: this lane has no wire change, which is the property that +lets it run beside the two lanes already in flight. + +## 7. Not in scope + +**Making raw `coroutine.yield` safe inside either dynamic scope** (§2, +rule 1). R46 forbids it by convention and the scheduler diagnoses it +after the fact; closing it properly means enforcement the runtime does +not have, and this lane claims only the two supported yield APIs. +**`owner`, in any spelling** — including `origin` or `subsystem` (§3). +The slot stays empty until P3 can fill it with a package signal; +nothing in this lane may be promoted into it later by use. +`Workspace` and `Location` fields (§7/§8 — the entities do not exist). +`parent`/`children` and the ownership tree (Stage 3, Q#W-5). Scoped +cancellation of any kind — cancel-all, by-kind, by-buffer, by-owner, +by-subtree (Stage 3; there is nothing to range over until identity +exists). The unified activity view joining the four planes (Stage 2). +Making terminal PTYs visible (Stage 2, Q#W-4). Widening `JobKind` or +making it open — third-party jobs are described by `purpose`, which is +the point, and reopening a closed wire-adjacent enum is a separate +decision. Latency classes and resource budgets (§9 names them; +neither has a consumer yet). Supersession coverage — §9 notes parse jobs +and MCP requests pass `None`, which is a real defect and a **different** +one. P3's package-ownership signal — §3 defers `owner` to it and makes +no claim of alignment with it. diff --git a/pmacs-gpu/src/main.rs b/pmacs-gpu/src/main.rs index 78eb64a..8f5d817 100644 --- a/pmacs-gpu/src/main.rs +++ b/pmacs-gpu/src/main.rs @@ -14939,6 +14939,95 @@ mod tests { assert_eq!(after[2].1, Color::rgb(20, 220, 40)); } + /// Worker identity Stage 1 (`docs/worker-identity-framing.md` §6): + /// the GPU half of "both frontends render the segment". + /// + /// The activity indicator adds no wire message — it rides the + /// existing `StatuslineSegments` vector as a fourth provider's + /// element. But that is a claim about the **producer**, and says + /// nothing about whether a consumer draws it, which is why this + /// exists on the consumer side. + /// + /// Two properties specific to this segment, neither of which the + /// existing rich-runs test covers: + /// + /// * its face (`ui.modeline.activity`) is **deliberately absent + /// from `ThemeFacts`** — no theme sets it, and `theme_facts_msg` + /// ships only faces that resolve — so a consumer that dropped + /// segments with an unknown face would silently lose the one + /// thing telling the user the editor is busy; + /// * its text leads with a non-ASCII `⋯`, which a byte-oriented + /// composition step would mangle. + #[test] + fn the_activity_segment_survives_an_unthemed_face_and_a_non_ascii_lead() { + let Some(mut state) = headless_or_skip(500, 280, "text") else { + return; + }; + let buffer_id = BufferId::next(); + state.current_buffer_id = Some(buffer_id); + state.status_facts = Some(status_facts(buffer_id, None)); + state.own_cursor = Some(OwnCursor { buffer_id, byte: 0 }); + // One themed face, and NOT the activity one: the point is that + // the theme has an opinion about some segments and none about + // this one. + apply_faces( + &mut state, + vec![theme_face( + "ui.modeline.lsp", + CellStyle { + fg: CellColor::Rgb(20, 220, 40), + ..CellStyle::default() + }, + )], + ); + apply_statusline( + &mut state, + buffer_id, + Vec::new(), + vec![ + statusline_segment("LSP:rust", "ui.modeline.lsp"), + statusline_segment("⋯2 lsp textDocument/definition", "ui.modeline.activity"), + ], + ); + + let right = state.compose_status_runs(); + let text: String = right.iter().map(|(text, _)| text.as_str()).collect(); + assert!( + text.contains("⋯2 lsp textDocument/definition"), + "the activity segment must reach the composed right runs \ + intact: {text:?}" + ); + let activity = right + .iter() + .find(|(run, _)| run.contains('⋯')) + .expect("activity run"); + assert_eq!( + activity.1, + state.status_right_base_color(), + "an unthemed modeline face falls back to the base colour \ + rather than dropping the segment" + ); + assert_eq!( + right[0].1, + Color::rgb(20, 220, 40), + "and its themed neighbour still takes its own colour" + ); + + // And it survives the real shaping pass, not only composition. + let _ = state.render_offscreen(); + let shaped: String = state + .status_runs + .as_ref() + .expect("right shaped") + .iter() + .map(|(text, _)| text.as_str()) + .collect(); + assert!( + shaped.contains("⋯2 lsp textDocument/definition"), + "{shaped:?}" + ); + } + #[test] fn modal_left_precedence_suppresses_custom_left_but_preserves_right() { let Some(mut state) = headless_or_skip(420, 260, "text") else { diff --git a/src/async_runtime.rs b/src/async_runtime.rs index 3620a32..284d2ee 100644 --- a/src/async_runtime.rs +++ b/src/async_runtime.rs @@ -58,8 +58,10 @@ //! search with cooperative cancellation and frame-boundary coalescing. //! Tree-sitter and LSP land in M4 on the same dispatch shape. +use std::borrow::Cow; use std::cell::{Cell, RefCell}; use std::collections::{HashMap, VecDeque}; +use std::fmt::Write; use std::path::{Path, PathBuf}; use std::rc::Rc; use std::sync::atomic::{AtomicU64, Ordering}; @@ -408,6 +410,47 @@ struct PendingJob { /// job→buffer link already lives in a side map and §9 names that as /// the defect. resource: Option, + /// What this job is doing, in words a user can read (worker + /// identity Stage 1, `COHERENCE.md` §9). + /// + /// **Not an owner.** It records *what work* is running and — when + /// the job was born inside a `pmacs.workers.dispatch` extent — the + /// registered handler name it ran under. Neither is the package + /// responsible for it; that slot is deliberately empty until P3 can + /// fill it with a real package signal (framing §3). + /// + /// Non-optional by construction: [`JobSpec`] has no `Default`, so a + /// dispatcher that supplies none does not compile. + purpose: String, +} + +/// Everything one job is born with. +/// +/// **Private, and deliberately so** (framing Q#W-1). The two-function +/// `allocate` / `allocate_with_resource` split existed only because one +/// prior lane needed one extra parameter; a second lane doing the same +/// produces `allocate_with_resource_and_identity`. Collapsing the pair +/// into a struct means the next field is a named literal at each of the +/// eleven construction sites rather than another positional parameter on +/// a public signature. +/// +/// **There is no `Default` impl, and that is the point.** `purpose` is +/// what makes the compiler — not a test — the thing that proves every +/// dispatcher supplied one (framing §6). A `Default` would let a new +/// dispatcher write `..Default::default()` and silently ship an empty +/// identity. +struct JobSpec<'a> { + /// Which builtin handler this job runs. + kind: JobKind, + /// Supersede key, if the dispatch opted into supersession. + supersede: Option<&'a str>, + /// `Some(max_batch)` marks this as a streaming dispatch. + stream: Option, + /// Filesystem mutation this job performs, for the settle-time + /// reconcile (dired Stage 2a). + resource: Option, + /// What the job is doing. See [`PendingJob::purpose`]. + purpose: String, } /// A settled filesystem mutation, with the paths the worker consumed @@ -490,6 +533,9 @@ pub struct ActiveJobInfo { /// True if this is a streaming dispatch (`emit_n`, `grep`, ...); /// false if it's request/reply (`sleep`, `compute_sum`). pub is_stream: bool, + /// What this job is doing (worker identity Stage 1). Rendered by + /// `*workers*` and by the statusline activity indicator. + pub purpose: String, } /// One row in the `*workers*` buffer's "completed" section: a job @@ -507,6 +553,8 @@ pub struct CompletedJobInfo { pub settled_age_ms: u64, /// Supersede key (if any) the job was dispatched under. pub supersede_key: Option, + /// What this job was doing (worker identity Stage 1). + pub purpose: String, /// Terminal outcome. `None` is unreachable here --- only /// settled jobs land in the completed ring. pub outcome: JobOutcome, @@ -543,9 +591,105 @@ struct CompletedSlot { dispatched_at: Instant, settled_at: Instant, supersede_key: Option, + purpose: String, outcome: JobOutcome, } +/// What the statusline activity indicator needs, and nothing more +/// (framing Q#W-3). +/// +/// A dedicated read surface rather than [`WorkersSnapshot`]: the +/// indicator is evaluated once per visible window per frame, and a +/// snapshot clones the whole completed ring (up to +/// [`COMPLETED_RING_CAP`] entries) that the indicator never looks at. +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct ActivitySummary { + /// How many jobs are in flight. Always ≥ 1 — an idle runtime + /// returns `None` rather than a zero count, because a segment that + /// is always present costs modeline width forever to say "nothing + /// is happening". + pub in_flight: usize, + /// The **oldest** in-flight job's purpose, already passed through + /// [`purpose_for_one_row`]. + /// + /// Oldest, not newest and not "busiest": jobs carry no cost + /// estimate, so "busiest" is not a defined quantity, while oldest + /// is computable from `dispatched_at` and answers the question a + /// user actually asks of a stuck editor. + /// + /// Escaped here rather than at the Lua provider because this struct + /// **is** the indicator's read surface — it exists for one consumer, + /// and that consumer has exactly one row. `workers_snapshot` is the + /// free-form path and stays raw. + pub oldest_purpose: String, +} + +/// A `purpose` rendered for a surface that gives it exactly **one row**. +/// +/// # A row must not be able to forge another row +/// +/// That is the property, and it is the only reason this exists. A +/// purpose is free-form text supplied by whoever dispatched the work, +/// and it is legitimately multi-line: a filesystem path may contain a +/// newline, and `pmacs-magit`'s spawn purpose is a whole argv. Rendered +/// raw into a row-per-job table, one such purpose becomes two physical +/// lines — the second of which the reader has no way to tell from a real +/// job row, because a real job row is just text in the same buffer. +/// The same applies to `\r`, which rewrites a rendered line in place on +/// a terminal, and to `\u{1b}`, which starts an escape sequence in one. +/// +/// # Escape, do not reject, and do not clip +/// +/// This follows the `#228` decision recorded on +/// [`crate::command::Command::description`]: the one-line constraint +/// belongs to the **surface that has it**, not to the registry that does +/// not. There, a free-form description is clipped by +/// `Command::description_first_line` at the two single-row consumers +/// while the registry keeps every line. Here the equivalent is escaping +/// rather than clipping, because a purpose's later lines are not +/// decoration — an argv's second word is as load-bearing as its first, +/// and a clip would silently drop the part that says which file. +/// +/// `pmacs.workers.snapshot()` is this lane's `describe-command`: it +/// hands Lua the raw purpose, so nothing is lost, only made safe where +/// a row boundary means something. +/// +/// # What is not escaped +/// +/// A backslash. Escaping it would make a purpose containing no control +/// characters **not** byte-identical after this call, and byte-identity +/// for ordinary text is a property worth more than distinguishing a +/// literal `\n` from an escaped newline — the ambiguity is cosmetic, +/// while forging a row is not, and no amount of literal backslashes +/// produces a second row. +#[must_use] +pub fn purpose_for_one_row(purpose: &str) -> Cow<'_, str> { + // `char::is_control` is the Unicode `Cc` category: C0 (`\0`–`\x1f`), + // `\x7f`, and C1 (`\u{80}`–`\u{9f}`, which includes NEL). Borrowing + // when there is nothing to do keeps the common path allocation-free + // AND makes the byte-identity property structural rather than + // asserted. + if !purpose.contains(char::is_control) { + return Cow::Borrowed(purpose); + } + let mut out = String::with_capacity(purpose.len() + 8); + for ch in purpose.chars() { + match ch { + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + other if other.is_control() => { + // `\u{1b}`, the same spelling Rust's own `escape_debug` + // uses, so the rendered form is one a reader can paste + // back into either language and get the byte returned. + let _ = write!(out, "\\u{{{:x}}}", other as u32); + } + other => out.push(other), + } + } + Cow::Owned(out) +} + /// One frame's worth of streamed items for a single stream id, /// returned by [`AsyncRuntime::take_stream_batches`]. T M3.5. #[derive(Clone, Debug)] @@ -610,6 +754,29 @@ pub struct AsyncRuntime { /// only contended at parse settle/take time --- never inside the /// editor's hot path. T M4.1. parse_handoff: Arc>>>, + /// Registered handler names of the `pmacs.workers.dispatch` calls + /// currently on the stack (worker identity Stage 1, Q#W-2). + /// + /// `pmacs.workers.dispatch(name, …)` looks `name` up, calls the + /// handler, and returns whatever it returns — **`name` is not a + /// parameter of any layer below that call**, and a handler that + /// reaches straight for `pmacs._async._dispatch_*` bypasses the Lua + /// wrapper layer entirely. So the name has to travel out of band, and + /// it is read here, at the one allocation funnel every job passes + /// through. + /// + /// A stack, not a slot: nesting is real (a handler may dispatch + /// through another registered handler) and innermost wins. + /// + /// **The extent is non-yieldable, and `async.lua` enforces it** — + /// both supported yield APIs refuse inside it, because parking a + /// coroutine with a name still pushed hands that name to whatever + /// allocates next. The one hole is a raw `coroutine.yield`, which + /// violates R46 and which no refusal sited in a yield helper can + /// intercept (the scheduler only sees the yielded value after the + /// coroutine has already suspended). That residual is recorded in + /// `docs/worker-identity-framing.md` §2, not claimed closed. + dispatch_names: RefCell>, } /// Default cap on stream items delivered in a single drain. 1024 @@ -650,6 +817,7 @@ impl AsyncRuntime { frame_target_ms: Cell::new(DEFAULT_FRAME_TARGET_MS), completed: RefCell::new(VecDeque::with_capacity(COMPLETED_RING_CAP)), parse_handoff: Arc::new(Mutex::new(HashMap::new())), + dispatch_names: RefCell::new(Vec::new()), } } @@ -733,34 +901,83 @@ impl AsyncRuntime { self.default_max_batch.set(n.clamp(1, 1_000_000)); } + /// Push a `pmacs.workers.dispatch` handler name for the dynamic + /// extent of that handler's call (worker identity Stage 1, Q#W-2). + /// + /// Paired with [`Self::pop_dispatch_name`] by + /// `pmacs.workers.dispatch`, which brackets the handler call under + /// `pcall` so a raising handler still pops. An unpaired push is the + /// failure mode that matters: it would poison every later dispatch + /// in the session with a stale name, and the feature would start + /// lying silently rather than loudly. + pub fn push_dispatch_name(&self, name: impl Into) { + self.dispatch_names.borrow_mut().push(name.into()); + } + + /// Pop the innermost dispatch-handler name. No-op when the stack is + /// already empty — an unbalanced pop is a Lua-side bug, and + /// panicking here would turn it into a torn editor rather than a + /// missing label. + pub fn pop_dispatch_name(&self) { + self.dispatch_names.borrow_mut().pop(); + } + + /// Whether a `pmacs.workers.dispatch` handler is on the stack. + /// + /// Read from Lua as `pmacs._async._in_dispatch_name_scope()`. Both + /// supported yield APIs refuse while it is set (Q#W-2 rule 1), for + /// the same reason `Handle:await` refuses inside + /// `pmacs.window.commit_to`: yielding would park the coroutine with + /// the name still pushed, and the next allocation — in any + /// coroutine, on any later tick — would inherit it. + #[must_use] + pub fn in_dispatch_name_scope(&self) -> bool { + !self.dispatch_names.borrow().is_empty() + } + + /// The innermost dispatch-handler name, if any. Nesting is a stack + /// and innermost wins (Q#W-2 rule 3). + #[must_use] + pub fn current_dispatch_name(&self) -> Option { + self.dispatch_names.borrow().last().cloned() + } + /// Register a fresh pending entry and return its id + cancel /// token. The token is what the worker closure polls; the entry /// is what `tick` updates on reply. /// - /// If `supersede_key` is `Some(key)`, any in-flight predecessor + /// **This is the single allocation funnel**: every job in the + /// system — the ten `dispatch_*` methods and + /// [`Self::register_external`] alike — is born here, which is what + /// makes the identity field reachable by construction rather than by + /// audit. + /// + /// If `spec.supersede` is `Some(key)`, any in-flight predecessor /// under the same key has its cancel token flipped *before* this /// allocation returns, and the `key → id` table is updated to /// point at the new id. The predecessor's pending entry is /// retained --- its worker will produce a `Cancelled` reply that /// `tick` then surfaces. - fn allocate( - &self, - kind: JobKind, - supersede_key: Option<&str>, - stream: Option, - ) -> (JobId, CancellationToken) { - self.allocate_with_resource(kind, supersede_key, stream, None) - } - - /// [`Self::allocate`], plus the filesystem mutation this job - /// performs. Only the two mutating fs dispatchers pass `resource`. - fn allocate_with_resource( - &self, - kind: JobKind, - supersede_key: Option<&str>, - stream: Option, - resource: Option, - ) -> (JobId, CancellationToken) { + /// + /// The recorded purpose **composes** with any dispatch-name ambient + /// rather than replacing it (Q#W-2 rule 6): `": "` + /// where the dispatcher described its own work, `""` where it + /// did not. Letting the dispatcher's purpose win would lose the + /// third-party caller all over again; letting the name win would + /// discard the only description of the actual work. + fn allocate(&self, spec: JobSpec<'_>) -> (JobId, CancellationToken) { + let JobSpec { + kind, + supersede: supersede_key, + stream, + resource, + purpose, + } = spec; + let purpose = match self.current_dispatch_name() { + Some(name) if purpose.is_empty() => name, + Some(name) => format!("{name}: {purpose}"), + None => purpose, + }; let id = self.next_job_id.fetch_add(1, Ordering::Relaxed); let cancel = CancellationToken::new(); if let Some(key) = supersede_key { @@ -788,6 +1005,7 @@ impl AsyncRuntime { kind, dispatched_at: Instant::now(), resource, + purpose, }, ); (id, cancel) @@ -801,7 +1019,13 @@ impl AsyncRuntime { /// dispatched under `key` is cancelled before this dispatch /// returns. T M3.4 / [spec §6.3]. pub fn dispatch_sleep(&self, ms: i64, supersede: Option<&str>) -> JobId { - let (id, cancel) = self.allocate(JobKind::Sleep, supersede, None); + let (id, cancel) = self.allocate(JobSpec { + kind: JobKind::Sleep, + supersede, + stream: None, + resource: None, + purpose: format!("sleep {}ms", ms.max(0)), + }); let bus = self.workers.clone(); let total = Duration::from_millis(ms.max(0).unsigned_abs()); self.pool.dispatch(move |_pool| { @@ -816,7 +1040,13 @@ impl AsyncRuntime { /// the granular cancel boundary. `supersede` follows the same /// rule as [`Self::dispatch_sleep`]. pub fn dispatch_compute_sum(&self, n: u64, supersede: Option<&str>) -> JobId { - let (id, cancel) = self.allocate(JobKind::ComputeSum, supersede, None); + let (id, cancel) = self.allocate(JobSpec { + kind: JobKind::ComputeSum, + supersede, + stream: None, + resource: None, + purpose: format!("sum 1..{n}"), + }); let bus = self.workers.clone(); self.pool.dispatch(move |_pool| { let kind = run_compute_sum(&cancel, n); @@ -842,7 +1072,13 @@ impl AsyncRuntime { max_batch: Option, ) -> JobId { let cap = max_batch.map_or_else(|| self.default_max_batch.get(), |n| n.clamp(1, 1_000_000)); - let (id, cancel) = self.allocate(JobKind::EmitN, supersede, Some(cap)); + let (id, cancel) = self.allocate(JobSpec { + kind: JobKind::EmitN, + supersede, + stream: Some(cap), + resource: None, + purpose: format!("emit {count} items"), + }); let bus = self.workers.clone(); self.pool.dispatch(move |_pool| { run_emit_n(&cancel, &bus, id, count); @@ -870,7 +1106,13 @@ impl AsyncRuntime { max_batch: Option, ) -> JobId { let cap = max_batch.map_or_else(|| self.default_max_batch.get(), |n| n.clamp(1, 1_000_000)); - let (id, cancel) = self.allocate(JobKind::Grep, supersede, Some(cap)); + let (id, cancel) = self.allocate(JobSpec { + kind: JobKind::Grep, + supersede, + stream: Some(cap), + resource: None, + purpose: format!("grep {:?} in {}", spec.pattern, spec.root.display()), + }); let bus = self.workers.clone(); self.pool.dispatch(move |_pool| { run_grep(&cancel, &bus, id, spec); @@ -897,7 +1139,13 @@ impl AsyncRuntime { /// in-flight predecessor under the same key has its cancel token /// flipped synchronously. T M4.1 / [spec §6.3]. pub fn dispatch_parse(&self, spec: ParseRequest, supersede: Option<&str>) -> JobId { - let (id, cancel) = self.allocate(JobKind::Parse, supersede, None); + let (id, cancel) = self.allocate(JobSpec { + kind: JobKind::Parse, + supersede, + stream: None, + resource: None, + purpose: format!("parse {}", spec.language_name), + }); let bus = self.workers.clone(); let handoff = self.parse_handoff.clone(); self.pool.dispatch(move |_pool| { @@ -922,7 +1170,13 @@ impl AsyncRuntime { tolerance: ReadDirTolerance, supersede: Option<&str>, ) -> JobId { - let (id, cancel) = self.allocate(JobKind::FsReadDir, supersede, None); + let (id, cancel) = self.allocate(JobSpec { + kind: JobKind::FsReadDir, + supersede, + stream: None, + resource: None, + purpose: format!("read_dir {}", path.display()), + }); let bus = self.workers.clone(); self.pool.dispatch(move |_pool| { let kind = run_fs_read_dir(&cancel, &path, tolerance); @@ -934,7 +1188,13 @@ impl AsyncRuntime { /// Dispatch a `stat(path)` job. Returns one [`FsDirEntry`] of /// metadata for `path`. T M8.1. pub fn dispatch_fs_stat(&self, path: PathBuf, supersede: Option<&str>) -> JobId { - let (id, cancel) = self.allocate(JobKind::FsStat, supersede, None); + let (id, cancel) = self.allocate(JobSpec { + kind: JobKind::FsStat, + supersede, + stream: None, + resource: None, + purpose: format!("stat {}", path.display()), + }); let bus = self.workers.clone(); self.pool.dispatch(move |_pool| { let kind = run_fs_stat(&cancel, &path); @@ -948,15 +1208,16 @@ impl AsyncRuntime { pub fn dispatch_fs_rename(&self, from: PathBuf, to: PathBuf, supersede: Option<&str>) -> JobId { // The closure below MOVES both paths; the pending entry is the // only thing that still knows them when the reply lands. - let (id, cancel) = self.allocate_with_resource( - JobKind::FsRename, + let (id, cancel) = self.allocate(JobSpec { + kind: JobKind::FsRename, supersede, - None, - Some(ResourceOp::Rename { + stream: None, + resource: Some(ResourceOp::Rename { from: from.clone(), to: to.clone(), }), - ); + purpose: format!("rename {} -> {}", from.display(), to.display()), + }); let bus = self.workers.clone(); self.pool.dispatch(move |_pool| { let kind = run_fs_rename(&cancel, &from, &to); @@ -967,7 +1228,13 @@ impl AsyncRuntime { /// Dispatch a `chmod(path, mode)` job. T M8.1. pub fn dispatch_fs_chmod(&self, path: PathBuf, mode: u32, supersede: Option<&str>) -> JobId { - let (id, cancel) = self.allocate(JobKind::FsChmod, supersede, None); + let (id, cancel) = self.allocate(JobSpec { + kind: JobKind::FsChmod, + supersede, + stream: None, + resource: None, + purpose: format!("chmod {mode:o} {}", path.display()), + }); let bus = self.workers.clone(); self.pool.dispatch(move |_pool| { let kind = run_fs_chmod(&cancel, &path, mode); @@ -978,12 +1245,13 @@ impl AsyncRuntime { /// Dispatch a `remove(path)` job. T M8.1. pub fn dispatch_fs_remove(&self, path: PathBuf, supersede: Option<&str>) -> JobId { - let (id, cancel) = self.allocate_with_resource( - JobKind::FsRemove, + let (id, cancel) = self.allocate(JobSpec { + kind: JobKind::FsRemove, supersede, - None, - Some(ResourceOp::Remove { path: path.clone() }), - ); + stream: None, + resource: Some(ResourceOp::Remove { path: path.clone() }), + purpose: format!("remove {}", path.display()), + }); let bus = self.workers.clone(); self.pool.dispatch(move |_pool| { let kind = run_fs_remove(&cancel, &path); @@ -1008,12 +1276,27 @@ impl AsyncRuntime { /// same supervisor (DAP, etc.) reuse this surface. /// /// `supersede` follows the same rule as the worker dispatchers. + /// + /// `purpose` is **required and has no derivable fallback** here, + /// which is why it is a parameter rather than something this method + /// composes for itself. The ten pool dispatchers each know what + /// their own job does; `register_external` knows only a `JobKind` + /// that is `McpRequest` or `LspRequest` — a category, not a + /// description. The caller is the only party that can say + /// `"lsp textDocument/definition"`. pub fn register_external( &self, kind: JobKind, supersede: Option<&str>, + purpose: impl Into, ) -> (JobId, CancellationToken) { - self.allocate(kind, supersede, None) + self.allocate(JobSpec { + kind, + supersede, + stream: None, + resource: None, + purpose: purpose.into(), + }) } /// Settle an externally-registered job with a JSON value. Wakes @@ -1206,6 +1489,7 @@ impl AsyncRuntime { dispatched_at: job.dispatched_at, settled_at: now, supersede_key: job.supersede_key.clone(), + purpose: job.purpose.clone(), outcome, }); } @@ -1243,6 +1527,7 @@ impl AsyncRuntime { supersede_key: j.supersede_key.clone(), cancel_requested: j.cancel.is_cancelled(), is_stream: j.stream_buffer.is_some(), + purpose: j.purpose.clone(), }) .collect(); // Stable order: oldest first. The buffer renderer renders in @@ -1262,12 +1547,54 @@ impl AsyncRuntime { .as_millis() as u64, settled_age_ms: now.saturating_duration_since(c.settled_at).as_millis() as u64, supersede_key: c.supersede_key.clone(), + purpose: c.purpose.clone(), outcome: c.outcome.clone(), }) .collect(); WorkersSnapshot { active, completed } } + /// What the statusline activity indicator shows, or `None` when + /// nothing is in flight (worker identity Stage 1, Q#W-3). + /// + /// `None` at zero is the contract, not an optimization: the + /// indicator renders **no segment at all** when idle, because a + /// statusline element that is always present costs modeline width + /// forever to say "nothing is happening". + /// + /// Scans the pending table rather than reusing + /// [`Self::workers_snapshot`]: this runs once per visible window per + /// frame, and a snapshot would clone the whole completed ring that + /// the indicator never reads. + #[must_use] + pub fn activity_summary(&self) -> Option { + let pending = self.pending.borrow(); + let mut in_flight = 0usize; + let mut oldest: Option<(&Instant, &str)> = None; + for job in pending.values() { + if !matches!(job.state, PendingState::Running) { + continue; + } + in_flight += 1; + // Strictly-earlier wins, so the first job seen holds the + // slot against later ties. `HashMap` iteration order is + // arbitrary, so two jobs dispatched in the same `Instant` + // resolve arbitrarily — a tie between simultaneous jobs has + // no right answer to lose. + if oldest.is_none_or(|(seen, _)| job.dispatched_at < *seen) { + oldest = Some((&job.dispatched_at, job.purpose.as_str())); + } + } + let (_, purpose) = oldest?; + Some(ActivitySummary { + in_flight, + // The modeline is one row and a segment is one line; + // `purpose_for_one_row` is what keeps a purpose carrying a + // newline (a path, an argv) from breaking it. + oldest_purpose: purpose_for_one_row(purpose).into_owned(), + }) + } + /// Drain the per-stream accumulators into one batch each. Each /// returned batch is bounded by the stream's `max_batch`; items /// beyond the cap stay in the accumulator until the next call. @@ -1876,23 +2203,25 @@ mod tests { fn tick_reports_resources_in_bus_arrival_order_not_allocation_order() { fn run(reverse: bool) -> Vec { let rt = AsyncRuntime::with_pool_size(1); - let (a, _) = rt.allocate_with_resource( - JobKind::FsRename, - None, - None, - Some(ResourceOp::Rename { + let (a, _) = rt.allocate(JobSpec { + kind: JobKind::FsRename, + supersede: None, + stream: None, + resource: Some(ResourceOp::Rename { from: PathBuf::from("/tmp/a-from"), to: PathBuf::from("/tmp/a-to"), }), - ); - let (b, _) = rt.allocate_with_resource( - JobKind::FsRemove, - None, - None, - Some(ResourceOp::Remove { + purpose: "rename a".to_owned(), + }); + let (b, _) = rt.allocate(JobSpec { + kind: JobKind::FsRemove, + supersede: None, + stream: None, + resource: Some(ResourceOp::Remove { path: PathBuf::from("/tmp/b-gone"), }), - ); + purpose: "remove b".to_owned(), + }); let order = if reverse { [b, a] } else { [a, b] }; for id in order { rt.workers @@ -1936,23 +2265,25 @@ mod tests { #[test] fn a_failed_or_cancelled_resource_job_is_not_harvested() { let rt = AsyncRuntime::with_pool_size(1); - let (failed, _) = rt.allocate_with_resource( - JobKind::FsRename, - None, - None, - Some(ResourceOp::Rename { + let (failed, _) = rt.allocate(JobSpec { + kind: JobKind::FsRename, + supersede: None, + stream: None, + resource: Some(ResourceOp::Rename { from: PathBuf::from("/tmp/nope"), to: PathBuf::from("/tmp/also-nope"), }), - ); - let (cancelled, _) = rt.allocate_with_resource( - JobKind::FsRemove, - None, - None, - Some(ResourceOp::Remove { + purpose: "rename nope".to_owned(), + }); + let (cancelled, _) = rt.allocate(JobSpec { + kind: JobKind::FsRemove, + supersede: None, + stream: None, + resource: Some(ResourceOp::Remove { path: PathBuf::from("/tmp/never"), }), - ); + purpose: "remove never".to_owned(), + }); rt.workers .send( ASYNC_REPLY_TOPIC, diff --git a/src/lsp.rs b/src/lsp.rs index f5630b6..43632f9 100644 --- a/src/lsp.rs +++ b/src/lsp.rs @@ -167,7 +167,11 @@ impl LspServerSpec { } fn to_process_spec(&self) -> ProcessSpec { - let mut p = ProcessSpec::new(format!("lsp:{}", self.label), &self.command); + let mut p = ProcessSpec::new( + format!("lsp:{}", self.label), + &self.command, + format!("language server for {}", self.label), + ); p.args.clone_from(&self.args); p.cwd.clone_from(&self.cwd); p.env.clone_from(&self.env); @@ -1587,9 +1591,15 @@ impl LspManager { uri: &str, ) -> JobId { let supersede = format!("lsp:{method}:{}:{uri}", sid.raw()); - let (job_id, token) = self - .runtime - .register_external(JobKind::LspRequest, Some(&supersede)); + // Worker identity Stage 1: `register_external` bypasses the + // worker pool, so its `JobKind` is the undifferentiated + // `LspRequest` for every method. The method and the document are + // the only thing that makes one row distinguishable from another + // in `*workers*`. + let purpose = format!("lsp {method} {uri}"); + let (job_id, token) = + self.runtime + .register_external(JobKind::LspRequest, Some(&supersede), purpose); self.pending_external.insert( (sid, req_id), PendingExternal { @@ -4538,7 +4548,8 @@ mod resource_reconciliation_tests { let runtime = mgr.runtime.clone(); let mut register = |rid: u64, uri: &str| { - let (job_id, token) = runtime.register_external(JobKind::LspRequest, None); + let (job_id, token) = + runtime.register_external(JobKind::LspRequest, None, format!("lsp hover {uri}")); mgr.pending_routes.insert( (a, rid), ResponseRoute::Hover { diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index b2de320..1a9795a 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -7566,6 +7566,99 @@ pub fn install_async( })?, )?; + // Worker identity Stage 1 (Q#W-2): the dispatch-name ambient. + // + // `pmacs.workers.dispatch(name, …)` is the one place a third-party + // job's own name exists, and nothing below it takes a name — the + // Rust dispatchers accept job arguments, a supersede key and stream + // data, and a handler reaching straight for `_dispatch_*` bypasses + // the Lua wrapper layer entirely. So the name travels out of band + // and is read at `allocate`, the single funnel every job passes + // through. + // + // Runtime-internal, underscore-prefixed: package code calls + // `pmacs.workers.dispatch`, which brackets these itself under + // `pcall`. A package pushing by hand and failing to pop would poison + // every later dispatch in the session with a stale name. + // + // `mlua::String`, not `String`: the parameter is a Lua BYTE string, + // so an `mlua`-driven `String` conversion would refuse a non-UTF-8 + // name with a generic message naming neither the argument nor the + // rule. `pmacs.workers.register` enforces the rest of the + // display-text standard (non-empty, no control characters) but + // cannot see UTF-8 validity from Lua 5.1, so the byte-level half is + // enforced here — the one point where Rust sees the name — with a + // message that names both. + // + // And it names the surfaces a JOB reaches, which are `*workers*` and + // the modeline activity indicator. The sibling refusal in + // `required_purpose` deliberately names a different one + // (`pmacs.process.list`), because a spawned process reaches neither + // of these in Stage 1. The two must not converge on one sentence: + // whichever wording won would be wrong on the other side, and a + // diagnostic that misdescribes the system sends the reader looking + // in the wrong place. + { + let rt = runtime.clone(); + async_mod.set( + "_push_dispatch_name", + lua.create_function(move |_, name: mlua::String| { + let Ok(text) = name.to_str() else { + return Err(mlua::Error::external( + "pmacs.workers.dispatch: handler name must be valid UTF-8 — it is \ + composed into every job's purpose, which is displayed to the user \ + in *workers* and in the modeline, and arbitrary bytes have no \ + display form there.", + )); + }; + rt.push_dispatch_name(&*text); + Ok(()) + })?, + )?; + } + + { + let rt = runtime.clone(); + async_mod.set( + "_pop_dispatch_name", + lua.create_function(move |_, ()| { + rt.pop_dispatch_name(); + Ok(()) + })?, + )?; + } + + // The refusal predicate, the sibling of `_in_commit_scope` above and + // enforced for the same reason: a coroutine that parks inside the + // extent leaves the name pushed, and every job allocated in the + // meantime — in any coroutine, on any later tick — inherits it. + { + let rt = runtime.clone(); + async_mod.set( + "_in_dispatch_name_scope", + lua.create_function(move |_, ()| Ok(rt.in_dispatch_name_scope()))?, + )?; + } + + // The statusline activity indicator's read surface (Q#W-3). Returns + // `nil` when nothing is in flight — the indicator renders no segment + // at all when idle, so "absent" has to be representable. + { + let rt = runtime.clone(); + async_mod.set( + "_activity_summary", + lua.create_function(move |lua, ()| { + let Some(summary) = rt.activity_summary() else { + return Ok(mlua::Value::Nil); + }; + let t = lua.create_table_with_capacity(0, 2)?; + t.set("in_flight", summary.in_flight)?; + t.set("purpose", summary.oldest_purpose)?; + Ok(mlua::Value::Table(t)) + })?, + )?; + } + { let rt = runtime.clone(); async_mod.set( @@ -7728,7 +7821,7 @@ fn workers_snapshot_to_lua(lua: &Lua, runtime: &SharedAsyncRuntime) -> mlua::Res let out = lua.create_table()?; let active = lua.create_table_with_capacity(snap.active.len(), 0)?; for (i, job) in snap.active.iter().enumerate() { - let row = lua.create_table_with_capacity(0, 6)?; + let row = lua.create_table_with_capacity(0, 7)?; row.set("id", job.id)?; row.set("kind", job.kind.label())?; row.set("age_ms", job.age_ms)?; @@ -7737,12 +7830,13 @@ fn workers_snapshot_to_lua(lua: &Lua, runtime: &SharedAsyncRuntime) -> mlua::Res } row.set("cancel_requested", job.cancel_requested)?; row.set("is_stream", job.is_stream)?; + row.set("purpose", job.purpose.as_str())?; active.set(i + 1, row)?; } out.set("active", active)?; let completed = lua.create_table_with_capacity(snap.completed.len(), 0)?; for (i, job) in snap.completed.iter().enumerate() { - let row = lua.create_table_with_capacity(0, 7)?; + let row = lua.create_table_with_capacity(0, 8)?; row.set("id", job.id)?; row.set("kind", job.kind.label())?; row.set("duration_ms", job.duration_ms)?; @@ -7750,6 +7844,7 @@ fn workers_snapshot_to_lua(lua: &Lua, runtime: &SharedAsyncRuntime) -> mlua::Res if let Some(key) = &job.supersede_key { row.set("supersede", key.as_str())?; } + row.set("purpose", job.purpose.as_str())?; let (status, value): (&'static str, mlua::Value) = match &job.outcome { JobOutcome::Complete(JobResult::Unit) => ("ok", mlua::Value::Nil), JobOutcome::Complete(JobResult::Sum(v)) => ( @@ -8677,9 +8772,93 @@ fn parse_restart(name: &str) -> mlua::Result { }) } +/// Read the **required** `purpose` out of a `pmacs.process.spawn` spec +/// (worker identity Stage 1, `COHERENCE.md` §9). +/// +/// An earlier revision of this lane defaulted the field to `label` so +/// that existing callers kept working. That preserved compatibility and +/// delivered nothing: §9's complaint about `ProcessSpec` is precisely +/// that `label` is "caller-supplied, unvalidated convention", so a +/// purpose defaulting to the label hands every caller back the +/// convention this lane exists to replace. +/// +/// The two fields answer different questions and neither substitutes for +/// the other. `label` **identifies** — `lsp:rust-analyzer`, a terminal's +/// buffer name — so that two processes running the same binary can be +/// told apart. `purpose` **describes**: it answers "what is happening", +/// which is the question §3's promise of visible asynchronous work is +/// about, and which a label chosen for uniqueness routinely does not +/// answer. +/// +/// # Errors +/// +/// Absent, empty, whitespace-only, non-string, or **not valid UTF-8**. +/// Empty and whitespace-only are rejected because they satisfy the type +/// and defeat the point exactly as copying the label across would — R42 +/// already rejects whitespace-only `description`s in the config registry +/// for the same reason. +/// +/// The UTF-8 case is a **reachable input class, not an internal +/// invariant**: Lua strings are byte strings, so `purpose = +/// string.char(255)` is a value a caller can write. Converting it with +/// `?` would surface mlua's generic conversion error *before* any of the +/// diagnostics below is constructed, and the caller would be told +/// neither the field nor the rule — so the conversion failure is mapped +/// onto this function's own message instead. +/// +/// That message names **`pmacs.process.list`**, which is the whole of +/// where a process's purpose surfaces in Stage 1. It deliberately does +/// *not* name `*workers*` or the modeline indicator: both are **job** +/// surfaces, a spawned process appears in neither, and joining the two +/// planes is Stage 2's work (framing §3, Q#W-4). A diagnostic that +/// named them would send the reader looking for their process somewhere +/// it will never appear — worse than a terse one. The job-side twin of +/// this refusal, on `_push_dispatch_name`, names those two surfaces for +/// the matching reason: a job really does reach them. +/// +/// The read is **raw**, matching the posture `stdin` and `group` already +/// document in [`lua_to_spec`]: a spec table is plain data, so a +/// metatable cannot smuggle a purpose in through `__index`. +fn required_purpose(table: &Table) -> mlua::Result { + let purpose = match table.raw_get::("purpose") { + Ok(mlua::Value::String(value)) => match value.to_str() { + Ok(text) => text.to_owned(), + Err(_) => { + return Err(mlua::Error::external( + "pmacs.process.spawn: purpose must be valid UTF-8 — it is displayed \ + to the user in pmacs.process.list, and arbitrary bytes have no \ + display form there.", + )); + } + }, + Ok(mlua::Value::Nil) => { + return Err(mlua::Error::external( + "pmacs.process.spawn: purpose is required — a short description of what \ + this process is DOING, e.g. purpose = \"running the project's test suite\". \ + It is not the label: the label identifies the process, the purpose says \ + what it is for.", + )); + } + Ok(other) => { + return Err(mlua::Error::external(format!( + "pmacs.process.spawn: purpose must be a string; got {}", + other.type_name() + ))); + } + Err(error) => return Err(error), + }; + if purpose.trim().is_empty() { + return Err(mlua::Error::external( + "pmacs.process.spawn: purpose must not be empty or whitespace-only", + )); + } + Ok(purpose) +} + fn lua_to_spec(table: &Table) -> mlua::Result { let label: String = table.get("label").unwrap_or_else(|_| "unnamed".to_owned()); let command: String = table.get("command")?; + let purpose = required_purpose(table)?; let args: Vec = table.get("args").unwrap_or_default(); let cwd: Option = table.get("cwd").ok().flatten(); let env_table: Option = table.get("env").ok().flatten(); @@ -8762,6 +8941,7 @@ fn lua_to_spec(table: &Table) -> mlua::Result { }; Ok(ProcessSpec { label, + purpose, command, args, cwd: cwd.map(std::path::PathBuf::from), @@ -8985,11 +9165,18 @@ pub fn install_process(lua: &Lua, supervisor: &SharedProcessSupervisor) -> mlua: .collect(); let out = lua.create_table_with_capacity(ids.len(), 0)?; for (i, id) in ids.iter().enumerate() { - let row = lua.create_table_with_capacity(0, 3)?; + let row = lua.create_table_with_capacity(0, 4)?; row.set("id", ProcessIdLua(*id))?; if let Some(spec) = sup.spec(*id) { row.set("label", spec.label.as_str())?; row.set("command", spec.command.as_str())?; + // Worker identity Stage 1: a new KEY on each + // existing row. The row COUNT is deliberately + // untouched — three acceptance suites assert on + // `#pmacs.process.list()` as a leak detector + // (framing Q#W-4), and widening what this + // enumerates would inflate all three baselines. + row.set("purpose", spec.purpose.as_str())?; } if let Some(state) = sup.state(*id) { row.set("state", state_to_lua(lua, state)?)?; diff --git a/src/mcp.rs b/src/mcp.rs index b1db5f4..f1085ca 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -175,7 +175,11 @@ impl McpServerSpec { } fn to_process_spec(&self) -> ProcessSpec { - let mut p = ProcessSpec::new(format!("mcp:{}", self.label), &self.command); + let mut p = ProcessSpec::new( + format!("mcp:{}", self.label), + &self.command, + format!("MCP server {}", self.label), + ); p.args.clone_from(&self.args); p.cwd.clone_from(&self.cwd); p.env.clone_from(&self.env); @@ -873,7 +877,9 @@ impl McpManager { } let req_id = next_request_id(client); let body = make_request(req_id, &method, params); - let (job_id, token) = self.runtime.register_external(JobKind::McpRequest, None); + let (job_id, token) = + self.runtime + .register_external(JobKind::McpRequest, None, format!("mcp {method}")); client.pending_external.insert( req_id, PendingExternal { @@ -948,7 +954,11 @@ impl McpManager { // (1) Cache hit. if let Some(ResourceCacheState::Cached { result }) = self.resource_cache.get(&key).cloned() { - let (job_id, _token) = self.runtime.register_external(JobKind::McpRequest, None); + let (job_id, _token) = self.runtime.register_external( + JobKind::McpRequest, + None, + format!("mcp resources/read {uri} (cached)"), + ); self.runtime.complete_external_ok(job_id, result); return Ok(job_id); } @@ -959,7 +969,11 @@ impl McpManager { // independently. if let Some(ResourceCacheState::InFlight { request_id }) = self.resource_cache.get(&key) { let in_flight_rid = *request_id; - let (job_id, token) = self.runtime.register_external(JobKind::McpRequest, None); + let (job_id, token) = self.runtime.register_external( + JobKind::McpRequest, + None, + format!("mcp resources/read {uri}"), + ); if let Some(p) = client.pending_external.get_mut(&in_flight_rid) { p.awaiters.push(Awaiter { job_id, token }); return Ok(job_id); @@ -974,7 +988,11 @@ impl McpManager { // (3) Cache miss: dispatch. let req_id = next_request_id(client); let body = make_request(req_id, "resources/read", json!({ "uri": uri })); - let (job_id, token) = self.runtime.register_external(JobKind::McpRequest, None); + let (job_id, token) = self.runtime.register_external( + JobKind::McpRequest, + None, + format!("mcp resources/read {uri}"), + ); client.pending_external.insert( req_id, PendingExternal { @@ -1063,10 +1081,13 @@ impl McpManager { // than referenced by `json!`); avoids a needless-pass-by- // value clippy complaint and matches `send_request`'s shape. let mut params_map = Map::new(); + let purpose = format!("mcp tools/call {name}"); params_map.insert("name".into(), Value::String(name)); params_map.insert("arguments".into(), arguments); let body = make_request(req_id, "tools/call", Value::Object(params_map)); - let (job_id, token) = self.runtime.register_external(JobKind::McpRequest, None); + let (job_id, token) = self + .runtime + .register_external(JobKind::McpRequest, None, purpose); client.pending_external.insert( req_id, PendingExternal { @@ -1125,10 +1146,13 @@ impl McpManager { } let req_id = next_request_id(client); let mut params_map = Map::new(); + let purpose = format!("mcp prompts/get {name}"); params_map.insert("name".into(), Value::String(name)); params_map.insert("arguments".into(), arguments); let body = make_request(req_id, "prompts/get", Value::Object(params_map)); - let (job_id, token) = self.runtime.register_external(JobKind::McpRequest, None); + let (job_id, token) = self + .runtime + .register_external(JobKind::McpRequest, None, purpose); client.pending_external.insert( req_id, PendingExternal { diff --git a/src/process.rs b/src/process.rs index c4a9277..17a5365 100644 --- a/src/process.rs +++ b/src/process.rs @@ -196,6 +196,23 @@ pub struct ProcessSpec { /// so multiple processes can run the same binary with /// distinguishable labels. pub label: String, + /// What this process is doing, in words a user can read (worker + /// identity Stage 1, `COHERENCE.md` §9). + /// + /// **Required, and not the same thing as [`Self::label`].** The + /// label is an *identity* — `lsp:rust-analyzer`, a terminal's buffer + /// name — spelled however the caller likes, so that two processes + /// running the same binary can be told apart. The purpose is a + /// *description*: it answers "what is happening", which is the + /// question §3's promise of visible asynchronous work is about and + /// which a label chosen for uniqueness routinely does not answer. + /// + /// **Not an owner**, in any spelling. It records what the process is + /// doing, not which package asked for it; `pmacs.process.spawn` is + /// callable by any package, so a value derived here would + /// misattribute third-party work to a builtin at exactly the point + /// §9 wants attribution (framing §3). + pub purpose: String, /// Program to execute. Looked up via the system PATH unless an /// absolute path is supplied. pub command: String, @@ -237,10 +254,21 @@ pub struct ProcessSpec { impl ProcessSpec { /// Construct a spec with the bare-minimum fields. Convenience /// for tests and one-off scripts. + /// + /// `purpose` is a parameter rather than something derived from the + /// label because it is a required field with no honest default + /// (worker identity Stage 1): deriving it from the label would make + /// every process claim its identity *is* its description, which is + /// exactly the conflation the field exists to undo. #[must_use] - pub fn new(label: impl Into, command: impl Into) -> Self { + pub fn new( + label: impl Into, + command: impl Into, + purpose: impl Into, + ) -> Self { Self { label: label.into(), + purpose: purpose.into(), command: command.into(), args: Vec::new(), cwd: None, @@ -2722,6 +2750,7 @@ mod tests { let spec = ProcessSpec::new( "unpublished-terminal", "/definitely/not/a/real/pmacs-terminal-program", + "test process", ); assert!(supervisor.spawn_terminal(spec).is_err()); supervisor.tick(); @@ -2732,7 +2761,7 @@ mod tests { #[test] fn spawn_pipes_lifecycle_started_then_exited() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("echo-test", "/bin/sh"); + let mut spec = ProcessSpec::new("echo-test", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "echo hello && exit 0".into()]; let id = sup.spawn(spec).expect("spawn"); let events = drain_until(&mut sup, id, Duration::from_secs(5), has_exited); @@ -2892,7 +2921,7 @@ mod tests { /// A plain PTY child, for tests that care about the PTY *branch* /// rather than about job control. fn spawn_live_pty(sup: &mut ProcessSupervisor, name: &str) -> (ProcessId, u32) { - let mut spec = ProcessSpec::new(name, "/bin/sleep"); + let mut spec = ProcessSpec::new(name, "/bin/sleep", "test process"); spec.args = vec!["30".into()]; spec.mode = ProcessMode::Pty { rows: 24, @@ -2943,7 +2972,7 @@ mod tests { sup: &mut ProcessSupervisor, name: &str, ) -> (ProcessId, u32, i32) { - let mut spec = ProcessSpec::new(name, BASH); + let mut spec = ProcessSpec::new(name, BASH, "test process"); spec.args = vec![ "--noprofile".into(), "--norc".into(), @@ -3194,7 +3223,7 @@ mod tests { #[test] fn a_pipe_child_still_renders_a_bare_leader_target() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("diag-pipe-leader", "/bin/sleep"); + let mut spec = ProcessSpec::new("diag-pipe-leader", "/bin/sleep", "test process"); spec.args = vec!["30".into()]; let id = sup.spawn(spec).expect("spawn"); let pid = spawn_started_pid(&mut sup, id); @@ -3227,7 +3256,7 @@ mod tests { let mut reports = Vec::new(); for signal in [Signal::SIGTERM, Signal::SIGUSR1] { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("diag-signal-name", "/bin/sh"); + let mut spec = ProcessSpec::new("diag-signal-name", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "sleep 30".into()]; spec.group = true; let id = sup.spawn(spec).expect("spawn"); @@ -3280,7 +3309,7 @@ mod tests { let mut sup = ProcessSupervisor::new(); let temp = tempfile::TempDir::new().expect("tempdir"); let ready = temp.path().join("usr1-trapped"); - let mut spec = ProcessSpec::new("diag-disposition-live", "/bin/sh"); + let mut spec = ProcessSpec::new("diag-disposition-live", "/bin/sh", "test process"); // Ignore USR1 so the successful non-fatal signal cannot end the // child and confuse the state assertion with a real exit — and // then WAIT for the child to say it has done so. `Started` is @@ -3344,7 +3373,7 @@ mod tests { let mut sup = ProcessSupervisor::new(); let temp = tempfile::TempDir::new().expect("tempdir"); let ready = temp.path().join("usr1-trapped"); - let mut spec = ProcessSpec::new("diag-trap-readiness", "/bin/sh"); + let mut spec = ProcessSpec::new("diag-trap-readiness", "/bin/sh", "test process"); spec.args = vec!["-c".into(), trapped_usr1_command(&ready, "sleep 1; ")]; spec.group = true; let id = sup.spawn(spec).expect("spawn"); @@ -3435,7 +3464,7 @@ mod tests { #[test] fn a_leader_directed_kill_failure_reports_the_fallback_branch() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("diag-leader", "/bin/sleep"); + let mut spec = ProcessSpec::new("diag-leader", "/bin/sleep", "test process"); spec.args = vec!["30".into()]; let id = sup.spawn(spec).expect("spawn"); let pid = spawn_started_pid(&mut sup, id); @@ -3484,7 +3513,7 @@ mod tests { #[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"); + let mut spec = ProcessSpec::new("diag-exited", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "exit 3".into()]; let id = sup.spawn(spec).expect("spawn"); // NOT `spawn_started_pid`: draining ticks, and this child exits @@ -3512,7 +3541,7 @@ mod tests { #[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"); + let mut spec = ProcessSpec::new("diag-disposition", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "sleep 30".into()]; spec.group = true; let id = sup.spawn(spec).expect("spawn"); @@ -3557,7 +3586,7 @@ mod tests { #[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"); + let mut spec = ProcessSpec::new("diag-one-event", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "exit 7".into()]; spec.mode = ProcessMode::Pty { rows: 24, @@ -3599,7 +3628,7 @@ mod tests { let mut sup = ProcessSupervisor::new(); // `sleep 30` is long enough that the test definitely needs // to terminate it deliberately. - let mut spec = ProcessSpec::new("sleeper", "/bin/sh"); + let mut spec = ProcessSpec::new("sleeper", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "sleep 30".into()]; let id = sup.spawn(spec).expect("spawn"); // Wait for Started so we have a pid. @@ -3628,7 +3657,7 @@ mod tests { // implementation blocked the caller in `write_all` here — // which in the editor was the main thread, wedging the frame // loop whenever an LSP server fell behind on its stdin. - let mut spec = ProcessSpec::new("stdin-ignorer", "/bin/sh"); + let mut spec = ProcessSpec::new("stdin-ignorer", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "sleep 30".into()]; let id = sup.spawn(spec).expect("spawn"); let _ = drain_until(&mut sup, id, Duration::from_secs(2), |evs| { @@ -3654,7 +3683,7 @@ mod tests { // payload back followed by a clean exit proves the writer // thread drains its queue before dropping the pipe (the // flush-then-EOF contract `close_stdin` documents). - let mut spec = ProcessSpec::new("cat-echo", "/bin/sh"); + let mut spec = ProcessSpec::new("cat-echo", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "cat".into()]; let id = sup.spawn(spec).expect("spawn"); let _ = drain_until(&mut sup, id, Duration::from_secs(2), |evs| { @@ -3700,7 +3729,7 @@ mod tests { fn restart_on_crash_respawns_after_nonzero_exit() { let mut sup = ProcessSupervisor::new(); sup.set_restart_backoff(Duration::from_millis(10)); - let mut spec = ProcessSpec::new("crasher", "/bin/sh"); + let mut spec = ProcessSpec::new("crasher", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "exit 7".into()]; spec.restart = RestartPolicy::OnCrash; let id = sup.spawn(spec).expect("spawn"); @@ -3731,7 +3760,7 @@ mod tests { #[test] fn restart_never_does_not_respawn_after_clean_exit() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("oneshot", "/bin/sh"); + let mut spec = ProcessSpec::new("oneshot", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "exit 0".into()]; let id = sup.spawn(spec).expect("spawn"); let _ = drain_until(&mut sup, id, Duration::from_secs(2), has_exited); @@ -3760,7 +3789,7 @@ mod tests { let pid = { let mut sup = ProcessSupervisor::new(); sup.set_grace_period(Duration::from_millis(200)); - let mut spec = ProcessSpec::new("victim", "/bin/sh"); + let mut spec = ProcessSpec::new("victim", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "sleep 30".into()]; let id = sup.spawn(spec).expect("spawn"); // Drain until Started so we know the pid. @@ -3798,7 +3827,7 @@ mod tests { #[test] fn pty_mode_child_sees_a_tty() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("ttytest", "/bin/sh"); + let mut spec = ProcessSpec::new("ttytest", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "tty".into()]; spec.mode = ProcessMode::default_pty(); let id = sup.spawn(spec).expect("spawn"); @@ -3835,7 +3864,7 @@ mod tests { #[test] fn m6_1_pty_resize_delivers_sigwinch_to_child() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("winch-watch", "/bin/sh"); + let mut spec = ProcessSpec::new("winch-watch", "/bin/sh", "test process"); // Trap WINCH, print READY for synchronization, then loop on // a short sleep so SIGWINCH can interrupt and fire the trap. spec.args = vec![ @@ -3880,7 +3909,7 @@ mod tests { #[test] fn m6_1_pty_mode_lifecycle_started_then_exited() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("pty-exit", "/bin/sh"); + let mut spec = ProcessSpec::new("pty-exit", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "echo done && exit 0".into()]; spec.mode = ProcessMode::default_pty(); let id = sup.spawn(spec).expect("spawn"); @@ -3915,7 +3944,7 @@ mod tests { #[test] fn m6_1_pty_raw_mode_disables_kernel_echo() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("raw-stty", "/bin/sh"); + let mut spec = ProcessSpec::new("raw-stty", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "stty -a".into()]; spec.mode = ProcessMode::default_pty(); // Raw by default. let id = sup.spawn(spec).expect("spawn"); @@ -3937,7 +3966,7 @@ mod tests { #[test] fn m6_1_pty_canonical_mode_keeps_kernel_echo() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("canon-stty", "/bin/sh"); + let mut spec = ProcessSpec::new("canon-stty", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "stty -a".into()]; spec.mode = ProcessMode::Pty { rows: 24, @@ -3991,7 +4020,7 @@ mod tests { // buffers. const TOTAL: usize = 10 * 1024 * 1024; let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("byte-flood", "/bin/sh"); + let mut spec = ProcessSpec::new("byte-flood", "/bin/sh", "test process"); spec.args = vec!["-c".into(), format!("head -c {TOTAL} /dev/zero")]; let id = sup.spawn(spec).expect("spawn"); @@ -4067,7 +4096,7 @@ mod tests { #[test] fn m6_2_pty_streaming_coalesces_per_tick() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("chunky-stream", "/bin/sh"); + let mut spec = ProcessSpec::new("chunky-stream", "/bin/sh", "test process"); // 1 MiB of zeros from /dev/zero. The reader thread reads in // [`BYTE_CHUNK_SIZE`] (8 KiB) chunks --- ~128 reads --- all // queued onto the bounded channel within microseconds of @@ -4116,7 +4145,7 @@ mod tests { #[test] fn m6_2_ansi_enabled_pty_emits_structured_events() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("ansi-stream", "/bin/sh"); + let mut spec = ProcessSpec::new("ansi-stream", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "printf '\\033[31mhi\\033[0m\\n'".into()]; spec.mode = ProcessMode::Pty { rows: 24, @@ -4198,7 +4227,7 @@ mod tests { let handle = std::thread::spawn(move || { let mut sup = ProcessSupervisor::new(); sup.set_grace_period(Duration::from_millis(300)); - let mut spec = ProcessSpec::new("forever-flood", "/bin/sh"); + let mut spec = ProcessSpec::new("forever-flood", "/bin/sh", "test process"); // Continuous writer; SIGTERM kills it (no signal handler). spec.args = vec!["-c".into(), "while :; do printf 'X'; done".into()]; let id = sup.spawn(spec).expect("spawn"); @@ -4324,7 +4353,7 @@ mod tests { let handle = std::thread::spawn(move || { let mut sup = ProcessSupervisor::new(); sup.set_grace_period(Duration::from_millis(300)); - let mut spec = ProcessSpec::new("orphan-holds-pipe", "setsid"); + let mut spec = ProcessSpec::new("orphan-holds-pipe", "setsid", "test process"); // `setsid --fork` forks and the parent exits, so the // *recorded* pid terminates promptly (letting `poll_one` // reach the teardown path) while `cat` survives holding the @@ -4412,7 +4441,7 @@ mod tests { // ----------------------------------------------------------------- fn sh_group_spec(label: &str, script: &str) -> ProcessSpec { - let mut spec = ProcessSpec::new(label, "/bin/sh"); + let mut spec = ProcessSpec::new(label, "/bin/sh", "test process"); spec.args = vec!["-c".into(), script.to_owned()]; spec.stdin = StdinMode::Null; spec.group = true; @@ -4546,7 +4575,7 @@ mod tests { ); // Control: a non-group child inherits the test process's // group instead of leading its own. - let mut plain = ProcessSpec::new("plain", "/bin/sh"); + let mut plain = ProcessSpec::new("plain", "/bin/sh", "test process"); plain.args = vec!["-c".into(), "sleep 30".into()]; let plain_id = sup.spawn(plain).expect("spawn plain"); let plain_events = drain_until(&mut sup, plain_id, Duration::from_secs(2), |evs| { @@ -5017,7 +5046,7 @@ mod tests { fn maybe_restart_inert_once_shut_down() { let mut sup = ProcessSupervisor::new(); sup.set_restart_backoff(Duration::from_millis(30)); - let mut spec = ProcessSpec::new("restarter", "/bin/sh"); + let mut spec = ProcessSpec::new("restarter", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "echo x".into()]; spec.restart = RestartPolicy::Always; let id = sup.spawn(spec).expect("spawn"); @@ -5158,7 +5187,7 @@ mod tests { #[test] fn group_and_null_stdin_rejected_under_pty() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("pty-null", "/bin/sh"); + let mut spec = ProcessSpec::new("pty-null", "/bin/sh", "test process"); spec.mode = ProcessMode::default_pty(); spec.stdin = StdinMode::Null; let err = sup @@ -5169,7 +5198,7 @@ mod tests { "error points at pipe mode: {err}" ); - let mut spec = ProcessSpec::new("pty-group", "/bin/sh"); + let mut spec = ProcessSpec::new("pty-group", "/bin/sh", "test process"); spec.mode = ProcessMode::default_pty(); spec.group = true; let err = sup diff --git a/src/terminal/session.rs b/src/terminal/session.rs index c731fb0..47fae56 100644 --- a/src/terminal/session.rs +++ b/src/terminal/session.rs @@ -305,7 +305,8 @@ impl TerminalManager { buffer.set_read_only(true); core.registry.borrow_mut().insert(buffer); - let mut process_spec = ProcessSpec::new(buffer_name, spec.command); + let purpose = format!("terminal running {}", spec.command); + let mut process_spec = ProcessSpec::new(buffer_name, spec.command, purpose); process_spec.args = spec.args; process_spec.cwd = spec.cwd; process_spec.env = spec.env; diff --git a/src/workers_buffer.rs b/src/workers_buffer.rs index 6a6eeb4..2a3bd59 100644 --- a/src/workers_buffer.rs +++ b/src/workers_buffer.rs @@ -14,19 +14,40 @@ //! ```text //! Workers (active: 2, completed: 5) //! -//! ID Kind Age Supersede Status -//! ------ ----------- -------- ---------- ---------- -//! #5 grep 412ms search running -//! #6 sleep 18ms running (cancel pending) +//! ID Kind Age Supersede Purpose Status +//! ------ ----------- -------- ---------- ------------------------ ---------- +//! #5 grep 412ms search search: grep "fn" in /x running +//! #6 sleep 18ms sleep 18ms running (cancel pending) //! //! Recent (newest first) //! -//! ID Kind Duration Supersede Outcome -//! ------ ----------- -------- ---------- ---------- -//! #4 grep 1242ms search cancelled (3s ago) -//! #3 compute_sum 2ms ok (3s ago) +//! ID Kind Duration Supersede Purpose Outcome +//! ------ ----------- -------- ---------- ------------------------ ---------- +//! #4 grep 1242ms search search: grep "fn" in /x cancelled (3s ago) +//! #3 compute_sum 2ms sum 1..100 ok (3s ago) //! ``` //! +//! # Purpose (worker identity Stage 1, `COHERENCE.md` §9) +//! +//! The `Purpose` column is what turns "twelve rows named `lsp_request`" +//! into a readable account of what the editor is doing. `Kind` names the +//! builtin dispatcher a job funnelled through, which for every +//! third-party job is a builtin's label rather than the caller's; the +//! purpose carries the work's own description and, under +//! `pmacs.workers.dispatch`, the registered handler name it ran under. +//! +//! It is placed **before** `Status` and padded, because `Status` is +//! variable-width (`running (cancel pending) [stream]`) and two +//! ragged trailing columns render as noise. An over-long purpose pushes +//! `Status` right rather than being truncated: losing the end of a path +//! is a worse failure than an uneven column. +//! +//! This table is **one row per job**, and the purpose is the only free +//! text in it, so every row goes through +//! [`crate::async_runtime::purpose_for_one_row`]: a row must not be able +//! to forge another row. See that function for why the escaping lives +//! here rather than as a rule on the purpose itself. +//! //! Lua reads the snapshot via `pmacs.workers.snapshot()`; the //! `pmacs.workers.show()` builtin invokes [`render`] on it and //! returns the buffer id. Auto-refresh hooks into @@ -35,7 +56,7 @@ use std::fmt::Write; use crate::async_runtime::{ - ActiveJobInfo, CompletedJobInfo, JobOutcome, JobResult, WorkersSnapshot, + ActiveJobInfo, CompletedJobInfo, JobOutcome, JobResult, WorkersSnapshot, purpose_for_one_row, }; use crate::buffer::{Buffer, BufferId, EditOp}; use crate::buffer_registry::BufferRegistry; @@ -43,6 +64,11 @@ use crate::buffer_registry::BufferRegistry; /// Canonical name for the workers observability buffer. pub const WORKERS_BUFFER_NAME: &str = "*workers*"; +/// Minimum column width the `Purpose` column is padded to. Purposes +/// longer than this push the trailing column right rather than being +/// truncated (see the module docs). +const PURPOSE_WIDTH: usize = 24; + /// Render `snapshot` into the `*workers*` buffer (creating it if /// absent), replacing its full contents. Returns the buffer id /// and the Edits produced by the replacement (zero, one, or two — @@ -119,13 +145,17 @@ fn format_snapshot(snapshot: &WorkersSnapshot) -> String { let _ = writeln!(text); let _ = writeln!( text, - "{:<7} {:<11} {:>9} {:<11} Status", - "ID", "Kind", "Age", "Supersede" + "{:<7} {:<11} {:>9} {:<11} {:9} {:<11} ----------", - "------", "-----------", "---------", "-----------" + "{:<7} {:<11} {:>9} {:<11} {: String { let _ = writeln!(text); let _ = writeln!( text, - "{:<7} {:<11} {:>9} {:<11} Outcome", - "ID", "Kind", "Duration", "Supersede" + "{:<7} {:<11} {:>9} {:<11} {:9} {:<11} ----------", - "------", "-----------", "---------", "-----------" + "{:<7} {:<11} {:>9} {:<11} {:9} {key:<11} {status}"); + let purpose = purpose_for_one_row(&job.purpose); + let _ = writeln!( + text, + "{id:<7} {kind:<11} {age:>9} {key:<11} {purpose:9} {key:<11} {outcome} ({age} ago)" + "{id:<7} {kind:<11} {duration:>9} {key:<11} {purpose: bool { #[test] fn m4_4_lifecycle_spawn_and_exit() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("hello", "/bin/sh"); + let mut spec = ProcessSpec::new("hello", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "printf hi && exit 0".into()]; let id = sup.spawn(spec).expect("spawn"); let evs = drain_until(&mut sup, id, Duration::from_secs(5), has_exit_event); @@ -983,7 +983,7 @@ fn m4_4_lifecycle_spawn_and_exit() { #[test] fn m4_4_lifecycle_signal_terminates() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("victim", "/bin/sh"); + let mut spec = ProcessSpec::new("victim", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "sleep 30".into()]; let id = sup.spawn(spec).expect("spawn"); let _ = drain_until(&mut sup, id, Duration::from_secs(2), |evs| { @@ -1020,7 +1020,11 @@ fn m4_4_lifecycle_signal_terminates() { fn m4_4_lifecycle_crash_surfaces_as_event() { let mut sup = ProcessSupervisor::new(); // Path that will reliably not resolve. - let spec = ProcessSpec::new("ghost", "/this/binary/does/not/exist/pmacs-m4-4"); + let spec = ProcessSpec::new( + "ghost", + "/this/binary/does/not/exist/pmacs-m4-4", + "test process", + ); let _ = sup.spawn(spec); // spawn returns Err but the event is still emitted sup.tick(); let evs = sup.take_all_events(); @@ -1037,7 +1041,7 @@ fn m4_4_lifecycle_crash_surfaces_as_event() { fn m4_4_restart_policy_on_crash_respawns() { let mut sup = ProcessSupervisor::new(); sup.set_restart_backoff(Duration::from_millis(10)); - let mut spec = ProcessSpec::new("flap", "/bin/sh"); + let mut spec = ProcessSpec::new("flap", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "exit 9".into()]; spec.restart = RestartPolicy::OnCrash; let id = sup.spawn(spec).expect("spawn"); @@ -1070,7 +1074,7 @@ fn m4_4_restart_policy_on_crash_respawns() { #[test] fn m4_4_restart_policy_never_does_not_respawn() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("oneshot", "/bin/sh"); + let mut spec = ProcessSpec::new("oneshot", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "exit 0".into()]; let id = sup.spawn(spec).expect("spawn"); let _ = drain_until(&mut sup, id, Duration::from_secs(2), has_exit_event); @@ -1099,7 +1103,7 @@ fn m4_4_no_zombies_after_editor_drop() { let pid: u32 = { let mut sup = ProcessSupervisor::new(); sup.set_grace_period(Duration::from_millis(200)); - let mut spec = ProcessSpec::new("zombie-test", "/bin/sh"); + let mut spec = ProcessSpec::new("zombie-test", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "sleep 60".into()]; let id = sup.spawn(spec).expect("spawn"); let _ = drain_until(&mut sup, id, Duration::from_secs(2), |evs| { @@ -1136,7 +1140,7 @@ fn m4_4_no_zombies_after_editor_drop() { #[test] fn m4_4_pty_mode_child_observes_a_tty() { let mut sup = ProcessSupervisor::new(); - let mut spec = ProcessSpec::new("ttytest", "/bin/sh"); + let mut spec = ProcessSpec::new("ttytest", "/bin/sh", "test process"); spec.args = vec!["-c".into(), "tty".into()]; spec.mode = ProcessMode::default_pty(); let id = sup.spawn(spec).expect("spawn"); @@ -1169,6 +1173,7 @@ fn m4_4_lua_surface_drives_lifecycle() { r#" local id = pmacs.process.spawn { label = "lua-hello", + purpose = "greeting the Lua surface end to end", command = "/bin/sh", args = { "-c", "printf hi-from-lua && exit 0" }, } diff --git a/tests/statusline_segments_acceptance.rs b/tests/statusline_segments_acceptance.rs index a4dc5f1..c577af4 100644 --- a/tests/statusline_segments_acceptance.rs +++ b/tests/statusline_segments_acceptance.rs @@ -136,7 +136,12 @@ fn a01_04_registry_contract_limits_epochs_and_results() { .iter() .map(|provider| provider.name.as_str()) .collect::>(), - ["mode", "terminal", "lsp"], + // `activity` is worker identity Stage 1's fourth adopter, and it + // sorts first because `async.lua` is loaded before `syntax.lua`, + // `terminal.lua` and `lsp.lua`. This is an INVENTORY assertion: + // it grows when a builtin provider is added, which is exactly + // what it is for. + ["activity", "mode", "terminal", "lsp"], "built-in providers are discoverable in registration order" ); let before_epochs = { diff --git a/tests/vterm_stage1_acceptance.rs b/tests/vterm_stage1_acceptance.rs index 489ca9a..0c7a312 100644 --- a/tests/vterm_stage1_acceptance.rs +++ b/tests/vterm_stage1_acceptance.rs @@ -398,7 +398,7 @@ fn editor_shutdown_kills_term_ignoring_terminal_child() { #[test] fn terminal_tick_does_not_take_non_terminal_process_events() { let mut state = EditorState::new_with_roots(&crate::iso::roots()); - let mut process = pmacs::process::ProcessSpec::new("ordinary", "/bin/sh"); + let mut process = pmacs::process::ProcessSpec::new("ordinary", "/bin/sh", "test process"); process.args = vec!["-c".into(), "printf ordinary".into()]; let ordinary_id = state .process_supervisor diff --git a/tests/worker_identity_acceptance.rs b/tests/worker_identity_acceptance.rs new file mode 100644 index 0000000..f5dcef7 --- /dev/null +++ b/tests/worker_identity_acceptance.rs @@ -0,0 +1,1266 @@ +// tests/worker_identity_acceptance.rs --- worker identity Stage 1. + +//! Worker identity Stage 1 (`docs/worker-identity-framing.md` §6, +//! `COHERENCE.md` §9). +//! +//! §9 grades the worker model **mechanism without identity**: a job +//! carries a `JobKind` naming the builtin dispatcher it funnelled +//! through, so every third-party job renders under a builtin's label, +//! and no progress indicator exists anywhere. This suite pins what +//! Stage 1 does about that — a required `purpose` on the job and the +//! process, the dispatch-name ambient that stops +//! `pmacs.workers.dispatch` discarding its handler name, and the first +//! indicator a user sees without running a command. +//! +//! # What is NOT here, deliberately +//! +//! **Presence is enforced by the COMPILER, not by anything below.** +//! `JobSpec::purpose` is non-optional and `JobSpec` has no `Default`, so +//! a dispatcher that supplies none does not build. A funnel test would +//! prove only that the funnel stores what it was handed, and would say +//! nothing about whether fourteen callers handed it anything meaningful. +//! Everything below is about *semantics*. +//! +//! **That a raw `coroutine.yield` inside either dynamic scope is +//! prevented — it is not.** R46 forbids package code from yielding +//! raw, but it is a convention, and the scheduler diagnoses a non-Handle +//! yield only *after* the coroutine has suspended (`async.lua` resumes, +//! then inspects what came back), so no refusal sited in a yield helper +//! is ever consulted. Rule 1 claims **the two supported yield APIs** and +//! nothing more. A test that "proved" coverage this design does not have +//! would be worse than the recorded gap, so the gap is recorded instead +//! (framing §2, §6, §7). +//! +//! **That background work is attributable from one place** (Stage 2's +//! unified view), **that a terminal PTY is visible anywhere** (Q#W-4), +//! or **that any job is attributed to the PACKAGE responsible for it**. +//! `purpose` records what work is being done and, under +//! `pmacs.workers.dispatch`, which registered handler it ran under. +//! Neither is package ownership, which waits for P3 — and there is no +//! `owner` field, in any spelling, for it to squat on (framing §3, §7). + +use std::collections::HashMap; +use std::time::{Duration, Instant}; + +use pmacs::async_runtime::JobKind; +use pmacs::cell::{Cell, CellGrid, CellSize, Glyph}; +use pmacs::editor::EditorState; +use pmacs::protocol::FrontendId; +use pmacs::statusline::{ + StatuslineEvaluationOutcome, StatuslineEvaluationTarget, StatuslineProviderId, + evaluate_statusline, +}; + +#[path = "common/iso.rs"] +mod iso; + +// --------------------------------------------------------------------------- +// Harness +// --------------------------------------------------------------------------- + +fn exec(state: &EditorState, source: &str) { + state.lua_host.lua().load(source.to_owned()).exec().unwrap(); +} + +fn eval(state: &EditorState, source: &str) -> T { + state.lua_host.lua().load(source.to_owned()).eval().unwrap() +} + +fn editor() -> EditorState { + let state = EditorState::new_with_roots(&iso::roots()); + exec(&state, "pmacs.lsp.config = {}"); + state +} + +/// Drive the async runtime until nothing is in flight and no coroutine +/// is parked. How many frames that takes is not knowable in advance, so +/// this never counts them. +/// +/// Quiescence is measured as **no `Running` job**, not as an empty +/// pending table. Most jobs here are dispatched and never awaited — +/// that is the shape the indicator exists to describe — and a settled +/// entry stays in the pending table until someone takes its result, so +/// `pending_count() == 0` would never come true. +fn pump(state: &mut EditorState) { + let deadline = Instant::now() + Duration::from_secs(10); + loop { + let idle: bool = eval( + state, + "return pmacs._async.parked_count() == 0 + and #pmacs.workers.snapshot().active == 0", + ); + if idle { + return; + } + assert!(Instant::now() < deadline, "async pump deadline exceeded"); + state.tick_async(); + } +} + +/// The purposes of every job the runtime currently has in flight. +/// +/// Read through the **Lua** snapshot surface, which is what `*workers*` +/// and any package consume, rather than through the Rust struct. +fn active_purposes(state: &EditorState) -> Vec { + eval( + state, + "local out = {} + for _, job in ipairs(pmacs.workers.snapshot().active) do + out[#out + 1] = job.purpose + end + return out", + ) +} + +fn paint(state: &EditorState, rows: u32, cols: u32) -> Vec { + let mut cells = vec![Cell::default(); (rows * cols) as usize]; + let mut grid = CellGrid { + cells: &mut cells, + stride: cols, + size: CellSize::new(rows, cols), + }; + let _ = pmacs::editor::paint_frame( + state, + FrontendId::LOCAL, + &HashMap::new(), + &mut grid, + CellSize::new(rows, cols), + ); + cells +} + +fn row_text(cells: &[Cell], cols: u32, row: u32) -> String { + (0..cols) + .map( + |column| match &cells[(row * cols + column) as usize].glyph { + Glyph::Char(ch) => *ch, + Glyph::Cluster(bytes) => std::str::from_utf8(bytes) + .ok() + .and_then(|text| text.chars().next()) + .unwrap_or(' '), + Glyph::Continuation => ' ', + }, + ) + .collect() +} + +/// The registration handle of the builtin activity provider. +fn activity_provider(state: &EditorState) -> StatuslineProviderId { + state + .statusline_registry + .borrow() + .providers() + .into_iter() + .find(|provider| provider.name == "activity") + .expect("builtin activity provider") + .id +} + +/// The activity provider's segment for `LOCAL`'s only window, or `None` +/// when it produced **no segment at all**. +/// +/// `Option`, never `String`, is the whole point of this helper: +/// "absent" and "empty" must be distinguishable, because a zero-width +/// segment still consumes a separator in the composed modeline. +fn activity_segment(state: &EditorState) -> Option { + let id = activity_provider(state); + let evaluation = evaluate_statusline( + state.lua_host.lua(), + &state.core, + &state.statusline_registry, + StatuslineEvaluationTarget::Grid { + frontend_id: FrontendId::LOCAL, + }, + ); + let StatuslineEvaluationOutcome::Ready(windows) = evaluation.outcome else { + panic!("statusline evaluation must be ready in a single-window editor"); + }; + windows + .iter() + .flat_map(|window| window.left.iter().chain(window.right.iter())) + .find(|segment| segment.provider_id == id) + .map(|segment| segment.text.clone()) +} + +/// Dispatch one job that will still be **in flight** when the caller +/// looks, without sleeping. +/// +/// A pending entry leaves `Running` only inside `AsyncRuntime::tick`, so +/// a dispatch with no intervening tick is in flight by construction — +/// no wall-clock race, and no worker left sleeping past the test. +fn dispatch_one_in_flight(state: &EditorState) { + exec(state, "IN_FLIGHT = pmacs.workers.sleep(50)"); +} + +// --------------------------------------------------------------------------- +// 1 — purpose reaches the three structurally distinct entry paths +// --------------------------------------------------------------------------- + +/// One per distinct **shape**, not one per dispatcher: a pool +/// dispatcher, an `register_external` job, and a spawned process. +/// +/// `register_external` is here because MCP and LSP bypass the worker +/// pool entirely — they are the likeliest paths for a later field to be +/// added to `PendingJob` and quietly missed — and because its `JobKind` +/// is the undifferentiated `LspRequest`/`McpRequest` for every method, +/// so `purpose` is the only thing that tells two of its rows apart. +#[test] +fn every_entry_shape_records_what_its_work_is() { + let mut state = editor(); + + // (a) A pool dispatcher. + dispatch_one_in_flight(&state); + let purposes = active_purposes(&state); + assert_eq!(purposes.len(), 1, "one job in flight: {purposes:?}"); + assert_eq!( + purposes[0], "sleep 50ms", + "a pool job records the work, not just its handler's name" + ); + + // (b) An externally-settled job. The purpose is a PARAMETER here + // because `register_external` has nothing to derive one from: its + // kind is a category, not a description. + let (job_id, _token) = state.async_runtime.register_external( + JobKind::LspRequest, + None, + "lsp textDocument/definition file:///tmp/x.rs", + ); + let purposes = active_purposes(&state); + assert!( + purposes + .iter() + .any(|p| p == "lsp textDocument/definition file:///tmp/x.rs"), + "an externally-registered job carries its caller's description: {purposes:?}" + ); + state.async_runtime.complete_external_cancelled(job_id); + + // (c) A spawned process. `label` keeps its existing meaning and its + // existing callers; `purpose` is the new, separate answer to "what + // is this doing". + exec( + &state, + r#"P = pmacs.process.spawn { + label = "sh-1", + purpose = "probing the repository for a build system", + command = "/bin/sh", + args = { "-c", "sleep 5" }, + }"#, + ); + let rows: Vec = eval( + &state, + "local out = {} + for _, row in ipairs(pmacs.process.list()) do + out[#out + 1] = row.label .. ' | ' .. row.purpose + end + return out", + ); + assert!( + rows.iter() + .any(|row| row == "sh-1 | probing the repository for a build system"), + "a spawned process carries a purpose ALONGSIDE its label: {rows:?}" + ); + exec(&state, "pmacs.process.terminate(P)"); + + pump(&mut state); +} + +/// **`pmacs.process.spawn` REFUSES a spec with no purpose, and spawns +/// nothing.** +/// +/// An earlier revision of this lane defaulted the field to `label` so +/// that existing callers kept working. That preserved compatibility and +/// delivered nothing: §9's complaint about `ProcessSpec` is precisely +/// that `label` is "caller-supplied, unvalidated convention", so a +/// purpose defaulting to the label hands every caller back the +/// convention this lane exists to replace. +/// +/// Six refusals, each asserted the same way — the call raises, **the +/// message names the field and the rule**, and the process list is +/// unchanged, because a validation that rejects after spawning has +/// already done the thing it was rejecting: +/// +/// * absent; +/// * empty, and whitespace-only — these satisfy the type and defeat the +/// point exactly as copying the label would (R42 rejects +/// whitespace-only config descriptions for the same reason); +/// * wrong type; +/// * **not valid UTF-8**. Lua strings are BYTE strings, so +/// `purpose = string.char(255)` is a value a caller can write, and +/// converting it with `?` would surface mlua's generic conversion +/// error before this lane's own diagnostic was ever constructed. The +/// refusal is not the interesting part — it refuses either way, and +/// nothing spawns either way — the MESSAGE is, which is why the +/// assertion is on content, and why the expected text now runs as far +/// as the **surface** the message names. Retyping this read as a bare +/// `?` breaks the row rather than silently degrading the error, and +/// naming the wrong surface breaks it too — see +/// `the_two_utf8_refusals_each_name_the_surface_their_own_text_reaches`; +/// * **metatable-provided**, which is the `stdin`/`group` raw-read +/// posture: a spec table is plain data, so a purpose cannot be +/// smuggled in through `__index`. +#[test] +fn spawning_without_a_real_purpose_is_refused_and_starts_nothing() { + let mut state = editor(); + let baseline: usize = eval(&state, "return #pmacs.process.list()"); + + for (label, spec, expected) in [ + ( + "absent", + r#"{ label = "x", command = "/bin/sh", args = { "-c", "sleep 5" } }"#, + "purpose is required", + ), + ( + "empty", + r#"{ label = "x", purpose = "", command = "/bin/sh", args = { "-c", "sleep 5" } }"#, + "must not be empty", + ), + ( + "whitespace-only", + r#"{ label = "x", purpose = " ", command = "/bin/sh", args = { "-c", "sleep 5" } }"#, + "must not be empty", + ), + ( + "wrong type", + r#"{ label = "x", purpose = 7, command = "/bin/sh", args = { "-c", "sleep 5" } }"#, + "purpose must be a string", + ), + ( + "invalid UTF-8", + r#"{ label = "x", purpose = "run " .. string.char(255), + command = "/bin/sh", args = { "-c", "sleep 5" } }"#, + "purpose must be valid UTF-8 — it is displayed to the user in \ + pmacs.process.list", + ), + ( + "metatable-provided", + r#"setmetatable( + { label = "x", command = "/bin/sh", args = { "-c", "sleep 5" } }, + { __index = function(_, k) + if k == "purpose" then return "smuggled" end + return nil + end })"#, + "purpose is required", + ), + ] { + let (ok, err): (bool, String) = eval( + &state, + &format!( + "local ok, err = pcall(pmacs.process.spawn, {spec}) + return ok, tostring(err)" + ), + ); + assert!(!ok, "{label}: spawn must refuse"); + assert!( + err.contains(expected), + "{label}: the refusal must name the field and the rule; got {err:?}" + ); + assert_eq!( + eval::(&state, "return #pmacs.process.list()"), + baseline, + "{label}: a refused spawn must start no process" + ); + } + pump(&mut state); +} + +/// Q#W-4's preservation half, pinned here as well as by the three +/// leak-detector suites: `purpose` is a new KEY on each existing row and +/// changes nothing about **which** processes `list()` enumerates. +/// +/// `m6_8_multi_repl_acceptance`, `compile_mode_acceptance` and +/// `lean4_stage1_acceptance` all assert on `#pmacs.process.list()` as a +/// leak baseline. If any of them needs editing, the design is wrong. +#[test] +fn process_list_still_hides_terminal_ptys() { + let mut state = editor(); + let before: usize = eval(&state, "return #pmacs.process.list()"); + exec( + &state, + "T = pmacs.terminal.open { command = '/bin/sh', args = { '-c', 'sleep 5' } }", + ); + let after: usize = eval(&state, "return #pmacs.process.list()"); + assert_eq!( + before, after, + "a terminal PTY must stay invisible to pmacs.process.list (Q#W-4)" + ); + assert!( + eval::(&state, "return pmacs.terminal.is_terminal(T)"), + "precondition: the PTY really was opened" + ); + // No explicit close: terminals have no Lua teardown surface, and + // `EditorState::drop` shuts the supervisor down with SIGTERM then + // SIGKILL, so the child cannot outlive the test. + pump(&mut state); +} + +// --------------------------------------------------------------------------- +// 2 — the dispatch-name ambient (Q#W-2) +// --------------------------------------------------------------------------- + +/// **A registered handler name is DISPLAY TEXT, and gets `purpose`'s +/// meaningful-value standard.** +/// +/// Before this lane the name died inside `dispatch` and a type check was +/// the whole of what it needed. It no longer dies there: the ambient +/// carries it into every job the handler allocates and composes it into +/// `purpose`, which `*workers*` and the modeline both render. So empty +/// and whitespace-only are refused here for the reason they are refused +/// in `required_purpose` — they satisfy the type and say nothing. +/// +/// **Refused at `register`, and asserted twice**: the call raises with a +/// message naming the rule, *and* nothing is installed under the name — +/// a validation that stored the handler first would have registered the +/// thing it was rejecting. +#[test] +fn a_handler_name_that_says_nothing_is_refused_at_registration() { + let mut state = editor(); + for (label, name_expr) in [("empty", r#""""#), ("whitespace-only", r#"" \t ""#)] { + let (ok, err): (bool, String) = eval( + &state, + &format!( + "local ok, err = pcall(pmacs.workers.register, {name_expr}, function() end) + return ok, tostring(err)" + ), + ); + assert!(!ok, "{label}: register must refuse"); + assert!( + err.contains("must not be empty or whitespace-only"), + "{label}: the refusal must name the rule; got {err:?}" + ); + let (dispatched, dispatch_err): (bool, String) = eval( + &state, + &format!( + "local ok, err = pcall(pmacs.workers.dispatch, {name_expr}) + return ok, tostring(err)" + ), + ); + assert!(!dispatched, "{label}: a refused name must install nothing"); + assert!( + dispatch_err.contains("unknown handler"), + "{label}: and the name must be genuinely absent from the table; \ + got {dispatch_err:?}" + ); + } + pump(&mut state); +} + +/// **Control characters in a handler name are refused at the source — +/// and this is the one rule `purpose` deliberately does NOT get.** +/// +/// The asymmetry is the design (`#228`'s shape). A purpose may +/// legitimately contain a newline: a filesystem path can, and +/// `pmacs-magit`'s spawn purpose is a whole argv — so its one-line +/// constraint is enforced by escaping at the surfaces that have one row. +/// A handler name is an identifier a package chooses for itself and +/// hands back to `dispatch`; a control character in one is a mistake or +/// an attempt at one, and refusing costs nobody anything. +/// +/// The rows are chosen to be **not** whitespace-only, so this test +/// cannot pass on the previous guard: a newline mid-word, an ESC (which +/// starts a terminal escape sequence), and a NUL. +#[test] +fn a_handler_name_with_control_characters_is_refused_at_registration() { + let mut state = editor(); + for (label, name_expr) in [ + ("newline", r#""index\ner""#), + ("escape", r#""index" .. string.char(27) .. "[31mer""#), + ("nul", r#""index" .. string.char(0) .. "er""#), + ] { + let (ok, err): (bool, String) = eval( + &state, + &format!( + "local ok, err = pcall(pmacs.workers.register, {name_expr}, function() end) + return ok, tostring(err)" + ), + ); + assert!(!ok, "{label}: register must refuse"); + assert!( + err.contains("must not contain control characters"), + "{label}: the refusal must name the rule; got {err:?}" + ); + let (dispatched, dispatch_err): (bool, String) = eval( + &state, + &format!( + "local ok, err = pcall(pmacs.workers.dispatch, {name_expr}) + return ok, tostring(err)" + ), + ); + assert!(!dispatched, "{label}: a refused name must install nothing"); + assert!( + dispatch_err.contains("unknown handler"), + "{label}: and the name must be genuinely absent from the table; \ + got {dispatch_err:?}" + ); + } + pump(&mut state); +} + +/// **The third rule a display-text name needs, enforced at the one +/// place that can see it.** +/// +/// Lua strings are BYTE strings and Lua 5.1 has no `utf8` library, so +/// `register` cannot tell a valid name from arbitrary bytes — +/// `string.char(255)` is neither whitespace nor a control character by +/// `%c`. The name crosses into Rust exactly once, at +/// `_push_dispatch_name`, and that is where the byte-level rule is +/// enforced. **This is the P2a class again**: an `mlua`-driven +/// conversion there would refuse with a generic message naming neither +/// the argument nor the rule, so the conversion failure is mapped onto +/// an owned diagnostic instead. +/// +/// The refusal lands at `dispatch` rather than at `register`, which is +/// later than ideal and is asserted as such — but it is still **before +/// the handler runs**, so a name that cannot be displayed never reaches +/// a job's purpose. +#[test] +fn a_handler_name_that_is_not_valid_utf8_is_refused_before_the_handler_runs() { + let mut state = editor(); + let (ok, err): (bool, String) = eval( + &state, + "RAN = false + pmacs.workers.register('bad' .. string.char(255), function() RAN = true end) + local ok, err = pcall(pmacs.workers.dispatch, 'bad' .. string.char(255)) + return ok, tostring(err)", + ); + assert!(!ok, "dispatch must refuse a name it cannot display"); + assert!( + err.contains("handler name must be valid UTF-8"), + "the refusal must name the argument and the rule; got {err:?}" + ); + assert!( + !eval::(&state, "return RAN"), + "and it must refuse BEFORE running the handler" + ); + assert!( + !eval::(&state, "return pmacs._async._in_dispatch_name_scope()"), + "a push that failed must leave no name on the stack" + ); + pump(&mut state); +} + +/// **A diagnostic that names the wrong surface is worse than a terse +/// one, and the two UTF-8 refusals do not name the same surface.** +/// +/// Both messages tell the caller *why* their bytes are refused: the text +/// gets displayed, and arbitrary bytes have no display form. But the two +/// values reach **different** places, and Stage 1 makes that difference +/// deliberately: +/// +/// * a **job**'s purpose — which a handler name is composed into — is +/// rendered by `*workers*` and by the modeline activity indicator; +/// * a **process**'s purpose is exposed through `pmacs.process.list` +/// and nothing else. Processes are kept out of `*workers*` and out of +/// the indicator until Stage 2's unified view (framing §3, Q#W-4). +/// +/// So the process-side message must not send a caller to `*workers*` to +/// look for a process that will never be listed there, and the job-side +/// message must not send them to an accessor that enumerates no jobs. +/// **Both directions are asserted, positive and negative**, because a +/// later edit that "unified the wording" would otherwise reintroduce +/// exactly one wrong sentence in exactly one of the two places and pass +/// every other test in this file. +#[test] +fn the_two_utf8_refusals_each_name_the_surface_their_own_text_reaches() { + let mut state = editor(); + + let (spawned, process_err): (bool, String) = eval( + &state, + r#"local ok, err = pcall(pmacs.process.spawn, { + label = "x", purpose = "run " .. string.char(255), + command = "/bin/sh", args = { "-c", "sleep 5" } }) + return ok, tostring(err)"#, + ); + assert!(!spawned, "precondition: the spawn must refuse"); + assert!( + process_err.contains("pmacs.process.list"), + "a process purpose reaches pmacs.process.list, and the refusal must \ + say so; got {process_err:?}" + ); + assert!( + !process_err.contains("*workers*") && !process_err.contains("modeline"), + "and it must NOT name the job surfaces a process never reaches; \ + got {process_err:?}" + ); + + let (dispatched, job_err): (bool, String) = eval( + &state, + "pmacs.workers.register('bad' .. string.char(255), function() end) + local ok, err = pcall(pmacs.workers.dispatch, 'bad' .. string.char(255)) + return ok, tostring(err)", + ); + assert!(!dispatched, "precondition: the dispatch must refuse"); + assert!( + job_err.contains("*workers*") && job_err.contains("modeline"), + "a handler name reaches both job surfaces, and the refusal must name \ + them; got {job_err:?}" + ); + assert!( + !job_err.contains("pmacs.process.list"), + "and it must NOT name the process accessor, which enumerates no jobs; \ + got {job_err:?}" + ); + + pump(&mut state); +} + +/// **Rule 7 + the defect itself.** A job dispatched through +/// `pmacs.workers.dispatch("name", …)` reports `"name"`. +/// +/// The witness is a handler **registered from Lua that calls a real +/// dispatcher**, not a synthetic push of the ambient. A test that +/// pushed the name by hand would prove the stack works and leave the +/// actual defect — `name` dying inside an arbitrary handler, three +/// layers above anything that takes a name — completely unwitnessed. +#[test] +fn a_dispatched_job_reports_the_registered_handler_name() { + let mut state = editor(); + exec( + &state, + "pmacs.workers.register('indexer', function() + return pmacs.workers.sleep(50) + end) + H = pmacs.workers.dispatch('indexer')", + ); + let purposes = active_purposes(&state); + assert_eq!(purposes.len(), 1, "one job in flight: {purposes:?}"); + assert!( + purposes[0].starts_with("indexer"), + "the third party's own name must survive the call chain: {purposes:?}" + ); + + // Rule 7: outside any extent, nothing changes. + exec(&state, "DIRECT = pmacs.workers.sleep(50)"); + let purposes = active_purposes(&state); + assert!( + purposes.iter().any(|p| p == "sleep 50ms"), + "a builtin invoked directly records its own purpose: {purposes:?}" + ); + pump(&mut state); +} + +/// **Rule 6 — COMPOSE, do not replace.** Both halves asserted, because +/// a test on the prefix alone passes when the description is dropped, +/// and a test on the description alone passes when the third party is +/// lost again. +#[test] +fn a_dispatched_job_composes_the_handler_name_with_the_work() { + let mut state = editor(); + exec( + &state, + "pmacs.workers.register('indexer', function() + return pmacs.workers.sleep(50) + end) + H = pmacs.workers.dispatch('indexer')", + ); + let purposes = active_purposes(&state); + assert_eq!( + purposes, + vec!["indexer: sleep 50ms".to_owned()], + "letting the name win discards the work; letting the work win \ + loses the third party" + ); + pump(&mut state); +} + +/// **Rules 3 and 4 — nesting is a stack (innermost wins) and fan-out +/// shares the name.** +#[test] +fn nesting_takes_the_innermost_name_and_fan_out_shares_it() { + let mut state = editor(); + exec( + &state, + "pmacs.workers.register('inner', function() + -- Fan-out: two jobs under one handler. + A = pmacs.workers.sleep(50) + B = pmacs.workers.sleep(51) + return A + end) + pmacs.workers.register('outer', function() + pmacs.workers.dispatch('inner') + -- Back in `outer`'s extent: the stack restored on return. + C = pmacs.workers.sleep(52) + return C + end) + pmacs.workers.dispatch('outer')", + ); + let mut purposes = active_purposes(&state); + purposes.sort(); + assert_eq!( + purposes, + vec![ + "inner: sleep 50ms".to_owned(), + "inner: sleep 51ms".to_owned(), + "outer: sleep 52ms".to_owned(), + ], + "innermost wins inside, and the outer name is restored after" + ); + pump(&mut state); +} + +/// **Rule 5 — unwind-safe, and this is the one that makes a naive +/// version worse than none.** +/// +/// A handler that raises must still pop. Otherwise one failure poisons +/// every subsequent dispatch in the session with a stale name, and the +/// feature stops failing loudly and starts lying silently — a +/// regression that would surface as intermittent misattribution long +/// after the lane landed. +#[test] +fn a_raising_handler_still_pops_its_name() { + let mut state = editor(); + exec( + &state, + "pmacs.workers.register('boom', function() error('handler failed') end) + OK, ERR = pcall(pmacs.workers.dispatch, 'boom')", + ); + assert!( + !eval::(&state, "return OK"), + "the handler's error must still reach the caller" + ); + assert!( + eval::(&state, "return tostring(ERR)").contains("handler failed"), + "and must reach it unchanged" + ); + + exec(&state, "LATER = pmacs.workers.sleep(50)"); + let purposes = active_purposes(&state); + assert_eq!( + purposes, + vec!["sleep 50ms".to_owned()], + "an unrelated later dispatch must not inherit the failed \ + handler's name: {purposes:?}" + ); + pump(&mut state); +} + +/// **Preservation.** `pmacs.workers.dispatch` was `return +/// handler(args, opts)` — a tail call that propagates **every** return +/// value. Bracketing it must not quietly truncate that. +/// +/// A `local ok, result = pcall(...)` bracketing would pass every other +/// test in this file and lose a two-value handler's second value with no +/// error anywhere, which is the shape of regression that surfaces months +/// later in somebody else's package. +#[test] +fn dispatch_still_propagates_every_value_the_handler_returns() { + let mut state = editor(); + let values: Vec = eval( + &state, + "pmacs.workers.register('multi', function() + return pmacs.workers.sleep(50), 'second', 'third' + end) + local a, b, c = pmacs.workers.dispatch('multi') + return { type(a), tostring(b), tostring(c) }", + ); + assert_eq!( + values, + vec!["table".to_owned(), "second".to_owned(), "third".to_owned()], + "a multi-value handler must survive the bracketing" + ); + pump(&mut state); +} + +/// **Rule 2 — work dispatched LATER is not covered, deliberately.** +/// +/// A job dispatched from an `on_complete` callback runs ticks later, +/// outside the extent, and carries only its own purpose. Asserted so +/// that the boundary reads as designed rather than as broken; covering +/// it would need the asynchronous lifetime mechanism Stage 3 owns +/// (Q#W-5). +#[test] +fn work_dispatched_from_a_completion_callback_carries_no_handler_name() { + let mut state = editor(); + exec( + &state, + "LATE = nil + pmacs.workers.register('deferred', function() + local h = pmacs.workers.sleep(1) + h:on_complete(function() + LATE = pmacs.workers.sleep(50) + end) + return h + end) + pmacs.workers.dispatch('deferred')", + ); + // One tick settles the first job and fires the callback; the job the + // callback dispatches is what this test is about, so do not pump to + // quiescence before reading it. + let deadline = Instant::now() + Duration::from_secs(10); + while !eval::(&state, "return LATE ~= nil") { + assert!(Instant::now() < deadline, "callback never fired"); + state.tick_async(); + } + let purposes = active_purposes(&state); + assert_eq!( + purposes, + vec!["sleep 50ms".to_owned()], + "the extent is the handler CALL, not the job's lifetime: {purposes:?}" + ); + pump(&mut state); +} + +// --------------------------------------------------------------------------- +// 3 — rule 1: the extent is non-yieldable, and that is ENFORCED +// --------------------------------------------------------------------------- + +/// **Rule 1, first supported yield API.** Two assertions, and the +/// second is the load-bearing one. +/// +/// A guard that raises but leaves the name pushed has converted a silent +/// misattribution into a silent misattribution *plus* an error. So the +/// witness dispatches again after the rejection and asserts the new job +/// carries no stale name. +#[test] +fn awaiting_inside_a_handler_is_refused_and_the_scope_restores() { + let mut state = editor(); + // The awaited handle is created OUTSIDE the extent on purpose: the + // second assertion below is about what a job allocated *after* the + // refusal carries, and a job the handler allocated for itself would + // legitimately wear the handler's name and blur that. + exec( + &state, + "OUTSIDE = pmacs.workers.sleep(1) + REFUSAL = nil + pmacs.workers.register('awaits', function() + local ok, err = pcall(function() return OUTSIDE:await() end) + REFUSAL = (not ok) and tostring(err) or '' + return OUTSIDE + end) + pmacs.async(function() pmacs.workers.dispatch('awaits') end)", + ); + let refusal: String = eval(&state, "return REFUSAL"); + assert!( + refusal.contains("cannot await inside") && refusal.contains("pmacs.workers.dispatch"), + "the refusal must name the rule it enforces; got {refusal:?}" + ); + assert!( + !eval::(&state, "return pmacs._async._in_dispatch_name_scope()"), + "a refused await must still leave the scope popped" + ); + + exec(&state, "AFTER = pmacs.workers.sleep(50)"); + let purposes = active_purposes(&state); + assert!( + purposes.iter().any(|p| p == "sleep 50ms"), + "and a later dispatch must carry no stale name: {purposes:?}" + ); + assert!( + !purposes.iter().any(|p| p.starts_with("awaits:")), + "no job allocated after the refusal may inherit the handler's \ + name: {purposes:?}" + ); + pump(&mut state); +} + +/// **Rule 1, unconditionally.** The refusal fires even when the awaited +/// handle has already settled. +/// +/// This is the case that separates an unconditional guard from one whose +/// behaviour depends on a race: a guard placed after the `_is_complete` +/// check would fire only when a yield would really occur, passing under +/// test and failing intermittently in production depending on whether +/// the job happened to finish first. +#[test] +fn the_await_refusal_fires_even_for_an_already_complete_handle() { + let mut state = editor(); + exec(&state, "SETTLED = pmacs.workers.sleep(0)"); + let deadline = Instant::now() + Duration::from_secs(10); + while !eval::(&state, "return SETTLED:is_complete()") { + assert!(Instant::now() < deadline, "the canary never settled"); + state.tick_async(); + } + + exec( + &state, + "REFUSAL = nil + pmacs.workers.register('awaits-settled', function() + local ok, err = pcall(function() return SETTLED:await() end) + REFUSAL = (not ok) and tostring(err) or '' + return pmacs.workers.sleep(50) + end) + pmacs.workers.dispatch('awaits-settled')", + ); + let refusal: String = eval(&state, "return REFUSAL"); + assert!( + refusal.contains("cannot await inside") && refusal.contains("pmacs.workers.dispatch"), + "a settled handle must be refused too, or the guard's behaviour \ + depends on a race; got {refusal:?}" + ); + assert!( + !eval::(&state, "return pmacs._async._in_dispatch_name_scope()"), + "and the scope must still be popped" + ); + pump(&mut state); +} + +/// **Rule 1, second supported yield API.** Guarding `:await()` and not +/// `yield_to_next_tick` would leave the extent open through a second +/// door — and Q#W-7 below is the proof that exactly that happens when +/// only one door is guarded. +#[test] +fn yield_to_next_tick_inside_a_handler_is_refused_and_the_scope_restores() { + let mut state = editor(); + exec( + &state, + "REFUSAL = nil + pmacs.workers.register('yields', function() + local ok, err = pcall(pmacs.async.yield_to_next_tick) + REFUSAL = (not ok) and tostring(err) or '' + return pmacs.workers.sleep(50) + end) + pmacs.async(function() pmacs.workers.dispatch('yields') end)", + ); + let refusal: String = eval(&state, "return REFUSAL"); + assert!( + refusal.contains("cannot yield inside") && refusal.contains("pmacs.workers.dispatch"), + "the second yield API must refuse too; got {refusal:?}" + ); + assert!( + !eval::(&state, "return pmacs._async._in_dispatch_name_scope()"), + "and must leave the scope popped" + ); + + exec(&state, "AFTER = pmacs.workers.sleep(51)"); + let purposes = active_purposes(&state); + assert!( + purposes.iter().any(|p| p == "sleep 51ms"), + "a later dispatch must carry no stale name: {purposes:?}" + ); + pump(&mut state); +} + +// --------------------------------------------------------------------------- +// 4 — Q#W-7: the same hole in `commit_to`, closed here +// --------------------------------------------------------------------------- + +/// **Q#W-7 — a pre-existing defect, found by reading and repaired in +/// this lane.** +/// +/// `Handle:await()` refuses inside `pmacs.window.commit_to` precisely so +/// a coroutine cannot park with the frontend scope pushed (Journey Stage +/// 1a, Q#JR14b). But `pmacs.async.yield_to_next_tick()` also yields, is +/// public, and carried **no** such refusal — so that invariant had a +/// second entrance. +/// +/// **Reachability by a real caller is UNPROVEN.** No production caller +/// is known to yield through this door inside a commit; this pins the +/// guard rather than reproducing a user-visible bug. +/// +/// Both halves asserted, for the same reason as rule 1's: a refusal that +/// leaves the scope pushed swaps a silent misrouting for a loud one and +/// fixes neither. +#[test] +fn yield_to_next_tick_inside_commit_to_is_refused_and_the_commit_scope_restores() { + let mut state = editor(); + let dir = tempfile::tempdir().expect("tempdir"); + std::fs::write(dir.path().join("alpha.txt"), b"alpha\n").expect("write"); + + // A GENUINE destination, produced by the production capture: the + // listener claims (returns false), so nothing commits and what lands + // in `dest` is exactly the userdata dired would have received. + // Nothing in a test can construct one. + exec( + &state, + "dest = nil + pmacs.hook.add('path.open-directory', function(_, d) dest = d return false end)", + ); + state.open_directory_target(dir.path()); + pump(&mut state); + assert!( + eval::(&state, "return dest ~= nil"), + "the chain must hand listeners a destination" + ); + + exec( + &state, + "REFUSAL = nil + pmacs.async(function() + local ok, err = pcall(pmacs.window.commit_to, dest, function() + pmacs.async.yield_to_next_tick() + end) + REFUSAL = (not ok) and tostring(err) or '' + end)", + ); + let refusal: String = eval(&state, "return REFUSAL"); + assert!( + refusal.contains("cannot yield inside") && refusal.contains("commit_to"), + "the second door into the commit scope must be shut; got {refusal:?}" + ); + assert!( + !eval::(&state, "return pmacs._async._in_commit_scope()"), + "and the commit scope must still be restored afterwards" + ); + pump(&mut state); +} + +// --------------------------------------------------------------------------- +// 5 — the statusline activity indicator (Q#W-3, Q#W-6) +// --------------------------------------------------------------------------- + +/// **Absent at zero, asserted as an absent SEGMENT rather than as an +/// empty string.** A zero-width segment still consumes a separator in +/// the composed modeline, so "returns nothing" and "returns nothing +/// visible" are different claims and only one of them is the design. +#[test] +fn the_indicator_produces_no_segment_at_all_when_nothing_is_running() { + let state = editor(); + assert_eq!( + activity_segment(&state), + None, + "an idle editor must produce NO activity segment" + ); +} + +/// **A count plus the oldest in-flight job's purpose, witnessed through +/// the real per-frame evaluation path.** +/// +/// Driven through `paint_frame`, not by calling the provider function +/// directly: a provider that works in isolation and never gets evaluated +/// is exactly the failure this must exclude. +#[test] +fn the_indicator_shows_a_count_and_the_oldest_purpose_in_a_painted_frame() { + let mut state = editor(); + exec( + &state, + "FIRST = pmacs.workers.sleep(50) + SECOND = pmacs.workers.grep({ root = '/tmp', pattern = 'zzz-no-match' })", + ); + + let cells = paint(&state, 24, 160); + let modeline = row_text(&cells, 160, 22); + assert!( + modeline.contains("⋯2 sleep 50ms"), + "the painted modeline must carry the count and the OLDEST job's \ + purpose (not the newest); got {modeline:?}" + ); + + // And the same value reaches the evaluator's segment vector, which is + // what the semantic frontend ships. + assert_eq!( + activity_segment(&state).as_deref(), + Some("⋯2 sleep 50ms"), + "the segment and the painted row must agree" + ); + + exec(&state, "SECOND:cancel()"); + pump(&mut state); +} + +/// **Q#W-6 — the setting, witnessed with work genuinely in flight.** +/// +/// The discriminating case: an assertion taken on an idle editor cannot +/// tell "disabled" from "nothing is happening", which is the only thing +/// this setting changes. +#[test] +fn the_indicator_honours_its_setting_while_work_is_in_flight() { + let mut state = editor(); + dispatch_one_in_flight(&state); + assert!( + activity_segment(&state).is_some(), + "precondition: work is in flight and the indicator is on" + ); + + exec(&state, "pmacs.config.set('ui.activity-indicator', false)"); + assert_eq!( + activity_segment(&state), + None, + "disabled means NO segment, with work still running" + ); + + exec(&state, "pmacs.config.set('ui.activity-indicator', true)"); + assert!( + activity_segment(&state).is_some(), + "and re-enabling brings it back without a restart" + ); + pump(&mut state); +} + +/// The setting is a real registry entry, not an ad-hoc global: it is +/// discoverable through `pmacs.config.describe` like every other +/// setting, which is what `COHERENCE.md` §11 grades. +#[test] +fn the_setting_is_registered_with_a_true_default() { + let state = editor(); + let (kind, default): (String, bool) = eval( + &state, + "local d = pmacs.config.describe('ui.activity-indicator') + return d.type, d.default", + ); + assert_eq!(kind, "boolean"); + assert!(default, "visible by default — no configuration, no command"); +} + +// --------------------------------------------------------------------------- +// 6 — `*workers*` renders the purpose +// --------------------------------------------------------------------------- + +/// The view §9 already has, now answering §9's question. +/// +/// `Kind` names the builtin dispatcher a job funnelled through, which +/// for a third-party job is a builtin's label rather than the caller's; +/// the `Purpose` column is what carries the caller's own account. +#[test] +fn the_workers_buffer_renders_the_purpose_column() { + let mut state = editor(); + exec( + &state, + "pmacs.workers.register('indexer', function() + return pmacs.workers.sleep(50) + end) + pmacs.workers.dispatch('indexer') + BUF = pmacs.workers.show()", + ); + let text: String = eval(&state, "return BUF:slice(0, BUF:len())"); + assert!( + text.contains("Purpose"), + "the active table must have a Purpose column:\n{text}" + ); + assert!( + text.contains("indexer: sleep 50ms"), + "and the row must render it:\n{text}" + ); + exec(&state, "pmacs.workers.hide()"); + pump(&mut state); +} + +// --------------------------------------------------------------------------- +// 7 — the display-text boundary: a row must not forge another row +// --------------------------------------------------------------------------- +// +// A purpose is free-form caller text and is legitimately multi-line — a +// filesystem path may contain a newline and `pmacs-magit`'s spawn +// purpose is a whole argv — so the one-line constraint belongs to the +// surfaces that have one line, not to the purpose. That is `#228`'s +// decision on `Command.description`, applied here as escaping rather +// than clipping, because a purpose's later words are load-bearing. +// +// Every test below drives the REAL rendering path. Calling +// `purpose_for_one_row` directly would prove the escaper escapes and say +// nothing about whether either surface calls it. +// +// `register_external` is the witness in all three because it is the one +// entry shape whose purpose is verbatim caller text: the pool +// dispatchers all `format!` their own, and `{:?}` in those formats +// already escapes, so a hostile purpose cannot reach a row through them. + +/// **The spoofing property, and the whole reason the escaping exists.** +/// +/// A purpose crafted to look like a row boundary followed by a plausible +/// job row does not produce a second row. Asserted by COUNTING the rows, +/// not by looking for the escape sequence: a renderer that dropped the +/// purpose entirely would satisfy "no forged row" while destroying the +/// feature, so the escaped text is asserted present in the surviving row +/// as well. +#[test] +fn a_purpose_shaped_like_a_row_boundary_does_not_produce_a_second_row() { + let mut state = editor(); + let (job_id, _token) = state.async_runtime.register_external( + JobKind::LspRequest, + None, + "lsp definition\n#99 grep 0ms forged \ + cancelled by nobody", + ); + exec(&state, "BUF = pmacs.workers.show()"); + let text: String = eval(&state, "return BUF:slice(0, BUF:len())"); + + let job_rows: Vec<&str> = text.lines().filter(|line| line.starts_with('#')).collect(); + assert_eq!( + job_rows.len(), + 1, + "one job must render as exactly ONE row:\n{text}" + ); + assert!( + job_rows[0].contains("lsp definition\\n#99"), + "and the break must be rendered, escaped, INSIDE that row:\n{text}" + ); + assert!( + !text.contains("\n#99"), + "no line may begin with the forged id:\n{text}" + ); + + // `#228`'s other half, and what makes this a rendering decision + // rather than data loss: the free-form surface still hands Lua every + // byte, unescaped. + let raw = active_purposes(&state); + assert!( + raw.iter().any(|purpose| purpose.contains('\n')), + "pmacs.workers.snapshot() is the raw path and must stay raw: {raw:?}" + ); + + exec(&state, "pmacs.workers.hide()"); + state.async_runtime.complete_external_cancelled(job_id); + pump(&mut state); +} + +/// **The modeline is one line, and that is enforced where the modeline +/// reads.** +/// +/// Two assertions, because they exclude different failures: the segment +/// carries no break at all (a composed modeline splicing one would +/// misplace every segment after it), and the escaped text survives the +/// real per-frame paint rather than only the evaluator. +#[test] +fn a_purpose_that_spans_lines_reaches_the_modeline_as_one_line() { + let mut state = editor(); + let (job_id, _token) = state.async_runtime.register_external( + JobKind::LspRequest, + None, + "lsp didOpen\nfile:///tmp/x.rs", + ); + + let segment = activity_segment(&state).expect("work is in flight, so a segment exists"); + assert!( + !segment.contains(['\n', '\r']), + "a modeline segment is ONE line: {segment:?}" + ); + assert_eq!( + segment, "⋯1 lsp didOpen\\nfile:///tmp/x.rs", + "and the break is escaped in place, not clipped away" + ); + + let cells = paint(&state, 24, 160); + let modeline = row_text(&cells, 160, 22); + assert!( + modeline.contains("⋯1 lsp didOpen\\nfile:///tmp/x.rs"), + "the painted modeline must carry it too; got {modeline:?}" + ); + + state.async_runtime.complete_external_cancelled(job_id); + pump(&mut state); +} + +/// **A purpose with no control characters is byte-identical after +/// escaping** — on both surfaces. +/// +/// The fixture is chosen to break a careless escaper: a literal +/// backslash (which a JSON-style escaper would double, and which is +/// deliberately NOT escaped here — no number of backslashes produces a +/// second row), a `\v` that is text rather than a vertical tab, quotes, +/// and a non-ASCII character. +#[test] +fn a_purpose_with_no_control_characters_is_unchanged_by_the_boundary() { + const PURPOSE: &str = r#"grep "fn \d+" in /tmp/pro—ject\v2"#; + + let mut state = editor(); + let (job_id, _token) = + state + .async_runtime + .register_external(JobKind::LspRequest, None, PURPOSE); + + exec(&state, "BUF = pmacs.workers.show()"); + let text: String = eval(&state, "return BUF:slice(0, BUF:len())"); + assert!( + text.contains(PURPOSE), + "the *workers* row must reproduce an ordinary purpose byte for byte:\n{text}" + ); + + assert_eq!( + activity_segment(&state).as_deref(), + Some(format!("⋯1 {PURPOSE}").as_str()), + "and so must the modeline segment" + ); + + exec(&state, "pmacs.workers.hide()"); + state.async_runtime.complete_external_cancelled(job_id); + pump(&mut state); +}