Review of the #233 implementation found two correctness defects. Both
are cases where the FIRST fix for #233 was itself wrong, which is worth
naming: this lane repaired absolute globs and, in the same change, broke
a case that had worked since May.
P1 --- `resolve_watcher` returned "absolute" for EVERY string, so the
form was carried but derived from the union arm rather than from the
pattern. A bare `*.txt` is a valid relative pattern (LSP 3.17 defines
`Pattern` relative to a base path; VS Code treats string watchers as
applying across workspace folders), and classifying it absolute matched
it against `<base>/foo.txt`, which `^[^/]*%.txt$` can never match. A
leading `/` is what makes a pattern absolute. The `filewatchflat` test
could not catch this: it sends the RelativePattern OBJECT form, so it
constrains the object arm, never the string arm the regression lived in
--- the framing's own F1 finding, repeating inside the lane that named
it.
P2 --- `scan_tree` awaits `read_dir` once per directory, so the watcher
coroutine sits suspended for most of a tick with `_sleep` already
cleared. A cancel arriving there sets `cancelled` and has no sleep to
interrupt, and the resumed scan ran on to `did_change_watched_files`:
one stale batch under the superseded pattern, which is a wrong-pattern
notification the server acts on. Cancellation and liveness are rechecked
after the scan.
Both tests are mutation-checked and each bite fails only its own defect.
Reverting P1 fails the bare-string test alone --- the absolute test
still passes, so the two readings are independent. Deleting P2's recheck
reproduces the defect verbatim: `.received = "1 file:///…/foo.txt"`,
a batch from a watcher that was already cancelled.
P2's witness needs a seam. The race is a cancel landing during one of
the scan's suspensions, which no arrangement of real timing produces on
demand --- the same situation, and the same device, as `git.lua`'s
`_deliver_status`. `pmacs.lsp._after_scan_for_tests` is nil in
production and is handed the SCAN RESULT deliberately: a test that
cancels on any other scan passes with the recheck deleted, because the
loop would break at the post-sleep check and emit nothing anyway.
Gates: 9/9 green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Implements D1 and D2 from docs/lsp-file-watcher-framing.md (revision 1,
approved 2026-08-10). The user ruled that the walking survives this
lane; D3 gets its own framing.
D1. resolve_watcher now returns (base, pattern, form) and the watch
record carries the form. Per LSP, a plain-string glob matches the
file's ABSOLUTE path while a RelativePattern's pattern is relative to
its base --- and resolve_watcher discarded the distinction, so every
downstream consumer matched relatively. Real servers send absolute
globs: rust-analyzer was never told about any file change (all six of
its globs absolute), and gopls saw go.mod but never a .go edit. The
match subject is chosen in start_file_watcher --- form "absolute"
matches base .. "/" .. rel, form "relative" matches rel unchanged.
scan_tree still walks in relative terms; only the string handed to the
matcher changes.
D2. register_file_watchers now cancels the outgoing record list before
file_watchers[skey][reg.id] = recs drops the only reference to it. The
cancel loop is factored into cancel_watch_records, shared with
unregister_file_watchers, so the two paths cannot diverge.
rust-analyzer registers the same id twice with no unregister between
--- previously 12 concurrent pollers, 6 permanently uncancellable.
Verification, per the framing's plan. Three new fake-LSP modes and
tests beside m4_24, each mutation-tested against the defect it names:
- filewatchabs registers a plain-string absolute glob whose relative
reading matches nothing. Red before D1 (reverting the match subject
fails exactly this test), green after.
- filewatchflat registers a RelativePattern without a leading **/
(*.txt at the base) --- F2's guard. Matching every form absolutely
fails exactly this test, so the obvious wrong fix cannot land green.
It also pins that a base-level pattern does not match into
subdirectories.
- filewatchrereg registers the same id twice (rust-analyzer's shape).
The witness is observable polling, not table shape: f.old exists on
disk before either .new event lands, so a leaked watcher at the same
250ms cadence reports it before the second positive. Reverting D2
fails exactly this test, the leaked .old event visible in .received.
m4_24 stayed green under all three mutations --- the framing's F1
finding (the existing test is insensitive to D1 in both directions),
confirmed rather than assumed. It is kept unchanged.
The framing doc records the approval and the answered ruling; the
active-work lane moves to IMPLEMENTED with the verification results.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The async completions rendered through whichever frontend was ambient
when git exited. Run `M-x git.status` in frontend A, let B become
active while `git status` runs, and A's panel opened in B. The
generation and the root were already captured at the keypress; the
frontend was the one input still read late.
Captured at invocation in all four entrances --- `git.status()`,
`_on_refresh`, `_deliver_root`'s hand-off, and the `git.diff-file`
command --- and threaded on the request table exactly as this module
already threads the generation and root. Committed under the profile
each surface actually takes: `"panel"` for `*git-status*`, `"document"`
for `*git-diff*`.
`set_status` moved INSIDE the status commit. Rows and message are now
computed first and emitted together, because a failure message
announcing a panel that the commit then refuses is the same misrouting
in its most confusing form.
A refused commit DROPS the render, which is the answer the
`expect_buffer` rule already gives when the panel a refresh belongs to
was killed. `commit_to` refuses before the body runs, so there is no
partial render to undo.
THE FIRST VERSION OF `g6_25` WAS WORTHLESS AND PASSED ITS OWN MUTATION.
`panel_text` finds `*git-status*` by NAME, which is global --- it
answers "does this buffer exist", not "which frontend is showing it"
--- so a render into the competitor satisfied it. Rewritten against
`side_window_for` and each view's active window, it now fails both
bites: removing the status commit grows a `*git-status*` panel in the
competing frontend; removing the diff commit hands it the document
window.
The merge also surfaced a cross-lane break invisible until the suites
ran: #232 made `purpose` required on `pmacs.process.spawn`, and this
module's spawn is on this branch, so it was never among the 11 sites
#232 updated. Each spawn now carries its own purpose and NOT the label
--- all three are labelled `git`, and only the purpose separates
resolving a repository from reading its status from diffing one file.
Existing tests that drive `_deliver_status` / `_deliver_diff` through
the internal seam now supply a captured destination. That includes the
one place it is load-bearing rather than uniform: `g6_23` reuses its
"stale" request table at the CURRENT ticket as its positive control,
which is the only one of those three deliveries that reaches a render.
Gates: 12/12 green --- git_status_stage1, destination_capture, journey
and worker_identity acceptance, plus the full sweep.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Merged rather than rebased, for the reason #231 was: nineteen commits
replayed against a ledger four lanes had rewritten is nineteen chances
to lose a lane entry, and the reviewed commits keep their SHAs. One
file conflicted.
The conflict was diff3-style with an EMPTY base --- both sides added
lanes at the same offset --- so it resolved by keeping both, this
branch's #227 lane ahead of main's. Worth noting because an empty base
section is exactly what produced the stray `|||||||` markers found
committed in this file earlier today: the base half of a diff3 conflict
reads as content if it is resolved by hand in a hurry.
This unblocks the lane. `pmacs.window.capture_destination` now exists
on the branch (`src/lua_bindings/window_panel.rs:638`), which is what
#227's review found missing and what #231 was split out to provide.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Merged rather than rebased. Eighteen commits replayed against a ledger
that three other lanes had rewritten meant eighteen conflict
resolutions in `docs/active-work.md`, each one a chance to lose a lane
entry; merging resolves it once, against the state that actually ships,
and leaves the reviewed commits' SHAs intact. Only one file conflicted.
`docs/ci-red-signatures.md` auto-merged **without a conflict** — the
same silent path that produced duplicate U4/U5 ids when #232 rebased.
Verified by hand afterwards: ids U1-U8 are disjoint. They are out of
numeric order (U6/U7 sit ahead of U4/U5) and are left that way rather
than moved, since the note at the U6 row explains the history and
relocating sixty lines inside a merge commit hides real changes.
Three leftover conflict markers were sitting in `docs/active-work.md`
on `main`, committed by an earlier lane's resolution. `git diff --check`
flags them — but only for a working-tree diff, which is why the gate's
`diff-check` step never saw them and they survived several merges.
Removed here.
The U4 row is corrected on evidence this lane produced:
- **Flavour was wrong as a matching key.** The row was filed from
#229's `lua54` red and put the flavour in the key; #231 reddened the
identical selector with the identical three fragments twice on
`luajit`. Matching as filed would have missed both.
- **A fourth sighting was a deliberate bite, not an occurrence** — the
defect reintroduced on purpose during the test's own development. It
is recorded for what it proves instead: the genuine defect and these
CI reds are signature-indistinguishable, same message class and same
full-timeout duration.
- **The control experiment is written down with its own bounds** — five
green base observations against 0/2, 4.8% under an equal-rate model,
and the two facts that bound it: attempt 5 reddened a different
selector, and the branch side was never resampled.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
`required_purpose`'s invalid-UTF-8 refusal told the caller that 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 `*workers*` — which lists async JOBS — and out of the
statusline activity indicator; a process's purpose is exposed through
`pmacs.process.list` and nowhere else, and joining the two planes is
Stage 2's work (framing §3, Q#W-4).
The refusal is correct and stays: a purpose that cannot be displayed
anywhere should still be refused, and nothing spawns either way. What
was wrong is the reason given to the user, which pointed them at two
places their process will never appear. A diagnostic that misdescribes
the system is worse than a terse one, because it sends the reader
looking in the wrong place.
The job-side twin diverges rather than converging. `_push_dispatch_name`
refuses a non-UTF-8 handler name for the same reason, and there
`*workers*` and the modeline are the RIGHT answer — the name is composed
into every job's purpose and a job renders in both. It said only "as
part of every job's purpose", which names no surface at all, so it now
names the two it reaches. The two messages must not collapse into one
sentence: whichever wording won would be wrong on the other side.
Verification. `the_two_utf8_refusals_each_name_the_surface_their_own_text_reaches`
asserts both directions, positive AND negative — the process message
contains `pmacs.process.list` and NOT `*workers*`/`modeline`, the job
message contains both of those and NOT `pmacs.process.list`. The
negative halves are the anti-collapse guard; without them a later
"unify the wording" edit reintroduces exactly one wrong sentence and
passes every other test in the file. 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, so the same edit breaks two tests.
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 could not see
it); restoring the job message's original vague wording fails it too.
The doc comments were fixed with the literals. `required_purpose`'s
rustdoc now states which surface its message names and why it names
neither of the other two, and the `_push_dispatch_name` comment states
the converse. A corrected string whose doc comment still argues the
other way is one refactor from reverting itself.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Review round 2, findings P2a and P2b, plus P3's stale recovery
summaries. Three defects, 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)` is a value a caller
can write, and `?` surfaced mlua's generic conversion error BEFORE this
lane's own diagnostic was ever constructed: the caller was told neither
the field nor the rule.
**This is the third time this project has hit the class** — an unowned
Lua string converted with `?` ahead of the owned message; the
destination-capture lane corrected the same shape two rounds ago. It
refused before spawning and nothing leaked, so the defect was the
message, not the outcome. The conversion failure is now mapped onto
this function's own message, and the new acceptance row asserts on
message CONTENT so retyping the read as a bare `?` breaks the test
rather than silently degrading the error.
Auditing the rest of the lane's diff for the same class turned up
exactly one more: `_push_dispatch_name` took `name: String`, so a
registered handler name that was not valid UTF-8 failed at first
dispatch with mlua's generic message. It now takes `mlua::String` and
maps that failure onto an owned diagnostic naming the argument and the
rule. Those are the only two Lua-string reads this lane added; every
other binding it adds takes `()`.
## P2b — no safe display-text boundary. Two halves, two different fixes
### Handler names are refused at the source
`pmacs.workers.register` type-checked its name and nothing more, which
was defensible while the name died inside `dispatch`. It no longer dies
there: the ambient carries it into every job the handler allocates and
composes it into `purpose`, which `*workers*` and the modeline both
render. So it now gets `purpose`'s meaningful-value standard —
non-empty, not whitespace-only — plus control characters, which have no
legitimate place in a registered identifier.
### Purposes are ESCAPED at presentation, not rejected at the registry
A purpose may legitimately contain a newline: a filesystem path can, and
`pmacs-magit`'s spawn purpose is a whole argv. **This is the shape of
the `#228` decision, and it is consistent with it** — the one-line
constraint belongs to the surface that has it, not to the registry that
does not. There, `Command.description` stays free-form and the two
single-row consumers clip with `description_first_line`. Here the
equivalent is escaping rather than clipping, because a purpose's later
words are load-bearing: an argv's second word says which file, and a
clip would drop it silently.
`purpose_for_one_row` states the property it exists for: **a row must
not be able to forge another row.** It escapes `\n`, `\r`, `\t` and the
rest of the Unicode `Cc` class (which covers ESC, so a purpose cannot
open a terminal escape sequence either), borrows unchanged when there is
nothing to escape — making byte-identity structural rather than
asserted — and deliberately does NOT escape backslashes: no number of
them produces a second row, and doubling them would cost byte-identity
for ordinary text.
Two surfaces call it: the `*workers*` rows, and `ActivitySummary`, which
exists for one consumer that has exactly one row.
`pmacs.workers.snapshot()` is this lane's `describe-command` and stays
raw, which is what makes this a rendering decision rather than data
loss — asserted, not assumed.
## P3 — two stale recovery summaries
`docs/worker-identity-framing.md` still said "Implementation may
proceed"; it is implemented. `docs/active-work.md` still said Stage 1
takes the "first two" of owner/purpose/parent — `owner` was REMOVED in
revision 2, so it takes one of the three, and the claim the whole
`owner` argument overturned was still standing in the volatile state of
record. Both fixed section-locally.
## Verification
`tests/worker_identity_acceptance.rs`, 18 -> 24 tests:
* invalid-UTF-8 purpose refused by THIS lane's message, asserted on
content, alongside the absent / empty / whitespace / wrong-type /
metatable rows;
* a whitespace-only handler name and a control-character one are each
refused AT `register`, asserted on the error and on the handler not
being installed (dispatch reports `unknown handler`);
* a non-UTF-8 handler name is refused before the handler runs, with the
dispatch-name stack left empty;
* a purpose containing a newline renders as ONE row in `*workers*` and
as one line in the modeline — through the real rendering path, the
latter through a painted frame as well as the evaluator;
* **a purpose crafted to look like a row boundary does not produce a
second row** — asserted by counting rows, with the escaped text
asserted present so a renderer that dropped the purpose entirely could
not pass;
* a purpose with no control characters is byte-identical on both
surfaces, fixtured with a literal backslash, a literal `\v`, quotes
and a non-ASCII character.
Mutation-checked, seven guards, each failing its own test and no other:
the purpose UTF-8 diagnostic; the `_push_dispatch_name` one; the
register whitespace guard; the register control-character guard; the
`*workers*` call site; the `ActivitySummary` call site; and
`purpose_for_one_row` itself neutered to the identity, which fails both
surfaces' tests and nothing else.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Review round 1 on worker identity Stage 1. The lane shipped `purpose` as
a required field on `ProcessSpec` but made it OPTIONAL at the
`pmacs.process.spawn` Lua surface, defaulting to `label`.
**That preserved compatibility and delivered nothing.** `COHERENCE.md`
§9's complaint about `ProcessSpec` is precisely that `label` is
"caller-supplied, unvalidated convention" — so a purpose defaulting to
the label hands every existing caller back the exact convention this lane
exists to replace. The approved framing said required; this makes it
required where callers actually are.
The two fields answer different questions and neither substitutes for the
other. `label` IDENTIFIES — `lsp:rust-analyzer`, a terminal's buffer
name — so two processes running the same binary can be told apart.
`purpose` DESCRIBES: it answers "what is happening", which is what §3's
promise of visible asynchronous work is about, and which a label chosen
for uniqueness routinely does not answer.
**The refusal covers five shapes, not one.** Absent; empty;
whitespace-only; wrong type; and metatable-provided. The middle two
matter 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, and a required
field that accepts `""` is not required in any sense a reader benefits
from. The read is RAW, matching the posture `stdin` and `group` already
document in the same function: a spec table is plain data, so `__index`
cannot smuggle a purpose in.
Every refusal also asserts **the process list is unchanged**. A
validation that rejects after spawning has already done the thing it was
rejecting.
**This is a BREAKING CHANGE to a public Lua API, taken deliberately and
now rather than later.** Weighed and reported rather than decided
silently: §10 grades extension trust "missing (one class)" and P7 package
lifecycle has not started, so the third-party population calling this
binding is ~zero and the cost of the change only rises from here. Checked
for a reason that would be wrong and found none — `pmacs.process.spawn`
has no API-reference documentation and no stability promise anywhere in
`docs/`; the guide's only mentions are an audit-rule classification and a
pointer to the bundled REPL, and its semver language governs *packages'*
own versioning, not pmacs's Lua surface. `lua_to_spec` has exactly one
caller, so the blast radius is this one binding.
Eleven executable call sites updated, each with a real description rather
than the label copied across — copying it would satisfy the type and
defeat the point as surely as the default did:
builtin/packages/repl/init.lua "interactive <interpreter> session"
builtin/runtime/compile.lua "compiling: <cmdline>"
builtin/runtime/lean.lua "checking the Lean toolchain version…"
tests/fixtures/pmacs-magit/status.lua the full argv, not just the
subcommand the label carries — "git
log" and "git log --oneline -20" are
one label and different work
tests/compile_mode_acceptance.rs (4), tests/m4_acceptance.rs (1),
tests/worker_identity_acceptance.rs (2)
`lean.lua`'s site is the clearest case for the field: its comment said
the label was where "a user wondering why their editor touched `lake`
finds an owner" — one string doing identity AND explanation, which is the
conflation being undone. The label stays a key; the purpose is now the
sentence.
Two references are deliberately NOT updated: `src/audit/mod.rs` and
`tests/m7_9_acceptance.rs` contain `pmacs.process.spawn("ls")` as **audit
fixture source text**. It is lexed by the audit engine, never executed,
and editing it would change what those rule tests scan.
`required_purpose` is extracted rather than inlined because inlining it
pushed `lua_to_spec` past the 100-line clippy bound — the validation has
its own rules and its own rationale, so it gets its own function instead
of an `#[allow]`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
`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. The mechanism half is solid —
cancellation, supersession, streaming, frame-aware draining, `*workers*`.
The identity half was absent: `PendingJob` carried no description of what
it was doing, `pmacs.workers.dispatch` discarded the registered handler
name three layers above anything that takes one, and §9's "no progress
indicator exists anywhere" was checkable and true.
Framing: `docs/worker-identity-framing.md` (revision 4, approved).
What lands:
**A required `purpose`, on the job and on the process.** Non-optional,
with no `Default`, so the compiler — not a test — is what proves every
dispatcher supplied one. `allocate` / `allocate_with_resource` collapse
into ONE private `JobSpec`-taking funnel (Q#W-1): the two-function split
existed only because one prior lane needed one extra parameter, and a
second lane doing the same produces `allocate_with_resource_and_identity`.
`register_external` gains a `purpose` parameter rather than deriving one,
because its `JobKind` is `McpRequest`/`LspRequest` for every method — a
category, not a description.
**A dispatch-name ambient (Q#W-2), read at that same single funnel.** The
capture point is Rust, not the Lua wrapper layer, because a handler
reaching straight for `pmacs._async._dispatch_*` bypasses the wrappers
entirely — and those are precisely the callers attribution exists for.
Seven rules; the ones that decide whether it is honest:
- **Rule 1 — the extent is NON-YIELDABLE, and that is ENFORCED.** Both
supported yield APIs refuse inside it, modelled on the `commit_to`
refusal already in `async.lua`. The guards reject BEFORE parking and
reject UNCONDITIONALLY: one placed after `_is_complete` would fire only
when a yield really occurred, passing under test and failing
intermittently in production.
- **A raw `coroutine.yield` is NOT covered, and nothing here claims it
is.** R46 is a convention, and the scheduler inspects the yielded value
only after `coroutine.resume` returns — by which point the coroutine has
already suspended — so no refusal sited in a yield helper is ever
consulted. The residual is recorded in the framing §2 and in the
suite's module docs rather than papered over with a test that would
imply coverage this design lacks.
- **Rule 5 — unwind-safe.** A raising handler still pops. A version that
did not would let one failure poison every later dispatch in the session
with a stale name: the feature would stop failing loudly and start lying
silently. The bracketing also has to preserve the tail call it replaced:
`dispatch` was `return handler(args, opts)` and propagated EVERY return
value, so the pop/rethrow runs behind a varargs boundary rather than a
`local ok, result = pcall(...)` that would silently truncate a
multi-value handler. Varargs rather than `table.pack`, because that is
Lua 5.2 surface and LuaJIT is this project's default backend.
- **Rule 6 — compose, do not replace.** `"<name>: <purpose>"`, because
letting the dispatcher's purpose win loses the third party again and
letting the name win discards the only description of the actual work.
**A statusline activity indicator** — the fourth `pmacs.statusline.register`
adopter, after `mode`, `terminal` and `lsp`. A count plus the OLDEST
in-flight job's purpose ("busiest" is not a defined quantity; jobs carry
no cost estimate), and **absent entirely** when idle rather than a
zero-width segment that costs modeline width forever to say nothing is
happening. Gated by one setting, `ui.activity-indicator` (boolean, default
true, Q#W-6) — a permanently-visible modeline element is a preference
someone genuinely holds on day one. No setting for purpose capture
itself: that is substrate.
**NO WIRE CHANGE.** The indicator rides the existing `StatuslineSegments`
vector, so a fourth provider adds an element, not a variant.
`PROTOCOL_VERSION` and `ADVERTISED_PROTOCOL_VERSION` are untouched — which
is the property that lets this run beside the two lanes holding the bump
slot.
**Q#W-7 — a pre-existing defect, repaired here, and NOT one anybody has
observed.** `Handle:await()` refuses inside `pmacs.window.commit_to`
precisely so a coroutine cannot park with the frontend scope pushed
(Journey Stage 1a, Q#JR14b). But `pmacs.async.yield_to_next_tick()` also
yields, is public, and carried no such refusal — so that invariant had a
second entrance, and a coroutine could produce exactly the misrouting the
`await` guard exists to prevent. It gains both refusals here: the same
supported yield helper, the same invariant, the same edit family, so
splitting it would have preserved a known hole without reducing
integration risk.
**Reachability by a real caller is UNPROVEN.** This was found by reading
the guard family while scouting rule 1, not by reproducing a fault. No
production caller is known to yield through that door inside a commit,
and the test pins the guard rather than reproducing a user-visible bug.
Nobody should later cite this commit as evidence the bug was observed in
the wild. Its witness is a PAIR, like rule 1's: the refusal fires **and**
the commit scope is restored afterwards — a guard that raises while
leaving the scope pushed converts a silent fault into a loud one and
fixes neither.
`journey_acceptance` carries the established `commit_to` pins —
forged-destination refusal, scope-and-restore on normal return and on
raise, the await refusal, delivery to the requesting frontend. It passes
**untouched**, which is what says this closed a gap in Journey Stage 1a's
semantics rather than altering them.
What is deliberately NOT here, and why it is worth saying:
- **No `owner`, in any spelling** — not `origin`, not `subsystem` (§3).
Populated from static per-subsystem constants it would be an origin,
not an owner, and 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. The slot stays empty until P3
can fill it with a real package signal.
- **No `parent`** (Q#W-5). An unpopulated field renders as `None`
everywhere and reads as "this job has no parent" rather than "this
system does not track parents". Stage 3 builds the lifetime model and
the field together.
Consequences worth recording:
- `ProcessSpec::new` takes a third argument. The 40-odd call sites are
almost all tests; the three production ones (LSP, MCP, terminal) supply
real descriptions. `pmacs.process.spawn`'s Lua surface keeps `purpose`
OPTIONAL, falling back to the label — requiring it there would break
every existing caller for no coverage the compiler is not already
providing, and a caller's own label is not a fabrication.
- `pmacs.process.list` gains a `purpose` KEY on each row and enumerates
exactly the same processes (Q#W-4). Terminal PTYs stay hidden: three
acceptance suites use `#pmacs.process.list()` as a leak baseline, and
widening the accessor would inflate all three. Stage 2's unified view
owns that decision.
- `statusline_segments_acceptance`'s builtin-provider inventory grows to
`["activity", "mode", "terminal", "lsp"]`. That assertion exists to
grow when a builtin provider is added.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
panel_commit_dedication_refusal matches on `fid` as well as on the
profile: a nested commit for a DIFFERENT frontend may dedicate that
frontend's own side slot, because resolve_placement consults only the
requesting frontend's panel_capable and its own one side window, so
nothing done to B can change where A's side request lands.
That promise was documented and unpinned. Both revision 9 nesting
tests drive a single frontend, so the comparison is trivially true
throughout them: deleting it, and making any outer "panel" contract
globally restrictive, passed the whole file.
a_nested_commit_for_another_frontend_may_dedicate_its_own_slot runs
two frontends. While an outer "panel" commit for A is in force, a
nested commit for B dedicates B's slot and is ALLOWED --- and B's
slot is asserted really dedicated afterwards, not merely unrefused.
The far side runs in the same test: A's slot stays undedicated and
A's result still lands in A's panel, so the row cannot pass by having
weakened the restriction generally.
This is the suite's only POSITIVE row; every other asserts a refusal,
which is the shape it was thinnest on. An exception only the doc
comment knows about is one review round from being simplified out.
Mutation-checked: deleting `&& contract.destination.frontend == fid`
fails ONLY this test. Both single-frontend nesting tests pass under
it, which is the evidence they are independent of the frontend match
rather than merely looking so. journey_acceptance (47),
dired_acceptance (31) and cargo test --lib (1920) stay green.
Two ledger corrections, both section-local:
* "Eight writes exist; five are reachable" then listed four. The
fifth is quit_window's QuitAction::Restore --- the site proved
unreachable and guarded anyway. It now appears in the list that
justifies it, and the bullet counts what actually matters: all five
are guarded.
* The revision 9 mutation paragraph had the preservation counts
REVERSED (journey 31 / dired 47). It is journey 47 / dired 31,
matching the bullet further up and measured per target. The same
reversal is in 394fa43's commit message; that is left as written
rather than rewriting a pushed commit, and the ledger now says so
where the numbers are, so a reader following the SHA takes the
corrected pair.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Revision 8 refuses, inside a "panel" commit_to, the mutations that
would make its relaxed preflight wrong. A nested commit_to REPLACED
the enclosing contract with its own and restored it afterwards, so
the outer restriction went out of force for the whole inner body:
commit_to(outer, function() -- "panel", relaxed preflight
commit_to(inner, function() -- "document", MASKS the outer
set_params(panel(), { dedicated = true }) -- ...and succeeds
end)
display(result, { side = "bottom" }) -- ...which now FALLS BACK
end, "panel")
Every step is legal on its own, and the outer commit then overwrote
a newer document buffer --- the P1a failure the lane exists to
remove, reached through one extra call.
What this invalidated, precisely: NOT the enumeration of dedication
write sites. Every site in it is real and still guarded. What was
wrong was the claim that the guard was in force for the whole outer
body. So the enumeration is inherited and qualified, not redone.
Contracts now COMPOSE rather than replace. The core holds a stack;
ScopedFrontendGuard pushes on entry and truncates back to its own
depth on every exit path; panel_commit_dedication_refusal consults
every contract in force rather than the innermost. The strictest
active restriction wins. Matching stays per frontend --- a nested
commit for a different frontend may dedicate its own side slot,
which cannot change where this frontend's side request lands.
Nesting itself is NOT forbidden, which was the other candidate fix.
It closes the hole by prohibiting a construction no rule objects to:
commit_to is public Lua API for saying where a continuation's result
belongs, and a body committing to a second destination (a diff
beside a status panel) is where #227's adoption is heading. Only the
restriction needed preserving. Detecting the dedication when the
outer commit resumed was not available either --- that is a late
refusal, which is what revision 7 was rejected for.
Two pins, and they are a pair rather than one test written twice:
* a_nested_commit_cannot_mask_an_outer_panel_restriction drives the
same four write-site rows through a nested, entirely valid
"document" commit, and asserts the attempt is refused, the slot is
still undedicated, and the outer commit's destination is intact.
* an_ordinary_nested_commit_still_runs_and_restores_the_outer_restriction
pins that nesting without dedication is accepted, that the
enclosing restriction is back in force once the nested commit
returns, and that outside every commit dedication is ordinary
again.
Mutation-checked: restoring the guard to the innermost contract
(.last(), exactly revision 8's swapped slot) fails only the first of
those. The other 13 pins, journey_acceptance (31), dired_acceptance
(47) and cargo test --lib (1920) all stay green. The ordinary-nesting
pin deliberately survives it --- it exists to fail the other
candidate fix.
Also sweeps the comments left by revision 7, which revision 8
superseded: no fallback_commit_refusal symbol remains, but six doc
sites still described placement-boundary enforcement as the
guarantee (ViewDestination, CommitProfile::Panel, CommitContract,
capture_view_destination, commit_destination_refusal,
panel_placement_can_fall_back), plus two comment blocks in the
commit_to binding and one stale mutation note in the acceptance
suite. Net rustdoc warnings down three.
Framing to revision 9; the active-work lane entry updated in place.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Revision 8 of `docs/destination-capture-framing.md`, replacing the
revision-7 design at `ca72461`, plus the invalid-UTF-8 profile hole.
The framing now carries §3's enumeration, performed.
THE BLOCKER, unchanged. The `"panel"` commit profile skips preflight
checks 2-4 on the claim that a panel result never touches a document
window. Panel placement FALLS BACK into an ordinary document window
when the frontend is not `panel_capable` or its one side slot is
dedicated elsewhere, and installs the result there --- so a `"panel"`
commit could replace a NEWER document with every stale-intent guard
skipped.
TWO REJECTED SHAPES, kept in the framing as the record of why not
those. Revision 6 predicted the fallback at preflight and argued the
body could not change it; false, because refusing `await` stops another
COROUTINE interleaving, not the body's own synchronous statements.
Revision 7 (`ca72461`) moved enforcement to the placement boundary;
that breaks the invariant `commit_to` exists for --- `docs/agent-handoff.md`
requires it to preflight BEFORE the callback, because a body creates
buffers, handles and paint long before it asks to display anything, so
"validating at display time is four mutations too late". A refusal
arriving after all of that is a partial commit with an error return.
REVISION 8 DOES NEITHER. The preflight stays exactly where it was, and
the mutations that would invalidate it are REFUSED AT THE ATTEMPT ---
the same shape as `Handle:await` being refused inside a commit scope,
for the identical reason: something that would invalidate the scope's
guarantee is rejected outright rather than predicted around or caught
late. With them refused, the fallback never comes into existence.
THE ENUMERATION, PERFORMED --- this is the load-bearing part, and it is
closed for a structural reason rather than because inspection ran out
of ideas. Full working in the framing §3.
`resolve_placement` reaches `Ordinary` from a side request through
exactly two branches, so only two pieces of state are levers at all:
`panel_capable`, and the one side window's `dedicated`.
`panel_capable` is UNREACHABLE from a body: written only where a
`FrontendView` is constructed, and nothing in `src/lua_bindings/`
constructs, registers or unregisters one --- `register_frontend_view`
has callers only in `daemon.rs` and core unit tests.
`dedicated` has eight writes. Five are reachable: `apply_placement`'s
`Side` created, replacing and non-replacing arms, and `set_params`. Two
`Ordinary` arms are harmless --- every `Ordinary` target is filtered
`!is_side`, and one only ever clears the flag. One is a unit test.
Closing the side window is NOT a route, checked rather than assumed:
with no side leaf `side_window_for` returns `None` and placement
CREATES a fresh panel instead of falling back. `panel_hidden` is not
consulted by placement, and `params.side` is unreachable.
`quit_window`'s `QuitAction::Restore { dedicated: true }` is
UNREACHABLE, and this was the surprise --- it looked like a route with
no `dedicated` argument at the call site at all. `Restore` is stored
only on a REPLACING side placement, and a dedicated slot can never be
the target of one: a side request with a different buffer falls through
to `Ordinary`, and an exact-target request is refused by
`window_accepts_buffer`. Guarded anyway, labelled defensive, because
its unreachability is emergent from two rules in another function.
GUARDS SITED WHERE THE PROPERTY CONVERGES. All three `Side` arms are
reached through `apply_placement`, which has EXACTLY ONE caller --- so
one guard in `display_buffer` covers every request-driven dedication,
including spellings that do not exist yet. `set_params` is a genuinely
separate write and is guarded separately; dedication does NOT converge
before the field itself, and that is stated rather than papered over.
`Window::params.dedicated` is a public field, so the compiler does not
enforce the funnel --- the acceptance rows are what would catch a new
direct writer.
WHAT IS DELIBERATELY NOT REFUSED. The document profile is untouched:
constraining its body would newly refuse dired's own documented panel
path, a preservation-suite stop signal. Dedicating a DOCUMENT window is
still allowed, since it cannot change which of panel-or-document a side
request resolves to. And falling back is still allowed --- a frontend
that cannot render a panel degrades gracefully exactly as today,
because this refuses the mutation that MANUFACTURES a fallback, never
the fallback itself.
THE SECOND HOLE. `commit_profile` did `name.to_str()?`, but Lua strings
are BYTE strings, so `string.char(255)` hit mlua's generic UTF-8 error
before `BAD_COMMIT_PROFILE` was constructed --- the same reachability
class as the `Option<String>` defect revision 5 fixed, one layer down.
Bytes now, with the row asserting on message content.
TESTS: 12 pins. The inside-the-body test is ONE ROW PER REACHABLE WRITE
SITE, not per call spelling, because one spelling reaches three
different writes: `set_params`, and `display{side, dedicated}` in each
of the created, replacing and non-replacing arms. Each asserts the
three things revision 8 requires --- the dedication call is refused, the
slot is still undedicated afterwards, and nothing partial was installed
(no `*result*` buffer, panel unchanged, document unchanged).
Mutation-checked per guard: deleting the `display_buffer` guard fails
all three display rows, verified INDIVIDUALLY by rotating each to the
front so the first failure cannot mask the rest; deleting the
`set_params` guard fails only that row.
THREE FRAMING CORRECTIONS ride along, all of them cases of the document
teaching something it later argues against. Section 3 stated the
disproved premise unconditionally --- "the panel case would inherit a
check about a window it never touches" --- a hundred lines before
correcting it, so a reader met the wrong claim first; it is now
qualified at the point of the claim, and section 2 carried the same
unconditional form one section earlier ("it lands in the bottom panel")
and now says it REQUESTS one. The handoff citation was written "section
748" twice when it is LINE 748, and this document's authority is that
its citations can be followed. And the "not asserted exhaustive" hedge
on the route list is retired: the enumeration is closed structurally,
because `resolve_placement` reaches `Ordinary` from a side request
through exactly two branches.
`journey_acceptance` (47) and `dired_acceptance` (31) pass UNCHANGED.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Revision 7 of `docs/destination-capture-framing.md`, closing the
correctness blocker review found in `0efc8c0` and the smaller
reachability hole beside it.
THE BLOCKER. The `"panel"` commit profile skipped preflight checks 2-4
on the claim that a panel result never touches a document window. That
claim is false: panel placement FALLS BACK to an ordinary document
window when the frontend is not `panel_capable` or its one side slot is
dedicated elsewhere -- `apply_placement` says so in its own comment --
and then installs the result there. So a `"panel"` commit could replace
a NEWER document with every stale-intent guard skipped: capture A, the
user opens B, the continuation lands, B is gone. That is the exact
failure `commit_to` exists to prevent, reached through the profile
meant to be the safe one.
WHY NOT A PREFLIGHT PREDICTION. Revision 6 proposed predicting the
fallback at preflight, arguing nothing could change in between because
the body cannot `await`. Refusing `await` prevents another COROUTINE
interleaving; it places no restriction on the body itself, which is
arbitrary Lua running synchronously and can invalidate the snapshot in
two statements -- take the panel, set it `dedicated`, then request a
side display. No preflight predicate closes that, however phrased: the
measurement is taken before the thing it measures is decided.
WHAT THIS DOES INSTEAD. `EditorCore::display_buffer` refuses between
`resolve_placement` and `apply_placement` when a side request resolved
to `PlacementKind::Ordinary` under an active `"panel"` contract whose
destination fails the document preconditions. That is the first moment
the fallback is a fact rather than a guess, and refusing before
`apply_placement` means a refused fallback mutates nothing. The
contract rides on the core, installed and restored by the same
`ScopedFrontendGuard` that scopes the frontend, so a profile can never
outlive the body that declared it; the field is crate-private, so Lua
cannot claim a profile for a placement it did not commit to.
The preflight predicate SURVIVES as an early refusal and not as the
guarantee. `panel_placement_can_fall_back` still gates the relaxation
in `commit_destination_refusal`, so the statically knowable case -- a
frontend that cannot render a panel at all, and will not acquire the
capability mid-body -- refuses before the body allocates a buffer,
registers a handle and paints. That is the same reason `commit_to`
preflights at all. Both layers are pinned, and neither pin subsumes the
other.
The four document checks now live once, in
`EditorCore::document_destination_refusal`: they are evaluated from two
sites, and two hand-written copies is how a backstop ends up weaker
than the thing it backs.
THREE DELIBERATE LIMITS, each a different decision rather than a
stricter version of this one. The document profile is untouched --
re-running its checks at placement would newly refuse dired's own
documented panel path, which is a preservation-suite stop signal. Only
a fallback is guarded, not every `Ordinary` placement -- a `"panel"`
body calling `display_file` is pinned as succeeding. And the refusal is
of the PLACEMENT, not of falling back: a `"panel"` commit with an
intact destination still degrades gracefully into the document window,
because turning graceful degradation into an error would regress every
consumer that works today on a frontend without panel capability.
THE SECOND HOLE. `commit_profile` did `name.to_str()?`, but Lua strings
are BYTE strings, so a `string.char(255)` profile hit mlua's generic
UTF-8 conversion error before `BAD_COMMIT_PROFILE` was ever
constructed -- the same reachability class as the `Option<String>`
defect revision 5 fixed, one layer down. The comparison is on bytes
now, and the invalid-UTF-8 row joins the number/table/boolean rows
asserting on message content.
FOUR DOC SITES repeated the false claim (`ViewDestination`'s own doc
twice, `capture_view_destination`, `ViewDestinationLua`) and are
corrected. Nothing else relied on it: dired, the only Lua `commit_to`
consumer, takes the two-argument document profile and already had all
four checks; `compile.lua`'s `already_in_panel` queries live state; and
the terminal adopter's rollback keys off `created_side`, already false
on a fallback.
Tests: 12 pins, up from 8. Three carry the enforcement split and none
subsumes another -- the pre-established fallback (both causes, the body
must not run), the inside-the-body transition (the body runs, the
result must not land), and the graceful fallback (a valid destination
still lands). Mutation-checked four ways; the pattern of which rows
survive each mutation is in `docs/active-work.md`.
`journey_acceptance` (47) and `dired_acceptance` (31) pass UNCHANGED.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
`docs/destination-capture-framing.md` §7, eight tests.
The one that decides the lane is `the_preflight_matrix_holds_in_both
_profiles`. Every cell Q#DC-2 marks "not applicable" for the panel
profile is asserted as NOT refusing, not merely left untested: a check
deliberately omitted and a check someone forgot look identical from the
outside, and the next reader restores the second one. The document
column re-asserts all four refusals in the same table, so a mutation
that collapses the two profiles fails one column or the other.
`a_bad_profile_is_refused_by_one_message_that_names_the_accepted_values`
is the guard on the argument's TYPE, not only on its behaviour. It
asserts the number, table and boolean cases produce the same message as
an unrecognized string --- which stops being true the moment the
argument is retyped to `Option<String>`, because mlua then rejects the
value during argument conversion and the pointed message is never
reached.
`a_captured_destination_survives_a_frontend_switch` runs under both
profiles. The panel profile drops three of the four preflight checks,
and a plausible way to implement that is to drop the frontend scope with
them --- which would leave a panel continuation resolving its target
from ambient state, the exact defect the lane removes.
`a_two_argument_commit_takes_the_document_profile` witnesses the default
through a check the panel profile omits (a stale buffer), because
asserting merely that a legacy call does not error would pass on one
silently downgraded to the panel profile.
ONE FINDING, RECORDED IN THE TEST RATHER THAN WORKED AROUND. Q#DC-4's
"a frontend with no document window" reads as a frontend showing only a
bottom panel, and that state is asserted impossible: Q#BP6 says a layout
always retains at least one non-side window, and `non_side_target`
carries a `debug_assert!` that fires under `cargo test` if one ever
does. So with Q#BP6 held, a registered frontend in a healthy editor
always has a live document window, and the absent document pair is a
DEFENSIVE branch rather than a routine one. The decision still stands
--- capture stays total, and an adopter with nowhere to land gets a
refusal naming that rather than permission to guess --- and the two
Q#DC-4 pins drive the reachable spelling of the same condition: a layout
whose document window has gone while the view remains. The helper says
so at its definition.
`tests/journey_acceptance.rs` (47) and `tests/dired_acceptance.rs` (31)
pass UNCHANGED, which is §7's stop signal and the reason the profile
default is the document one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
PR #228 review found a correctness gap this lane made reachable. The
GPU dropdown derives its height, its visible window and its
selection-highlight offset from `rows.len()` — ONE logical row per
candidate — while a detail carrying a line break shapes into more
physical lines than that. One such row misaligns every row below it
and the highlight with it. The grid TUI has the same exposure from the
other side: it writes the description into a single-row suffix on the
minibuffer band.
## Why not reject CR/LF at registration
That was the obvious fix. It was implemented, measured, and abandoned
on evidence.
MCP tool registration renders a whole schema block into
`Command.description` — tool text, blank line, `Arguments:`, then one
line per argument (`tests/fixtures/pmacs-mcp-tools/init.lua:272`, a
`table.concat(lines, "\n")`, used at `:496`). And
`tests/m9_6_acceptance.rs:583-598` ASSERTS four of those lines. A
one-line guard in `CommandRegistry::define` fails 36 tests across
`m9_6` (19/25), `m9_7` (16/19) and `m9_8` (1/17), in both feature
configurations, and could only be made green by deleting a shipped
acceptance criterion.
So the one-line constraint goes where the constraint actually is: the
surfaces that have one row. `Command.description` stays free-form,
which it legitimately is.
## The change
`Command::description_first_line` clips to the first CR **or** LF — a
lone CR ends a line too, and an LF-only clip would pass a bare `\r`
straight through to the same surface. Both single-row consumers call
it: the semantic producer filling `MinibufferRow.detail`
(`src/semantic_render.rs`) and the TUI suffix (`src/editor.rs`). A
first line that is empty ships as `None` rather than `Some("")`, which
would draw trailing padding.
No ellipsis or truncation marker, matching the in-tree precedent and
the minibuffer's own width rule.
`describe-command` and `help.list-commands` are untouched and still
report every line. That is what makes this a rendering decision rather
than data loss, and it is asserted, not assumed.
## Precedent, already in this tree
The same MCP fixture clips a tool RESULT to its first line because
"a multi-line set_status would corrupt the row layout"
(`init.lua:277-285`), leaving width clipping to the frontend. Same
hazard class, same resolution.
## Verification
`src/command.rs`: a schema block registers AND clips, in all three
break forms; a single-line description is byte-identical after the
clip; an empty first line clips to empty.
`tests/discovery_stage2_acceptance.rs`: an MCP-shaped description
reaches the TUI band and the GPU row as one line, through the real
prompt path — with the full text still reachable via
`describe-command` asserted alongside, so a clip that deleted the
schema block everywhere would fail rather than pass.
`pmacs-gpu`: one physical shaped line per logical candidate row — the
geometry invariant the dropdown depends on.
Mutation-checked: neutering `first_line` to the identity fails all
four new break-handling tests
(`a_multi_line_description_registers_and_clips_to_its_first_line`,
`a_description_whose_first_line_is_empty_clips_to_empty`,
`a_multi_line_description_reaches_the_tui_band_as_one_line`,
`a_multi_line_description_reaches_the_gpu_row_as_one_physical_line`)
and leaves the two "did not tighten past purpose" tests green.
`Command.description`'s doc comment claimed "one-line", which the MCP
path openly violates. It now states the real contract and records why
a registration guard must not be re-proposed.
`m9_6`/`m9_7`/`m9_8` pass COMPLETELY UNTOUCHED, and are now named
gate suites so that stays on the record.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
`Command.description` has always been required and has always been
rendered by `help.list-commands`. It was missing at the one moment it
would change a decision: the M-x row. This carries it there.
COHERENCE.md §5's clause "M-x rows are still bare names", per
docs/discovery-stage2-framing.md revision 3.
## The wire half is additive, and the old variant is FROZEN
postcard is not self-describing: enum variants encode by index and
fields by position. Widening `MinibufferPrompt.candidates` in place
would make every v12–v22 peer MIS-DECODE the bytes rather than ignore
them — and gating the widened form at `>= 23` would not rescue them
either, because with only one variant to gate they would receive no
minibuffer message at all. Compatibility requires the old shape to
still exist AND still be sent.
So `MinibufferPrompt` is retained unchanged for `12..=22`, and
`MinibufferPromptRows { prompt, input, cursor, rows, selected, total }`
is APPENDED as the final variant, carrying `MinibufferRow { label,
detail: Option<String> }`. A new row type, not `CompletionPopupRow`,
whose `kind` is an LSP `CompletionItemKind` code with no honest value
for a command (Q#D2-1).
Exactly one of the two reaches any peer, ever. The producer selects on
the session's negotiated version, so the CLOSE necessarily uses the
same family as the OPEN — a rows session closed by a legacy clear
leaves the dropdown on screen forever. The daemon's write loop gates
both directions again, with the legacy gate written as a RANGE
(`12..MINIBUFFER_ROWS_MIN_VERSION`) rather than a floor, so a v23 peer
cannot receive both and double-render.
`ADVERTISED_PROTOCOL_VERSION` stays 20, untouched.
## The TUI half involves no wire at all
`src/editor.rs` contains zero references to `MinibufferPrompt`:
`paint_minibuffer` reads `core.minibuffer` directly. So it reads
`Command.description` from the registry in-process, which is why this
half is independent of the bump.
Clipping is three ORDERED steps (§3.4), and the guarantee is "never a
PARTIAL name", not "the name always survives" — the prompt and typed
input consume the budget first, so the remainder can be too small even
for the bare name. If the whole name does not fit, the suffix is
omitted entirely; only once it fits is a description attempted; a
description that does not fit whole is dropped, leaving today's
`[name]`. No ellipsis stub, and no prefix of a name is ever emitted.
## Verification
`src/protocol.rs` gains this repo's FIRST literal postcard byte
fixtures: `minibuffer_prompt_v12_wire_bytes_are_frozen`, open and
cleared. A round-trip freezes nothing — it encodes and decodes with
the same types, so a field addition leaves it passing while every
shipped peer breaks. Bite-verified: reordering two fields of
`MinibufferPrompt` leaves `minibuffer_prompt_round_trips_through_postcard`
green and fails the fixture.
`line_wrap_facts_encoding_is_unchanged_by_the_v23_build` pins the
PREVIOUS final variant, per the handoff §4 rule that an appended
variant's own round-trip cannot detect a discriminant shift.
`tests/discovery_stage2_acceptance.rs` runs ONE daemon serving a v22
and a v23 session simultaneously, through the real M-x key path, and
asserts each receives its own variant AND ONLY its own — open and
close alike — by collecting every minibuffer message rather than
filtering for the expected one.
No cross-version cache test, deliberately (§3.2):
`SemanticRenderState::for_peer` bakes the negotiated version in at
attach and is dropped at detach, so a cache cannot span two versions.
A test for an impossible condition passes forever while teaching the
next reader that the hazard is real.
Five version assertions updated, each read before editing:
`src/protocol.rs` (the `PROTOCOL_VERSION` tripwire, renamed; and the
v6-floor ladder's accepted/rejected ranges),
`tests/statusline_segments_acceptance.rs`,
`tests/bottom_panel_stage2b_gpu_acceptance.rs`,
`tests/vterm_stage3_acceptance.rs`. No `ADVERTISED_PROTOCOL_VERSION`
assertion fired.
Gates: `scripts/gate --protocol --acceptance discovery_stage2_acceptance`
— all ten green, including the strengthened two-configuration sweep.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
`latex_within_boundary` answered a question about path COMPONENTS with
string arithmetic:
dir:sub(1, #boundary + 1) == boundary .. "/"
With a `/` boundary the needle is `"//"`, which no canonical path
begins with. Every ancestor was therefore judged out of bounds, the
marker walk never examined a single directory, and each chapter of a
thesis got its own root — two texlab processes for one document tree.
The lane's headline behaviour, silently off, with all fourteen shipped
tests still green because every one of them clamps the boundary to its
own tempdir.
Fixed by comparing segments rather than characters, so the root is a
boundary with zero segments — containing everything by construction
instead of by a special case, and tolerating a trailing separator for
free. The same root-is-special trap sat at two other points on the same
path and is closed with it:
* `latex_parent_of` returned nil for a top-level directory, making `/`
the one directory the walk could never examine — the identical bug
from the far end. It now yields `/`, matching `walk_for_marker`'s
`Path::ancestors` on the Rust side, and still terminates because `/`
has no component to strip.
* `latex_root_for` sliced `/paper.tex` to an EMPTY directory, which
canonicalizes to nothing and made the resolver DECLINE — and a
decline is the one path that falls through to `pmacs.project.detect`,
whose walk includes `.git`. A document at the filesystem root now
roots at `/`.
* `latex_marker_in`'s join is guarded for `dir == "/"`, which this
change makes reachable for the first time; the naive form produces
`//name`, the one spelling POSIX leaves implementation-defined.
Two new pins plus a strengthened one, 16 tests:
* `two_chapters_share_one_server_under_a_root_search_boundary` — the
defect end to end through ATTACH, not on the predicate, because the
symptom is two servers rather than a wrong string. Restoring the old
comparison fails exactly this test, with the two-server output.
* `latex_root_walk_stops_at_the_search_boundary` now asserts BOTH
directions. "Stops at the boundary" is also satisfied by a walk that
never runs — which is precisely what a `/` boundary produced — so the
hermeticity property (R8's shape: a stray `latexmkrc` above the
tempdir must stay invisible) is now paired with the walk still
climbing to, and examining, the boundary directory itself.
* `latex_root_for_a_document_at_the_filesystem_root_is_the_root`.
Also corrects `docs/active-work.md`: §3 no longer awaits a revision 3 —
`b5eaf27` IS revision 3 — and the lane entry now records that boundary
handling has been this resolver's interesting part twice, so a reader
weighing whether to trust it knows where to look first.
Gates: ALL GREEN via `./scripts/gate --acceptance lsp_latex_acceptance`.
`/tmp` on this machine is a near-full tmpfs and three grep tests fail
there with `QuotaExceeded`; the green run used
`TMPDIR=/home/jeans/build/pmacs-gate-tmp/lsp-latex`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Framing: `docs/lsp-language-coverage-framing.md` (revision 2, §3 and
§6). One `pmacs.lsp.config.latex` entry — command `texlab`, args none,
a function-valued `root`, and deliberately nothing else.
STEP ZERO: the §3 caveat, discharged by observation.
Revision 2 marked marker 1 (`.texlabroot`) UNVERIFIED and blocking:
only texlab's version and CLI had been checked, and the CLI exposes
just `run` / `inverse-search`. Driving a minimal LSP client against
`texlab run` by hand, plus reading texlab's own source at the exact
installed tag, settles it — and corrects the premise underneath it.
`.texlabroot` IS a real marker. `crates/distro/src/language.rs` at
v5.25.1 maps `.texlabroot`/`texlabroot` -> Root, `Tectonic.toml` ->
Tectonic, `.latexmkrc`/`latexmkrc` -> Latexmkrc, and
`ProjectRoot::walk_and_find` (`crates/base-db/src/deps/root.rs`) walks
ancestors testing all three, innermost wins. So the shipped marker set
is texlab's own rather than a plausible-looking guess, and marker 1
stays.
But texlab CANNOT apply that walk to rescue a root pmacs gets wrong.
Each arm of `walk_and_find` searches `workspace.iter()` — documents
already loaded — and the workspace is built from the folders the CLIENT
supplies. Live sessions confirm it: with `rootUri` at a `chapters/`
subdirectory, no marker above it (`.texlabroot` included) widened
texlab's view, and its dependency graph never reached the parent
document; with `rootUri` at the marker directory the parent resolved,
marker present or not. texlab honours the root it is handed and never
corrects a too-narrow one.
That inverts the significance of the resolver rather than weakening it:
whatever `config.latex.root` returns *is* the project scope. It is the
whole value of the lane, not a nicety.
Also observed, because the entry depends on it: bare `texlab` serves
LSP over stdio — `initialize` returns `TexLab 5.25.1` with no
subcommand — so `args = {}` is right and `run` is not needed.
WHY `.git` IS EXCLUDED, AND WHY THAT IS NOT AN OMISSION.
texlab wants the document root; a thesis inside a monorepo must not get
the monorepo. The subtlety is that leaving `.git` out of the marker
list does not achieve this on its own. `project_root_for` falls through
to `pmacs.project.detect` when a resolver returns nil, and that walk
does include `.git` — so a resolver that declined on a markerless file
would hand texlab the repository root by the back door. The resolver
therefore never declines for a file that has a directory: no marker
means the file's own directory, which is also framing marker 4. The
acceptance pins this end to end through attach, not just on the
resolver's return, and asserts in the same fixture that the shared
detector really would have answered the repository root.
NO FILETYPE MAPPINGS, per revision 2 §2 — verified, not inherited.
`src/syntax.rs` already declares `name: "latex"` with `extensions:
["tex", "latex", "sty", "cls"]`, and grammar-extension detection sits
ahead of the LSP filetype map in `detect_buffer_language`
(`syntax.lua`). A `.tex` buffer already resolves to `latex`. The suite
asserts both halves — the extensions resolve, and `pmacs.lsp.filetypes`
is empty for them — so a later "helpful" addition cannot be mistaken
for the thing that made attach work.
Q#LX1: no `settings`, no `init_options`. Build-on-save and
forward-search are both opinionated and forward-search needs a
configured viewer.
Fixtures bound detection with `pmacs.project.set_search_boundary` and
assert the boundary took — R8's hazard is exactly this fixture's shape,
and one test pins the walk stopping at the boundary directly. Attach
fixtures use `pmacs_fake_lsp`, and the missing-server fixture an
asserted-absent path: texlab is installed on this machine, so relying
on either its presence or its absence would behave differently here and
in CI.
Verification: fourteen tests, one per §6 bullet plus the boundary and
decline cases. Seven mutations each fail the suite — resolver declining
on no marker (6 tests), no marker walk (4), a redundant `filetypes.tex`
(1), boundary ignored (1), `io.open` truthiness so a directory counts
as a marker (1), marker set narrowed (4), command renamed with
opinionated settings added (1).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
The lane asserted that real git emits no 2 C record even under
status.renames=copies, and a test MEASURED it. The measurement was
real; the proposition drawn from it was too broad.
git-status(1) documents C as "copied (if config option status.renames
is set to copies)", so git does emit it. What the fixture establishes
is narrower: with ITS copy source left unchanged, git reports 1 A. and
emits no 2 C record. That is a fact about the fixture.
It is still sufficient reason to craft the row through _deliver_status
--- a weaker and true justification replacing a stronger false one ---
so no behaviour changes and no test changes. The score-based runtime
fix was never in question.
Six sites narrowed, not the two that review cited: the framing bullet,
the ledger bullet, the g6_4b doc comment, the premise comment, the
assertion message, and the rename_and_copy_repo fixture doc. Each also
records the retraction, so the old claim cannot be reconstructed from a
stale copy that outlived the correction.
What is deliberately NOT claimed anywhere: WHY an unchanged source is
not offered as a copy candidate. There is a plausible mechanism and it
was never established, and replacing one overreach with a smaller one
is how this class of error survives.
The root cause is worth recording: this claim entered the lane as a
dispatch instruction stated as settled fact, and the implementing agent
did exactly what it was asked --- measured one fixture. A measurement
cannot be broader than its fixture, however carefully it is run.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Porcelain v2's `2` record covers renames AND copies --- the `<Xscore>`
field leads with `R` or `C` --- and the diff header said "renamed from"
for either. A copied file was therefore reported to the user as a
rename, which is a different fact about their tree.
The information was already retained: `parse_status` captures `score`.
Nothing new is parsed; the header reads the byte it already has.
`kind` stays `"rename"` for both, deliberately. Every BEHAVIOUR keyed on
it is identical --- including the two-path `git diff HEAD -- <orig>
<current>`, which is right for a copy as much as for a rename. Splitting
the kind would oblige every consumer present and future to spell
`kind == "rename" or kind == "copy"`, and an arm forgotten anywhere
silently drops copies back to the one-path diff: the exact regression
this fix exists to avoid. The consumers are few and all were checked ---
`diff_plan` is the tree's only `kind == "rename"` branch,
`status_line_text` keys off `row.orig`, and the two tests that name the
kind are `g6_1`'s corpus and `g6_8`'s unborn-unreachability assertion.
`score` has no other reader anywhere.
Read from `score` rather than from `row.x`: the score names
rename-vs-copy whichever side detected the change, while `X` carries the
letter only for an index-side one.
The status ROW is UNCHANGED, and that is a decision rather than an
omission. Its `XY` prefix already reads `R.` against `C.`, out of the
same byte, in the porcelain vocabulary every other row in the panel is
read in --- so the distinction is already on screen, and a second
vocabulary beside it would be a wider surface for no new fact. `g6_4b`
asserts both prefixes so the claim is checked.
Unborn `HEAD` needs nothing, confirmed rather than assumed: `diff_plan`'s
rename branch sits inside `if not unborn`, and `g6_8` already pins that
no `2` record can occur there.
`g6_4b` is a parser/presentation test and says so. Real `git` emits no
`2 C` record --- the test MEASURES that under `-c status.renames=copies`
rather than recalling it --- so the copy row is supplied through
`_deliver_status`, the seam `g6_2b`/`g6_17`/`g6_21` already use.
Everything downstream is real: repository, panel, `d` dispatch, spawned
`git diff`, rendered buffer. Both crafted rows name paths that exist in
the fixture, so each drives a real two-path diff. Both classes are
asserted, and so is the argv --- a fix to what the user is TOLD must not
reach what the module DOES.
Mutations, each caught: header always "renamed" fails only the copy
half; header always "copied" fails only the rename half; dropping
`row.orig` from the steps fails the argv equality.
Gate: `scripts/gate --acceptance git_status_stage1_acceptance
--acceptance listview_acceptance --acceptance config_registry_acceptance`
--- all eleven steps green, acceptance 34/34.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Both macOS legs of PR #227's matrix failed `g6_2` deterministically
while Linux stayed green. The behaviour under test was correct; the
FIXTURE was unportable. `g6_2` built `bad\xFF.txt` on disk with
`std::fs::write`, and APFS/HFS+ validate pathnames as UTF-8 and reject
an invalid one at the syscall with errno 92, EILSEQ, "Illegal byte
sequence". Linux's VFS treats a filename as opaque bytes.
The claim from Q#G-8 is unchanged and had to survive whole: a non-UTF-8
path is PARSED and DISPLAYED, so the user is not lied to about what is
modified, and RET and `d` REFUSE it with a message, because
`pmacs.process.spawn` takes `args: Vec<String>` and
`pmacs.buffer.find_or_open` takes `path: String`.
So the coverage is split along the line the PLATFORM draws, rather than
`#[cfg]`-skipped — a behaviour that vanishes on one platform is exactly
how a boundary stops being tested at all:
g6_2 parse + display, driven from the PAYLOAD BYTES directly. No
repository, no filesystem, runs everywhere. This half never
needed a file: git hands the module bytes and `parse_status`
takes a string.
g6_2b the gestures refusing, over a REAL repository the platform can
create, with the unrepresentable row delivered through
`_deliver_status` — the seam `g6_17` and `g6_21` already use,
for the same reason: a chosen completion is not otherwise
expressible. The repository, panel, keymap and dispatch are all
real; only the row bytes are supplied. Runs everywhere.
g6_2c the one thing a payload cannot witness — that real `git` emits
these bytes at all. LINUX-ONLY, named for the limitation, with
a comment stating exactly what is uncovered on macOS and why
nothing there could cover it.
`g6_2b` also covers the rename ORIGIN, which the old single test never
did: `d` on a rename passes the origin to `git diff` as an argument too,
so a check written on `row.path` alone lets it through.
WHY g6_2c CANNOT BE MADE PORTABLE. The name cannot be reached around
the filesystem either. Putting it only in the index (`update-index
--index-info` plus `write-tree`, never touching the worktree) does not
help: `git status` lstats every index entry, and on macOS that lstat
fails with EILSEQ rather than ENOENT, which git reports on stderr and
SKIPS — so the row would be absent rather than unrepresentable, and the
test would assert a different thing while looking the same. There is no
macOS arrangement in which real `git status` names a non-UTF-8 path.
What that gate leaves uncovered on macOS is the PROVENANCE of the bytes
and nothing else. The remaining link — that the spawn pipe carries bytes
rather than text — is structural: `event_to_lua` in
src/lua_bindings/mod.rs builds the stdout chunk with
`lua.create_string(bytes)`, and git.lua only concatenates chunks.
New fixture mechanism, `lua_bytes` / `z_payload_bytes`: a `-z` payload
whose paths are not UTF-8 cannot be spelled as a Rust `&str` at all, so
it is assembled as raw bytes and handed to Lua as one literal, with
every non-printable byte spelled as a THREE-DIGIT decimal escape. Three
digits always — Lua's decimal escape consumes up to three, so a shorter
one swallows the digit that follows it, which is the same hazard
`z_payload`'s comment records for `{:?}`-rendered NULs.
Verified here: the full gate suite green; 33/33 under both LuaJIT and
Lua 5.4; the two portable tests still green with `g6_2c` compiled out,
leaving no dead-code warning behind; and three mutations each caught by
`g6_2b` — removing the RET check, removing the `d` check, and removing
only the origin clause. Reasoned about rather than executed: the macOS
EILSEQ behaviour itself. After this change nothing macOS runs depends on
a filesystem accepting such a name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
`strip_output_terminator` stripped `\r?\n$`, and a carriage return is as
legal a byte in a POSIX directory name as a newline is. For a repository
rooted at a directory named `trailing\r`, `rev-parse --show-toplevel`
prints the path's own `0d` and then its own `0a` terminator, and a strip
tolerant of an optional preceding carriage return cannot tell those two
bytes apart --- so it took both. The root resolved as `trailing`, and
every command afterwards ran with a `-C` and a cwd naming a directory
that does not exist: the same defect the previous commit at this call
site fixed, one byte over.
Exactly one trailing `\n` is now removed, by an explicit last-byte test
rather than an anchored pattern. Both of this function's bugs lived in a
pattern, and the third answer to the same question should not be a
cleverer one.
`-z` was CHECKED against the installed git rather than assumed, and must
NOT be used. `git rev-parse` has no `-z` option at all on git 2.55: it
is absent from the manual, `--parseopt -z` errors with "unknown switch",
and in ordinary mode `rev-parse` treats `-z` as an unrecognized flag
argument and echoes a literal `-z\n` onto stdout AHEAD of the toplevel
--- exit code 0, corrupted output, silent. `--show-toplevel` applies no
C quoting either, not even under `core.quotePath=true`. There is
therefore no unambiguous output representation to prefer over removing
the one byte git appended.
`first_line` is untouched, for the reason the previous commit recorded:
its three callers all feed the single-line status band, where taking the
first line is right.
Witnessed by `g6_14d` end to end, not at the parser: the fixture really
creates directories named `trailing\r` and `nl\nand-trailing\r`, the
real `git` resolves them, and the assertion is on the cwd of the status
spawn the module actually made, plus real rows in the panel and a RET
that opens the file the row names. The second case sends both hazards in
together because a root may hold both and neither fix may mask the
other. `g6_14c` now reaches that chain through the shared
`assert_root_resolves_whole` rather than keeping a second copy of it,
and the helper binds `pmacs.project.set_search_boundary` to the fixture
through `open_panel` --- R8's lesson, and a root fixture is exactly that
hazard's shape.
Mutation-verified: restoring `\r?\n$` fails `g6_14d` at `<tmp>/trailing`
against `<tmp>/trailing\r` while `g6_14c` still passes, which is exactly
the byte that separates the two fixes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
The lane exists to guarantee two things --- that the crdt sweep is
preceded by the build producing its binary, and that a build failure is
attributed to `build-crdt` rather than to `sweep-crdt`. It shipped with
neither guaranteed, because NEITHER WITNESS COULD SEE A NAME.
--print-plan renders `emit_plan | cut -f2-`, so the ordering test
compared commands and never saw the names beside them.
--self-test hardcodes the string `build-crdt` inside its OWN synthetic
plan, so it proves things about the runner and nothing about the real
emitter.
Review demonstrated the consequence: renaming the real build step to
`sweep-crdt` left both tests passing --- a plan that would report a
build failure under the sweep's name, sitting green, which is exactly
the misattribution the separate step exists to prevent.
--print-plan-named prints emit_plan VERBATIM: the same `name<TAB>command`
text the runner reads back from PLAN_FILE. The new assertion compares
WHOLE LINES against it, so name and command are pinned together and a
rename of either step fails. The sweep's own pair is asserted too ---
asserting only the build's name leaves the identical hole open in the
other direction.
WHY A RENDERING AND NOT A SEAM. PLAN_FILE stays uninjectable: a test
that supplied the runner's plan would turn its `eval` into a general
command executor, the same class of defect this script's own review
caught in --acceptance and fixed with a parse-time refusal. Re-deriving
the plan test-side would be a second implementation of the thing under
test, which is the failure being repaired one level up. A distinct mode
rather than a --with-names modifier leaves --print-plan's contract ---
runnable lines --- exactly as it was, and defines no flag combination
that has no meaning.
--self-test STAYS. It witnesses the runner: failure naming, the FAILED:
list, log paths, non-zero exit, and continuation past a failure via the
sentinel. That is a different thing from attributing the real step, and
what it may no longer do is stand in for it.
A second test pins that the two renderings are one plan --- the stripped
one is the named one minus its names --- so a later edit giving either
mode its own text is caught rather than leaving an assertion on a name
the runner never uses. It also pins the `name<TAB>command` shape the
runner's `IFS=<tab> read` depends on.
Both new tests stay on the no-gates paths, so the suite stays cheap.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
`git.diff-file` started a plan with no request generation while every
plan writes the SINGLETON `*git-diff*` buffer through
`show_diff_buffer`. Press `d` on A, then `d` on B before A finishes: if
A completes last, A's diff replaces B's. The newest invocation loses to
the slowest subprocess --- the same defect the status channel had before
`ffe5ae2`, on the one surface that had never been given the fix.
Every finding on this lane has now been one shape: module-level mutable
state read or written at CONTINUATION time without an invocation-time
ticket. So the rule gets ONE implementation rather than a fourth
hand-rolled counter. `new_channel()` hands out a ticket at the command
and answers "is this still the request in force?" at the completion;
`state.generation` and `reserve_generation` are gone.
TWO channels, and that is a design decision rather than an oversight: a
single module-wide counter would make `d` cancel an in-flight `g` and
vice versa. The status panel and the diff view are independent things a
user asks for, so each gets its own "newest wins" ordering. What is
shared is the MECHANISM, not the counter. A channel spans a whole
request rather than one process --- a status open is `rev-parse` then
`status`, and a diff is one or two `git diff` runs --- so `_deliver_root`
and `_deliver_status` correctly share one ticket while the diff plan
gets its own. `g6_23` asserts the separation directly: two `d` presses
leave `_generation()` untouched.
The plan is restructured into the request shape the other two
continuations already use. `step_done`'s closure becomes
`pmacs.git._deliver_diff(request, step, res)`, exposed for exactly the
reason `_deliver_status` and `_deliver_root` are: no arrangement of real
subprocess timing can make two `git diff` runs finish in a chosen order,
and the contract is about the order the caller did NOT choose. The
ticket check sits at the single point a plan re-enters from a
continuation, so one check covers everything downstream --- no further
spawn, no buffer write, and no status message, since a status line from
a replaced invocation is as wrong as a buffer from one.
Witnessed by `g6_23` in two halves. The real half presses `d` twice with
nothing pumped between, so two plans are genuinely in flight and each
really reserved its own ticket. The driven half then completes the OLDER
request after the newer one has rendered --- the ordering that is the
whole contract, and the one real timing will not produce on demand,
since the first plan spawned normally finishes first and that order
passes on the broken code. A superseded FAILURE is asserted too, since a
buffer-only check would miss the status-message half. The positive
control at the current ticket makes the discards attributable to the
ticket rather than the payload.
Mutation-verified: removing the ticket check fails `g6_23` --- and in
that run it failed at the REAL half, the older plan having overwritten
the newer one's patch before the fabricated delivery was ever reached.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Found by mutation-testing the assertion rather than by reading it.
Emitting `build-crdt` AFTER `sweep-crdt` does fail the test --- so the
position criterion was never vacuous --- but it failed by panicking
inside the slice with
begin > end (427 > 282) when slicing `cargo fmt --check ...`
which names neither step and reads as a bug in the test. A gate test
whose failure has to be decoded is a gate test nobody trusts, and this
suite exists precisely to be trustworthy about the gate.
An explicit ordering assertion ahead of the slice says what is wrong:
the build must run before the sweep, because a sweep that builds its
own precondition afterwards has already failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
`scripts/gate --protocol` emitted `sweep-crdt` with no build step. The
crdt sweep spawns `pmacs-gpu` as a process, and nothing in a
`cargo test` run produces that binary --- `pmacs-gpu` has no `tests/`
directory, so cargo never uplifts its bin to `debug/pmacs-gpu`. On a
cold target directory the sweep therefore fails twelve
`gpu_invocation_acceptance::crdt::*` tests on "build pmacs-gpu before
this acceptance suite".
The hazard was never the red gate. Before per-worktree target
directories (#225) every worktree shared one, which nearly always
already held the binary, so the precondition was satisfied BY ACCIDENT
for the whole life of that arrangement --- a GREEN `--protocol` run
whose crdt sweep was decided by the state of the build directory rather
than by the diff.
Q#GR-1 SETTLED BY OBSERVATION, not by reading. On a disposable target
directory with `debug/pmacs-gpu` asserted ABSENT before each run
(recorded, not assumed), each sweep run alone from the same cold state:
default cargo test --workspace --no-fail-fast -- --skip basedpyright
exit 0, 114 test targets green, and `debug/pmacs-gpu` was
STILL ABSENT afterwards --- the default sweep never builds it
and never needs it.
crdt cargo test --workspace --features crdt --no-fail-fast
-- --skip basedpyright
exit 101, exactly twelve failures, all
`gpu_invocation_acceptance::crdt::*`, matching the signature
handoff section 5 recorded.
So the step is conditional on `--protocol`, which the framing voted for
on an inference this run confirms rather than assumes.
Also observed, and worse than the twelve: `a54_real_daemon_real_pty_and_
headless_gpu_render_one_panel_hosted_terminal` reported `ok` in that
same cold crdt sweep. Its only path that does not spawn `pmacs-gpu` is
its skip branch, so a test whose whole purpose is real wgpu rendering
passed having rendered nothing. The missing build does not only fail
twelve tests --- it silently voids coverage in tests that report green.
A NAMED STEP, NOT A FOLDED COMMAND. `cargo build ... && cargo test ...`
would report a BUILD failure under the name `sweep-crdt`, a wrong
attribution in the one place this script exists to be trustworthy
about.
`--self-test` is how that attribution is witnessed at all. The existing
suite drives only no-gates paths, so plan assertions can prove a step's
name and order and NOTHING about what the runner does when a step
fails. The mode runs a HARDCODED three-line synthetic plan through the
real runner: a passing step, a failing one named `build-crdt`, and a
passing SENTINEL after it. The sentinel is load-bearing --- with the
failure last, an aborting runner and a continuing one produce identical
output, so the witness would pass on a runner doing the opposite of the
stated policy.
The plan is a literal inside the script. Making `PLAN_FILE` injectable
would work and would turn the runner's `eval` into a general command
executor --- the same defect this script's own review caught in
`--acceptance` and fixed with a refusal at parse time.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
`rev-parse --show-toplevel`'s output was parsed with `first_line`, which
takes `^[^\r\n]*`. A newline is a legal byte in a POSIX path, so a
repository rooted at `/tmp/a\nb` resolved to `/tmp/a` --- and every
command afterwards ran with a `-C` and a cwd naming a directory that
does not exist, turning a working repository into a wall of exit-128
failure rows.
Fixed with a SEPARATE helper, `strip_output_terminator`, used at that
one call site. `first_line` is deliberately left alone: its other three
callers --- the spawn-error text, the stderr detail, and the
`display_file` error string --- all feed the single-line status band,
where a multi-line message corrupts the row layout, so taking the first
line is exactly right for them. Folding the two together would fix one
caller and break three. Both functions now say at their definition which
kind of text they are for and why the other exists.
Exactly one trailing newline is stripped, with an optional preceding
carriage return, because that is what git emits as a terminator; a
second newline would be output rather than a terminator. The trailing
whitespace trim `first_line` also did is NOT carried over --- a path may
legally end in a space.
Witnessed end to end by `g6_14c`, not at the parser: the fixture really
creates `<tmp>/nl\nroot`, the real `git` resolves it, and the assertion
is on the cwd of the status spawn the module actually made, plus real
rows in the panel and a RET that opens the file the row names.
Mutation-verified --- restoring `first_line` there resolves the root to
`<tmp>/nl` and fails the test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
`run_diff_plan`'s `next_step` read the module-level `state.root` each
time it started a step, so a multi-step plan could change repositories
halfway through.
An unborn `AM`/`AD` row produces a TWO-STEP plan --- staged patch, then
unstaged patch --- and the second step is spawned from the first one's
completion callback. `state.root` is reassigned by `_deliver_root`,
which runs whenever a concurrent `git.status` for another repository
finishes resolving its worktree. Start a diff in A, run `git.status` in
B before the first patch lands, and the plan's second step runs with B
as its cwd and A's path: git there matches nothing, so the unstaged half
silently renders "(no changes)" instead of the worktree delta it exists
to show.
The root is now captured at the keypress and threaded through the plan
as a parameter; `state.root` is not read inside the plan at all. Same
shape as the generation counter fixed in `ffe5ae2` --- capture at the
INVOCATION, never at the continuation --- and the third instance of it,
`state.branch`, is already read at the keypress on the same line.
Witnessed by `g6_22`: an unborn `AM` row's two-step plan with
`state.root` reassigned between the keypress and the first step's
completion, asserting BOTH spawned diff argvs carry the originally
captured root and neither carries the other repository's. Driven through
`_deliver_root` because no arrangement of real subprocess timing can
guarantee the interleaving, and nothing is pumped between the keypress
and the reassignment, so step 1 is genuinely in flight.
The argv assertion is the load-bearing half. A test that checked only
the first step, or only that a diff rendered, passes on the broken code:
step 1 is spawned synchronously from the keypress, and step 2 against
the wrong repository exits 0 with empty output rather than failing.
Mutation-verified --- restoring the `state.root` read fails the test on
the second argv.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
PR #227 review, P2. `check_key_collisions` compared RAW TOKENS, but the
key parser canonicalizes first: `parse_key_code` (src/key.rs) uppercases
and folds `RET`/`RETURN`/`ENTER`, `SPC`/`SPACE`, `ESC`/`ESCAPE`,
`BS`/`BACKSPACE`, `DEL`/`DELETE` onto one `KeyCode`. So
`keys = { RETURN = ... }` compared unequal to every fixed token, passed
preflight, and was refused by `Keymap::bind` instead --- after the
buffer had been created, made read-only, marked round-trip and given the
fixed keymap, and before the panel was registered.
The old rollback unbound only the newly added keys, so the BUFFER
survived, owned by no `panels` record: unreachable, un-editable, and
findable by name --- which made the next `open` for that name
disambiguate itself to `<2>`. A rejected `keys` table silently renamed
the panel.
The comment directly above the check claimed the opposite guarantee ---
"Reject collisions BEFORE anything is created or bound, so a bad `keys`
table leaves no half-built panel behind" --- and that is corrected here
too, since a comment stating a belief is not code enforcing it.
APPROACH: the second of the two the review offered --- full teardown ---
rather than canonicalizing in preflight. Two reasons, both in the code:
* **A Lua canonicalizer would be a second copy of a Rust rule.** It
would have to restate `parse_key_code`'s alias table, and the day the
Rust table gains a name the Lua copy silently stops seeing that alias
--- reintroducing exactly this bug for it. Deferring to `Keymap::bind`
cannot go stale, because it IS the thing that decides.
* **There is no Lua-reachable canonicalization to use anyway.**
`display_sequence` escapes only through `describe.key` and
`keymap.list`, both of which require the sequence to be BOUND already.
Reported rather than worked around, and no new binding added.
So the raw-token preflight stays, demoted to what it actually is: a
first pass that buys a better message ("that is the panel's own `g`")
and not safety. The construction block is now all-or-nothing, and the
teardown is `pmacs.buffer.kill`, which through `after_buffer_removed`
prunes the buffer's keymap scope, config locals and folds. `install_keys`
drops its own per-key rollback: two cleanup mechanisms for one failure
is how the weaker one came to be the only one that ran.
Witness: `g6_10c_an_alias_spelling_is_rejected_and_leaves_no_orphan_buffer`
walks every alias the parser folds onto a key the panel owns, asserting
after EACH that the buffer count is unchanged, then that a subsequent
legitimate open gets the plain name rather than `<2>`.
Bite: removing the teardown while keeping the raise fails it at the
buffer count (2 vs 1). Asserting only the error message would have
passed on the broken code --- it raised too; it just left wreckage.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
PR #227 review, P1. Two `git.status` invocations against different
repositories were ordered by which `rev-parse` returned first, not by
which the user asked for last.
`git.status` started an UNVERSIONED `rev-parse`, and the generation was
minted later, inside `start_status`, which runs from that lookup's
completion callback. So invoke status in repo A, then repo B: if B's
root resolved first and A's resolved second, A claimed the NEWER
generation and replaced B. The counter that exists to make the newest
INVOCATION win instead made the slowest SUBPROCESS win --- and it did it
silently, since both requests looked well-formed.
The fix is the ordering, not a new check:
* `reserve_generation()` is called at the point the user ASKS ---
`git.status` after its early returns and before the spawn, `g` at the
keypress. An invocation that starts no work reserves nothing, so it
cannot invalidate one already in flight.
* `start_status` takes the reserved generation as a PARAMETER instead of
minting its own, so the value survives the round trip through the root
lookup.
* The root-lookup completion is now `pmacs.git._deliver_root`, and it
drops a superseded result before any effect: no status spawn, no
`state.root` write, and no status-line message. A message from an
invocation the user has already replaced is as wrong as a panel from
one, and the previous shape would have written both.
Exposed for the same reason `_deliver_status` is exposed: the contract
is about completions arriving in an order the caller did not choose, and
no arrangement of real subprocess timing can guarantee two `rev-parse`
runs finish in a chosen order.
Witness: `g6_21_a_superseded_root_lookup_does_not_spawn_its_status`
drives two real invocations, then completes their ROOT LOOKUPS out of
order --- newer first, older second --- and asserts the superseded one
spawns nothing at all, comparing the status-spawn count before and
after. `g6_17` cannot see this: it drives the STATUS completions out of
order, by which point the generation each carries is already fixed.
Bite: restoring the old ordering (mint on arrival, no staleness check)
fails `g6_21` and nothing else. A test that merely hoped for the bad
subprocess order would have passed on the broken code about half the
time, which is why this one drives the completion directly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Implements docs/git-integration-framing.md revision 5 (approved
2026-08-09). COHERENCE.md section 15's largest named gap gets something
to attach to: a user can now answer "what have I changed?" without
leaving the editor.
*git-status* a `pmacs.listview` panel over
`git --no-optional-locks -C <root> status
--porcelain=v2 --branch -z`. RET visits the file, `d`
shows its diff, `g` refreshes.
*git-diff* the file-level diff, in a generated buffer rendered as
plain text --- there is no bundled `diff` grammar and no
hunk model anywhere in the tree.
NO WIRE CHANGE: no pmacs-protocol edit, no PROTOCOL_VERSION bump, no
DecorationKind variant. That is load-bearing for scheduling, not a
coincidence --- gutter markers (Stage 2) need all three and must be
scheduled alone, while this lane could run beside another.
One additive `listview` change, and the framing was wrong to say there
would be none: an optional `keys` table on the open spec. `d` cannot be
bound from outside the primitive safely, because a name collision
disambiguates to `<2>` and the name a consumer passed is not necessarily
the buffer it got. Keys are INSTALLED ONCE with the panel's buffer and
COMPARED on reopen: `Keymap::bind` refuses duplicates, and the async
completion model re-opens on every refresh, so a naive implementation
would have errored on every successful refresh.
One config-registry setting, `git.enabled`, through `pmacs.config.define`.
Facts measured against real git rather than reasoned about, each pinned
by a test:
* `ProjectKind::Git` means a BARE repository, and a language marker
beside `.git` wins --- so pmacs reports `kind = "rust"` for its own
repository. This module never asks pmacs whether something is a repo;
it runs `rev-parse --show-toplevel` and lets a non-zero exit answer.
* `git diff --no-index` implies `--exit-code`: exit 1 means it
SUCCESSFULLY found differences. The untracked predicate is exit in
{0,1}; only >= 2 is failure. Under the naive predicate every untracked
diff --- the case `--no-index` exists for --- would render a failure.
* An unborn HEAD makes `git diff HEAD` exit 128. Detected from
`# branch.oid (initial)` in output already being parsed, never from a
second `rev-parse`. `AM`/`AD` carry both states and get two labelled
patches; rename/copy is asserted UNREACHABLE, because `git mv` on a
staged-but-uncommitted file yields `1 A.`, not a `2` record.
* Under `-z` a rename's origin is the NEXT NUL-terminated field, not a
tab-joined suffix, so the record tokenizer is new rather than ported
from `tests/fixtures/pmacs-magit/`. What ports is that fixture's
SEPARATION --- pure `parse_*` over a string --- and its case coverage.
The fixture is untouched: it exists to prove the package system can
host this, and bundled code becoming its dependency would make
`m8_6_acceptance` test less than it claims.
Coherence impact, stated per CLAUDE.md:
* Section 14: `*git-status*` is the FIFTH `listview` call site and the
first outside `lsp.lua` --- the evidence P5 asked for that the
primitive generalizes past its first consumer.
* Section 6: no new interaction island. `d` is an ordinary buffer-local
binding through the primitive's own path, so `describe-key` reports
the truth and `init.lua` can rebind it. The count stays at six.
* Section 9: NEGATIVE, and named as such. A spawned process does not
appear in `*workers*` --- that view is `async.lua`'s job list. This
adds a fifth background thing with no single place to see it. Every
spawn is labelled, which is better than anonymous, but a label is not
attribution. Accepted only because these are short-lived reads.
* Journey: no step added. Git is not a journey step and this does not
make it one.
Section 15's "no Git integration at all ... anywhere in the tree" is
narrowed here. It was literally false when written ---
`tests/fixtures/pmacs-magit/` is a tracked, installable package that
spawns git and parses porcelain v2 --- and the product gap it described
is what this closes.
Five things found by biting the suite rather than by reading, recorded
in docs/active-work.md: `listview.open`'s `seat_cursor` walks DOWN from
wherever the cursor is (so a re-opened panel lands one row low, and the
completion handler seats unconditionally from line 0); a selection test
that inserts ONE row above the selection is vacuous against exactly that
off-by-one; `{:?}` on a Rust string cannot build a `-z` fixture, because
Lua's decimal escape swallows the digit after `\0` --- which made one
test pass while parsing nothing; a path may contain a newline, so rows
escape it; and untracked rows sort after every tracked row.
Gates: scripts/gate --acceptance git_status_stage1_acceptance
--acceptance listview_acceptance --acceptance config_registry_acceptance.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
* build: scripts/gate — a target dir per worktree, and one gate suite
Parallel worktrees do not work on this machine, and the reason is one
exported variable: every checkout builds into one CARGO_TARGET_DIR, and
cargo takes an EXCLUSIVE LOCK on it. Two lanes building at once do not
run in parallel — the second blocks — and they invalidate each other's
artifacts, so alternating between them recompiles from scratch. Parallel
development under that arrangement is slower than serial.
MEASURED, BECAUSE THE FIRST PLAN WAS WRONG. The shared directory is
285G, which drove a proposal to add sccache so per-worktree directories
would not lose artifact sharing. That number is years of accumulation
across TWO projects (pmacs and levcs share it). Measured directly: a
cold `cargo test --workspace --no-run` is 80s and 19G. And sccache
across two target directories hits 50% on C/C++ and **0.00% on Rust** —
rlibs embed their target-dir path, so dependency artifacts are not
bit-identical between directories and `--extern` hashes cascade into
misses. There is no sharing worth buying back. sccache stays configured
and earns its keep on C/C++; it is not what makes parallel lanes work.
The script also owns the FIXED gates, because a procedure living only in
prose gets executed differently each time — twice in the session that
motivated this:
- a sweep run with `--tests` instead of `--workspace`, silently
dropping pmacs_protocol and pmacs_gpu, including protocol tests that
same lane had just written;
- a sweep piped through `grep` before anyone read it, so an
intermittent red could not be matched against ci-red-signatures —
a row needs its fragments. That is registry note U2, and then U3
when it happened AGAIN.
Hence durable per-gate logs with the sweep paths printed. The remedy is
real: this lane's own run diagnosed its failures from the log without
re-running anything.
WHAT THE SCRIPT IS NOT AUTHORITATIVE FOR. Handoff §3 keeps policy and
keeps CHOOSING the touched acceptance suites, which arrive only via
`--acceptance`. No script can infer those from a working tree, and one
that guessed would report coverage it does not have.
THREE HAZARDS SPECIFIED RATHER THAN LEFT TO CHANCE:
- `cmd | tee log` reports TEE's status, so a failing gate would exit 0
and the suite would read green. `pipefail` is not POSIX.
- `cmd > log; rc=$?` never reaches the assignment under `set -eu`
(which scripts/bite already uses) — the shell exits at the failing
command, so nothing prints which gate failed or where its log is,
destroying the point of capturing it. The runner is therefore an
`if` condition, the only `set -e` exemption.
- CARGO_TARGET_DIR (env) OVERRIDES build.target-dir in config.toml, so
a per-worktree config file silently does nothing. Only a
per-invocation value beats it.
Pruning is dry-run by default, `--force` to delete, and refuses any
directory without a `.pmacs-gate-target` marker. "Live" means a git
worktree record carrying NO `prunable` line — git keeps listing a
worktree whose directory was deleted without `git worktree remove`, and
treating listed as live would make exactly the reclaimable directories
permanently ineligible.
ONE HONEST FINDING FROM MUTATION TESTING. Three mutations came back
vacuous, and all three are redundant defences rather than test holes:
git already returns resolved physical paths from both
`rev-parse --show-toplevel` and `worktree list --porcelain`, so canon()
is belt-and-braces; and the prune path guards the marker twice. Recorded
in the script and the tests so a later reader does not mistake a
"vacuous" result for a gap — or delete a defence because a test did not
notice.
VERIFICATION. 11 acceptance tests over the no-gates paths (running the
script for real inside the suite would recurse), each pointed at a
tempdir via PMACS_GATE_TARGET_ROOT so the real managed root is
unreachable — a prune bug is unrecoverable. Mutation-tested: `--tests`
in the sweep, an unconditional CRDT sweep, and pruning on a dry run all
fail their intended test.
Observed in a real run, which is how the framing said to confirm the
parts a test cannot: the failed-gate names and log paths print, the
ambient directory is created and reaped by the exit trap, and every log
appears. The run exits non-zero because of R8 — the pre-existing,
merge-base-confirmed listview failure — which means `scripts/gate`
cannot go green on this machine until R8 is diagnosed. That is a
property of the tree, not of this change.
Framing: docs/gate-script-framing.md (revision 4, approved).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
* fix(gate): two ways the script could do harm, and four smaller defects
Review round 1 on #225. Neither blocking finding was a design gap ---
both were the implementation failing to honour its own framing, which
is the case a framing document cannot prevent by itself.
PRUNE COULD DELETE EVERY MANAGED DIRECTORY. §2.6 requires the live
worktree set to be ESTABLISHED. The code piped `git worktree list`
straight into awk and the caller masked the result with `|| true`, so
running from outside any repository produced an EMPTY live set --- and
an empty live set means "every managed directory is an orphan", so
`--prune --force` would have deleted all of them, live lanes' artifacts
included. The failure mode was silent and total.
Two refusals now, and they are deliberately redundant: not inside a
worktree, and the enumeration itself failing. `live_worktrees` captures
git's output and returns non-zero rather than emitting nothing, so
"I cannot tell what is live" is unrepresentable as "nothing is live".
An empty porcelain listing counts as failure too --- a repository always
has at least its own worktree.
--ACCEPTANCE WAS SHELL-INJECTABLE. The name is interpolated into a
command the runner evaluates, and nothing validated it, so
`--acceptance 'x; rm -rf ~'` would have run. Now an allowlist of what a
cargo test target can actually be named --- letters, digits, underscore,
hyphen --- refused at parse time, before any gate. Rejection rather than
escaping: there is no legitimate suite name that needs quoting.
FOUR SMALLER ONES:
- Log directories carried a whole-second timestamp, so two runs in the
same worktree within one second shared one and could overwrite each
other's evidence --- reintroducing U2/U3 through a naming choice.
The PID is now part of the name.
- The ownership marker is DOCUMENTED as one line, so it is enforced as
one line instead of read head-first. Acting on the first line of a
file we did not understand is how a corrupted marker authorises a
deletion.
- The `prunable` test returned green when `git worktree add` failed,
so the only coverage of that rule could silently never run. It now
fails loudly.
- Its cleanup ran after the assertions, so a panicking assertion would
have left the real repository carrying a stale worktree record. Now
a `Drop` guard.
MUTATION TESTING, HONESTLY REPORTED. The injection and marker fixes bite
individually. The two prune guards do NOT --- each alone satisfies the
outside-repo test, so mutating one at a time reads as vacuous. Removing
BOTH fails the test, which is what establishes that the test detects the
unsafe state rather than being blind to it. Recorded in the test so a
later reader does not delete one guard on the grounds that nothing
noticed.
ALSO: handoff §3's ambient-root caveat still said "until the
ambient-root isolation lane lands". #206 merged; the five variables are
now belt-and-braces for external and integration paths, and `scripts/gate`
sets them regardless.
R8 PROMOTED. `docs/ci-red-signatures.md` gains the reason it stops being
a catalogued curiosity: with the gate suite reduced to one command, R8
makes that command exit non-zero on a clean tree EVERY TIME, and a gate
that is always red is a gate nobody reads. `docs/active-work.md` gains a
lane. It is still not a regression from #223 or #225 --- the merge-base
control says so --- and the lane's first job is diagnosis, because a
change that made the assertion pass without explaining the prefix strip
would convert a visible failure into an invisible one.
15 acceptance tests. Observed run re-confirmed: failed gates named with
log paths, ambient directory created and reaped, distinct log directory,
exit 1 from R8 alone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
* docs: the #225 lane, and R8 diagnosed to a stray /tmp/.git
TWO LEDGER GAPS, both found by review.
and — the part that matters — an explicit GATE STATUS: NOT GREEN
section. `scripts/gate` exits 1 on this branch and on a clean `main`
because R8 fails m4_acceptance and therefore the sweep. That is a merge
blocker under the standing rule, and #225 is the worst possible lane to
grant a silent exception to: it is the lane that makes the gate suite
authoritative, and a tool shipping with its own gate red teaches the
opposite of what it exists to teach.
The lane also records that it was written after the PR existed, again,
because review asked again. Two lanes in a row now. The correction from
only evidence of that.
R8 DIAGNOSED, and the `TMPDIR` hypothesis was right:
1. `display_path` (builtin/runtime/lsp.lua:2397) shortens a location
against the DETECTED PROJECT ROOT before rendering it.
2. `project.detect` walks UPWARD for a marker; from
/tmp/.tmpXXXX/r.rs it reaches /tmp.
3. This machine has a stray `/tmp/.git` — an EMPTY DIRECTORY, not a
repository. The `.git` marker is directory-only, so an empty
directory still matches.
4. Root resolves to /tmp, the prefix is stripped, and the rendered row
is exactly the observed `.tmpXXXXXX/r.rs:12:3`.
Controlled, not inferred: the same test with TMPDIR outside /tmp PASSES.
THE CODEBASE ANTICIPATED THIS BY NAME. src/project.rs:208 documents
`detect_project_within(start, markers, stop_root)` as existing "so a
stray marker in a temp-dir's ancestor (e.g. a developer's /tmp/.git)
can't leak into a fixture that lives below it." The mechanism exists;
this fixture does not use it.
So the row splits, and the halves need different fixes. The failure is
ENVIRONMENTAL — nothing about pmacs is wrong when a real project root
sits above a file, that is the feature, and removing /tmp/.git makes the
gate green immediately. The fixture being ENVIRONMENT-DEPENDENT is a
real defect, and bounding its detection is what retires the row.
PROVENANCE UNRESOLVED, and I am not going to assume in my own favour:
/tmp/.git is dated 2026-08-07 23:17, inside this session's window, and
may have been created by this session's own work — a stray git
invocation from /tmp would do it. The earlier merge-base control stays
valid as "this tree has it" but says nothing about WHEN the environment
acquired the marker, so "pre-existing" must not be read as
"long-standing".
Nothing deleted: /tmp/.git is outside the repository and I cannot
confirm I created it, so removing it is the user's call.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
* docs: rebase onto the R8 fix; scripts/gate now exits 0
#226 (`dcb852e`) retired R8 by bounding the LSP fixture's project
detection. This branch rebases onto it, and the thing that was blocked
is now demonstrable: **`scripts/gate` exits 0** --- all nine gates green
in one command, the first time the tool has passed the suite it exists
to run. That is #225's own acceptance criterion, and it could not even
be stated while the script did not exist on `main`.
REBASE RESOLUTION, per the standing rule that #226's R8 documentation is
authoritative. Every conflict was in R8 text this branch wrote while the
row was still an open investigation:
- two in `docs/ci-red-signatures.md`, both resolved to #226's retired
row with this branch's pre-fix copy dropped;
- the framing-doc pair --- e71e1bd added `docs/r8-fixture-boundary-
framing.md`, 7cfba73 removed it --- both SKIPPED. They are net-zero
here and `main` owns that file authoritatively; replaying the second
would have deleted `main`'s copy, which is the one failure mode a
mechanical "resolve each conflict in turn" would have walked into.
TWO STALE LANES REMOVED. This branch's "R8 --- NEEDS A LANE"
investigation block describes a diagnosis that has since happened and a
fix that has since landed. And #226's own lane arrived through the
rebase still saying "OPEN, HELD FOR REVIEW"; Rule 4 retires it now that
it has merged, its durable facts already being in the retired registry
row and the handoff section 6 census. Leaving either would have left the
ledger asserting that a merged fix was still an open investigation.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs: frame the R8 fixture-boundary fix (revision 2)
R8 fails m4_acceptance deterministically on one machine, and the
diagnosis is that the fixture never bounds its own project detection.
`display_path` (builtin/runtime/lsp.lua:2397) shortens a location
against the DETECTED PROJECT ROOT. `pmacs.project.detect` walks upward
for a marker; from /tmp/.tmpXXXX/r.rs it reaches /tmp, where this
machine has a stray EMPTY `.git` directory. The `.git` marker is
directory-only, so an empty directory matches, the root resolves to
/tmp, and the prefix is stripped.
THE PRODUCT BEHAVIOUR IS CORRECT AND IS NOT CHANGING. Shortening a
location against its project root is the feature. The defect is that
the fixture's assertion depends on whether the developer's /tmp happens
to contain a `.git`.
THE MECHANISM ALREADY EXISTS AND THIS SUITE ALREADY USES IT.
`src/project.rs:208` documents `detect_project_within(.., stop_root)` as
existing "so a stray marker in a temp-dir's ancestor (e.g. a
developer's /tmp/.git) can't leak into a fixture that lives below it."
It is exposed to Lua as `pmacs.project.set_search_boundary`; eight test
files make fourteen real calls to it, five of them in m4_acceptance
itself --- one carrying that same hazard as a comment. `open_against_fake`
(tests/m4_acceptance.rs:7985) is one helper that missed the pattern.
THE WITNESS PLANTS ITS OWN HAZARD, so the proof is not a property of
this machine: an empty `.git` in a temporary ancestor, the file one
level below, boundary at the file's parent. With the boundary the row
renders absolute; reverting it strips the prefix deterministically on
every machine, including CI where /tmp/.git does not exist. The
/tmp/.git observation stays as corroboration, not as the bite.
`scripts/gate` is deliberately NOT a criterion: this lane branches from
main, where that script does not exist (it is unmerged on #225). Naming
it would make this lane depend on an artifact absent from its own base.
R8 lands first on its own merits; #225 then rebases and takes "gate runs
green" as ITS criterion.
Q#R8-1 records a limitation rather than discovering it later:
parent-as-boundary is correct only while fixtures put the file as a
direct child of the fixture root. A future nested fixture cannot fix
itself by passing a deeper path --- the boundary is DERIVED from the
parent, so a deeper path clamps sooner, never later.
Provenance of /tmp/.git is left permanently unresolved, and the document
says why no timestamp is treated as authoritative.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
* fix(tests): bound the LSP fixture's project detection — retires R8
`open_against_fake` never set a search boundary, so the panel tests'
rendered paths were shortened against whatever project root detection
found ABOVE their tempdir. On a machine with a stray `/tmp/.git` that
meant `/tmp` --- and the assertion that spells a path out failed
deterministically. Registry row R8.
THE PRODUCT BEHAVIOUR WAS NEVER WRONG AND IS NOT CHANGED. Shortening a
location against its project root is the feature; a file that really is
inside a project really should render relative to it. What was wrong is
that a fixture's assertion depended on the contents of the developer's
/tmp.
THE MECHANISM WAS ALREADY THERE. `src/project.rs:208` documents
`detect_project_within(.., stop_root)` as existing "so a stray marker in
a temp-dir's ancestor (e.g. a developer's /tmp/.git) can't leak into a
fixture that lives below it" --- naming this exact hazard. It is exposed
to Lua as `pmacs.project.set_search_boundary`, eight test files make
fourteen real calls to it, and five of those are in this same file, one
carrying that hazard as a comment. This was one helper that missed a
pattern its own file already used.
THE WITNESS PLANTS ITS OWN HAZARD, so the proof is not a property of one
machine. `a_planted_ancestor_marker_does_not_reach_the_rendered_row`
creates an empty `.git` in a temporary ancestor with the file one level
below, and asserts the row stays absolute. Reverting the boundary fails
it with `proj/r.rs:12:3` --- relative to the PLANTED marker, not to
/tmp, because the nearer ancestor wins. That is what makes it bite in
CI, where no /tmp/.git exists; confirmed by also running it with TMPDIR
outside /tmp.
Resting the bite on /tmp/.git would have been the same mistake as a test
that passes only where the developer happens to be standing.
/tmp/.git IS DELIBERATELY LEFT IN PLACE. Deleting it would hide the
hermeticity defect rather than fix it, its provenance is unresolved, and
it is the only thing on this machine that reproduces the row --- which
makes it useful, not merely untouchable. The R8 fix is verified WITH it
present.
VERIFICATION. The R8 test passes on the machine that reproduces it. Full
m4_acceptance 151/0. `--lib` 1920, `--lib --features crdt` 2105,
`-p pmacs-gpu` 241, fmt, clippy, `git diff --check`. The full workspace
sweep exits 0 across 113 targets --- the first fully green local sweep of
this session, R8 having been the only obstacle.
`scripts/gate` is deliberately not a criterion: this branches from main,
where it does not exist. #225 rebases onto this and takes a green gate
run as ITS criterion.
R8 is RETIRED CAUSALLY --- mechanism removed plus a discriminating,
portable witness --- and moved to the retired section with its
disposition. What the retirement does NOT claim is stated there: 113
`new_with_roots` constructions in this suite alone, an unknown number
equally unbounded, harmless only while their assertions do not render a
path. That census is now a named §6 follow-on, because the next one will
otherwise look like a fresh mystery rather than a known class.
Framing: docs/r8-fixture-boundary-framing.md (revision 2, approved).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
* docs(tests): reunite the listview doc comment with its test; state the PR
Three review findings, one of which is mine to own plainly.
I REPORTED A SHA I NEVER VERIFIED. The previous message named the PR
head as `21f0ed1`. That object does not exist in this repository. The
true head is `78d8e1c` --- local tip, `githubsucks/r8-fixture-boundary`,
and the PR all agree, and it is what was reviewed. No command in that
turn ever printed `21f0ed1`; I asserted an identifier instead of
reading one, which is precisely the failure a head-SHA check exists to
catch. Verified this time before writing it down.
THE DOC COMMENT DOCUMENTED THE WRONG TEST. Inserting the new witness
anchored on `#[test]\nfn flat_listview_...`, which sits BELOW that
test's 17-line doc comment --- so the comment about outline and flat
listview consumers ended up introducing the planted-marker test, which
touches neither, while the test it was written for was left bare. Moved
back. No behaviour change; both tests still pass.
That is a general hazard of anchored insertion worth naming: anchoring
on the `fn` line silently steals whatever documentation precedes it.
STALE STATE IN TWO DOCS. The framing still said "Pre-implementation.
Awaiting approval" after being approved and implemented, and the ledger
lane said "PR PENDING" after #226 opened. Both now record approval,
implementation, the PR link, and that it is held for review with no
merge authorization.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The suite header still said "the four call sites", written before the
selection painter joined `Viewport::visible_cols`. Four is now the count
of decorator FAMILIES — syntax/LSP styling, diagnostic underlines,
search washes, `BufferStyleOverlay` — and five is the count of adopters,
selection being the fifth.
Also points at where selection's own witnesses live, since a reader of
this file would otherwise look for them here and find nothing:
`paint_local_selection` is private, so they are in `src/editor.rs`.
Comment only; no behavior and no assertion changed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Stage 4 of the QoL arc, framing revision 4 (approved). Under
`truncate`, text past the right edge was UNREACHABLE; moving the cursor
now brings it into view. Automatic only — no commands, no bindings, no
new interaction island (Q#HS2).
THE CONTRACT. `view_left` is an unsnapped per-window display column
(Q#HS7(a)), and each line derives its own effective edge during the
walk it already performs from column 0. Starting at 0 is not laziness:
tab expansion depends on the absolute column from the line start, so a
walk beginning at the edge would put tab stops in the wrong place. The
walk stays line-absolute and only the emit translates.
Where the edge bisects a wide glyph on a given line (Q#HS7(c′)), its
trailing cell paints a styled BLANK rather than a `Continuation` — that
glyph means "the cell before me is a wide glyph's head", and here that
cell is off-screen, so emitting it would name a cell nobody painted.
The mapping designates that cell to the glyph's START byte, which keeps
`byte_at_place` total over visible cells and makes the character the
user scrolled toward clickable. Tabs keep FORWARD rounding (Q#HS7(c″))
— preserved, not chosen.
DECORATIONS TRAVEL WITH THE TEXT. The first version of this commit
translated the base glyph walk and nothing else, which split the frame
in half: at `view_left = 10` a glyph from source column 10 painted at
screen column 0 while its syntax style, diagnostic underline, search
wash and `BufferStyleOverlay` span painted at screen column 10 — or
vanished. Decorations drifting off the characters they describe,
silently, and only once a window had been scrolled.
Every such site carried the same two lines (`start_col.min(max_cols)`,
`end_col.min(max_cols)`), correct only while the left edge was pinned
at zero. `Viewport::visible_cols` is now the one rule all FIVE adopters
share — syntax/LSP styling, diagnostic underlines, search washes,
`BufferStyleOverlay`, and the selection painter — so a future decorator
inherits the translation instead of re-deriving it. It also subsumes
the old `end_col <= start_col` guard rather than sitting beside it.
`StyleSpanOverlay` and `VirtualCellOverlay` are deliberately untouched:
they are documented as viewport-relative, so translating them would be
the mirror defect.
The selection painter was nearly a sixth site with its own copy of the
rule, which I justified by a width it supposedly needed and the
viewport lacked. That was FALSE — the render viewport's
`cell_size.cols` is already `rect.size.cols - gutter_w` and its origin
already sits past the gutter. It now takes that same viewport and drops
its `rect`/`gutter_w` parameters entirely. A canonical rule with one
honest exception is not canonical.
The selection painter had the same defect with a worse failure mode: it
asked `pos_to_display` through the LIVE context, which returns `None`
for a position left of the edge, so a selection beginning off-screen
and reaching into view took `continue` and painted NOTHING. That is the
common shape, not an edge case — select rightward from column 0 past
the window width and the view scrolls with the cursor.
TWO THINGS THE TESTS FOUND, both in `pos_to_display`. My framing note
said a caret sits between characters so never lands inside a glyph;
true for the caret, false for the DESIGNATION direction — the glyph's
start byte must map to its visible trailing cell, so `screen_col` needs
the straddle rule and not a bare subtraction. And the `take == 0` early
return short-circuited the translation entirely, so byte 0 looked
visible at every offset.
`view_left` is inert under `wrap` BY CONSTRUCTION —
`LayoutCtx::effective_left` and `Viewport::left_edge` return 0 while
wrapping — rather than by every caller remembering.
Persisted per leaf at DESKTOP_VERSION 1 (Q#HS5) with both approval
conditions: `#[serde(default)]` and a literal v1 JSON fixture omitting
the field, hand-written because a generated one would gain the field
and prove nothing.
Also: `view_left: window.view_left` in the render viewport, not a
literal 0. My mechanical fill put 0 there and it is EXACTLY the
`aa3cd4d` defect — coordinates and the indicator following the scroll
while the painter stays pinned at column 0.
BITE, per clause. Forcing `bisected = false` fails the multi-line
straddle witness; dropping the backward designation fails the
round-trip witness; removing `#[serde(default)]` fails the v1 fixture;
pinning `visible_cols` to an absolute clamp fails all three decorator
witnesses; restoring the selection painter's live-context lookup fails
the off-screen-start selection witness. Each alone. And with selection
now reading the shared helper, pinning `visible_cols` to an absolute
clamp fails the selection witnesses TOO — which is the check that the
duplication is really gone rather than merely reworded.
One unrelated red, logged as R7 in ci-red-signatures.md — the first
this session with a COMPLETE signature, so a matchable row rather than
a U note. `pmacs-gpu`'s managed-retry attach hit a BrokenPipe once
under full-sweep load and did not reproduce (6 isolated runs plus a
clean 113-target sweep). Per the rerun rule that is intermittence only,
and the row explicitly does not claim harmlessness. Not attributed to
this lane: Stage 4 touches no `pmacs-gpu` file and adds no wire
surface.
Gates: fmt; clippy --workspace --all-targets -D warnings, both
configurations; `cargo test --workspace --no-fail-fast -- --skip
basedpyright` 113 targets exit 0, and the same with --features crdt,
113 targets exit 0; git diff --check. No protocol change, so no version
bump and no protocol-bump matrix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
CI red on #221: all five Test jobs, one identical test, every platform
— deterministic, not a flake. The production code was never wrong.
WHY MY GATES MISSED IT. The standing gate is "the touched acceptance
suites", selected from the diff. A PROTOCOL_VERSION bump breaks
version-assertion tests that appear nowhere in it. Worse, CI showed
only ONE of the eight, because cargo stops at the first failing
target; the rest surfaced only under `--tests --no-fail-fast`, and one
at a time would have cost four more red rounds.
Three of the eight were invisible even to that, because they are
crdt-gated real-daemon tests asserting on a live socket. Found by
`--tests --features crdt --no-fail-fast`. That is the handoff's
existing "a local sweep is blind to whichever configuration it does
not build" lesson, hit again by a different lane.
THREE TRIPWIRES, WORKING AS DESIGNED. `assert_eq!(PROTOCOL_VERSION,
21)` in statusline_segments, bottom_panel_stage2b_gpu, and
vterm_stage3 are meant to fire and take a deliberate edit; each says
so in its own comment. Updated to 22 with the reason recorded. Worth
noting the pin that must NEVER be edited —
ADVERTISED_PROTOCOL_VERSION == 20 — did not fire, which is the
mechanism behaving exactly as designed.
FIVE DEFECTS, ONE SHAPE: an absolute contract expressed as arithmetic
on, or equality with, a MOVING constant. Each was true when written
and silently false afterwards.
- `PROTOCOL_VERSION - 1` meaning "below the panel version". Held
only while PROTOCOL_VERSION == PANEL_MIN_VERSION; at v22 it
equalled PANEL_MIN_VERSION exactly, so the fixture's "old" peer
became panel-capable and the daemon correctly sent it a frame.
Now `PANEL_MIN_VERSION - 1`.
- `assert_eq!(PANEL_MIN_VERSION, PROTOCOL_VERSION)` — a coincidence
true only while panels were the newest feature. Replaced by the
two durable bounds: above the advertised floor, at or below this
binary's wire.
- `assert_eq!(PROTOCOL_VERSION, 21)` in a test named
`the_panel_stage_takes_protocol_v21` — the current wire as a proxy
for the panel stage's own version, in a test whose name says which
one it means. Now PANEL_MIN_VERSION.
- `session_protocol_version == "21"` in two real-daemon probes. What
the counter-offer activates is THIS BINARY's wire, so the literal
was only ever right by accident. Now PROTOCOL_VERSION, plus an
explicit `>= PANEL_MIN_VERSION` for the panel capability the
literal had been carrying implicitly.
The codebase already had the right idiom: src/daemon.rs and
pmacs-gpu/src/main.rs spell it `PANEL_MIN_VERSION - 1` in five places.
Every outlier was in tests/.
ALSO LOGGED, NOT FIXED: U2 in ci-red-signatures.md.
`process::tests::m6_1_pty_raw_mode_disables_kernel_echo` failed once
during a full corpus run and did not reproduce (108 targets exit 0,
plus 3 isolated --lib runs at 1917/0). It is in no registry row, so it
is a new incident, and leaked `pmacs --daemon` processes remain an
unexcluded rival explanation. Recorded with a selector this time —
unlike U1, whose name I destroyed by piping through `tail`.
Gates: fmt; clippy --workspace --all-targets -D warnings, both
configurations; --lib 1917/0; --lib --features crdt 2102/0; --tests
--no-fail-fast 108 targets exit 0; --tests --features crdt
--no-fail-fast 108 targets exit 0; PMACS_REQUIRE_GPU=1 -p pmacs-gpu
228/0; git diff --check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Closes Stage 3's remaining obligations.
THE PTY TEST. Every other test in this lane checks a mechanism.
tests/long_line_readable_acceptance.rs checks the complaint: the
shipped binary, a real 80x24 PTY, one source line 200 columns wide,
and an assertion that its tail marker reaches the host. It bites —
`scripts/bite HEAD~2 src/editor.rs --test long_line_readable_acceptance`
against the pre-aa3cd4d editor never paints TAILZQX in 20s.
Its truncate control (an isolated init.lua pinning the mode) is what
makes that marker discriminating, and it is also the honest statement
of what truncate costs today: those bytes are not off-screen, they are
unreachable until Stage 4.
What it does not prove: the workspace has no screen model and no
vt100/termwiz/vte, so this shows the tail was WRITTEN to the terminal,
not which row a human would point at. That is nonetheless the whole of
the report — under truncation the bytes are never emitted at all.
§1.1 WAS WRONG, FOR NINETEEN REVISIONS. `editing.fill-column` is not
an orphaned registry setting "of the exact shape Stage 1 just fixed".
Both cited occurrences are inside `#[cfg(test)] mod tests` — fixture
names in round-trip tests covering one setting per ConfigKind. Two of
those five names are real; three, including this one, are defined
nowhere else in the tree. There is no shipped setting, so the Q#LL4
deliverable "sharpen its description" had no object.
The mechanism is worth more than the correction. A grep hit at a src/
path, a genuine `r.define(...)` call that is real API usage rather
than a mock, and `#[cfg(test)]` about fifty lines above the citation.
Every later revision inherited the conclusion instead of the evidence,
and three review rounds reasoned about the consequences of an orphaned
setting rather than re-checking that it existed. A file:line citation
is not a substitute for reading the scope it sits in.
Had it gone unchecked into implementation, Stage 3 would have shipped
an edit to a unit-test fixture believing it was rewording a
user-visible setting — a no-op with a misleading commit message.
§1.1 is withdrawn in place, keeping the original text and the
reasoning that produced it; §6's answer is unchanged (a setting that
does not exist is a stronger reason not to adopt it) and its premise
corrected. Both fixture sites now say they are fixtures. The approval
is not reopened: nothing else in the document rested on §1.1, which
argued for a display setting separate from fill-column — which is what
shipped.
AND ONE UNCLASSIFIABLE RED, logged as U1 in ci-red-signatures.md. A
`-p pmacs-gpu` run went 227/1 once; every run since is 228/0. The
failing test name was NOT captured, because I piped that command
through `tail -3` and discarded the failure block above the summary.
36 later runs are clean, 6 under deliberate concurrent load — which
per the rerun rule establishes intermittence only, and without a
selector not even that. Deliberately NOT matched against A1 despite
A1 also being GPU-headless-under-load: matching requires an exact
selector and every required fragment, and calling a shapeless red
"probably the known one" is the reputation-by-adjacency that file
exists to deny.
Gates: fmt; clippy --workspace --all-targets -D warnings; --lib
1917/0; --lib --features crdt 2102/0; line_wrap 6/0;
long_line_readable 2/0; folding 21/0; folding_stage2 48/0;
full_grid_resync 1/0; config_registry 16/0; m4 150/0;
PMACS_REQUIRE_GPU=1 -p pmacs-gpu 228/0 (see U1); git diff --check.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
The frame resolved ui.line-wrap, recorded it on the window, and fed it
to the coordinate mapping and the scroll indicator --- while the
viewport handed TextView::render a hard-coded Truncate. With the
default of wrap, that meant the cursor was placed for wrapped text and
the indicator reckoned against wrapped rows while the text was still
clipped at the edge. The worst possible split: every part that reports
where things are agreed, and the part that draws them did not.
The viewport now reads window.last_wrap, so it consumes the same single
resolution as everything else rather than resolving again.
How it survived: the edit was in a script that raised on a LATER
assertion, so nothing was written; a follow-up script's replace then
matched nothing and silently did nothing. Every test I had asked "is
the mode right?" and none asked "is the text wrapped?", so all of them
passed.
Hence the new witness reads the GRID. the_default_actually_wraps_the_
painted_text goes through RenderState and reconstructs rows from the
emitted CellDelta spans, for two reasons: the defect lived in the
DRIVER, between the resolved mode and the viewport it built, so a test
building its own viewport would have passed against it --- and the
spans are what a TUI actually consumes. truncate_clips_the_painted_text
is its control, so the pair is discriminating rather than merely true.
It bites: restoring the hard-coded Truncate fails the wrap witness
while all five other tests keep passing, which is exactly the shape
that let it through.
Gates: fmt, workspace clippy -D warnings, diff --check, --lib 1917/0,
crdt 2102/0, line_wrap 6/6, tab_width 2/0, folding 21/0,
full_grid_resync 1/1.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
ui.line-wrap is buffer-local, and the toggle was built from
pmacs.config.get(name) and pmacs.config.set(name, value) --- both of
which address the GLOBAL layer. The registry's buffer-local surface is
get(name, buf) and set_local(buf, name, value), and the command used
neither.
The result was wrong in both directions at once, which is why it needed
two witnesses rather than one. In a buffer pinned to truncate, the
toggle would read "wrap" from the global layer, decide the next mode is
truncate, and leave that buffer exactly as it was --- while writing
truncate globally and flipping every buffer that had no override of its
own. The command that changes nothing here and everything elsewhere.
Now resolves the buffer ONCE and uses it for both calls. Once matters:
resolving twice would be a narrower version of the same bug, since the
active buffer can change between two calls.
Two witnesses in a new acceptance suite, and both bite against the code
review rejected --- restoring the global-layer toggle fails both while
the default and enum tests keep passing, which is exactly how it
shipped. the_toggle_moves_this_buffer_and_leaves_the_other_alone covers
the leak outward: a second buffer and the global layer must be
untouched. a_pinned_buffer_toggles_from_its_own_value covers the miss
inward: a buffer whose value differs from global must toggle from ITS
value.
Note on the second buffer: there is no Lua buffer-switch, so "the
other" is a buffer that exists but is not shown. That is the case that
matters anyway --- a global write reaches every buffer without an
override, shown or not.
Gates: fmt, workspace clippy -D warnings, diff --check, --lib 1916/0,
line_wrap_acceptance 4/4.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
pos_to_display and display_to_pos now take a LayoutCtx and DisplayCoord
carries a sub_row. The asymmetry is the whole audit strategy, and it
played out as designed.
Breaking on the way in: adding a required parameter turned the audit
into 61 compiler errors instead of a grep. Additive on the way out:
sub_row defaults to 0, so overlay_paint's `row - view_top` and vertical
motion's bounds check stayed CORRECT rather than merely findable ---
neither needed touching. row is still the source line. Redefining it as
a visual row would have broken both silently.
Two structural gaps this surfaced, neither in the framing:
Window recorded last_visible_rows but no width, so the coordinate
callers --- vertical motion, paging, overlay placement --- had nothing
to build a context from. Added last_content_cols, taken from the
viewport the renderer actually used rather than recomputed: a second
derivation could disagree, and the disagreement would show only as a
cursor on the wrong row. Content width, not window width, because the
gutter grows at the line-count digit boundary.
The mode had the same problem one level up. It is buffer-local and the
registry has no ambient buffer, so only the driver can resolve it ---
but every consumer holds a window, not a registry. Window::last_wrap is
recorded beside the width and read through Window::layout_ctx(), so
there is ONE resolution consumed everywhere. When ui.line-wrap is
registered, only the driver changes and all twenty call sites become
wrap-aware together. The alternative, each caller resolving for itself,
is how two callers end up disagreeing about one buffer.
One real regression, caught by the render tests rather than reasoning:
generalising row_of_byte into place_of_byte lost the boundary rule. A
byte landing exactly on a row edge reported (row, max_cols) instead of
(row+1, 0), so a viewport anchored there painted the wrong row. The fix
is the rule framing section 7 already settled --- the wrap position is
owned by column 0 of the NEXT row, because that cell always exists and
(row, max_cols) does not.
Five coordinate witnesses. Identity on every cursor boundary of a line
containing a tab and a CJK glyph, across four widths; projection to the
codepoint start for interior bytes, unchanged by wrapping; the two
distinct adjacent codepoints across a break mapping distinctly; row
staying the source line; and a truncate control. They bite --- forcing
the wrap branch off fails three, including the round trip.
Gates: fmt, workspace clippy -D warnings, diff --check, --lib 1912/0,
crdt 2092/0, tab_width 2/0, folding 21/0, gui_zoom 15/15.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Stage 2 (GUI zoom, #220) landed. Stage 3 shares no code with it, so this
is a currency merge rather than a dependency --- taken now so the
Stage 3 PR opens against a base it has already been tested on.
# Conflicts:
# docs/active-work.md
Inert by construction. Adds the type and the Viewport field, sets all
31 construction sites to Truncate, and changes no rendering: --lib is
1900/0 and crdt 2085/0, the same counts as the parent commit.
The field is required rather than defaulted on purpose. A default would
have let 31 sites stay silent about which behavior they meant; a
required field makes each one state it, so the pre-existing sites now
read as deliberately unwrapped rather than merely untouched. The
compiler enumerated them, including five integration tests --- Viewport
is public API, so this is a real break, and the break is the point.
The render driver is pinned to Truncate too. The wrap path does not
exist yet, and exposing a mode before the cursor mapping honors it
would ship a setting that renders one thing and navigates another ---
the shape of defect this lane exists to remove, not add.
Two notes on getting here, since both were nearly landed:
The first mechanical patch matched every `folds,` line and put a wrap
field into function call sites and a FoldStore literal. Scoping the
insertion to Viewport literals cut it from 40 sites to 31. The compiler
caught it, but only because a struct field cannot be mistaken for an
argument; a same-arity call would have compiled.
While rewriting the character walk I changed the wide-character edge
case --- a double-width glyph with one cell left now breaking instead
of painting a lone lead cell. That is arguably better behavior and it
is NOT this commit's to make: Truncate must be byte-identical, and an
"improvement" smuggled in beside a refactor is how identity cases stop
being identity cases. Reverted; the walk is untouched.
Gates: fmt, workspace clippy -D warnings, diff --check, --lib 1900/0,
crdt 2085/0, tab_width 2/0, listview 26/0, compile_mode 73/0,
folding 21/0.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Review caught the explanation of the 0.015 round-trip break, not the
fix. Three copies of it claimed 16.015 rounds up while 16.005 rounds
down --- "opposite directions". Both round UP.
Verified rather than reasoned about: at the point the quantizer sees
them, 16.015 * 100 is exactly 1601.5 and 16.005 * 100 is exactly
1600.5. Both are exact ties, and half-up sends both away from zero.
So the mechanism is not opposed rounding, it is that half-up is not
symmetric under negation. Rounding up on the way in adds half a
centi-pixel; rounding up on the way out adds another, so the two
errors ACCUMULATE instead of cancelling, and 16.00 -> 16.02 -> 16.01
ends one centi-pixel high. "Opposite directions" would have predicted
them cancelling, which is the reverse of what happens.
Corrected in all three places that carried it: the module comment, the
test's doc comment, and framing section 3.2.
Comments only --- no behavior change, and the witness values in the
tests were already right. Zoom suite still 15/15.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
THE BOUNDS WERE NOT SUFFICIENT AND THE TEST COULD NOT SEE IT.
`ConfigKind::Number` validates finiteness and bounds and nothing else,
and `on_change` listeners are notified after a value is stored — they
cannot veto. So 0.015 is a perfectly settable step and nothing in the
registry can refuse it.
Used raw it breaks the framed guarantee, because each operation rounds
independently and 16.015 and 16.005 round in OPPOSITE directions:
step 0.015: 16.00 -> 16.02 -> 16.01 broken
step 0.37 : 16.00 -> 16.37 -> 16.00 holds
The existing round-trip test used 0.37 — centi-pixel representable — so
it passed against the defect. Bitten now: with the raw value the new
case lands on 16.01, while the 0.37 case still passes, which is exactly
why it needed to be its own witness.
QUANTIZED WHERE USED, not at `set`. Sizes live in integer hundredths
end to end and `validate_font_size` already range-checks the original
and then rounds to the nearest hundredth; rounding the step is that
same operation one level up. A step of 0.015 is not a finer step in
this domain, it is 0.02 written imprecisely.
Enforcing at set time was considered and rejected: the registry cannot
express a precision constraint, and a validating wrapper is bypassed by
a direct `pmacs.config.set` — the seam `autosave` already documents
about its own interval_ms wrapper. Quantizing at the point of use
cannot be bypassed. Both descriptions say "quantized to hundredths", so
`describe-setting` shows it.
The framing header also still said "proposed, awaiting approval" while
the lane and this PR recorded it approved and implemented. Revision 5,
with §3.2 recording the gap and why quantization rather than rejection
closes it.
Verified: fmt, clippy, diff-check, --lib 1900/0, gui_zoom 15/15.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ctrl +/- had no effect whatsoever in the GPU frontend. Stage 1 (#219)
fixed what zoom did to the TUI; this is the other half.
NO RENDERING WORK. FontMetrics::scale already derived every GUI
dimension — code size, line height, status band, divider, menu rows,
minibuffer dropdown, gutter advance — and apply_font_facts already
re-metriced all seven buffers in one transaction. This drives the
preference that existed: two settings, three commands, and a restore.
Q#Z1 = (c). Relative zoom needs an origin and the daemon is built never
to know one — font_pref.rs is explicit that it "never learns metrics,
advances, or what resolves". Hardcoding 16.0 would put a pixel constant
on the daemon side; always sending a size would destroy the `None`
state for everyone who never zooms. A configured base is the only
option where the daemon still infers nothing, and the untouched path
stays byte-identical.
THREE THINGS REVIEW CAUGHT THAT REVISION 1 HAD WRONG.
Q#Z3 was not implementable as framed. `keymap_stack::Scope` is
Buffer | Mode | Global and carries no frontend identity, so "bind on
GPU frontends only" does not exist; and FrontendEvent has no
command-invocation variant, so the GPU cannot ask for a command by name
either. A global binding would capture the chord in the TUI and take
away the terminal's own zoom — the very thing the user is pressing it
for. Commands ship; the binding waits on capability-aware keymap
resolution, which is now a named follow-on rather than something
smuggled in here.
The restore seam did not exist. Builtins and init.lua both run BEFORE
install_state_dirs, so a pmacs.state.read at module load returns
nothing, always. saveplace and recentf never meet this because both
read lazily inside functions; zoom must apply with no user action,
which makes it this project's first eager state consumer. Restore lives
at the end of install_state_dirs — by definition the moment state
becomes readable, so it cannot be ordered wrongly and a future third
startup path gets it without knowing it had to ask.
Every size write clobbered the family. set_font replaces both fields
unconditionally, so { size = n } alone silently cleared a configured
family until restart.
BITTEN, THREE WAYS. Dropping family preservation fails 3 tests.
Reverting to the framing's own first parser `^(%d+)$` fails 4 including
the seam restore — it anchors to end-of-subject and rejects the
newline-terminated file the writer emits, which is the contradiction
review caught in the framing before it reached code. Hardcoding the
16.0 origin fails the base test.
Also recorded: a loaded crdt run failed two m6_1 PTY tests with
`stty -a output was: ""`. That is R4/R6's empty-content readiness
family, and it means the readiness-helper audit's scope is wider than
three wait_for_file copies under tests/ — src/process.rs's own tests
carry the shape. Undiagnosed, load-sensitive, green isolated and on a
quiet full run; a scope note for that lane, not a registry row, since
the registry judges red CI runs and these were local.
Verified: fmt, clippy, diff-check, --lib 1900/0, crdt 2085/0,
gui_zoom 13/13, journey 47/0, m4 150/0, gpu 221/0, full_grid 1/1.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Zooming a terminal with Ctrl +/- left the TUI showing the previous
frame through the new one. Q#FG1 = A, as approved.
THE RULE WAS ALREADY WRITTEN DOWN, ON A PRIVATE FIELD.
src/instance_render.rs:36 says remote frontends "must blank their local
buffer before applying the deltas" — the binding contract, in the one
place a consumer author will never look. The protocol type said only
that full_grid marks "the initial sync ... versus an incremental
frame": a label, from which no obligation follows. So FG-INV now lives
on InstanceMessage::CellDelta, where whoever writes the next frontend
reads it. A resync is a picture of the screen's INK, not of the screen.
The producer diffs against a blank grid, so a cell that should be blank
produces no span. src/frontend.rs then took `CellDelta { spans, .. }`
and discarded the flag. That was correct for exactly one frame — the
fresh-attach frame, which follows Frontend::new's Clear — and wrong for
every resize after, which follows nothing. A font-size change is the
worst case because the terminal reflows in place rather than dropping
content, so the maximum number of stale glyphs survive.
emit_cell_delta joins emit_span and emit_status_overlay as a pure
helper over a writer; apply_message routes through it. No struct
change, no generic parameter, no new pattern.
WHY SEVEN TESTS MISSED IT. Every one asserts the producer SETS the
flag; none asserted a consumer ACTS on it, and no runtime reader
existed workspace-wide. "Add a test for the flag" had already been
done and did not help. Handoff §5's enforcement-vs-documentation drift,
in a second register.
Three unit witnesses, each bitten independently. The empty-spans case
earns its own test rather than folding into the others: under the
plausible `spans.is_empty()` early return the ordering test still
PASSES and only that one fails — and an empty resync is exactly the
frame whose entire content is the blanking.
The PTY acceptance drives a real SIGWINCH, and its mark is anchored to
CONTENT rather than time. A time-based settle was written first and is
unusable: a settled pmacs screen emits per-frame bytes forever, so
"output stopped growing" never becomes true. Anchoring just past the
first painted byte excludes both startup clears by construction —
Frontend::new clears before any frame exists, and the first frame is
itself a resync whose clear precedes its own spans. Bitten against the
original defect: 34,831 bytes after the first painted frame, no CSI 2 J
anywhere in them.
What it does not prove, stated here rather than found in review: the
suites assert on raw bytes, with no screen model and no vt100/termwiz/
vte dependency. This shows pmacs emitted a blank at the right moment,
not that the screen ended correct.
Verified: fmt, clippy, diff-check, --lib 1900/0, crdt 2085/0, m4 150/0,
gpu 221/0, and the grid-driving suites — full_grid_resync 1/1, vterm
1/2/3 9+9+5, m5_5 15, m5_8 5, bottom_panel_stage1 47.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The scalar contract said "identity" and enforced only "scalar", so two
ways to hold an id that is not one survived.
NaN passes `type(x) == "number"` and then errors at
`p.collapsed[row.id]` with "table index is NaN" — the one scalar Lua
accepts as a number and refuses as a key. Bitten with the check
removed, it reports exactly that, from inside listview, naming no row.
DUPLICATES do not merely collide. Every lookup here — `line_of_id`, and
toggle's scan for the row index — resolves an id to the FIRST row
bearing it, so selecting the second such row toggles the first and
re-seats the cursor onto it: a stray jump with nothing pointing at the
id. Bitten with the check removed, nothing is raised at all.
Both are enforced in `check_ids`, where rows already enter, so the
error names the offending row (and, for a duplicate, both of them)
instead of surfacing as a low-level error or a wrong jump later. The
error text says why, not just what, since the reason is not guessable
from the rule.
Verified: fmt, clippy, diff-check, --lib 1897/0, crdt 2082/0, listview
26/26, m4 150/0, gpu 221/0, bottom_panel_stage1 47/47.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TWO REVIEW FINDINGS, both real, and neither reachable from the existing
tree tests.
1. `item` WAS EFFECTIVELY REQUIRED. `render` writes `line_to_item[n] =
row.item`, so that map is SPARSE whenever a row omits the optional
`item` — and `seat_cursor` took `#` of it. A display-only tree (a
grouping node with `on_visit` unused, which the API explicitly allows)
made that length 0, so the cursor never left the header, TAB found no
row, and folding was unusable. It now counts visible rows explicitly.
The old tests could not catch this because every one of them supplies
`item`: under the reverted fix `tr_5` fails `left: 0 / right: 1` while
`tr_1` still passes.
2. THE ID CONTRACT WAS TWO CONTRACTS. The docs said "opaque, compared
by equality". Selection does compare with `==`, honouring `__eq` — but
collapse state stores ids as TABLE KEYS, and Lua indexes tables by raw
identity, consulting no metamethod. So a table id would satisfy one
half and quietly fail the other: after a refresh minted fresh id
tables, the cursor would be restored and the fold silently lost. A
divergence that shows up as a missing fold, arbitrarily later, with
nothing pointing back at the id.
Narrowed rather than generalized. Equality-aware collapse lookup is the
alternative and it is worse: `hidden_by_ancestor` runs per row, so it
turns a linear render quadratic to support a key type no consumer has
asked for. The contract is now the one both halves can honour — string
or number, compared by value — enforced by `check_ids` where rows enter
(`open` and `refresh`), so a bad id is a named error at the call site
instead of a lost fold much later. Q#TR3 in the framing records the
narrowing and why.
Verified: fmt, clippy, diff-check, --lib 1897/0, crdt 2082/0, listview
24/24, m4 150/0, gpu 221/0. Both fixes bitten independently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#216 landed while this branch was open. Both conflicts are in the
ledger and the handoff, and both sides had independently written up the
same shared-CARGO_TARGET_DIR hazard.
THE TWO WRITE-UPS ARE NOT ABOUT THE SAME OCCURRENCE, and merging them
carelessly would have been a real error. Stage 2's is established: seven
failures against a clean baseline, failure text naming its own cause,
pgrep confirmation, and a dedicated-target-dir re-run at 41/41. This
lane's is the one whose signatures were destroyed before being read —
it has no captured text to match against Stage 2's, and it keeps two
non-causal hypotheses. A mechanism established in one occurrence is not
evidence about a different occurrence that was never characterized, so
the merged bullet says so explicitly rather than letting proximity
imply it.
The ledger records #216 merged and stops saying Stage 2 is in flight.
It does NOT retire the arc, though rule 4 now would: R1 belongs to the
async-runtime lane and R3 is an unresolved possible product defect for
the process-signal lane, and neither has a block yet. Re-homing them is
an absorption pass — not something to fold into a feature PR.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Levi Neuwirth