Documentation only; no code, no protocol change.
Handoff §1 anchors on `main` @ `fe8b8ba` and leads with terminal copy
mode (#178) and its landed-doc pair (#168). The active-work terminal lane
flips to BOTH STAGES MERGED, and the #166 lane records #168 landing with
what it measured: 264 tests dark workspace-wide for want of `crdt` in CI,
and the vterm audit's honest count of acceptances that drive a real
daemon (2, not 3).
The substantive addition is a new §4 substrate invariant for
`Buffer::set_generated_contents`, because it is now the one authorized
write for every generated buffer and three of its properties are
non-obvious enough that four review rounds were spent finding them:
- an intercept is not read-only — `Buffer::undo` reaches the rope through
`ensure_writable` without consulting the intercept chain, and rebinding
the undo chords does not help because `M-x buffer.undo` needs no
keymap;
- a bare `set_read_only` would be worse than nothing, since it also
refuses the owner's refresh — the pairing is the primitive, which is
why no Lua `set_read_only` exists;
- a rope write is only half of an edit: the returned `Edit` must be
fanned out, or a displaying window paints the new rope with stale line
offsets and replica mirrors never import the write.
Also recorded: history clearing must clear whichever history the buffer
has (the v0.1 stacks are bypassed in CRDT mode), that `*compilation*` and
listview have NOT adopted the primitive and remain emptiable by
`M-x buffer.undo`, and that this does not replace `set_round_trip_input`
— the two guards cover different copies, and a daemon refusal arrives
after the frontend has already painted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gGQC6eqHJVbZJ5Hg7aLer
Re-integrated after #168 landed the ledger refresh. Five conflicts, none
resolved by taking a side wholesale:
- Both head-of-main anchors take main's a27f646 wording (written by
#168); the ledger keeps this branch's caveat about lanes naming an
older base.
- The dired lane section is pruned, which is this PR's purpose, but main
had meanwhile pruned the GPU terminal input lane as landed — so the
union drops both stale lanes rather than resurrecting either.
- "Closed since the last snapshot" keeps BOTH new entries: dired Stage 1
#165 from this branch and GPU terminal input #166 from main.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RuhVYUPHXMHG8r2z4tsDPR
Same one-line ledger conflict as the previous integration, now against
the refreshed main (#168): keep this lane's PTY terminate section, take
main's newer Lean 4 lane heading.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RuhVYUPHXMHG8r2z4tsDPR
The handoff's "last updated" paragraph conflicted; resolved by writing
the current statement rather than taking either side, and the same pass
clears the staleness the ledger had accumulated while five PRs contended
for it:
- Both head-of-main anchors (handoff §1 and the ledger's canonical base)
now name a27f646, and list terminal config #173, the bottom-panel
Stage 2 framing #175, Stage 2A #177 and Lean 4 Stage 4a #179 — plus
the CRDT undo repro #157 and the inline-math landed-doc refresh #172,
which are merged but had dropped out of both lists.
- The bottom-panel lane records Stage 2A as MERGED (#177 @ 0a3fcd1)
instead of IN REVIEW, and states what 2B is: approved, unstarted,
branching from main rather than stacking, with §7.2's criteria and
§5.3's three-boundary trap named.
- The Lean 4 lane records Stage 4a as MERGED (#179 @ a27f646); its
"framing-only, no code, awaiting approval" bullet was false.
The dired lane is left alone deliberately: #169 is the PR that records
dired Stage 1 as landed, and owns that heading.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RuhVYUPHXMHG8r2z4tsDPR
Three conflicts, resolved by taking the newer statement on each side
rather than either side wholesale:
- COHERENCE.md journey row 7 keeps this branch's text (dired #165 is
merged, not a PR); row 8 takes main's, which records terminal config
#173.
- The ledger's canonical-base line takes main's d400f30 anchor and keeps
this branch's caveat about lanes that name an older base.
- agent-handoff's two anchors take main's d400f30 wording, with the CRDT
undo repro #157 and the inline-math landed-doc refresh #172 added
back — both are merged and main's summary had dropped them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RuhVYUPHXMHG8r2z4tsDPR
Review round 4, P2. A fix can invalidate a test that was never written.
Criterion 17 still specified the pre-round-2 world: remove
`set_round_trip_input` and the optimistic op "passes `ensure_writable()`
and mutates BOTH sides, silently, with no divergence to notice". That
was true while no Lua binding set `read_only`. Since
`set_generated_contents` does, the daemon refuses the op — so only the
frontend's own mirror mutates, and the copies diverge.
The gap matters precisely because 17 is unpinned. A real-GPU test
written to the old spec would hunt for a daemon-side edit that can no
longer occur and pass for the wrong reason, quietly readmitting the
round-2 regression through a test not yet built. The specification is
the artifact under review here, not the code.
Restated around unauthorized MIRROR mutation plus daemon refusal —
divergence — in all four places carrying the obsolete claim: the
criterion itself, the Q#TC6a heading, the acceptance-16 doc comment, and
the bite roster. The heading's "ONLY thing" now says what it is the only
thing FOR: the replica's own mirror. `docs/active-work.md` also still
described acceptance 16b as asserting `is_read_only()` is false, which
round 2 flipped.
Why round-trip input stays load-bearing rather than redundant, now
stated wherever the daemon guard is mentioned: a refusal arrives after
the frontend has already applied optimistically and painted. It buys
divergence instead of silent agreement; it does not prevent the mutation
the user is looking at.
Also recorded, after capturing it properly this time: the gate-run flake
in `cargo test --lib --features crdt` is
`process::tests::setsid_escapee_is_not_reaped_and_teardown_reclaims_readers`
(`active_reader_probe` -> None, "live runtime probe"), ~1 run in 5.
Pre-existing and unrelated — this branch does not touch
`src/process.rs`, the test passes 10/10 standalone and 2017/2017 at
`--test-threads=1`, and it is another instance of the known `drain_until`
trap: draining for `Started` also ticks, and a tick reaps the leader.
That also explains the unattributed "2 failed" run noted in round 2.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gGQC6eqHJVbZJ5Hg7aLer
Resolves the ledger conflict: keep this lane's PTY terminate section and
take main's newer Lean 4 lane heading verbatim. No content of either
lane is rewritten here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RuhVYUPHXMHG8r2z4tsDPR
Q#LN10 still said a throwing consumer "fails the fan-out for everyone."
It does not: `run_all_must_succeed` (src/hook.rs:332) collects the error
and continues to the hook's remaining subscribers, so `lsp.lua` still
flushes didChange. The throw stops every LATER consumer in the chain,
which is a narrower consequence and still worth containing — the
failure is silent exactly where the abandoned consumers registered.
The module comment, criterion 46d, the test, and the ledger were all
corrected in the previous commit; Q#LN10 is the decision they descend
from, so leaving it stale would have made the disproven claim the
authoritative one. Also records the protected-rendering rule there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011LFvC4FQtux4y32KuevZ7B
Review round 3, on the round-2 primitive itself. One lesson covers all
three findings: a rope write is only half of an edit, and "discard
history" means whichever history the buffer actually has.
P1 — the binding swallowed the edit. `set_generated_contents` returned
`()`, so nothing reached `notify_buffer_edit_to_windows`. Two
consequences, both reproduced by the reviewer. In the default build a
window showing the buffer kept a `TextView` line index describing the
PREVIOUS contents, and the next paint indexed the new rope with stale
ranges — `assertion failed: end <= self.len()` in `src/rope.rs`. In the
CRDT build `pending_crdt_ops` stayed empty, so replica mirrors never
imported the owner's write and their optimistic edits were generated
against content already replaced. The `delete`+`insert` pair this
replaced had done that fan-out for free.
Now applies ONE whole-buffer `Replace`, returns its `Edit`, and notifies
from the binding. The doc comment states the obligation, because the
next owner to adopt the primitive inherits it.
P2 — "discard history" was false in CRDT mode. The v0.1 stacks are
bypassed entirely there; the history lives in loro's `UndoManager`.
`read_only` stops the replay but not the retention, which is the memory
cost the contract claims to eliminate. `UndoManager` exposes no clear,
but needs none: it records only what happens after it is constructed,
the same property `CrdtState::from_bytes` already uses to keep the seed
insert out of undo. `CrdtState::clear_undo_history` rebinds a fresh
manager to the same doc.
P2 — the docs described the pre-fix architecture. Q#TC6a said no Lua
binding sets `read_only` and round-trip input is the only guard; the
acceptance text still said `is_read_only() == false` while 16b had been
flipped to true; `terminal.lua`'s comment repeated the obsolete claim.
The architecture is layered and now says so: rope-level read-only
protects the daemon copy, round-trip input protects the replica's
optimistic mirror, and neither substitutes for the other. Q#TC6a keeps
its analysis under a superseded-in-part box rather than being silently
rewritten — its conclusion survives, two of its premises do not.
New pins. acc16d paints the window after a SHRINKING generated write:
stale offsets then point past the buffer end, so the failure is the
reported crash rather than merely stale pixels. acc16e asserts the
refresh is queued for mirrors, through the real copy-mode path;
`crdt`-gated and therefore dark in CI, which is why 16d drives the
binding rather than the terminal. Plus a CRDT unit test that ten renders
leave the `UndoManager` with nothing recorded.
Bites: dropping the notify panics acc16d at `rope.rs:145` and fails
acc16e with `queued: []`; dropping the `UndoManager` rebind fails the
new unit test on `can_undo`.
Still open, and recorded in COHERENCE.md §14: the fan-out obligation
makes `*compilation*`/listview adoption more than a one-line swap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gGQC6eqHJVbZJ5Hg7aLer
Five defects in the chain itself, plus the stale handoff state.
Each consumer now gets its own shallow copy of the typed-edit record.
Handing everyone the same table let a DECLINING consumer rewrite
provenance for the ones behind it, and pairing decides what to close
from `rec.char` — so a forged `char` turned a typed `x` into `x)`.
Every field is a scalar or an opaque id, so a shallow copy is complete.
The fan-out iterates a snapshot of the consumer list. It was iterating
the same array `add_consumer` mutates: a consumer that registered a
lower-priority one shifted itself forward under `ipairs` and ran twice,
and re-registering made that unbounded. Registrations and removals made
during a fan-out now take effect on the next one, stated as a contract
and pinned in both directions.
`tostring` on the caught error moved inside the containment. A Lua
error may be any value, including a table whose `__tostring` throws —
rendering it outside the `pcall` reintroduced exactly the escape the
containment exists to prevent.
Priorities are validated as finite integers in i32 range, matching
`pmacs.completion.register`. NaN is a number and every ordered
comparison with it is false, so a NaN consumer landed wherever the
insertion scan gave up and silently voided the lowest-first ordering
that Q#LN22 depends on.
`add_consumer` returns a handle and `remove_consumer` unregisters it,
reporting whether it was live. Without teardown the chain inherited the
`pmacs.hook.add` callback leak COHERENCE.md §13 already records, and
spread it to every consumer.
Also corrects the rationale the containment was documented with, in the
module, the test, and the framing: an uncontained throw does NOT take
the fan-out's other subscribers down. `run_all_must_succeed`
(src/hook.rs:332) collects errors and continues, so lsp.lua still
flushes didChange. The containment is still required — the throw skips
every later consumer in the chain — but the reason is narrower than
rev 7 claimed.
Criteria 46f (record isolation), 46g (snapshot iteration), and 46h
(lifecycle and priority validation) added; 46d's rationale corrected.
Four new tests, all bite-verified by mutation, each failing only its
target: shared record table (1), live-array iteration (1), unprotected
tostring (1), bare number check (1), no-op removal (2). The suite also
runs green under `--features lua54`.
docs/agent-handoff.md said Stage 4a was awaiting approval while this
branch had it implemented and in review.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011LFvC4FQtux4y32KuevZ7B
**P1-1 — layout invalidation could suppress the authoritative clear.**
Real bug. Both render paths resolved the document identity AFTER the
evaluator ran callbacks, but BOTH outcome arms carry PHASE-1 contexts.
A provider that closes the primary document split changes
`primary_document_window` mid-evaluation, so the filter compared
phase-1 contexts against a replacement identity, matched nothing, and
emitted no clear — leaving stale statusline text on the wire forever.
The identity is now captured BEFORE `evaluate_statusline` runs and
threaded through both paths (the terminal path via `terminal_chrome`).
Pinning it took three attempts, and the two failures are the useful
part:
- `pmacs.window.close()` takes no argument — it closes the ACTIVE
window. The first version passed a window id that was silently
ignored, so it closed the panel instead of the document.
- The Lua window API acts on the ACTIVE FRONTEND, so driving it against
a synthetic semantic view changed nothing at all.
- Closing the only document window is structurally REFUSED (Q#BP6
forbids a lone side window as a resting state), so the fixture needs
TWO document windows for the close to be legal.
The test now asserts its own precondition — that the callback really
changed the identity — before asserting the clear, and reproduces the
reported symptom (no `StatuslineSegments` at all) when the fix is
reverted.
**P1-2 — #21 was pinned at the helper, not the producer.** Confirmed:
reverting only the call site inside
`publish_buffer_snapshot_to_replicas` left both the helper test and the
existing socket-pair test green. The helper assertions are removed (with
a note saying why) and replaced by
`snapshot_publication_follows_the_document_under_a_focused_panel`, which
drives the real producer over socket pairs and asserts BOTH directions:
the document buffer's snapshot is delivered while a panel holds focus,
and a panel-only buffer's is not.
Biting that test exposed a defect in the test itself: the delivery read
had no timeout, so a regression made it HANG rather than fail. A hanging
test is strictly worse than a red one — every read now has a timeout.
Gates: fmt clean; workspace clippy clean; 1,832 default + 2,015 CRDT
library; Stage 2A 17; Stage 1 46; statusline 8; m11_5 2; GPU initial
target 14; terminal config 12; folding Stage 2 48; vterm 1/2 10 / 6;
M4 121; required GPU 202; `git diff --check` clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review round 2, P1. Undo could empty the "read-only" snapshot.
`render_snapshot` wrote with bypass_intercept, which leaves ordinary
undo history behind, and `Buffer::undo` reaches the rope through
`ensure_writable` without ever consulting the intercept chain. So a
single `C-/` — or `M-x buffer.undo`, which needs no keymap at all —
replaced a freshly rendered snapshot with an empty buffer.
`set_round_trip_input` does not help: it routes the key into the daemon
command path, which is exactly where undo runs.
Rebinding the undo chords buffer-locally would not have closed this,
and `compile.lua` already says so in a comment: "command/menu undo
stays dispatchable". `*compilation*` and listview panels therefore
carry the same latent defect today.
Adds `Buffer::set_generated_contents` (Lua:
`pmacs.buffer.set_generated_contents`): lift `read_only`, replace the
contents skipping intercepts, discard the resulting history, re-assert
`read_only`. This ships the framing's deferred immutability lane as ONE
primitive rather than exposing the setter — a bare `set_read_only`
would let a caller lock a buffer it can no longer refresh, which is
precisely why that lane was deferred. Discarding history is
load-bearing twice: it removes what undo would replay, and it stops a
periodically refreshed buffer accumulating rope clones that `read_only`
guarantees nothing can ever pop.
New acceptance 16c drives the real M-x path
(`command.invoke_interactive`), the chord, and redo, and asserts the
owner's own refresh still works — the operation plain `read_only` would
have broken. Acceptance 16b flips from asserting `is_read_only()` is
false to true, because the property it documented is the one that was
wrong. Three `buffer.rs` unit tests cover the primitive directly,
including that ten refreshes leave an empty undo stack.
Bite: restoring the delete+insert render reproduces the report exactly
— `left: Some("")` against the full snapshot — failing 16c and 16b.
Still open, and now named in the framing, COHERENCE.md §14 and the
ledger: `*compilation*` and listview have not adopted the primitive and
remain emptiable by `M-x buffer.undo`; a streaming-friendly variant is
needed for the append case. In CRDT mode `read_only` is what refuses
undo, since loro's UndoManager exposes no clear through `CrdtState`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gGQC6eqHJVbZJ5Hg7aLer
Verification describes the pushed tree, per the standing rule. Includes
the bite that was worthless as first written: moving only typed_edit.lua
past lsp.lua broke the runtime load instead of testing flush ordering.
A bite that kills everything has not isolated anything.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011LFvC4FQtux4y32KuevZ7B
`pmacs.editor.take_typed_edit()` is one-shot and per-frontend (Q#AP9):
the first `buffer.after-edit` callback to call it clears the slot, and
every later callback in the same fan-out sees nil. That was survivable
only because auto-pairing was the sole consumer — never a property
anyone chose. A second independent caller would get nil or steal the
record from pairing depending on hook registration order, and
registration order is not a contract.
This makes it one. `builtin/runtime/typed_edit.lua` owns the single
after-edit subscriber that reads the record, and offers that one read to
consumers registered through `pmacs.typed_edit.add_consumer{ name,
priority, fn }`: lowest priority first, ties by registration order, and
the first consumer to return truthy claims the edit and stops the chain.
`pair.lua` becomes that chain's only consumer, at priority 100.
No Lean content. Stage 4b's abbreviation expander is what needs the
ordering guarantee (64 of its 1,855 keys contain a `lean4` pair-set
character, so pairing running first corrupts them), but the chain is
substrate every language runs through, which is why it ships alone —
framing Q#LN10, and §4's rule that no PR in this arc mixes a
cross-cutting substrate change with Lean feature content.
Three design points worth review attention:
- Consumers are called even when the record is nil. "This fan-out
carried no typed edit" is information a consumer acts on: it is how
pairing's test seam observes a non-event, and how Stage 4b will
abandon a pending abbreviation an unrelated edit invalidated. Three
existing auto-pairing tests fail if the chain skips consumers on nil.
- The chain pcalls each consumer. `buffer.after-edit` is
all-must-succeed, so a throwing consumer would otherwise fail the
fan-out for every other subscriber, including lsp.lua's didChange
flush. Behavior-preserving for pairing, which already never throws.
- Ordered insertion, not `table.sort`, which is not stable in Lua —
"ties by registration order" is a stated contract, not a coincidence.
`tests/auto_pair_acceptance.rs` is UNCHANGED — zero lines — and its 45
tests pass. That is criterion 46 and the whole no-behavior-change claim;
a suite edited to accommodate the refactor would prove nothing.
`tests/typed_edit_chain_acceptance.rs` adds 9 tests for criteria
46a-46e. Every one is bite-verified by mutation: appending instead of
ordered insert (5 fail), `>=` for the tiebreak (1), re-taking per
consumer (4), ignoring the claim (1), dropping the pcall (1), skipping
nil fan-outs (1 here plus 3 in the untouched auto-pair suite), and
loading the chain after lsp.lua (the Q#AP7 flush test fails, alongside
the two existing pairing ones).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011LFvC4FQtux4y32KuevZ7B
Advance the Stage 4 framing to revision 8. Keep pending abbreviation
state frontend-owned while conservatively invalidating it after any
intervening shared-buffer edit, make the revision token explicit, and
rewrite acceptance 45i around that contract.
Correct the active-work multi-codepoint count and the stale coherence
revision label.
Four findings, all real, and they rhyme in pairs. Two implementation
defects and two vacuous pins, all four tracing to one root: a name is
not an identity, and a context-free readout is not a state observation.
A foreign buffer carrying the snapshot's name was adopted and then
overwritten. `pmacs.buffer.create` accepts any caller-chosen name and
snapshot writes use bypass_intercept, so found-by-name adoption
clobbered user data — the reviewer reproduced "do not clobber" becoming
23 newlines. Now follows dired's F7 rule: ownership means "in copy
mode's own handle table", never "found by name", and a taken name yields
a `<2>` variant.
Snapshot identity was keyed by terminal NAME.
`TerminalManager::open` uniquifies only the derived name — an explicit
`name = ...` is inserted verbatim — so two valid terminals can share
one, and a name-keyed table handed them a single snapshot: the second
invocation retargeted it, `q` returned to the wrong terminal, and
killing either removed the shared buffer. Identity is now the terminal
buffer, compared in an array, because BufferIdLua implements `__eq` but
each wrapper is a distinct table key: comparison works, hashing does
not. The kill-with-terminal callback now closes over its own record
rather than looking the name up again.
The refresh pins were vacuous. Acceptance 19 compared a quiet
terminal's snapshot against itself and 18 counted buffers, so both
passed with render_snapshot replaced by a no-op. The child is
`exec cat`, so the tests now type a marker into the focused terminal,
require it ABSENT from the existing snapshot, and only then refresh —
via `g` and via re-invocation respectively.
The tail-follow pin could not observe view state.
`TerminalManager::snapshot(buffer_id)` is context-free and always
returns the live screen, so it reported "at the tail" even for a view
forced to the oldest retained row. Now read through
`snapshot_for_view`'s at_bottom and its projected cells.
Adds acceptance 18a (a foreign same-named buffer is never adopted or
clobbered) and 18b (two same-named terminals get two independent
snapshots, each `q` returns to its own source, and killing one leaves
the other's snapshot alive).
Four new bites, all discriminating: restoring adopt-by-name fails 18a
AND 18b; restoring name-keyed identity fails 18b; making
render_snapshot a no-op fails BOTH 18 and 19, which is the vacuity
demonstrated rather than argued; and forcing the view off the tail
fails 20.
Criterion 17 stays a named follow-up, per review agreement, until the
real GPU probe is non-skipping and CI-executed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gGQC6eqHJVbZJ5Hg7aLer
The 4a/4b split held; five P1s against rev 6's own content, all real,
all reproduced. Four share a root: rev 6 verified its external facts and
under-verified its internal ones.
1. Stage 4a's declared footprint excluded the tests its own acceptance
required. 46a-46e cannot live in tests/auto_pair_acceptance.rs,
which criterion 46 requires byte-identical. Footprint now names
tests/typed_edit_chain_acceptance.rs and gates on it.
2. Pending abbreviation state had the wrong owner. pmacs is
multi-frontend: EditorCore.views is per-FrontendId with its own
active window, take_typed_edit is already frontend-keyed, and
buffer.after-switch fires with no arguments — so a buffer-keyed
clear-on-switch lets any frontend discard another's pending
abbreviation. Now keyed (frontend, buffer) with a window check,
frontend-scoped clearing, a frontend.detached purge, and acceptance
45i, which the buffer-keyed design passes every other criterion
without.
3. The shortest-match rule was missing its tie-break: upstream keeps
declaration order among equal-length shortest keys, and 101 prefixes
have equal-shortest candidates resolving to different symbols (f
picks f< over f>). A pairs-iterated Lua map cannot express this, so
the vendored artifact is now an ordered sequence and resolution sorts
by (#key, source rank). Rev 6 missed this because it declared the
package ships no README after a 404 on the package root, with the
directory listing showing src/README.md already in hand — a 404 on a
guessed path is not evidence of absence, and the README states the
rule in one sentence.
4. The generator's rejection rule rejected the current table: \ is a key
and " begins eleven, while acceptance 45d requires \ to work.
Replaced with canonical lossless escaping; aborts only on duplicate
keys, invalid UTF-8, and a failed self-round-trip. 45g no longer
claims to diff against abbreviations.json, which is not shipped.
5. Durable and volatile state were not reconciled. agent-handoff.md
anchored main at d152120 with neither #167 nor #170 and no Lean arc
bullet at all; active-work.md kept 407 lines of merged Stage 1/2/3a/3b
history against its own instruction to prune merged entries, under a
stale snapshot date. Durable facts moved to the handoff; the ledger
keeps only the unlanded Stage 4 lane.
Also corrected: 119 multi-codepoint symbols (26 with $CURSOR), not 93;
three backslash values, not two; Q#LN22 now states the terminating-\
reprocess rule acceptance 45d depended on; acceptance 38 says the
terminator is retained, so undo restores "\alpha " with its space;
coherence cites golden-journey step 5, not step 4; and the
config-registry prior art points at Q#LN22.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011LFvC4FQtux4y32KuevZ7B
**P1-1 — the `Invalidated` arm published the panel context on the
document wire.** Real bug, and the live half of the routing defect: the
semantic peer has ONE statusline slot, so emitting an
authoritative-empty payload for every context replaced the document's
with the panel's. Now filtered by document-window identity exactly like
the `Ready` arm; a panel's own clear belongs to `PanelFrame` in 2B.
Pinned by `invalidated_statusline_clears_only_the_document_not_the_panel`,
which reproduces the reported shape — two targets instead of one — when
the filter is removed.
Honest note on the `Ready` arm: its identity selector is **defensive**,
not independently falsifiable today, because the document context is
captured first so "first context for my frontend" happens to pick it.
Rather than leave that as a silent dependency,
`the_semantic_fan_out_captures_the_document_first` pins the order and
says why it matters.
**P1-2 — round-1 finding 3 was not closed; four of my pins were
vacuous.** All four confirmed and fixed:
- The statusline consumer test discarded `render_frame`'s output. It now
observes the WIRE payload from a v18 peer with a registered provider,
and asserts non-emptiness so it cannot pass by emitting nothing.
- The terminal test compared two NON-terminal buffers, so both routings
answered `false`. The document window now holds a REAL terminal, so
the routes disagree; reverting `semantic_terminal_key` fails it.
- The decorations test used different buffers and an empty selection —
again the same answer either way. The panel now displays the declared
buffer with a non-empty selection while the document has none.
- #1/#3/#21 had no discriminating pin at all. Their only production
caller is `dispatcher_loop`, which no test can drive, so this extracts
three named seams the loop calls — `document_buffer_to_follow`,
`document_cursor_byte`, `peer_displays_buffer_as_document` — and pins
each.
Also newly pinned: #2 the lazy CRDT upgrade (the census's sharpest
case), #7 `Viewport` aligning WITHOUT taking focus, and #9 a focused
terminal panel not suppressing the document viewport.
**Every one of the nine pins was falsified by revert.** Two needed a
second attempt after the first bite came back green.
**P2-3 — stale docs.** `StatuslineEvaluationTarget::Semantic`'s
documentation described evaluating only the focused window; it now
describes the document-plus-side fan-out, the capture order, the
identity-selection requirement, and that `active` reports actual focus.
The ledger's Stage 2A entry is corrected to five commits, 2,014 CRDT
tests, and 16 acceptance tests.
Two clippy findings the refactor introduced were fixed:
`document_buffer_to_follow` is `crdt`-gated to match its only caller,
and the `CursorByte` guard collapses into one `if`.
Gates: fmt clean; workspace clippy clean; 1,832 default + 2,014 CRDT
library; Stage 2A 16; Stage 1 46; statusline 8; m11_5 2; GPU initial
target 14; terminal config 12; folding Stage 2 48; vterm 1/2 10 / 6;
M4 121; required GPU 202; `git diff --check` clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stage 2 of docs/terminal-config-and-copy-mode-framing.md (rev 4,
approved). `M-x terminal.copy-mode`, or `C-t` in a terminal buffer —
physically `C-c C-t`, since every unescaped key goes to the child —
materializes the retained scrollback into an ordinary read-only,
path-less buffer, with `g` to re-snapshot and `q` to return.
No protocol change.
Materializing is the whole design. isearch, motion, selection and the
kill ring work with no new substrate because the snapshot is a rope, so
SearchStore and the existing match painting apply unchanged. And "keys
must not reach the child" dissolves structurally rather than being
guarded: the transport arm keys on is_terminal(buffer_id), and a
snapshot is not a terminal, so the arm never fires. The
dispatch-shadow count stays at six and describe-key keeps telling the
truth — asserted directly, since that is the observable difference
between the buffer-local idiom and a shadow.
One serializer, not two (Q#TC7). `copy_retained` builds a whole-range
selection and hands it to `copy_selection_bytes`; a second walk would
re-derive soft-wrap joining, wide-glyph continuation, cluster bytes and
per-row trailing-blank trimming, and the two would drift. Four unit
pins in view.rs assert exact bytes against the same projection fixtures
that pin the serializer itself.
Q#TC6a is implemented as two calls, and the second is the load-bearing
one: an intercept guards dispatch only, and no Lua binding sets
Buffer::read_only, so set_round_trip_input is what keeps a replica
frontend from applying optimistically and emitting an op that would
pass ensure_writable and mutate both sides. Acceptance 16 pins that
UNGATED, because CI never compiles the crdt feature.
Eight of nine criteria. Criterion 17's semantic-frontend end-to-end pin
is deliberately absent: the optimistic apply lives only in
pmacs-gpu/src/main.rs and the headless SemanticClient has no optimistic
path, so a faithful test needs the real GPU binary — the a37
foundation, which CI never compiles, silently returns ok when the
binary is unbuilt, and is load-sensitive. Both halves of the mechanism
are pinned ungated instead (16, and 16b for the hazard); the wire-level
half stays an explicit obligation of the CI crdt-coverage lane.
Substrate fact found while wiring lifecycle: TerminalManager::prune
REACTS to a buffer already gone from the registry rather than removing
one, so a child exiting leaves both the terminal and its snapshot
alive. That is why on_removed is a sound teardown hook, and why a
finished command's output stays readable.
Five bites, five different wrong implementations, each failing exactly
one test: removing set_round_trip_input fails acceptance 16 in the
DEFAULT configuration; a naive independent serializer fails all four
unit pins, with the diffs naming each drift mode; making re-invoke
create a fresh buffer fails 18; dropping the kill-with-terminal
teardown fails 18; removing the intercept fails 16b.
COHERENCE.md: §6 gains this as the worked example that a modal-looking
feature need not become a shadow; §11 records the scope="global"
deferral's second live case, making the argument for both registry
deferrals cumulative; §2 step 8 gains copy mode and keeps the
still-missing close command named.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gGQC6eqHJVbZJ5Hg7aLer
Stages 3a and 3b landed (#167, #170). Re-scouting Stage 4 against main
@ d400f30 produced six findings that change the plan and three that
confirm it. The pmacs-side facts were verified in a worktree at that
commit; the upstream facts by reading leanprover/vscode-lean4 @ 17d1d08.
The split: Stage 4's risk column read "refactors pair.lua's provenance
read" — every language's auto-pairing — for a stage the prose called the
Lean input method, which is exactly the rule §4 states and exactly what
round 4 found for Stage 3. Rev 5 had noticed the shape and answered it
with a commit boundary; a commit boundary is not a review boundary.
Stage 4a is now the typed-edit consumer chain (substrate, no Lean) and
4b the input method.
Rev 5's expansion semantics were wrong in three ways. Resolution is the
shortest key having the input as a prefix (\al yields ∀ from `all`, not
`alpha`); there is no terminator list at all ('+ ' is a key, so space
extends after \+; '\' is a key, so \\ yields \); and an unmatchable tail
is appended rather than dropped (\alp7 yields α7).
Three further findings. There is no cursor-motion hook, so acceptance 43
as written was not buildable and abandonment is lazy. dispatch_key is
only half of 4b's production path — \ and the letters are not excluded
from the optimistic classifier, and that producer is crdt-gated, so a
crdt-gated integration test is dark in CI and dark in the gate list. And
the whole expansion has cross-peer-degraded undo, a wider bite than
Q#LN6's three bracket pairs; set_round_trip_input would fix it and is
rejected with reasons.
New decisions Q#LN21 (undo degradation) and Q#LN22 (the state machine);
Q#LN10 and Q#LN11 rewritten; §2.11 records the upstream algorithm; §9.1
states the coherence impact for both stages. Acceptance keeps its
existing numbers and adds letter suffixes on both sides of the split.
Citation sweep per COHERENCE §25: five live citations moved in the 50
commits since rev 5.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011LFvC4FQtux4y32KuevZ7B
Integrates canonical `main` @ `cf54270` and closes every finding.
**P1-1 — a stale document `Pointer` stole focus from the panel.** Real
bug. `align_primary_document_window`'s unknown-buffer arm returned
`Some(window)` despite aligning nothing, so #8's activation focused the
document *before* `dispatch_pointer` rejected the mismatched buffer. It
now returns `None`: alignment did not happen, so no caller may treat it
as a document gesture.
Pinned through `handle_dispatcher_event` — the real dispatcher seam —
because the defect lived in the PAIR of alignment and activation, not
in either alone. **The first version of that test was vacuous**: an
unregistered session is dropped at `daemon.rs:1962` (#148's
membership check) before the aligner runs, so it passed with the bug
restored. It now registers a real semantic session and fails with
exactly the reported symptom, focus moving `WindowId(2)` →
`WindowId(3)`.
**P1-2 — the approved A2A-2 fan-out was missing.** The semantic target
returned one context. It now captures the primary document PLUS the
visible side window, each provider invoked once, with a
derived-hidden side omitted (Q#BP2b — no mode line to paint, so no
callback should run for it). The acceptance asserts `windows.len() == 2`.
This exposed a second defect the finding did not name: the consumer
selected segments with `.find(|w| w.context.frontend_id == frontend_id)`
— the FIRST context for the frontend. With two contexts that silently
depended on capture order and could have shipped the panel's mode-line
text as the document status band. `emit_statusline_segments` now takes
the document `WindowId` and selects on window identity.
**P1-3 — the census suite tested the authority, not the consumers.**
Confirmed: reverting a producer to `active_window_for` left all ten
tests green. Added consumer-level pins that drive the real producers
through `SemanticRenderState::render_frame` with a panel focused, plus
the terminal-declaration guard. Bite-verified: reverting the
`LineNumbers` routing now fails
`consumer_line_numbers_follow_the_document_not_the_focused_panel`.
**P1-4 — main integrated.** The textual conflict was `docs/active-work.md`
(both lanes rewrote the same region; the terminal-config lane is kept
whole and the bottom-panel heading updated). `src/editor.rs` auto-merged,
and the full gate suite was rerun on the merge result.
**P2-5 — the painter test was vacuous.** A fixed-point check that
survived deleting `window.text_view.render`. It now asserts each of the
four extracted outputs actually appears: buffer TEXT, the line-number
GUTTER (with line numbers explicitly enabled, rather than dropping the
assertion), the window MODE LINE, and a returned caret. Bite-verified
by deleting the render call.
**P2-6 — the stale fold-projection claim is corrected.**
`src/window.rs`'s `fold_projection` doc no longer asserts that a
semantic session never enters `paint_frame`; it records that the panel
band breaks that premise and that the extracted painters take the map
as a parameter.
Gates on the merge result: fmt clean; workspace clippy clean; 1,832
default + 2,010 CRDT library tests; Stage 2A acceptance 13; Stage 1 46;
statusline 8; m11_5 2; GPU initial target 14; terminal config 12;
folding Stage 2 48; vterm 1/2 10 / 6; M4 121; required GPU 202;
`git diff --check` clean. `vterm_stage3_acceptance::a37` remains the
pre-existing flake measured on the base commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Integrates githubsucks/main @ cf54270 (terminal config Stage 1, #173).
Both conflicts were additive appends to the same lists — the handoff
preamble and the ledger's "Closed since the last snapshot" — so both
sides are kept and the preamble now leads with #173.
Also records what gating #173 measured about a37, because it changes
this lane's proposed fix rather than merely annotating it:
a37 reports `ok` without running whenever `pmacs-gpu` is absent from
the target directory. A fresh worktree reports the Stage 3 suite 9/9 in
0.17 s having never executed the arc's only real-daemon/real-PTY/real-
wgpu path; a genuine run takes about four seconds. Only
PMACS_REQUIRE_GPU=1 promotes that skip to a failure, and the standing
gate list applies that flag to `cargo test -p pmacs-gpu`, a different
package. So fix-shape part 2 must state the flag as a requirement of
the gpu-render job: a crdt leg added to the plain `test` job would run
a37 vacuously and report green.
a37 is also load-sensitive — it passed at d152120 and failed at that
same commit twenty minutes later under machine contention — which makes
a red first CI run ambiguous by construction. The lane now says to
re-run on the merge base before believing a failure, and to prefer
serialized execution over retry-until-green.
The vterm audit's "only 3 of 9 Stage 3 tests drive a real daemon" is
corrected: without the frontend binary the honest number is 2.
Ledger: terminal config Stage 1 flipped from IN REVIEW to MERGED.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gGQC6eqHJVbZJ5Hg7aLer
The parallel sweep failed one of the new tests for a real reason, not a
flake: drain_until ticks, and a tick can reap an immediately-exiting
child before the diagnostic runs. Recorded with the matched-load
measurement that shows the fix is load-bearing (0/15 fixed vs 1/10
unfixed under full saturation), and the final sweep numbers.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk
Make command-time attachment healing cancel an armed terminal restart
before replacing the server, while keeping request-only lookup pure and
restart-safe.
Track config-driven server ownership privately, bound every fallback
server per SID, scope no-swap retirement to the failed root, and route
the shipped Lean diagnostics command through the safe resolver while
waiting for initialization.
Add direct acceptance counterexamples for all five review findings and
record the sixth-round verification and vacuity lesson.
The parallel workspace sweep failed
observing_the_leader_does_not_consume_the_exit_event with "process
ProcessId(26) is not running". A real defect in the test, not a flake.
The helper that fetched the pid drained for the Started event, and
draining ticks. A tick can observe an immediately-exiting child and
transition the record out of Running, after which signal returns "is not
running" and never reaches the diagnostic -- so the loop spun to its
10 s bound and panicked. It passed standalone because the drain returned
on Started before poll_one saw the exit; only the sweep's load shifted
the timing enough to lose that race.
Fast-exiting children now read the pid straight from the supervisor
record, which does not tick. The bounded loop also fails fast when the
record has left Running, so a future recurrence is diagnosed in one line
rather than surfacing as a timeout.
Verified under matched load: 15/15 green with all 16 cores saturated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk
The ledger's own update protocol requires a lane for volatile work, and
PR #176 had none: branch, worktree, review state, and verification were
all missing.
Records why the lane ships a diagnostic rather than a fix -- three
rejected tolerance designs, the two facts that killed the original
argument (group=true is rejected for PTY mode so the reap ledger never
applies to that path, and the ledger comment asserts EPERM cannot happen
rather than ruling that it means dead), and that the CI evidence never
established the child had exited.
Also records the round-1 test fixes and the four verified bites, so a
reader can tell which assertions are load-bearing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk
Round-1 review found both test weaknesses.
The exited-child tests used a fixed 300 ms sleep as proof the child had
exited, which on a loaded runner can be false and would turn them into
spurious failures. nix's waitid is unavailable on macOS and libc::waitid
would need unsafe, which the crate forbids, so the tests now synchronise
on the observation under test: a bounded loop that drives the production
diagnostic until it reports the leader as exited. Each failing attempt
leaves the record untouched because the failure path returns before any
bookkeeping, so the loop is side-effect free, and it is strictly stronger
than a sleep because it observes the actual state rather than assuming it.
The assertions were substring checks -- target=-, expected_group=-,
leader=exited( -- which a hardcoded target or a wrong exit code would
satisfy. They are now exact message equality built from the pid the
kernel actually assigned and the errno's own Display, and the one-event
test asserts the surviving event carries exit code 7 rather than any
terminal event. The group test also spawns /bin/sleep directly rather
than through a shell, since a shell may place the command in a different
foreground process group than the one being asserted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk
Ledger entry for the in-flight Stage 2A branch, plus three findings the
gate run produced that are worth carrying regardless of this PR:
- The structural test comparing the two authorities directly did NOT
catch the focus-class bite; only the consumer-level assertion did.
Both kinds are needed, and the distinction generalizes.
- `vterm_stage3_acceptance::a37` is badly flaky on this machine —
6/8 failures on the BASE commit against 7/8 on the branch in matched
isolated samples, so it is pre-existing rather than a regression. It
also returns `ok` without running unless `pmacs-gpu` is built.
- `m11_5_semantic_acceptance` reports 0 tests and
`gpu_initial_target_acceptance` reports 1 without `--features crdt`.
Both are semantic-census suites, so gating Stage 2A in the default
config alone would exercise almost none of its relevant coverage.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bottom-panel Stage 2A, second half (Q#BP8, Q#BP17). Still no protocol
change and no behavior change: `paint_frame` builds the same fold map
it always did and passes it in, so grid rendering is unchanged.
Two extractions, both taking the fold map as a **parameter** rather
than building it:
- `prepare_window_cursor_visible` — the active-window auto-scroll
clamp. The panel band (2B) runs this for its own window when that
window owns focus, and leaves a passive panel's `view_top` alone.
- `paint_window_content` — the per-window document body: text, gutter,
overlays, selection, and the mode line. The panel paints into a
panel-sized grid at the same origin-agnostic `Viewport`, so this is
that body lifted out, not a second painter (Bet B2').
The parameter is the point (Q#BP17). Folding built its per-window map
ungated on the premise that "a semantic session never enters
`paint_frame`", which the panel band breaks. The panel path must pass
`None` for a frontend whose `fold_projection` is false, and must not
call `EditorCore::fold_map_for_window` — that gates on the **active**
frontend, which is right for command-time reckoning and wrong for
painting another frontend's panel.
`tests/bottom_panel_stage2a_acceptance.rs` — 10 tests. The negative
half is the load-bearing half, so Projection assertions are paired
with focus-class assertions taken in the SAME state:
- `focus_and_projection_disagree_in_the_same_state` is the key one:
with a panel focused, the focus authority must name the panel while
the projection authority names the document. Routing the focus class
through `primary_document_window` fails this even though every
Projection test still passes.
- The statusline pair pins the split: the LOOKUP resolves the document
window while `active` reports actual focus, with a non-vacuity twin
that flips `active` back to true when focus returns.
- The extraction pair pins cells, the returned cursor, the focused
window's `view_top`, AND a passive window's untouched scroll —
identical cells alone would not catch a clamp that moved to the
wrong window on a single-window frame.
- `the_panel_fixture_really_builds_a_side_window` pins the fixture's
own precondition, since every other test is worthless if
`focused_panel` silently produced an ordinary split.
One crdt-gated caller of the old `align_semantic_window_to_buffer` was
updated; it compiles only under `--features crdt`, which is the
config CI never runs.
1,832 default + 2,009 CRDT library tests, 10 new acceptance; fmt and
workspace clippy clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bottom-panel Stage 2A, first half. Every consumer the framing classifies
**Projection** now resolves the frontend's primary document window or
buffer instead of its focused one; every consumer classified focus,
focus-chrome, or focus/session is deliberately left alone.
No protocol change, no behavior change for any frontend today: with
`panel_capable = false` for semantic sessions, `primary_document_window`
returns `view.active` for every existing configuration, so this is a
seam adoption that becomes load-bearing in 2B.
Projection consumers routed:
- **#1** semantic buffer-follow / `BufferSnapshot` re-send
- **#2** the lazy CRDT upgrade — the sharpest case, since it BROADCASTS
to every replica, so keying it on focus would let focusing a fresh
generated panel buffer swap every peer's document mirror
- **#3** `CursorByte`
- **#4** `LineNumbers` mode
- **#5** selection decorations
- **#6/#10/#11** the full-window semantic terminal declaration, its
snapshot/sync, and terminal-frame suppression, via the shared
`semantic_terminal_key` resolver
- **#9** the `Viewport` terminal-context gate — a focused terminal panel
must not suppress the still-visible document's viewport
- **#12** the semantic statusline target LOOKUP
- **#21** the `BufferSnapshot` publication recipient filter
`align_semantic_window_to_buffer` splits per Q#BP14, which is the
distinction that makes rejecting panel-named events insufficient on its
own:
- `align_primary_document_window` (**#7**, `Viewport`) — aligns the
document window and **never touches `view.active`**.
- `align_and_activate_primary_document_window` (**#8**, `Pointer`) —
aligns and then activates, because a click in the document area means
"work here". This is the one place projection and focus legitimately
move together.
`dispatch_semantic_terminal_pointer` (**#11**) gains the same rule: an
accepted non-`Move` gesture activates the document window before the
gesture replays, while bare hover neither focuses nor claims.
The statusline change is deliberately a HALF change (parent acceptance
42): the window LOOKUP resolves the primary document window, but
`active` still reports **actual focus**, so a document provider can
truthfully observe `active = false` while a panel owns focus.
Untouched, and that is the load-bearing negative: #13 remote-op
validation, #14 `dispatch_idle_for`, #15 presence, #16-#19 search /
menu / minibuffer / completion chrome, #20 terminal bell drain, and #23
remote-op application all still resolve the actually focused window.
#16-#19's Q#BP14b routing table needs `PanelFrame` and lands in 2B.
1,832 library tests pass; fmt and workspace clippy clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five findings, all real. The blocker and both majors are the same
mistake in three places: a claim asserted somewhere cheaper than where
it actually lives.
COHERENCE.md was stale in four places, not the three reported. Step 8
still read "no keybinding" and §11 still read "five settings", but §6's
dispatch table also still cited `is_terminal_escape_chord` — a symbol
this branch deletes. §25 requires that update to ride the PR, so a PR
changing audited ground truth has to re-grep the audit for its own
symbols, not only for its topic.
Acceptance 5 asserted a registry round-trip, which is a test of the
registry: it stayed green with the setting's only consumer deleted. It
now opens a real terminal whose child overflows the 24-row screen,
scrolls the view to its oldest retained row, and asserts LINE001 is
present at 10,000 and absent at 0.
Acceptance 8a waited for the session count to fall, which the rejected
editor-side cache map satisfies exactly — a map with no purge hook
leaks while sessions drain. Adds `TerminalManager::escape_caches()`, the
lifetime half of Q#TC4c's contract that `escape_parses` cannot cover.
`table.sort` over `pmacs.terminal.profiles` raised "attempt to compare
number with string" on the unknown-profile path whenever the user's
table held both a string and a numeric key, replacing the exact
diagnostic being asked for; `%q` raised likewise on a non-string
`profile` argument. Both are partial functions applied to user input on
a diagnostic path.
Also corrects the framing's status line, and a status message whose
embedded whitespace run had survived a rustfmt reflow.
Three new bites, each falsified by revert: deleting the scrollback
consumer fails acc5 and only acc5; restoring the raw-key sort
reproduces the comparison error verbatim; and implementing the rejected
map fails the new acc8a at left: 2, right: 1 while passing the old
session-count version.
Merges githubsucks/main @ ccf29e3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gGQC6eqHJVbZJ5Hg7aLer
Round 5 review: one P1, a frontend scope hole, and three P2s.
**1. A fallback that SPAWNS and then dies retried forever.** The
once-per-buffer guard bounds calls to `_attach_buffer`, not the server
those calls produce. `ensure_server` still never forwards `cfg.restart`,
so the fallback inherits `OnCrash`; an executable that exits before
`initialize` is respawned by the manager with no attempt ceiling —
silently, because `latched` has already disabled the primary's failure
poll. The fallback now gets its own one-shot die-before-initialize
watch, which retires it (ending the respawn loop) and reports.
The prior failing-fallback test used a NONEXISTENT executable, so it
only ever exercised synchronous ENOENT. To reach "spawned, then died"
the fixture has to actually spawn.
**2. Simultaneous frontends.** Both repair triggers read the ambient
`pmacs.window.buffer()`, and the daemon restores `active_frontend` to
the last-dispatched frontend before `tick_processes` — so a Lean buffer
active in ANOTHER frontend receives no `buffer.after-switch` here and
stays stale after its server is globally retired.
Fixed at the seam that is frontend-agnostic: **make consumption safe.**
`attached_for_active` now rebuilds rather than returning a record whose
server is dead, and `attachment_for_request` reports none (it must not
perturb LSP state, so it cannot rebuild). Whichever frontend runs a
command is the active one while it runs, so healing at the point of use
reaches every buffer no eager sweep can. This also closes the half where
a dead attachment was handed to a command and the request vanished.
**3. The retirement sweep stopped user-managed servers.** Selecting on
`language_id == "lean4"` also names servers the user spawned from
`init.lua`, which are not derived from `pmacs.lsp.config.lean4`. It now
keys on the `default-lean4` label `ensure_server` stamps — the
derivation discriminator.
**4. Repair ran even when no swap occurred.** `swap_to_fallback()`
returning false left `latched` true, so the next tick retried the
UNCHANGED configuration and reported it as a fallback failure. Split
into `probe.fallback_installed`: repair exists to apply a swap, so no
swap means nothing to apply.
**5. The once-per-buffer assertion counted table keys**, which cannot
distinguish "once per buffer" from "every tick for one buffer" —
cardinality stays 1 either way. Replaced with a numeric attempt counter;
the bite reports 174 attempts against the expected 1.
Five bites, each against 7c37bdc: no fallback watch -> attempt reaches
4; retire by language_id -> the user's server is stopped; gate repair on
`latched` -> a repair is attempted with no swap; drop the
once-per-buffer guard -> 174 vs 1; hand back a dead attachment -> a
command receives a `stopped` server.
Two more vacuity shapes recorded in the ledger (8 and 9): counting
distinct keys cannot bound repeated work, and a nonexistent executable
cannot reach any post-spawn failure.
A failing kill in ProcessSupervisor::signal reported an errno and
nothing else, which is not enough to diagnose the macOS CI failure that
prompted this lane: three different hypotheses about that EPERM produce
the same message, and the fix each one implies is different.
The error now carries five facts as separate fields: the target source
(which branch of signal_target ran), the target kind and value, the
spawn-time group for a group-directed signal, the errno, and the
spawned leader's real try_wait state.
Keeping the target and the leader apart is the whole point. For a PTY
the signal goes to the terminal's foreground process group, read from
the tty at signal time, while the leader is the child that was spawned.
Those are different entities whenever job control has moved the
terminal, and three rejected designs for this code were unsound
precisely because they concluded something about one from the other.
The report states both and concludes nothing.
The disposition is unchanged. Every call that failed before still
fails, with no state transition and no reap-ledger arming. That is
asserted directly rather than assumed, because it is what separates
this from the tolerance rules review rejected.
Q#PD3, stated narrowly: this is not a pure message change. Consulting
try_wait reaps an exited child and caches its status, so the child may
be reaped earlier than it otherwise would be. That is observably safe
because portable-pty 0.9.0 returns a std::process::Child on Unix and
delegates try_wait straight to it, so the status is cached and poll_one
still sees it -- but safe by argument is not safe by assertion, so a
test forces a kill failure against the real PTY child and then checks
that exactly one terminal event survives.
Q#PD4: the test seam injects the kill attempt's result only, never the
observation. Target selection, the real ChildHandle::try_wait against
the real child, and the error construction all run unmodified; a
stubbed observation would bypass the code path under test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk
Three design blockers, four cleanups, and the staging call taken. Round
3's theme: rev 3 named the right seams but sized two of them from a
partial inventory, and one promise was still stronger than its mechanism.
All three verified against c93f9ee.
H1 — the modified-buffer delete check races the syscall. Rev 3's
"immediately before each syscall" was wrong about where the boundary is:
pmacs.fs.remove DISPATCHES A WORKER, so the interval to remove_blocking's
remove_file is wide open, and acceptance 20 (edit before y) could never
have detected it. NARROWED to a TOCTOU-bounded pre-dispatch check, the
same honest framing G6 forced on R, rather than inventing a reservation
primitive inside a dired stage. The residue is stated precisely: the
buffer survives with its contents (that half IS robust — it runs at drain
time), the file does not. So the orphan deferral rev 3 scoped to the LSP
path now covers dired too, as one deferral rather than two. Acceptance 20
says outright that the interval has no test because it is not closed.
H2 — the LSP teardown inventory was a third of the real one. LspManager
holds FOURTEEN URI-bearing store families (lsp.rs:741-819), not five, plus
the `documents` text map didChange diffs against — a stale entry there is
a correctness problem, not a leak — plus pending_routes, whose
ResponseRoute variants CARRY THE URI at fifteen insert sites, so an
in-flight response repopulates the old key AFTER any clear. Rev 4 gives
the full table and one manager-level forget_uri(sid, uri) that purges
routes, drain-cancels the matching awaiters (the existing contract at
:799-803 already requires that wherever routes are purged), and clears all
fourteen plus documents — handling locations_store's kind key and
symbol_store's scope key specially. Modelled on the server-scoped
teardown at :1316-1331. Also records the surprise found on the way: that
teardown clears routes and documents but NOT the fourteen stores.
H3 — the diagnostic-view seam is now chosen, not either/or. Verified the
constraints: DiagnosticView.uri is private and immutable, View has no
downcast, and _attach_view takes active_window_mut() and ERRORS otherwise,
so it reaches one window and cannot drive a per-window loop from Lua; and
a remove-and-re-push loses composition order in an ordered
Vec<Box<dyn View>>. The seam: a View::rename_resource default-no-op hook,
joining overlay_identity and clone_for_split — the family #113 round 6
added for exactly this class — swept over core.windows.values_mut() the
way overlay disposal already is (mod.rs:2016-2019). In-place mutation, so
order is preserved by construction, the field stays private, and future
URI-bearing overlays opt in by overriding rather than growing a special
case. Acceptance 30 now needs TWO windows and an order assertion; a new
item 31 pins the store inventory and the in-flight repopulation.
Staging: TOOK THE FURTHER CUT as directed. Three PRs — 2a the
reconciliation transaction with no dired surface, 2b marks and operations,
2c the new fs primitives. 2a leads with the two defects it closes on main
today (an LSP-authored delete that destroys unsaved work; a workspace-edit
phantom buffer), neither of which needs dired to be worth fixing. Named
for the substrate per #161's precedent. §10 states the cost: three review
cycles, and 2a ships nothing visible.
Cleanups: item 35→40 (now 41), acceptance 27→30 and 28→32 (now 33), and
the §10 table's obsolete rename-only-Rust description, replaced by a
per-PR breakdown of what each actually carries.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0126d2sikA6jZpFin3rtLCSK
Levi Neuwirth