Merge pull request #232 from levineuwirth/worker-identity-stage1
feat(workers): a required purpose on every job and process — worker identity Stage 1
This commit is contained in:
commit
3cc1b85108
|
|
@ -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" },
|
||||
|
|
|
|||
|
|
@ -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, `"<name>: <purpose>"`) 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 `"<name>: <purpose>"`, 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.
|
||||
|
|
|
|||
|
|
@ -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" },
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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
|
||||
`"<name>"` instead of `"<name>: <purpose>"` 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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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<ResourceOp>`, 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<WorkspaceId>` 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 `"<name>: <purpose>"`; where it did not, the
|
||||
recorded value is `"<name>"`. 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 `"<name>: <purpose>"`
|
||||
— 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 <the new suite>`. **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.
|
||||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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<ResourceOp>,
|
||||
/// 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<usize>,
|
||||
/// Filesystem mutation this job performs, for the settle-time
|
||||
/// reconcile (dired Stage 2a).
|
||||
resource: Option<ResourceOp>,
|
||||
/// 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<String>,
|
||||
/// 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<String>,
|
||||
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<Mutex<HashMap<JobId, Arc<ParseTreeBundle>>>>,
|
||||
/// 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<Vec<String>>,
|
||||
}
|
||||
|
||||
/// 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<String>) {
|
||||
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<String> {
|
||||
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<usize>,
|
||||
) -> (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<usize>,
|
||||
resource: Option<ResourceOp>,
|
||||
) -> (JobId, CancellationToken) {
|
||||
///
|
||||
/// The recorded purpose **composes** with any dispatch-name ambient
|
||||
/// rather than replacing it (Q#W-2 rule 6): `"<name>: <purpose>"`
|
||||
/// where the dispatcher described its own work, `"<name>"` 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<usize>,
|
||||
) -> 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<usize>,
|
||||
) -> 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<String>,
|
||||
) -> (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<ActivitySummary> {
|
||||
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<ResourceOp> {
|
||||
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,
|
||||
|
|
|
|||
21
src/lsp.rs
21
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 {
|
||||
|
|
|
|||
|
|
@ -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<RestartPolicy> {
|
|||
})
|
||||
}
|
||||
|
||||
/// 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<String> {
|
||||
let purpose = match table.raw_get::<mlua::Value>("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<ProcessSpec> {
|
||||
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<String> = table.get("args").unwrap_or_default();
|
||||
let cwd: Option<String> = table.get("cwd").ok().flatten();
|
||||
let env_table: Option<Table> = table.get("env").ok().flatten();
|
||||
|
|
@ -8762,6 +8941,7 @@ fn lua_to_spec(table: &Table) -> mlua::Result<ProcessSpec> {
|
|||
};
|
||||
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)?)?;
|
||||
|
|
|
|||
38
src/mcp.rs
38
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 {
|
||||
|
|
|
|||
|
|
@ -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<String>, command: impl Into<String>) -> Self {
|
||||
pub fn new(
|
||||
label: impl Into<String>,
|
||||
command: impl Into<String>,
|
||||
purpose: impl Into<String>,
|
||||
) -> 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
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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} {:<PURPOSE_WIDTH$} Status",
|
||||
"ID", "Kind", "Age", "Supersede", "Purpose"
|
||||
);
|
||||
let _ = writeln!(
|
||||
text,
|
||||
"{:<7} {:<11} {:>9} {:<11} ----------",
|
||||
"------", "-----------", "---------", "-----------"
|
||||
"{:<7} {:<11} {:>9} {:<11} {:<PURPOSE_WIDTH$} ----------",
|
||||
"------",
|
||||
"-----------",
|
||||
"---------",
|
||||
"-----------",
|
||||
"-".repeat(PURPOSE_WIDTH)
|
||||
);
|
||||
if snapshot.active.is_empty() {
|
||||
let _ = writeln!(text, "(no active jobs)");
|
||||
|
|
@ -139,13 +169,17 @@ fn format_snapshot(snapshot: &WorkersSnapshot) -> String {
|
|||
let _ = writeln!(text);
|
||||
let _ = writeln!(
|
||||
text,
|
||||
"{:<7} {:<11} {:>9} {:<11} Outcome",
|
||||
"ID", "Kind", "Duration", "Supersede"
|
||||
"{:<7} {:<11} {:>9} {:<11} {:<PURPOSE_WIDTH$} Outcome",
|
||||
"ID", "Kind", "Duration", "Supersede", "Purpose"
|
||||
);
|
||||
let _ = writeln!(
|
||||
text,
|
||||
"{:<7} {:<11} {:>9} {:<11} ----------",
|
||||
"------", "-----------", "---------", "-----------"
|
||||
"{:<7} {:<11} {:>9} {:<11} {:<PURPOSE_WIDTH$} ----------",
|
||||
"------",
|
||||
"-----------",
|
||||
"---------",
|
||||
"-----------",
|
||||
"-".repeat(PURPOSE_WIDTH)
|
||||
);
|
||||
if snapshot.completed.is_empty() {
|
||||
let _ = writeln!(text, "(no recent completions)");
|
||||
|
|
@ -169,7 +203,11 @@ fn write_active_row(text: &mut String, job: &ActiveJobInfo) {
|
|||
if job.is_stream {
|
||||
status.push_str(" [stream]");
|
||||
}
|
||||
let _ = writeln!(text, "{id:<7} {kind:<11} {age:>9} {key:<11} {status}");
|
||||
let purpose = purpose_for_one_row(&job.purpose);
|
||||
let _ = writeln!(
|
||||
text,
|
||||
"{id:<7} {kind:<11} {age:>9} {key:<11} {purpose:<PURPOSE_WIDTH$} {status}"
|
||||
);
|
||||
}
|
||||
|
||||
fn write_completed_row(text: &mut String, job: &CompletedJobInfo) {
|
||||
|
|
@ -179,9 +217,10 @@ fn write_completed_row(text: &mut String, job: &CompletedJobInfo) {
|
|||
let key = job.supersede_key.as_deref().unwrap_or("");
|
||||
let outcome = format_outcome(&job.outcome);
|
||||
let age = format_duration_ms(job.settled_age_ms);
|
||||
let purpose = purpose_for_one_row(&job.purpose);
|
||||
let _ = writeln!(
|
||||
text,
|
||||
"{id:<7} {kind:<11} {duration:>9} {key:<11} {outcome} ({age} ago)"
|
||||
"{id:<7} {kind:<11} {duration:>9} {key:<11} {purpose:<PURPOSE_WIDTH$} {outcome} ({age} ago)"
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -287,6 +326,7 @@ mod tests {
|
|||
supersede_key: Some("search".to_string()),
|
||||
cancel_requested: false,
|
||||
is_stream: true,
|
||||
purpose: "grep pattern".to_string(),
|
||||
}],
|
||||
vec![],
|
||||
);
|
||||
|
|
@ -309,6 +349,7 @@ mod tests {
|
|||
supersede_key: None,
|
||||
cancel_requested: true,
|
||||
is_stream: false,
|
||||
purpose: "grep pattern".to_string(),
|
||||
}],
|
||||
vec![],
|
||||
);
|
||||
|
|
@ -326,6 +367,7 @@ mod tests {
|
|||
duration_ms: 25,
|
||||
settled_age_ms: 200,
|
||||
supersede_key: None,
|
||||
purpose: "sum 1..10".to_string(),
|
||||
outcome: JobOutcome::Complete(JobResult::Sum(55)),
|
||||
}],
|
||||
);
|
||||
|
|
@ -368,6 +410,7 @@ mod tests {
|
|||
supersede_key: None,
|
||||
cancel_requested: false,
|
||||
is_stream: true,
|
||||
purpose: "grep pattern".to_string(),
|
||||
}],
|
||||
vec![],
|
||||
);
|
||||
|
|
|
|||
|
|
@ -1836,7 +1836,8 @@ fn r1f6_wrong_spec_types_error_instead_of_defaulting() {
|
|||
&s,
|
||||
r#"
|
||||
local ok, err = pcall(pmacs.process.spawn,
|
||||
{ label = "t", command = "/bin/true", stdin = true })
|
||||
{ label = "t", purpose = "type-check probe", command = "/bin/true",
|
||||
stdin = true })
|
||||
return ok, tostring(err)
|
||||
"#,
|
||||
);
|
||||
|
|
@ -1846,7 +1847,8 @@ fn r1f6_wrong_spec_types_error_instead_of_defaulting() {
|
|||
&s,
|
||||
r#"
|
||||
local ok, err = pcall(pmacs.process.spawn,
|
||||
{ label = "t", command = "/bin/true", group = "true" })
|
||||
{ label = "t", purpose = "type-check probe", command = "/bin/true",
|
||||
group = "true" })
|
||||
return ok, tostring(err)
|
||||
"#,
|
||||
);
|
||||
|
|
@ -2234,7 +2236,8 @@ fn r3f3_spec_fields_are_raw_reads_metatables_not_honored() {
|
|||
&s,
|
||||
r#"
|
||||
local spec = setmetatable(
|
||||
{ label = "mt", command = "/bin/sh", args = { "-c", "sleep 30" } },
|
||||
{ label = "mt", purpose = "raw-read probe", command = "/bin/sh",
|
||||
args = { "-c", "sleep 30" } },
|
||||
{ __index = function(_, k)
|
||||
if k == "group" then return true end
|
||||
return nil
|
||||
|
|
@ -2265,7 +2268,8 @@ fn r3f3_spec_fields_are_raw_reads_metatables_not_honored() {
|
|||
&s,
|
||||
r#"
|
||||
local spec = setmetatable(
|
||||
{ label = "mt2", command = "/bin/sh", args = { "-c", "exit 0" } },
|
||||
{ label = "mt2", purpose = "raw-read probe", command = "/bin/sh",
|
||||
args = { "-c", "exit 0" } },
|
||||
{ __index = function() error("hostile spec metatable") end })
|
||||
local ok = pcall(pmacs.process.spawn, spec)
|
||||
return ok
|
||||
|
|
|
|||
|
|
@ -54,6 +54,10 @@ function M.run_git(args, opts)
|
|||
opts = opts or {}
|
||||
local id = pmacs.process.spawn {
|
||||
label = "git " .. (args[1] or ""),
|
||||
-- Worker identity Stage 1: `purpose` is required. The full argument
|
||||
-- vector, not just the subcommand the label carries -- "git log" and
|
||||
-- "git log --oneline -20" are the same label and different work.
|
||||
purpose = "git " .. table.concat(args, " "),
|
||||
command = "git",
|
||||
args = args,
|
||||
cwd = opts.cwd,
|
||||
|
|
|
|||
|
|
@ -952,7 +952,7 @@ fn has_exit_event(events: &[ProcessEvent]) -> 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" },
|
||||
}
|
||||
|
|
|
|||
|
|
@ -136,7 +136,12 @@ fn a01_04_registry_contract_limits_epochs_and_results() {
|
|||
.iter()
|
||||
.map(|provider| provider.name.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
["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 = {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
Loading…
Reference in New Issue