Revisions 2 and 3 said "the profile is declared at commit_to" and never
said how. That is not a detail. The binding accepts exactly (dest,
body) today, so without a specified form #227 has no stable API to
adopt against, and the promise that existing callers keep their
semantics was a hope rather than a contract.
Q#DC-5: commit_to(dest, body [, profile]). An optional TRAILING string,
typed Option<String>, so there is no arity sniffing and no
table-or-function dispatch on argument 2 --- the existing binding chose
Value over AnyUserData specifically to keep its error message reachable
and naming the rule, and a polymorphic second argument would undo that.
Trailing reads badly after a long inline closure, but that is not the
call shape in use: dired defines a named local commit at dired.lua:670
and calls commit_to(opts.dest, commit) at :717. Verified, not assumed.
Against a named body the trailing profile reads fine.
The value set is CLOSED --- document and panel, exactly Q#DC-2 two
profiles. A third is a decision, not a spelling.
Omitted means document, and that is the load-bearing part: every
existing two-argument call keeps all four preflight checks by
definition of the signature, so journey_acceptance passing untouched
follows from the API shape rather than from care.
An unrecognized profile is an ERROR naming the accepted values, not a
silent fallback to document. A fallback would hand a caller stricter or
looser checks than it asked for, which is the failure the whole
parameterization exists to prevent. Its witness asserts the legacy
two-argument form through a check the panel profile OMITS --- a
stale-buffer refusal --- because asserting merely that it does not
error would pass on a call silently downgraded to panel, which is the
regression that would quietly void Journey Stage 1a guarantees.
Git mapping settled here rather than rediscovered during adoption:
*git-status* takes panel, *git-diff* takes document.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Q#DC-4 contradicted Q#DC-2, and on the primary panel API. Q#DC-2
concluded a panel profile depends only on a live frontend, so it can
commit with no document window at all; Q#DC-4 still voted to return nil
in exactly that case and told git to fall back to ambient behaviour.
Those cannot both hold, and the fallback advice was independently
wrong: falling back to ambient IS the P1a bug this lane exists to
remove.
Decided rather than voted on, since it is the primary API. The
destination document pair is optional; capture_destination() is
profile-blind and argument-free, because making capture profile-aware
would force a caller to know at capture time what it will do at commit
time, which is the opposite of why capture exists. The profile is
declared at commit_to, where Q#DC-2 parameterization already lives, and
a document-profile commit with no document pair is refused alongside
the other four preflight refusals. Capture never returns nil while a
frame exists.
Section 4 outline and Q#DC-1 were updated to match rather than left to
disagree --- Q#DC-1 no-arguments answer is now load-bearing rather than
incidental, because no arguments is what keeps capture profile-blind.
The ledger gate line said "new suite plus dired". --acceptance is
repeatable, so it now carries the executable command including
journey_acceptance and dired_acceptance, both named as preservation
suites and a stop signal. A volatile ledger that understates required
coverage is how a recovering machine runs a weaker gate than the lane
agreed to.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Three review findings.
Q#DC-2 parameterization was incomplete. Revision 1 said only "skip the
stale-buffer check for a non-replacing continuation", but a panel
result does not depend on the captured document window at all: it does
not replace that window buffer (check 3), does not occupy it (check 4),
and does not need that specific window to exist (check 2). Retaining
any of the three can reject git.status for an unrelated document-window
change; dropping them without an explicit profile risks weakening
document replacement. The question now carries a four-row matrix with
two profiles, and check 1 --- the requesting frontend still has a
layout --- is the entire panel profile.
That has a consequence the framing now states rather than leaving to be
discovered: if the panel profile needs only the frontend, a frontend
with no document window can still host a panel, so Q#DC-4 return-nil
rule is right for the document profile and possibly wrong for the panel
one. Settled as part of answering Q#DC-2, not after it.
tests/journey_acceptance.rs joins dired as a named preservation suite
and stop signal. It carries 27 commit_to references across nine named
pins --- forged destination, scope-and-restore on normal return and on
raise, await refusal, delivery to the requesting frontend, the
declining-listener redirect guard, and two already named preservation_*
--- and Journey Stage 1a own framing treats it as a required gate. A
lane that generalizes its substrate does not get to relax that. The
stop signal now covers both suites: a suite edited to accommodate the
change under test has stopped being evidence.
The coherence-impact section was missing entirely. CLAUDE.md and
COHERENCE.md section 25 both require one for coherence-affecting work,
and this lane qualifies twice over --- new Lua API surface, and a
generalization of a Journey substrate. Section 16 is the section it
serves. Journey steps: none added, one protected. Islands, config
registry: none. Section 9: neutral, and stated precisely, because
knowing which frontend a result belongs to is NOT knowing who asked for
it --- that is the worker-identity arc and the two should not be
conflated just because both concern async continuations.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
PR #227 review found that git async completions surface in whichever
frame is active when git exits, and named the right mechanism:
commit_to exists for exactly this continuation boundary, built by
Journey Stage 1a Q#JR14 because the work settles a tick or more later,
by which time the ambient frontend, window and buffer may all name
something else.
The fix is not available to git, which is why this is a lane rather
than a line in #227. commit_to takes a DirectoryDestinationLua that is
nonconstructible from Lua by deliberate design, and the only site that
mints one is inside the path.open-directory listener dispatch, from a
pub(crate) capture. Any async Lua continuation that is not a directory
open has no way to say where its result belongs.
The captured data is already generic --- frontend, window, buffer, with
nothing directory-specific in it. Only the name and the capture site
are, and the rename is 8 references across 4 files, counted rather than
estimated.
The substantive question is Q#DC-2, and scouting is what surfaced it.
Git two continuations are different in kind. *git-status* goes to the
bottom panel, because listview.open resolves display with a "panel"
default. *git-diff* replaces a document window, deliberately, so the
status panel it was invoked from stays visible beside it. The
stale-intent check that commit_to preflight runs --- the window still
shows the captured buffer --- is right for the second and wrong for the
first: the panel never touches that window buffer, so refusing because
the user switched files there is a refusal with no relationship to what
the continuation does. One shape either over-refuses the panel case or
under-checks the document case, and the framing votes for a
parameterized preflight while holding that vote loosely.
No adopter in this lane. Git adoption is #227 work after this lands; a
prerequisite that also converts its first consumer makes the two
impossible to review separately.
Verification carries a stop signal rather than a target: if any
existing dired test needs editing, the generalization changed Journey
Stage 1a semantics and that is cause to stop, not to adjust a test.
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
The lane heading said "no PR yet". It also needs to carry WHY the PR is
merge-blocked, because a reader who finds only "blocked" will treat it
as backlog hygiene and unblock it by rerunning the gate.
The problem is gate integrity. --protocol promises the CRDT workspace
sweep, that sweep has a documented precondition (handoff section 5),
and the script does not run it --- confirmed by reading the plan
emitter, not inferred from the failure. So a --protocol result can be
decided by whether the build directory happened to contain pmacs-gpu
rather than by the diff under test.
It was latent until #225 gave each worktree its own target directory. A
shared target dir usually already had pmacs-gpu built, which satisfied
the precondition by accident and hid the omission.
Unblocking needs both halves recorded: the scripts/gate repair as its
own framing and PR, and then a fresh-target rerun of this branch
protocol gate under the repaired script. A rerun alone would reproduce
the same accident.
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
Revision 3 was approved 2026-08-09 at 6a9a25d, after the three stale
internal references were cleared. The lane heading and the framing
status line both still said "in review", which is the kind of
contradiction that makes a reader trust whichever line they hit first.
The lane entry now also records WHAT the three review rounds found,
rather than only how many there were. A count says the doc was
reviewed; the list says what class of error it was reviewed for, which
is the part a later reader can act on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Revision 3 corrected the cache model in section 3.2 and the clipping
rule in 3.4, then failed to propagate either. Internal contradictions in
a framing document are worse than the original errors: a later reader
resolves them by picking whichever statement they read first.
- Q#D2-4 still summarised the design as "per-variant cache keys",
contradicting 3.2's corrected one-per-peer model. Now names the
`peer_knows_minibuffer_rows` producer gate and the single per-peer
cache, with a parenthetical recording that the sentence was stale
rather than silently rewriting history.
- Section 6 still required the cross-version cache test that
revision 3 removed as unfalsifiable. Replaced with what IS
assertable --- a v22 and a v23 peer attached simultaneously each
receive their own variant and only their own --- and the bullet
says why the other test is absent, so it does not get "restored"
later by someone who notices the gap.
- The active-work lane still called the framing revision 2.
Swept for the same class of residue from the other two corrections
("the name must survive", "both frontends render"); the remaining hits
are the notes ABOUT those corrections, which are deliberate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Three corrections, each to something revision 2 asserted without
checking the mechanism it was reasoning about. All three verified in
the tree.
A ROUND-TRIP FREEZES NOTHING. Revision 2 proposed pinning the legacy
variant with an encode/decode round-trip. That test uses the SAME types
on both sides, so adding a field to `MinibufferPrompt` leaves it
passing while every v12-v22 peer in the field breaks. The existing
`minibuffer_prompt_round_trips_through_postcard` (src/protocol.rs:2363)
is precisely that shape, and there are NO literal byte fixtures
anywhere in the protocol tests today --- checked, not assumed. Only
`assert_eq!(encoded, LEGACY_BYTES)` against bytes captured now can fail
when the encoding changes. Two fixtures, open and clear, matching the
shapes the semantic test already covers so the corpus is not a fresh
judgement call.
THE CACHE HAZARD I DESCRIBED CANNOT HAPPEN. Revision 2 argued for a
per-variant cache key on the grounds that a peer reconnecting at a
different version could have its first message suppressed as a
duplicate. `SemanticRenderState::for_peer(frontend_id,
negotiated_protocol_version)` is created PER PEER with its version
baked in on attach (src/daemon.rs:2080) and removed on detach (:1591),
so a cache never spans two versions and reconnecting creates a fresh
state. One per-peer cache stands; the selection is a producer gate,
`peer_knows_minibuffer_rows`, alongside the four such gates that
already exist (:1410-1435). The test for the impossible condition is
REMOVED rather than written --- a test that cannot fail passes forever
and teaches the next reader that the hazard is real.
The matched open/close family requirement is independent of caching and
stands unchanged.
"THE NAME MUST SURVIVE" IS NOT ACHIEVABLE. Prompt plus typed input
consume the width budget first, so the remaining suffix space can be
too small for even the bare name --- and revision 2's rule would then
have forced a partial name, which reads as a DIFFERENT command. The
rule is now ordered: if the whole name does not fit, omit the suffix
entirely; only once it fits is a description attempted; if the
description does not fit whole, drop it. The guarantee becomes "never a
partial name", which is achievable, rather than "the name always
survives", which is not. Witnessed at three widths.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
COHERENCE.md section 5 grades discoverability Partial after Stage 1 and
names "M-x rows are still bare names". The descriptions ALREADY EXIST
--- `Command.description` is required, and `help.list-commands` renders
them --- so this is substrate without surface in its purest form: the
information is present, surfaced elsewhere, and absent from the one
moment it would change a decision.
TWO REVISION-1 CLAIMS WERE WRONG, both checkable in the tree:
- "Change `candidates` in place and gate at >= 23." Postcard is NOT
self-describing: fields encode positionally, so a v22 peer decoding
`Vec<MinibufferRow>` where it expects `Vec<String>` mis-reads the
bytes rather than skipping them. And gating would not have rescued
it --- with only one variant to gate, a v12-v22 peer would have
received NO MINIBUFFER AT ALL. That variant goes to every peer
negotiated >= 12 (src/daemon.rs:1472).
Revision 2 is additive: `MinibufferPromptRows` APPENDED to the enum
(indices are positional; inserting renumbers everything),
`MinibufferPrompt` frozen for v12-v22, per-peer selection, per-
variant cache keys, and close matching the open's family --- a
mismatched close is how a popup stays on screen forever.
- "Both frontends render label + detail." The grid TUI never reads
`MinibufferPrompt`; src/editor.rs contains ZERO references to it.
It paints from `core.minibuffer` and renders the selected candidate
as `format!(" [{cand}]")` (src/editor.rs:5484). The rich wire
reaches pmacs-gpu only.
So the TUI half is a LOCAL formatting change --- it is in-process
with the core and reads `Command.description` from the registry
directly, with no wire involvement. The contract is pinned
including clipping: THE NAME SURVIVES AND THE DESCRIPTION IS
DROPPED at narrow widths, because a clipped name is strictly worse
than today's bare one. A multi-row TUI chooser is explicitly not
this lane.
This lane HOLDS THE BUMP SLOT. Git Stage 1 is no-wire and runs beside
it; git Stage 2 needs a bump and must wait.
Q#D2-5 records a trap that arrives with the feature: richer rows make
M-x LOOK like a closed set, inviting someone to make acceptance reject
unmatched input. Completion is assistance, not validation ---
`resolve_accepted_value` returns literal typed text by design --- so
that would be a behaviour change, not a rendering one, and it is out of
scope.
Co-Authored-By: Claude Opus 5 (1M context) <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
The .texlabroot caveat is discharged, and discharging it falsified the
reasoning behind it.
.texlabroot is real, and texlab marker set is wider than this document
listed --- the bare texlabroot and latexmkrc spellings count too, and
the walk takes the innermost. The implementation ships texlab own set.
But the caveat framed marker 1 as conditional on texlab honouring the
FILE, which was the wrong question. Every arm of texlab ancestor walk
searches documents already loaded, and that workspace is built from the
folders the CLIENT supplies. Observed: with rootUri at chapters/, no
ancestor marker widened texlab view and its dependency graph never
reached the parent document; with rootUri at the marker directory the
parent resolved either way.
So texlab honours the root it is handed and never corrects a too-narrow
one. config.latex.root IS the project scope, which makes the resolver
the whole value of the lane for a multi-file thesis rather than a
nicety --- the opposite of how section 2 "Slice 1 is one config entry"
reads.
Second correction, and this one was a real trap. Revision 2 said .git
is deliberately excluded from the walk and stopped there. Omitting it
is not sufficient: project_root_for falls through to
pmacs.project.detect when a resolver returns nil, and that walk lists
.git among its markers at src/project.rs:184. A resolver that politely
declined on a markerless file would hand texlab the monorepo by the
back door, with the exclusion looking correct at every line you would
think to read. The resolver never declines for a file with a directory,
and the fixture asserts the shared detector really would have answered
the repo root so the pin cannot pass vacuously.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Section-local edit to the lane's own block in `docs/active-work.md`.
The heading moves from "BRANCHED, framing in review" to "IMPLEMENTED,
gates green, no PR yet", and the §3 `.texlabroot` caveat is written up
as discharged rather than merely resolved: what texlab actually does,
how it was observed, and the one place the framing now reads stale.
The substantive finding recorded here is not "the marker works". It is
that texlab's own root walk only sees markers belonging to documents
already in its workspace, and the workspace comes from the folders the
client supplies — so texlab honours the root pmacs hands it and never
corrects a too-narrow one. That makes `config.latex.root` the project
scope rather than a hint, which is worth carrying forward whether or
not anyone rereads the framing.
Also recorded: the shared-`CARGO_TARGET_DIR` trap, because a bare
`cargo test` in this worktree fails with compile errors from a sibling
lane's code and reads exactly like a broken branch.
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
Recorded as "implementation authorized" rather than "approved", because
that is what happened: the user authorized dispatch after a summary of
revision 2 four corrections, not after returning findings on the
document the way they did for the other four lanes.
The distinction matters for one reason. Section 3 .texlabroot caveat is
unverified and blocking --- texlab 5.25.1 is installed and its version
and CLI were checked, but the CLI exposes only run and inverse-search,
so its LSP-level behaviour was never established. A framing marked
plainly "approved" invites a reader to treat that caveat as settled
prose. It is step zero of the work.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Revision 1 was untracked, on main, in a single checkout. By the handoff
own rule --- work is portable only after it is committed and pushed ---
it did not travel. Committing it here is the first fix.
Three factual corrections, all from checking rather than reading.
haskell-language-server IS installed on this machine, along with its
wrapper. Revision 1 said it was not, and that claim was the entire
basis of its Slice 1 / Slice 2 split. Only OCaml still lacks a server.
The argument for leaving Haskell out survives, but on use evidence ---
the .hs files are a rarely-edited Hakyll generator --- not on
dependency cost.
Slice 1 is smaller than framed. Revision 1 proposed filetype mappings
for .tex/.latex/.sty/.cls "so highlighting and LSP agree on what a
LaTeX file is". They already agree, by construction: the grammar
carries exactly those extensions at src/syntax.rs:1111,
grammar-extension detection sits AHEAD of the LSP filetype map in the
precedence chain per the merged grammar framing at :166-171, and
lsp.lua:267-270 calls the filetype map "mainly the LSP-only fallback".
The real missing piece is one config entry.
Q#LX3 deferral argument read a stale line. COHERENCE.md:124 and :867
both record multi-root LSP affinity as merged in #161; only :1669 still
says "first slice in flight", contradicting the same document twice.
The one item revision 1 said could justify deferring therefore
dissolves. The COHERENCE.md inconsistency is real and is left for
whoever next touches section 20 rather than smuggled in here.
Q#LX2, which revision 1 called the question most likely to make the
entry wrong in practice, is now answered rather than shrugged at. An
upward marker walk through config.latex.root, which already accepts a
resolver function. .git is deliberately excluded: a repository root is
the wrong answer for LaTeX, since texlab wants the document root, and
this is the one place where copying the other fourteen entries
instinct would be actively wrong.
The .texlabroot marker is marked UNVERIFIED and blocking. texlab 5.25.1
is installed and its version and CLI were checked directly, but the CLI
exposes only run and inverse-search, so its LSP-level behaviour was not
established. Confirm against a live session before implementing. Same
discipline the gate-protocol-build lane applies to its own
precondition: the thing the design rests on gets observed, not
reasoned about.
Also renumbered Q#HS1 to Q#LX4, because revision 1 Q#LX2 collided with
a live question ID in the merged grammar framing for the same language.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Three corrections. The first two are the same error in two rows.
I wrote each control as if it DECIDES, when each is informative in only
one direction.
U4: extending the deadline establishes "emitted late" IF the clear
arrives. If it does not arrive, that establishes only "not observed by
the longer deadline" --- not "never emitted" --- because transport loss
produces the same absence. No deadline, however long, separates
non-emission from transport loss. That needs producer-side emission
evidence, did pmacs write the clear, cross-checked against the
collected stream. The row now states both branches and names what the
negative branch cannot conclude.
U5: one isolated run cannot decide whether the gate suite is
implicated. A matching isolated RED proves the gate suite is not
necessary for the failure. An isolated GREEN proves nothing beyond that
run, because the failure is intermittent and absence under one run is
not evidence of dependence. I had written it as though either outcome
settled the question.
This is worth naming as a class rather than two typos: a control whose
positive branch is conclusive and whose negative branch is not, written
up as though both were, is how an inconclusive result gets recorded as
an exclusion. Two rows in this file had it.
Third, minor: "three docs" was accurate at the occurrence tip and is
not now --- #229 has since added this registry file. Replaced with
"documentation" so the claim does not rot again with the next commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Four review findings, all mine.
Normalization. The byte count and the :LINE suffix are
occurrence-specific --- the count is the collected suffix length, which
varies per run, and the line moves with the file --- so neither can be
a required fragment. Both rows now carry stable fragments and an
explicit NOT-fragments field naming what must not be matched on.
The LuaJIT-pass argument was the rejected overreach again. I used a
passing sibling leg as a STRUCTURAL exclusion; a deterministic defect
can be Lua-flavour-specific, so it is corroboration only. The row now
says so in those words. The real grounds are stronger anyway and were
sitting there: the workflow never invokes scripts/gate, and
full_grid_resync_acceptance runs BEFORE the changed gate suite, which
closes even the leaked-state path.
Three contradictions inside U4, each removed.
"It never emitted the blank" asserts a mechanism the next field
simultaneously calls open. Now: no blank was OBSERVED after the mark
within the deadline.
The 25,362 bytes were not a capped window. suffix.len() is the ENTIRE
post-mark output; only the displayed head is truncated, to 400 bytes.
Verified in the test source. So my control --- capture the full stream
rather than the window --- was solving a gap that does not exist. The
gap is arrival TIME, and the control now instruments that instead.
The ~20s failure duration IS the fixed Duration::from_secs(20) timeout,
so the ratio against a fast pass is mechanically determined and is not
independent timing evidence. Also verified in the source.
U5 control relabelled. Running m5_8_acceptance alone decides whether
the gate suite is implicated --- cross-suite attribution --- and
nothing more. It cannot separate injected-before-raw-mode from
raw-mode-lost from a third cause, and another isolated pass cannot
either however often it is repeated. Mechanism discrimination needs
readiness and raw-mode state observed AT INJECTION, which is now a
second, separately labelled control.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
PR #229 first CI run went red on Test (macos-latest / lua54) with
a_pty_resize_blanks_the_host_before_repainting. The rerun turned that
selector green in 0.50s against 20.26s failing, and went red on a
DIFFERENT selector, ctrl_c_during_reconnect_sleep_yields_clean_exit.
Per this file matching rule that is a new incident, not the resize
signature occurring twice, so they are two rows.
U4 records the resize failure with its exact fragments. The diff is
excluded on two structural grounds that do not depend on a rerun: #229
touches no src/ at all and no test but gate_script_acceptance, and the
sibling luajit leg passed on the same commit. A deterministic platform
defect fails both flavours --- that is how #227 non-UTF-8 fixture
presented.
U5 records the Ctrl-C failure at deliberately WEAKER exclusion
strength, and says so in its own field. The changed gate suite ran
earlier in the same job and creates worktrees and directories. No
leaked child or persistent signal-state mutation was observed, but "the
diff touches no src/" is not the same argument here as for U4, because
cross-suite leaked state is a path reachability reasoning does not
close. Its control is to run m5_8_acceptance alone, without the gate
suite ahead of it, before attributing anything either way.
Neither row claims a mechanism. The exit status shows only that Ctrl-C
arrived as SIGINT rather than as the raw-mode key event the test
drives; whether injection preceded raw mode, raw mode was lost, or
something else happened is open, and the fragment does not separate
them.
Also flagged: worker-identity-stage1 independently defines its own U4
and U5. This lane merges first, so that branch must renumber on rebase
--- a conflict resolved textually without renumbering would leave two
different incidents sharing an id, which is the failure the matching
rule exists to prevent.
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
Run 31330601204 at `e816812`: all 14 jobs green, including both
`Test (macos-latest / …)` legs — the two that were deterministically red
on `g6_2`. The macOS half of the fix is therefore OBSERVED, not inferred.
What stays reasoned about is only the explanation — `EILSEQ` itself and
the `lstat`-vs-`ENOENT` argument for why `g6_2c` cannot be made portable
— which a green run can neither confirm nor refute. Kept separate on
purpose: conflating "the suite passes" with "the cause is understood" is
what put an unportable fixture in the suite in the first place.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
PR #227's CI round, in the lane entry: both macOS legs failed `g6_2`
deterministically while Linux stayed green, the fix at `4b82d1e`, and
the durable portability fact behind it — APFS/HFS+ validate pathnames as
UTF-8 and reject an invalid one with errno 92, EILSEQ, so a non-UTF-8
filename is a Linux-only fixture, and it cannot be reached around the
filesystem either because `git status` lstats every index entry and
EILSEQ is not ENOENT.
Also records the coverage split (parse+display and gesture refusal
everywhere; provenance Linux-only and loudly gated), the new
`lua_bytes`/`z_payload_bytes` fixture mechanism and its three-digit
escape rule, what was verified locally versus reasoned about, and the
latent sibling at tests/gpu_invocation_acceptance.rs:621 — which writes
a non-UTF-8 filename but sits behind `#[cfg(feature = "crdt")]`, and the
`crdt-test` job is ubuntu-only, so it is not red today and would be the
day that job gains a macOS leg.
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
The lane entry now carries round 3's single P2 --- a repository root
ending in a carriage return, truncated by round 2's own `\r?\n$` --- and
the tip moves to `39ad43d`. Section-local; nothing outside the Git
integration Stage 1 block is touched, and the P1a merge block is
unchanged and still the reason this lane cannot merge.
What is worth carrying beyond the fix itself is the `-z` finding, so the
next reader does not re-derive it: `git rev-parse` has no `-z` option on
git 2.55, and asking for one makes rev-parse echo a literal `-z` line
ahead of the toplevel at exit code 0. It was checked against the
installed git rather than assumed, which is the whole reason the fix is
a correct strip rather than a different output representation.
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
Revision 5's ledger edits were written while the witness was still the
open blocker, and landed alongside the commit that closed it --- so the
entry asserted both at once: an OPEN BLOCKER bullet saying this lane
"currently ships without the regression guard it was created to
provide", and, further down, that same gap closed at 677fd25. A
recovering machine reads the top of an entry first, so the stale half
is the half that gets acted on. Reconciled in place: the heading, the
framing bullet and the blocker bullet now say re-opened by review and
CLOSED at 677fd25, and point at the bullet that closed it.
The script header cited framing revision 4; it is revision 5.
Also recorded, from auditing whether any OTHER assertion in that suite
is detached from the thing it names: renaming every other plan step ---
fmt, clippy, lib, m4, gpu, sweep, diff-check, acceptance-<suite> ---
leaves all 20 tests green. For most that is only a log filename and a
FAILED: entry. `sweep` is not: the runner's end-of-run listing globs
*-sweep.log and *-sweep-crdt.log, so renaming that step silently
empties the "read these, do not re-run and grep" listing that is the
U2/U3 remedy, with the suite still green. Left open deliberately and
said so --- that listing exists only on the RUN path, and every test in
this file is no-gates by design, so there is no cheap witness for it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Revision 4 said it would close the rename hole for the sweep as well as
the build, and then did not: its verification bullet required only a
singular real-plan pair in the context of build-crdt. The hole is
symmetric --- renaming sweep-crdt slips through exactly the same gap
--- so section 7 now pins BOTH emitter pairs explicitly, name and exact
command, asserted from the emitter where the name still exists.
Two older bullets also still claimed named steps appear in
--print-plan. They do not; that mode prints commands only, which is the
wording that let the attribution witness drift away from the step it
names in the first place. --print-plan is now described as the command
and order witness, and nothing more.
The ledger recorded revision 4 as approved and implemented, and
presented the synthetic self-test as the attribution witness. Both were
read as done. A recovering machine or a PR preparation from that entry
would have shipped this lane without the regression guard it exists to
provide.
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
Review mutated the REAL build step name to sweep-crdt and both existing
witnesses still passed. That is the gap: --print-plan strips names
before printing, so the order assertion sees only commands, and
--self-test hardcodes build-crdt inside its own synthetic plan. Neither
witness is connected to the step it claims to describe, so this lane
shipped without the regression guard it exists to provide.
Section 7 now requires asserting the real emitter (name, command) pair
together, so a rename cannot pass. The synthetic failure and
continuation test stays --- it tests the runner, which is a different
thing --- but it can no longer stand in for attribution of the actual
step.
Also fixed the header, which read "Pre-implementation. Awaiting
approval" through three revisions while the ledger recorded this lane
as approved and implemented. That is the same contradiction class this
project keeps correcting elsewhere, left standing in the document that
keeps correcting it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
The acceptance criterion is witnessed: `scripts/gate --acceptance
gate_script_acceptance --protocol` on a target root that did not exist
beforehand goes green in all eleven steps, with `09 build-crdt ok`
producing `debug/pmacs-gpu` and `gpu_invocation_acceptance` at 15
passed / 0 failed where the same suite is 3 / 12 without the build
step. No manual build anywhere, which is the thing that was false.
An EARLIER attempt at that same cold run went red, and it is recorded
rather than dropped once a later run was green. Fifty failures across
m5_5/m5_6/m5_7/m5_8 --- all real-daemon suites --- with the signature
"daemon exited with exit status: 101 before socket appeared;
socket=/tmp/.tmpXXXX/pmacs.sock" and an EMPTY daemon stderr. Not the
pmacs-gpu signature, and no row in docs/ci-red-signatures.md matches
it.
Re-running the same test binary from the same target directory gave
36/36. Per that registry's own rule a green rerun establishes
INTERMITTENCE ONLY, never environmental cause, so this is left open
rather than blamed on the load it happened under.
What DOES rule out this lane's change is a construction argument, not
the rerun: the root crate declares `default = ["luajit"]`, so
`--no-default-features --features luajit,crdt` enables exactly the same
feature set as the sweep's `--features crdt`. `build-crdt` cannot hand
the sweep a differently-featured binary, so it has no mechanism by
which to break a daemon suite.
Also records that the new assertions were mutation tested --- wrong
features, wrong position, unconditional emission, an aborting runner,
and the build folded into `sweep-crdt` each fail the suite.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
The diff channel's missing ticket, and --- because this was the fourth
recurrence of one shape --- a census of every async continuation in
`git.lua` rather than another instance-by-instance note. Three
continuations, one dispatcher, one synchronous impostor, each with
whether it carries an invocation-time ticket, whether it needs one, and
what shared state it writes.
Also states why the fix is two channels sharing one mechanism rather
than one shared counter: a single counter would make `d` cancel an
in-flight `g`. And records that `state.diff_buffer` is deliberately
still read at continuation time --- "do I already have a live diff
buffer?" is a question about now, not about the invocation --- so it is
not a fifth instance.
P1a's citations re-pointed at `723afa7` and the untouched claim
tightened from "no diff line reaches these names" to something
checkable: `show_diff_buffer`'s body and `open_status_panel`'s
`listview.open` are byte-identical to `4002734`, and no commit on this
branch adds a `commit_to` call anywhere.
Section-local: nothing outside this lane's entry is touched or
reflowed.
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
The durable half of this lane is a boundary question, not the missing
line. `scripts/gate`'s header names handoff section 3 as the owner of
its reasoning, and this precondition lived in section 5's hazard
register --- a coherent cause for the omission rather than mere
oversight. A requirement the script was never told to encode is one it
will keep not encoding.
So section 3 gains it NORMATIVELY: the build joins the protocol-bump
block as a third line, with its own load-bearing bullet covering the
mechanism (pmacs-gpu has no tests/ directory, so cargo never uplifts
its bin), the measurement that makes it conditional, and why it was
latent until per-worktree target directories stopped hiding it.
Section 5 keeps the INCIDENT and its signature, which is history rather
than contract, and now says so: twelve
`gpu_invocation_acceptance::crdt::*` failures on a target directory
with no `debug/pmacs-gpu`, first seen on PR #228's first gate run.
Recast so that seeing the signature again reads as "the script was
bypassed", not "the requirement moved".
The script's header keeps citing section 3 and ONLY section 3. Citing
both would split one executable contract across two homes and weaken
the script's only clean boundary at the same time as Q#GR-4 declines to
build any automated check for prose drift. A boundary that is neither
enforced nor singular is not a boundary.
The ledger records Q#GR-1's observed answer rather than the question:
both sweeps run alone from the same cold disposable target with
`debug/pmacs-gpu` asserted absent beforehand --- default exit 0 with
the binary still absent afterwards, crdt exit 101 with exactly twelve
failures --- plus the silent-skip finding, which is the part nobody was
looking for: a54 reported `ok` in that cold crdt sweep because its only
non-spawning path is its skip branch.
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
Two more P2s, both the same shape as round 1's P1 --- module-level
mutable state read at continuation time instead of captured at
invocation time --- plus the third instance of that shape, which is
still open and which no review finding covers: the diff path has no
generation counter at all, so two `d` presses in flight together are
last-writer-wins on the single `*git-diff*` buffer.
Also corrects a citation round 1 got wrong. The P1a block names two
lines that must not be touched, and its second one (`:854`) pointed at
`local unstaged = …` inside `diff_plan`, not at a display call. The site
was always `show_diff_buffer`'s `pmacs.window.display`. A stale pointer
in a block whose entire purpose is "leave these alone" is worse than
none, so it is corrected rather than silently re-numbered.
Section-local: nothing outside this lane's entry is touched or
reflowed.
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
Two findings, both about a claim that could not be falsified.
The --self-test plan put the failing step last. With the failure last,
a runner that ABORTS on failure and one that CONTINUES produce
identical output, so the witness for Q#GR-2 policy --- the suite keeps
going --- would have passed on a runner doing the exact opposite. The
plan is now three lines with a passing SENTINEL after build-crdt,
asserted to have written its own log. That is the only thing that
distinguishes the two behaviours, and it turns Q#GR-2 from a declared
policy into an observed one.
The plan test also now pins the EXACT command, not only the step name
and its position. A build-crdt running plain cargo build would leave
the gate exactly as unsound while looking repaired --- the crdt sweep
needs those specific features, which is the whole defect.
The ledger still recorded the superseded boundary decision: "section 3
gains it, section 5 keeps the incident, and the script cites both".
Revision 2 replaced that with section 3 as the sole normative home and
the script citing section 3 alone. active-work.md is the volatile
cross-machine record, so a recovering machine reading the stale entry
would have rebuilt revision 1 wrong boundary. Now updated, and it says
which decision it supersedes rather than silently replacing it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
The lane said "normal review; no merge authorization", which is no
longer the state: two of three blockers are fixed and the third makes
the PR merge-blocked behind the destination-capture lane.
P1a is recorded as deliberately NOT fixed rather than outstanding.
commit_to is the right mechanism and is not Lua-reachable outside a
directory open --- DirectoryDestinationLua is nonconstructible by
design and minted only in the path.open-directory dispatch --- so the
fix is a prerequisite lane and this one adopts it afterwards. Recorded
with the mechanical check that no diff line in either fix reaches the
four named symbols, so a later reader does not have to take it on
trust.
The P2 entry keeps the reasoning for teardown over a canonicalized
preflight, because the rejected option is the one that looks obviously
better: a Lua canonicalizer would be a second copy of the Rust alias
table and would go stale the day that table gains a name,
reintroducing this exact bug for the new alias. Keymap::bind is the
authority because it is what decides. Also recorded: there is no
Lua-reachable canonicalization to build on, verified, and no binding
was added to invent one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
Three review findings.
The normative build requirement goes entirely into handoff section 3.
Revision 1 proposed section 3 gaining it while the script header cited
both sections, which splits one executable contract across two homes
and weakens the single clean boundary the script has --- at the same
time as Q#GR-4 declines to build any automated check for prose drift. A
boundary that is neither enforced nor singular is not a boundary.
Section 5 keeps the incident and its signature, which is history rather
than contract.
Q#GR-1 observation procedure was unsafe and insufficient. "Delete
pmacs-gpu from a target directory" mutates a live worktree build
directory, and removing one binary does not establish that the other
artifacts and feature permutations are cold --- a stale dependency
graph can satisfy the run for reasons the experiment never sees. Now: a
disposable target, the binary asserted ABSENT before each run as a
recorded precondition, and the two sweeps run separately so neither can
be explained by the other having built the binary first. That last
point is the same accident that hid this defect for the whole life of
the shared target dir.
The attribution criterion had no feasible witness. gate_script_acceptance
deliberately runs no gates, so plan assertions prove name and order and
nothing about runtime behaviour. The obvious seam is a trap: making
PLAN_FILE injectable would turn the script into a general command
executor through its runner eval --- the same class of defect this
script own review already caught in --acceptance and fixed with a
parse-time refusal. Reintroducing it one lane later, in the tool whose
purpose is to be trustworthy, is not a trade worth making.
Q#GR-5 proposes --self-test over a HARDCODED two-line synthetic plan,
true and false, with the failing one named build-crdt. No injection,
no real gate, and it tests the thing actually under test: whether the
runner names the right gate when a command fails. Whether cargo build
really fails is cargo business.
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
--protocol promises the CRDT workspace sweep. That sweep documented
precondition is cargo build --workspace --no-default-features
--features luajit,crdt (handoff section 5:532-535), and the plan
emitter at scripts/gate:187-204 has no build step at all --- read from
the source, not inferred from the failure.
The interesting part is why it stayed invisible. Before #225 every
worktree on this machine resolved to one shared CARGO_TARGET_DIR, which
almost always already contained a pmacs-gpu binary, so the precondition
was satisfied by accident on essentially every run. Per-worktree target
dirs start empty. So this is not a bug #225 introduced; it is a
pre-existing gap in the documented procedure that #225 stopped hiding.
That also decides the urgency. A red gate is fine --- it stops you. The
hazard is the reverse: a GREEN --protocol run whose crdt sweep was
decided by what happened to be in the build directory rather than by
the diff. A gate reporting coverage it does not have is exactly what
#225 exists to prevent, so the tool shipping with this gap teaches the
opposite of what it is for.
Observed on PR #228 first gate run: twelve
gpu_invocation_acceptance::crdt::* failures, all "build pmacs-gpu
before this acceptance suite", with debug/pmacs-gpu absent from the
fresh target dir.
The durable half is a boundary question rather than a missing line. The
script header names handoff section 3 as the owner of its reasoning,
and this precondition lives in section 5 --- a coherent cause for the
omission, not oversight. Q#GR-3 proposes section 3 gains it, section 5
keeps the incident and its signature, and the script stops naming
section 3 as its only source.
Q#GR-1 is marked as the one thing this lane will not accept on
reasoning: whether the default sweep also needs the binary must be
established by deleting it and running both sweeps. The whole defect is
a precondition nobody checked, and establishing its replacement by
reading would repeat the error at one remove. The mechanism section
states its own inference (the failing tests are namespaced ::crdt:: and
so are probably feature-gated) and marks it unverified.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai
The lane heading still said the PR was not opened. Per the standing
correction from #171 and #215, the PR number belongs in the ledger when
the PR exists, not when review asks for it.
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
Levi Neuwirth