Normalize batch dependency paths through the registry's lexical
canonical form so equivalent URI spellings do not revive the
initial-state preflight bug.
Separate execution-started state from the count of completed plan
items. Preflight failures retain the no-mutation guarantee, while
runtime failures conservatively acknowledge that the failing item may
itself have changed a buffer or the filesystem.
Add real-server-pump acceptance for dot-path dependency aliases,
partial text edits within one item, and resource-operation side
effects, and record the review-round corrections in the framing.
One conflict, in `docs/active-work.md`, with an extra strand: main
gained #188's lane while this branch had removed the documentation
lane, so the two sides disagreed about a region neither had edited
against the other.
Resolved by construction rather than by editing markers --- main's file
taken whole, the documentation lane removed, this lane's block
inserted before the parked lane. Verified against both parents: exactly
26 lines removed relative to main, which is the documentation lane and
nothing else, and the lane-2 block byte-identical to this branch's.
Records, without fixing, that #188's own lane now reads "OPEN,
PROPOSED" on a merged PR. Rule 4 forbids relabelling and allows removal
only once the facts reach `docs/agent-handoff.md`, which #188 did not
touch. That absorption belongs to the immutability arc's next PR, not
to a testing lane reaching across into it.
Bring the approved generated-buffer immutability framing onto the Stage
1 branch and update both active-work lanes to the landed #188 state.
Main @ 27b1185 changes documentation only relative to the prior base.
Record the revision-7 selection and acceptance reconciliation, the
exact code checkpoint, the non-vacuous fan-out bite, and the final gate
results. Keep PR 188's proposed status and merge ordering explicit.
Adopt Q#GB6's clamp-or-clear rule in both window-coordinate
normalization paths. Preserve shortened selections, clear only those
collapsed by a moved endpoint, and pin both outcomes through the real
generated-write and view-rebuild callers.
Make listview refresh rely on the generated-write notification before
reseating, so Stage 1 criterion 7's fan-out mutation bites both
adopters. Align criteria 5, 11, and 12 with framing revision 7.
Review round 2 found no new defects; this is the one durable item it
asked to be carried further than a commit message.
`cmd | tail -2` returns TAIL's exit status, not `cmd`'s, in fish and
bash alike. So a gate chain of `cargo test ... | tail -2 && ... && echo
"ALL GATES CLEAN"` prints the clean line even when a suite failed, and
that is what happened while gating this lane: a `pmacs-gpu` failure was
summarized as clean. The point worth keeping is that this is not
carelessness a closer read would catch --- the verdict is structurally
absent from the summary the PR then cites. §5 now says to check
`$pipestatus[1]`, or better to redirect each gate to a file and read it
afterwards, which also preserves the full log that section already asks
for. Filed beside the skip-reports-`ok` lesson, which is the same
family: the thing that summarizes a gate must not be able to lose the
gate's verdict.
Also fixes the doc-comment splice in `tests/support/mod.rs`, where the
why-two-directories paragraph landed mid-sentence and left the
include-mechanics explanation stranded inside it. Cosmetic, and review
called it not worth a round on its own --- folded in here because the
file was being touched anyway.
P1 --- the workflow contradicted this lane's own claim. `components:
rust-analyzer` rode the shared `dtolnay/rust-toolchain` step, which
matrixes over ubuntu AND macos, so the binary would have been present
on both. PRESENCE, not PMACS_REQUIRE_LSP, is what decides whether a
gated test body runs --- the unset variables on macOS only meant
absence would be tolerated there, not that the tests stay skipped. Two
rust-analyzer tests would therefore have executed on macOS for the
first time ever, on the legs that are simultaneously the CI critical
path and the documented flake surface, one of them doing real indexing,
none of it covered by the Linux-only local runs behind this lane. Moved
into the Linux-gated step so the text and the workflow agree.
P2 --- the ledger promised a base and pasted nothing. Worse than the
review knew: a script was run to substitute it, reported success,
matched no text, and the result was never re-read. The claim shipped on
the strength of a tool's exit status. The entry now carries the pasted
base and a recovery command, and records the lesson in the terms this
ledger keeps relearning --- asserting a measurement is not making one,
and a tool reporting success is not the measurement either.
P3 --- tool versions pinned (gopls v0.16.2,
vscode-langservers-extracted 4.10.0, yaml-language-server 1.15.0), so
CI no longer drifts with upstream publishes and a break has a commit
here to bisect against. `tests/support/` now states why it exists
beside `tests/common/`: the latter re-exports daemon and PTY
machinery, and pulling that into three binaries that spawn neither to
reach a six-line environment check is the wrong trade. Recorded as a
cost, with the rule that a third such directory means consolidating
rather than continuing.
Also recorded, because §1.2 is NOT fully closed by this lane: the
guards arm the ENTRY skip only, and m4_acceptance's mid-test
"workspace likely still indexing; skipping" survives --- so even armed,
that test's one assertion can still vanish under load, which is exactly
when a regression would show. Mid-test skips want their own pass. And
the follow-up this lane creates: removing the documentation lane
removes the only pointer to githubsucks/handoff-2026-07-20, so that
branch needs deleting after merge or nothing will ever mention it
again.
P4 --- the consolidated lua skip message names PMACS_TEST_LUAJIT again;
the `or_else(locate_shell("luajit"))` path still honours it and the
escape hatch had become undiscoverable. Double blank line before the
parked lane collapsed.
One gate note worth carrying rather than burying. A `PMACS_REQUIRE_GPU=1
cargo test -p pmacs-gpu` run failed once during this round, naming the
`--bin pmacs-gpu` target, and my gate chain still printed a clean
summary because `cmd | tail -2` in an `&&` chain returns TAIL's exit
status, not cargo's. Four subsequent runs pass 202/202 and the failure
has not reproduced, so it is recorded as observed-and-unreproduced
rather than explained. The masking is the durable part: a gate summary
assembled through a pipe can report success over a failure.
Gates re-run after the fix: fmt; clippy -D warnings; --lib 1863; --lib
--features crdt 2048; m4_acceptance 121; m6_5 11; m6_8 8; required GPU
202 (x4); git diff --check clean.
Absorb #193 without rewriting the reviewed revision 6 or revision 7
commits. Resolve docs/active-work.md by retaining PR #188's proposed
lane and accepting main's removal of the landed bite lane.
Record that the intervening base change touches no product source, so
revision 7's source readings remain current.
Lane 2 of the testing arc (`TEST_IMPROVEMENT.md` §1.2, §5.4).
The shape being fixed reports GREEN when the tool is missing:
let Ok(_) = which_binary("gopls") else {
eprintln!("gopls not on PATH; skipping");
return;
};
CI installed none of these tools, so a block of real-language-server
and multi-shell tests had never once executed their bodies while
reporting success on every run. A suite that cannot distinguish
"passed" from "never ran" is worse than a missing suite, because it
reads as coverage in exactly the place someone would go looking for it.
The fix is this project's own pattern rather than a new one:
PMACS_REQUIRE_GPU already turns a missing adapter into a hard failure
for the headless render job. This adds PMACS_REQUIRE_LSP,
PMACS_REQUIRE_SHELLS and PMACS_REQUIRE_LUA, and the CI step that
installs the tools they promise. Per-tool variables rather than one
blanket flag, so a tool that must stay unarmed keeps that decision
visible at the call site instead of buried in a workflow file.
basedpyright is deliberately NOT installed and NOT armed. Its test has
no timeout and hangs forever; the root cause is the non-interruptible
reader-thread join in `RuntimeHandles::drop`, already a named deferral
in `src/process.rs`, and the `test` job has no `timeout-minutes`.
Arming it today would trade a vacuous green for a six-hour hang across
four legs. PMACS_REQUIRE_PYRIGHT exists and is never set, so the flip
is one line after the hang fix and the CI timeouts land.
A trap found while writing the workflow rather than after: the natural
Actions idiom
PMACS_REQUIRE_LSP: ${{ runner.os == 'Linux' && '1' || '' }}
sets the variable to the EMPTY STRING on every other platform, and
`var_os(..).is_some()` is true for `Some("")`. That would have armed
the guard on precisely the runners with none of the tools installed
and failed every one of them. The helper treats empty as unset, which
makes the common spelling safe instead of subtly wrong.
The helper is SHARED via `#[path = "support/mod.rs"]` rather than
copied into three test binaries. `m6_8_multi_repl_acceptance.rs`
carried a comment saying cross-test-binary sharing "would need a
fixture crate"; it does not, and a correct helper in one file beside a
degraded copy in another is this suite's most repeated defect.
Verified by execution in all three states, using a tool genuinely
absent from this machine (vscode-json-language-server): unset skips
green; armed fails hard, naming the CI step that should have installed
it; empty string skips green. On `main` the armed state cannot fail at
all, because no guard exists.
And the question none of this could answer until now --- whether the
tests pass when they actually run --- is answered: armed locally, 11
m6_5 and 8 m6_8 REPL tests are green, and all six real-LSP tests
(clangd x2, gopls x2, rust-analyzer x2) pass individually. The coverage
was real the whole time. It just never ran.
Linux only for now, deliberately: macOS needs the brew equivalents and
roughly doubles install cost on the slowest matrix leg. The variables
stay unset there, so those tests skip cleanly as before.
Also removes the documentation lane from the ledger. Its disposition
was left undecided pending confirmation that its branch carried
nothing unique; measured, `githubsucks/handoff-2026-07-20` is 1 ahead
and 365 behind, and its whole unique diff is four doc files at 42
insertions against 88 deletions --- merging it would REVERT current
documentation. The section asked whoever confirmed that to remove it.
Gates: fmt; clippy -D warnings; --lib 1863; --lib --features crdt
2048; m4_acceptance 121 (unarmed, per CLAUDE.md); m6_5 11; m6_8 8;
PMACS_REQUIRE_GPU=1 -p pmacs-gpu 202; git diff --check clean.
The entry pasted `4 0`, measured immediately before the commit that
wrote it; at the pushed head the same command said `5 0`. An
ahead-count is self-referential in the file that carries it, so the
entry now pastes only the merge-base — which is stable — and says to
run the count on demand. Behind-count stays the thing that matters,
and the merge-base tells you whether it can still be zero.
Close review round 6 by moving CRDT quarantine to common divergence
detection, guarding every snapshot and later owner-write route, and
making the acceptance pre-image classifications honest.
Advance the active-work lane to the actual integration checkpoint and
replace the obsolete delete-to-insert recovery rule.
The count in the previous commit was measured against `64883eb` and
was stale the moment that merge landed. Re-measured and pasted as
command output. `main` moved twice during this round; the entry now
says so, and says why a stale base matters under branch protection.
Resolution is a deletion, not a rewrite: `main` REMOVED the
`scripts/bite` lane entry when #192 landed, so this side drops it too
rather than resurrecting a merged lane. This lane keeps its own entry;
its pasted position measurement is re-taken in the next commit,
because the merge itself moves it.
The previous commit pasted `git rev-parse`/`git log`/`merge-base` output
showing `main` at `64883eb` and "main IS integrated". #193 merged between
writing that and pushing it, so the pasted output was false in the tree
that carried it --- and "main is integrated" is exactly the kind of claim
this review round was about: the tree asserting something the record does
not support.
Re-measured and re-integrated, and the bullet now says the thing the
first attempt got wrong. Pasting command output is necessary and NOT
sufficient: a base measured before the last push is a claim about a tree
nobody has, so it has to be re-measured at push time and treated as
expired on sight. `main` moved three times inside this one lane
(#187 -> #192 -> #193).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
Four findings, all reproduced by the reviewer, all accepted. Two of
them are one defect class — a guard whose scope was REASONED ABOUT
rather than enumerated — so both are recorded in the framing's new §9
together with a sweep of every other place this lane decides something
is "affected".
P1 — the delete preflight broke ordered resource operations
(§9.3). Every delete was judged against the filesystem's INITIAL
state, at plan-construction time, so a valid `create X -> delete X`
was refused with a fabricated NotFound about a path the batch was
about to create; likewise `rename A -> B -> delete B`. This was a
regression this lane introduced, not a pre-existing defect.
Decision: DEFER, do not simulate. A delete whose target is related by
component-aware path containment to a path an EARLIER op in the same
plan creates, renames onto, renames away from, or removes is not
judged at plan time; the primitive judges it when it runs. Q#RD3
already calls this check a filter, not a transaction, so declining to
judge is inside its contract and refusing a legal batch is not.
Simulating instead would mean modelling filesystem presence AND the
registry's path bindings across create/rename/edit — the transaction
Q#RD3 declines to build — and a wrong simulation emits false `clear`
verdicts, which is the dangerous direction. `edit` ops are excluded
from the deferral set on purpose: an edit changes no path's existence,
so it can only turn a plan-time `clear` into a primitive-time
refusal, which Q#RD3 already documents and accepts. The
buffer-and-filesystem half therefore still fires early for any target
no prior op touches, which is what criterion 11c pins.
P1 — the production-boundary acceptances are landed (§9.5).
Criteria 11, 11a-11d, 12 (both directions), 13 and 15 now drive a real
`pmacs_fake_lsp` child over a real transport. One parameterized mode,
`applyeditplan`, replaces the eight the framing named: it reads its
whole WorkspaceEdit from a test-written file and publishes the
client's response to a sink, so each of the eight fixtures sits next
to the assertions that depend on it instead of being mirrored across
two files. Fail-closed — an unreadable plan sends no applyEdit and
reports itself through the sink, so a broken fixture cannot read as a
pass — and the sink is written `.part`-then-rename so a polling reader
never sees a partial record. There is no skip-and-return-ok arm
anywhere: `fake_lsp_path` resolves through `env!("CARGO_BIN_EXE_...")`,
a compile-time constant, so a missing binary is a build failure.
P1 — mid-batch failures were misreported as complete aborts (§9.4).
`apply_workspace_edit` now returns `nil, message, applied_op_count`,
and ONE renderer serves both the user-facing status line and the
server's `failureReason`, so the two cannot disagree. All three
callers are updated, not only the rename one.
P2 — non-recursive deletes inspected descendants (§9.2). `recursive`
is now a parameter of the shared query and descendant matching is
reserved for recursive deletes. The old doc comment argued at length
for the wrong behaviour and is replaced by the counterexample that
falsifies it: a modified buffer at `tree/gone.rs` whose file is
already gone blocked a non-recursive delete of the now-EMPTY `tree/`,
an op that would have succeeded and that removes none of that
buffer's contents. This narrows the Q#RD6 query #171 adopts.
Criterion 3's stated bite: fixed by fixing the SETUP, not the doc.
The first commit's test comment carried a correction saying the
framing's wording was wrong. It was wrong only against that setup —
and §9.2's narrowing would then have left the setup with no bite at
all, since a non-recursive delete no longer inspects a descendant.
So the buffer is now bound to the EXACT deleted path: a file is
opened, then replaced on disk by a non-empty directory, and
`remove_dir` fails with ENOTEMPTY deterministically under any uid.
Both of the framing's stated pre-images now bite, so the framing
needed no amendment there. The correction is recorded in §9.1 rather
than only in a test comment, which is where the review asked for it.
WHY THE SHIPPED SUITE PASSED WHILE FINDINGS 1 AND 4 WERE LIVE — two
coverage facts for the next lane. Every delete criterion drove the
PRIMITIVE directly, so nothing in the suite ever built a multi-op plan
and the preflight's plan-time behaviour had no test at all; the only
batch test, `m4_15`, happens to delete a path no earlier op touches.
And every recursive-delete criterion (7, 8, 9) passes `recursive =
true`, while every non-recursive one binds its buffer to the exact
target, so no test in the suite ever combined a non-recursive delete
with a descendant buffer — the exact cell finding 4 lives in.
Sweep, per the review's request. Seven sites decide something is
"affected"; the table is in framing §9.7. Three were the defects
above. Two are unchanged by design and named so they are not mistaken
for oversights: phase-4 reconciliation compares paths RAW via
`BufferRegistry::find_by_path`, which Q#RD10 pins as "exactly today's
behaviour" and which correcting would widen reconciliation — the one
thing Q#RD5 forbids; and `delete_verdict` stats the raw path while
comparing normalized ones, a latent inconsistency whose every branch
fails safe and which matches the primitive's own `remove_file`. Two
are consistent: the `_delete_verdict` binding defaults `recursive` and
`ignore_if_not_exists` the same way the primitive does, and the
deferral set is enumerated (create: 1 path; rename: 2; delete: 1;
edit: excluded, with the argument written down) rather than reasoned
about. Nothing else in the lane decides an affected set.
Bites. Every row was RUN, with the positive control `scripts/bite`
gained in #192 (merged into this lane), and every ref-based row below
reports `OK (assertion)` rather than `OK (COMPILE)`. `1873be6` is this
lane's own first commit: findings 1, 3 and 4 were introduced by it, so
`main` cannot falsify their pins.
rd11a builtin/runtime/lsp.lua @ main OK (assertion)
rd11b builtin/runtime/lsp.lua @ main OK (assertion)
rd11c builtin/runtime/lsp.lua @ main OK (assertion)
rd11d builtin/runtime/lsp.lua @ main OK (assertion)
rd12a builtin/runtime/lsp.lua @ main OK (assertion)
rd12b builtin/runtime/lsp.lua @ main OK (assertion)
rd13 builtin/runtime/lsp.lua @ main OK (assertion)
rd15 builtin/runtime/lsp.lua @ main OK (assertion)
rd18 src/lua_bindings/mod.rs @ 1873be6 OK (assertion)
rd19a builtin/runtime/lsp.lua @ 1873be6 OK (assertion)
rd19b builtin/runtime/lsp.lua @ 1873be6 OK (assertion)
rd19c builtin/runtime/lsp.lua @ 1873be6 OK (assertion)
rd20 builtin/runtime/lsp.lua @ 1873be6 OK (assertion)
Two rows need their weakness stated rather than hidden.
rd11 is VACUOUS against `main`'s `lsp.lua` and the script says so — a
preflight-less applier passes it, which is expected, because rd11 is
the direction that asserts the guard does NOT over-refuse (the same
shape as criteria 2, 7, 9 and 14). It bites two other ways, both run:
`OK (assertion)` against `main`'s `src/lua_bindings/mod.rs`, where the
primitive's absent-plus-ignore branch destroys the buffer; and against
a hand mutation dropping `ignore_if_not_exists` from the preflight
call, which is the pre-image the framing actually names for it.
rd3's two pre-images are designs never committed, so no ref carries
them and `scripts/bite` cannot be used. Hand-mutated instead:
reconciliation moved ahead of the filesystem mutation makes rd3 fail
on exactly its stated assertion (and rd4 with it). On this setup that
mutation and "validation that removes rather than inspects" are the
same mutation, because the buffer is bound to the exact deleted path —
stated because the first shipped setup could see neither.
The eight rows against `main`'s `lsp.lua` all fail by TIMEOUT rather
than by a value assertion, and that is the pre-image behaviour, not a
flaky harness: on `main` the primitive's raise escapes the applier,
escapes `handle_server_requests`, is swallowed by the
`pcall(handle_server_requests)` at the bottom of the file, and the
server is never answered at all. The sink is therefore never written.
That unanswered request is the defect criterion 13 exists to pin.
Gates: fmt; clippy -D warnings; --lib 1863; --lib --features crdt
2048; m4_acceptance 146 (was 132); lsp_dispatch_seams_acceptance 15;
dired_acceptance 25 and autosave_acceptance 29 (the framing's watch
items); PMACS_REQUIRE_GPU=1 -p pmacs-gpu 202; git diff --check clean.
No protocol change.
The lane itself carried two instances of the defect the review found in
the tests, so it is rewritten rather than appended to.
- Its "two framing criteria were wrong and the tests say so" bullet was
a restated contract in the ledger, which is the same shape as the one
in the test comments. Replaced by a statement of the boundary --- #188
owns the acceptance contract, this lane adopts it and files findings
there --- and by what each finding actually was.
- Its bite list said "five criteria are falsified by revert" over six
enumerated items, and counted the disambiguated-panel criterion among
them. That criterion fails on `main` only at its fixture premise, so
it is a mutation bite. Both errors corrected, and the list is now
split by falsification method rather than by a single count.
The base is measured in the lane with the command output pasted, and
labelled a reading rather than a constant: `main` moved twice while this
lane was open, so a SHA quoted forward from an earlier message would
already be wrong.
Adds the sweep result (7 sites, 4 named by the review, 3 found by the
sweep), the re-run bite results under `scripts/bite`'s new positive
control, and the one deliberately-recorded VACUOUS result --- criterion
7's listview half, which passes under the criterion's own mutation
because `window.switch_buffer` rebuilds the `TextView`. That is measured,
not inferred: the same mutation reports VACUOUS against the listview half
alone and BITES against the dired half.
Also records that the dired 200 ms perf test is load-sensitive rather
than regressed, with the pre-image comparison: 0.09 s either way over
five runs each, so the whole-buffer conversion costs nothing measurable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
Answers review round 5 on cab3404 -- five P1, two P2. All seven confirmed
against the tree. Withdraws two of revision 5's decisions and corrects one
assertion it made about containment.
Numbering: the review asked for "revision 5", but the reviewed commit
already carried revision 5 (238fd04, closing round 4), so this is revision
6. Recorded in the document rather than silently renumbered.
Base measured at the moment of writing, not carried from a briefing:
githubsucks/main is 64883eb, merged here at 76cfaac. Every count and every
pre-image is stated as a reading at 76cfaac rather than a constant, because
the base has moved four times while this document has been open. Verified
rather than assumed that 300cbc4..64883eb touches only docs/ and
scripts/bite -- no product code -- which is what lets the source citations
survive the move.
P1-1, criterion 5 impossible. begin_edit and apply_edit each call
ensure_writable as their first statement, so a locked rope returns ReadOnly
and no intercept runs. The named-error requirement moves behind a Rust-side
lift, where the intercept genuinely is the guard; criterion 5 splits into
(a) refused-and-unchanged and (b) refused-by-the-intercept-under-a-lift,
and (b) is where the original bite survives. Consequence revisions 1-5 all
missed: after adoption the intercept is unreachable on the ordinary path
including in the shipped precedent -- terminal.lua's has been dead since
#178.
P1-2, Diverged classifier. Revision 5 got three of seven cases wrong, not
one: export_updates_since runs after every successful CRDT mutation for all
three op shapes, and revision 5 mapped all three to Rejected, whose cleanup
restores a fresh buffer to writable while CRDT and rope disagree -- the
exact harm round 4's P1-3 withdrew the four-variant fallback to prevent.
Replaced by a crdt_mutated flag carried out of the routing function, total
by construction rather than an enumeration a future failure point escapes.
P1-3, containment. Revision 5's "strongest available containment" was
wrong; neither export path reads read_only and neither can, since read_only
is about inbound mutation. New crdt_quarantined flag with three consumers
-- both export sites and queue_daemon_origin_crdt_op, the third not named
by the review. Repair stays deferred and quarantine's cost is stated.
P1-4, stale views. The review named one broadcast site; the sweep found
four, and two it did not name are load-bearing here: buffer.rs:1250 is the
arm every NoOp takes, including Q#GB5's ensure_slot lock, and :1033 is how
a locked buffer's write reaches a replica. All four adopt
continue-and-retain-first-error. Criterion 15a gains a RecordingView after
FailingView, without which it cannot see the gap.
P1-5, selection anchor. The only one of the three coordinates that panics.
Clamp-or-clear in both clamp sites, for every affected window -- and the
rule is not invented: src/terminal/view.rs:715-721 already solved the
identical problem for TerminalSelection, collapsed_by_clamp included. Also
found: a stale anchor reaches the presence broadcast, not only the local
slice.
P2-6, pre-images. Four named, six wrong. Relabelled with a table stating
what the base actually does per criterion, re-derived at 76cfaac and
labelled as a reading at that commit.
P2-7, unfold before refusal. A read-only generated_preflight runs exits 1-4
before the unfold; apply_generated_edit still re-checks all four because
the borrow is released between and Buffer stays the only authority.
Sweep F, recovery mechanisms audited for unenumerated failure points: 10
examined, 4 broken (all four are the review's), and within them three exits
the review did not name.
Sweep G, pre-images re-derived from what the base does: 41 criteria
audited, 6 mislabelled. Arithmetic shown, since a count asserted without
one is what the sweep exists to catch.
Citation audit: PR #187 moved four source files, and 15 stale citation
instances across 9 distinct citations were found and corrected. The tell
was internal -- revision 6's new text cited editor_core.rs:1836 and :1865
for the same two functions older text still cited as :1814 and :1843. A
citation is a reading too, and this document had not been re-reading them.
Records the #188/#191 acceptance-contract boundary verbatim, and the two
divergences it has already had to settle.
Status stays PROPOSED. No runtime code, no protocol change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
Review finding 1 on PR #191. `notify_buffer_edit` clamped `cursor` and
`view_top` but not `win.selection.anchor`, and `rebuild_views_for` had
the same gap. Clamping the cursor is not enough to make the region safe:
`Window::region` orders `(anchor, cursor)`, so a stale anchor above a
clamped cursor is still the region's high end and `region_bytes` slices
the rope with it. Reproduced before the fix as
`assertion failed: end <= self.len()` at `src/rope.rs:145`, reached from
`EditorCore::clipboard_copy` after a generated rewrite.
The anchor is DROPPED, not clamped. A window must always have a cursor,
so clamping one is the only available answer; a window need not have a
selection, and a clamped anchor asserts a region boundary the user never
placed --- after a wholesale rewrite the surviving offsets address
unrelated bytes. This is not a new rule: `window.quit`'s restore already
answers the same question the same way with
`selection.filter(|sel| sel.anchor <= len)` (`src/editor_core.rs:3259`).
One rule, now three call sites.
Both exits are pinned separately, because fixing one and trusting the
other is how the gap arose: `acc16h` drives `notify_buffer_edit` through
a generated write, `acc16i` drives `rebuild_views_for` through
`pmacs.help.show_command`, which is the `*help*` renderer's real path.
Deleting either call site fails only its own test. The pin also
discriminates DROP from CLAMP, because that is the decision a revised
Q#GB6 could overturn.
The wording is marked PROVISIONAL in both the implementation and the
pins. The rule belongs to Q#GB6, and PR #188's approved revision 5 does
not mention the anchor; a revision request carrying this defect is with
that lane. If the landed revision says clamp or translate, this changes
to match rather than standing as a third description.
Also in this commit, review findings 2 and 3 --- the tree asserting what
the record does not support:
- Criterion 5's restatement is withdrawn in BOTH suites. The tests now
quote the approved criterion, are renamed `*_provisional_*`, and say
they do not satisfy it; the evidence (`ensure_writable` precedes the
intercept chain, with the measured `ReadOnly` message) is recorded as
what was sent to #188, not as a replacement contract. The framing's
own bite is unchanged and still fails them.
- Criterion 7's "for each adopter" is restored: the listview half now
exists as its own test. Its inability to carry the framing's mutation
bite --- `window.switch_buffer` rebuilds the `TextView`, verified by
applying the mutation and watching this half stay green while the
dired half fails --- is recorded in the test and filed with #188,
not resolved here.
- Criteria 11 and 12 are relabelled from `main` bites to mutation bites.
Both fail on `main` only at their disambiguation premise and never
reach the assertions they exist for, so a revert is not evidence for
what they assert.
How a restated contract passed the previous gate run, since the next
lane can use this: nothing in the gate suite reads a framing document,
so a test that quietly narrows its criterion is indistinguishable from
one that satisfies it --- both are green, and `scripts/bite` only proves
an assertion bites some pre-image, never that the assertion is the one
that was approved. The gate can catch a test that does not bite; it
cannot catch a test that bites the wrong contract, so that check has to
happen where the criterion is read.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
Clean auto-merge of #192. Carries the absorption that merge left owed:
#192's own lane still sat in the ledger describing an open PR, the
fourth time today a PR has merged carrying its own pre-merge lane text.
Removal is legal here without further work because #192 put its durable
facts into `docs/agent-handoff.md` §5 in the same commit --- the
positive control, the two-assertion rule, the NO CONTROL and
assertion/COMPILE/MIXED labels, and the correction to the false
`git checkout --` restore claim. Rule 4's precondition is already
satisfied, so the lane goes rather than being relabelled.
This PR is the natural carrier: it is docs-only, it touches no file any
open PR touches except this one, and it was already integrating.
Five parallel read-only audits --- dark tests, unit-coverage map, the
Lua layer, test quality, and the CI-versus-gates delta --- organized
into findings (§1-4), CI hardening and speedups (§5-6), ten prioritized
candidate lanes (§8), and a re-measurement appendix, because every
count in it drifts with each merge.
Committed now, ahead of the lanes it scopes, for a reason the ledger
already enforces: it was untracked and machine-local, so a lane entry
citing "TEST_IMPROVEMENT.md §8" was a dangling reference on any other
machine. Review of PR #192 caught exactly that, and that entry was
rewritten to be self-contained rather than left pointing at a file no
one else could read. Later lanes need to cite §1.2's silent-skip
inventory and §3's race shapes directly, so the document has to exist
in the repo first.
Two findings in it correct documents already on `main`, and both were
hand-verified rather than taken on the audit's word:
* `docs/agent-handoff.md` claimed `scripts/bite` restores via
`git checkout --`, destroying uncommitted work. False --- it has
always restored from a mktemp copy under a trap. PR #192 carries
the correction.
* `scripts/bite` had no positive control, so a test that fails
everywhere reported `bite: OK`. Also PR #192.
Kept at the repository root beside `CLAUDE.md` and `COHERENCE.md`,
where it was written, rather than moved under `docs/`. It is an audit
that scopes an arc rather than a framing for one lane, and moving it
would break every reference already made to it.
Docs only. No product code, no tests, no CI change.
P2 --- the MIXED arm said "OK" and exited 0 while its likeliest cause
is vacuity. Reaching it means cargo failed, at least one `test result:`
line exists, and none says FAILED --- i.e. the named tests may have run
CLEAN against the old tree, the vacuous outcome, with an unrelated
failure supplying the exit status. The message admitted as much while
the exit code contradicted it, so a scripted caller, or a framing doc
quoting "bite exited 0", would record a certified bite. It now exits 4
and is labelled INCONCLUSIVE rather than OK. The exit ladder in the
header gains the code.
While exercising that arm I found the review's stated trigger --- and
my own comment repeating it --- is WRONG, which is worth more than the
fix. `--test A --test B`, where B's swapped file will not build, does
NOT produce MIXED: cargo builds every named target before running any,
so B's build failure stops A from running too and the swapped run
emits no summary at all. Verified by running exactly that shape
against an ancient `tests/m4_acceptance.rs`; it lands in COMPILE, as
it should. What genuinely reaches MIXED is a run that prints summaries
and *then* fails --- doc-tests failing to compile after the lib tests
pass, or a harness dying after its summary line.
So the arm stays as defence but is documented as defensive, and the
ledger lists it as the one path NOT reproduced rather than implying
the validation was complete. Manufacturing a doc-test failure to reach
it would test the arm at the cost of a contrived fixture; saying which
paths were executed and which were not is the more useful record.
Two wording fixes from the same round. The COMPILE message said the
file "does not build against the current tree", but the no-summary
condition also covers a harness that built and then died before
printing one; it now says "produced no test summary (build failure, or
a harness that died before printing one)" so the label never overstates
the observation. And the colour comment no longer overclaims: exporting
CARGO_TERM_COLOR=never defeats an ambient variable, but an explicit
`--color always` in the passed-through args still beats the
environment. Self-inflicted and fail-closed, and now said so.
P1 --- the classifier could mislabel a genuine bite, and this repo
manufactures the trigger. It grepped the swapped run's output for
`^error\[E...\]` / `^error: could not compile`, but a failing test can
print those lines itself: `tests/compile_mode_acceptance.rs:572` has a
fixture that emits `error[E0308]: mismatched types` at column 0, and
any suite that dumps buffer or PTY text on failure does the same. A
clean assertion failure --- the strong evidence --- would then be
stamped `OK (COMPILE)`, "weaker evidence", while the new handoff text
tells the reader to trust the label. The verdict was never wrong; the
label was.
Classification now reads libtest's own summary instead:
`test result: FAILED` present means the harness ran and tests failed;
no `test result:` line at all means nothing ran. A third `MIXED` arm
covers "some target ran clean but cargo still failed", which the
two-way split silently folded into the wrong side. This also removes
two latent portability bugs for free: `\|` alternation in a BRE is a
GNU extension, so on BSD grep every failure would have labelled as
assertion; and cargo's coloured `error:` output would have evaded the
grep entirely. libtest's summary lines stay uncoloured when piped, and
`CARGO_TERM_COLOR=never` is now exported so `--color always` in the
passed-through args cannot break the parse either --- that would
otherwise have zeroed `count_passed` and produced a spurious
NO CONTROL.
Verified rather than reasoned: a test made to print BOTH
`error[E0308]: mismatched types` and ``error: could not compile
`probe` `` at column 0, then failed by swap, is still classified
`OK (assertion)`.
P2 --- the ledger cited a file that is not in the tree. The lane
scoped itself by `TEST_IMPROVEMENT.md` §8, which is untracked and
machine-local: not on `main`, not in this PR's tree. That is a
dangling reference for any other machine, which is the failure this
ledger exists to prevent. The entry is now self-contained and says so;
if that audit is to scope later lanes it needs committing in its own
PR first.
P3 --- a comment records that both runs are captured rather than
streamed, and why, so nobody "fixes" it back to streaming and breaks
the parser. Double blank line before the parked lane collapsed.
Also, on review's prompting: the #165 wiped-round incident is now
UNEXPLAINED, and the handoff says so rather than leaving a gap the
next reader will fill by re-deriving the false mechanism. Work was
lost; not by the cause this file blamed. SIGKILL bypassing the trap
and a stash collision in the same round are both candidates, and the
note declines to pick one.
Lane 6 of the test-improvement arc, pulled ahead of its own priority
list because every later lane's evidence is bite-shaped. A verifier
that cannot fail is worth no more than the claims it certifies.
The defect. The script ran only the SWAPPED tree, so "the tests failed
against the old file" was the only thing it ever checked. It could not
distinguish a load-bearing fix from a broken test: a typo, an unrelated
compile break, or a filter matching nothing all make the swapped run
fail, and each printed `bite: OK`.
The control now asserts two things, because passing alone is not
enough --- `cargo test` with a filter matching zero tests exits 0, and
a compile error in the old tree would then still read as OK:
* the named tests PASS against the working tree, and
* at least one test actually RAN,
exiting 3 as NO CONTROL otherwise. The swapped run additionally
distinguishes `OK (assertion)` from `OK (COMPILE)`, since an old file
that will not build against the current tree is much weaker evidence:
the tests may never have run at all.
Validated on all three paths rather than the happy one alone. A
zero-match filter reports NO CONTROL. A genuine bite --- PR #190's
`rd1`, which is known to fail against `main` --- reports `control OK
--- 1 test(s) pass` then `OK (assertion)`. A test deliberately broken
so it fails on BOTH trees reports NO CONTROL at exit 3; the previous
script printed `bite: OK` for exactly that case.
Handoff correction, verified rather than inherited. §5 claimed the
script "restores by `git checkout --`, which reverts the file to HEAD",
destroying uncommitted work, and cited a wiped review round during
#165. The mechanism description is false: the script copies the file to
a mktemp path before swapping and restores from that copy under an
`EXIT INT TERM` trap, touching no git state beyond a read-only
`git show`. The commit-before-gating rule is kept --- it stands on
gate-results-describe-the-pushed-tree --- but the false mechanism is
removed, because a reader who believes bite eats uncommitted work will
reach for `git stash` to protect themselves, which is the repo-global
trap the script exists to avoid.
No product code. No tests changed.
An open PR is exactly the volatile work this file records, so it gets a
lane the moment it is opened — and the lane rides on its own branch, not
on `main`, because with several PRs open a lane written there
re-conflicts on every merge.
Records the branch and its base (`githubsucks/main` @ `300cbc4`), the
code checkpoint the verification numbers describe, what Stage 1 ships,
what Stage 2 still owes, the two framing criteria that turned out to be
unimplementable as written and what replaced them, the bite result for
every criterion, and the recovery commands.
The canonical-base line above is deliberately left at `7586905`: this
lane names its own base, which is the case that paragraph already covers
("lanes below that name an older base have not been re-based"), and
editing the shared snapshot line while other PRs are open is the
contention this file warns about.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
Stage 1 of generated-buffer immutability
(docs/generated-buffer-immutability-framing.md, revision 5). Closes the
two families the bug is reachable on WITHOUT `M-x`: `compile.lua` and the
search panel rebind all seven undo chords to a no-op, but `dired.lua` and
`listview.lua` rebind nothing, so a bare `C-/` emptied a listing and a
panel. The cheap half is also the exposed half.
An intercept is not read-only. `Buffer::undo` reaches the rope through
`ensure_writable` and never consults the intercept chain, so the
erroring-intercept-plus-`bypass_intercept`-over-a-writable-rope idiom
guarded the edit path and left the history path open. Rebinding chords
does not close it: `M-x buffer.undo` is dispatchable on every buffer in
the tree.
- `dired.lua`'s `paint` and `listview.lua`'s `render` write through
`pmacs.buffer.set_generated_contents` — lift the lock, whole-buffer
replace skipping intercepts, discard history, re-assert the lock, fan
the `Edit` out. Zero `bypass_intercept` writes remain in either file.
- Both keep their named erroring intercept and `set_round_trip_input`.
The layering at `terminal.lua:351-366` is unchanged: the rope lock
protects the daemon copy, round-trip input protects a semantic
frontend's own mirror, and neither substitutes for the other.
- Q#GB13 — `listview.ensure_panel` stops adopting a same-named foreign
buffer. Ownership is the `panels` table; a collision disambiguates
`<2>`..`<99>` and raises at the limit, matching `dired.lua:476-504`.
This is a prerequisite of the lock, not a follow-up: the arc removes
the `M-x buffer.undo` that was the only recovery from a clobber.
- Q#GB18 — `panels` becomes a compacting list keyed by identity. It was
written under the requested name and read back under the actual name,
which a disambiguated panel breaks: `RET`, `g` and `q` fail closed and
silently, and `listview.open`'s capture guard fails OPEN, capturing a
panel as its own `q` target — the chained-panel loop its comment says
it prevents. Ships in the same commit as the disambiguation by the
framing's ordering constraint.
- Q#GB6 — `EditorCore::notify_buffer_edit` clamps each window coordinate
against its own post-edit bound, unconditionally. `cursor` is a byte
position bounded by `Buffer::len`; `view_top` is a line index bounded
by `TextView::line_count`, and a replace can grow in bytes while
collapsing lines, so "the buffer shrank" is not a usable trigger. This
fixes a shipped defect that reaches terminal copy mode.
- Q#GB16(a) — locking these families disables fold CREATION on them,
because `document_bytes` is spelled `is_read_only()`. Accepted and
stated rather than shipped silently; the status string now names the
read-only lock instead of claiming "not a document buffer".
Acceptance: 10 new criteria in `listview_acceptance` (16 total), 6 in
`dired_acceptance` (31 total), 2 in `terminal_copy_mode_acceptance`.
Every criterion's falsifying mutation was run: 5 bite by revert against
`githubsucks/main`, 9 by a named one-line mutation.
Two framing corrections, both recorded in the tests rather than worked
around silently:
- Stage 1 criterion 5 is unreachable as written. `Buffer::apply_edit`
(`src/buffer.rs:773`) and `begin_edit` (`:725`) call `ensure_writable`
as their FIRST statement while the intercept chain runs later inside
`apply_edit_inner` (`:1072`), so once this arc's lock is installed an
ordinary edit can never reach the intercept. Restated at the one point
where the two are distinguishable — the lock lifted — which is the
state the intercept genuinely still covers.
- Criterion 7 cannot bite at the listview adopter. `listview.refresh`
and `listview.open` both follow `render` with `window.switch_buffer`,
which rebuilds the `TextView` from scratch and masks a dropped
fan-out. `dired.revert` does not, so the dired half carries the bite;
it fails under the mutation with the reported
`assertion failed: end <= self.len()`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
Implements the framing merged as #186. On `main` today,
`pmacs.buffer.apply_resource_op`'s delete arm removes a file and then
removes any buffer bound to it, with no dirty check at any link in the
chain — so a server-driven delete destroys unsaved edits, and the
`ignore_if_not_exists` arm destroys them having done no filesystem work
at all.
Layer 1 — the primitive. The delete arm becomes four ordered phases:
stat/no-op decision, enumerate and validate, mutate the filesystem,
reconcile the registry. Validation inspects and removes nothing, so a
filesystem failure leaves every buffer intact automatically rather than
by compensation, and `on_removed` still observes the path already gone
because reconciliation stays last.
`delete_verdict` is the single shared query. It scans *every*
path-bound buffer rather than the first match, because `find_by_path`
is first-match-only and `pmacs.buffer.from_file` makes duplicates
reachable — a clean first match could otherwise hide a modified second.
It normalizes both sides before comparing and uses component-aware
`starts_with`, so `/tree` does not match `/tree-sibling`. It stats with
`symlink_metadata`, not `canonicalize`, which reports a dangling
symlink as absent and would disagree with the primitive on exactly the
input `ignore_if_not_exists` turns on.
Layer 2 — the applier and the server-request boundary.
`apply_workspace_edit` gains a plan-time delete precondition check
driven by the same Rust helper, so the two layers cannot drift. It is a
filter, not a transaction, and the code says so: `documentChanges` are
sequential, so an earlier edit can dirty a buffer a later op deletes.
The applier is now total — every failure becomes `nil, message`, and
the origin buffer is restored on the failure path as well as the
success path. At the boundary, parse *and* apply are wrapped:
`_parse_workspace_edit` sits one line above the applier and is
fallible, so a parse failure previously escaped, was swallowed by
`pcall(handle_server_requests)`, and left the server unanswered — the
defect being fixed, one line out of scope. Failures now also append one
labelled record to `*errors*`.
Scope, stated plainly rather than implied by what is present:
* Acceptance criteria 1-10, 14 and 16 land here — 11 tests driving
the primitive directly. Criteria 11, 11a-11d, 12, 13 and 15 do
NOT: they exercise Layer 2 through a real server pump and need
`pmacs_fake_lsp` modes that do not exist yet. Criterion 13
explicitly rejects a direct-call test as insufficient, so the
Layer 2 code currently has no production-path pin. That is a real
gap and the reason this is not the whole lane.
* The framing's §8 branch plan said the implementation would land on
#186 itself. #186 merged as framing-only, so it gets its own
branch and PR. No decision changes.
* Criterion 3's stated bite in the framing is wrong. It claims to
fail against buffer-first ordering; it does not, because the
deleted path is a directory no buffer is bound to, so the
reordering never fires on that input. It does fail against
validation that removes rather than inspects. Checked by mutation
rather than trusted, and the test comment carries the correction.
Bite: criteria 1, 5, 6, 8 and 10 fail against `githubsucks/main` under
`scripts/bite`. Criteria 3 and 4 pin phase ordering against designs
never committed, so `main` cannot falsify them; both were verified by
hand mutation instead. Criteria 2, 7, 9 and 14 assert preserved or
deliberately-unchanged behaviour and pass against `main` by design —
2 is criterion 1's opposite direction, 9 pins today's imperfect
orphaning so widening cannot happen silently.
Gates: fmt; clippy -D warnings; --lib 1863; --lib --features crdt
2048; m4_acceptance 132; lsp_dispatch_seams_acceptance 15;
dired_acceptance 25 and autosave_acceptance 29 (the framing's watch
items); PMACS_REQUIRE_GPU=1 -p pmacs-gpu 202; git diff --check clean.
No protocol change.
Same file, same anchor, third time: `docs/active-work.md`. #187 removed
the #186 and #171 lanes — legally, having first put their facts in
`docs/agent-handoff.md` §1 — and rewrote both the ledger intro and the
bottom-panel lane. This branch still carried all three of those blocks
from the previous integration, so the two sides disagree about most of
the file's middle.
Resolved the same way as that integration, by construction rather than
by editing markers: main's file taken whole, with this branch's #188
lane block inserted between the folding lane and the parked lane, where
it sat before. Verified against both parents — zero lines removed
relative to main, so nothing #187 landed was lost, and the #188 block
byte-identical to this branch's.
Two claims elsewhere in the file were falsified by this merge rather
than conflicting with it, which is the failure mode a marker-by-marker
resolution misses:
* #187's intro said "one open PR has no lane here: #188". True when
written, false the moment this merge lands. Rewritten to state the
reason the arrangement is deliberate, which is what the sentence
was actually for.
* this lane claimed integration "to `7586905`". It is now `300cbc4`,
by way of #189, then #186 and #171, then #187 — three conflicts in
this one file, all at the same anchor. Recorded rather than
silently corrected, because the count is the evidence for keeping
lanes on their own branches.
No framing content changed. No runtime code.
Merges `main` (PRs #186 and #171) into the 2B-2 branch — a clean
auto-merge, no conflict — and carries the ledger absorption those two
merges left owed.
`main` currently describes three lanes wrongly. The #186 lane still
says "OPEN, PROPOSED, DO NOT MERGE" and the #171 lane still says
"AT REVISION 9, AWAITING APPROVAL", because each PR merged carrying its
own pre-merge lane text; #187's lane says "OPEN" for the same reason
this commit exists. Rule 4 forbids relabelling a landed lane and allows
removing it only once its durable facts reach `docs/agent-handoff.md`,
so no earlier commit could legally fix any of them. This one does both
halves together.
Handoff §1 gains three arc bullets and an advanced anchor (`7586905` ->
`6c9e765`):
* bottom-panel Stage 2B-2, recording what the review round taught
rather than what the feature does — a durable transition
implemented as a per-frame effect is a bug shape and four of five
findings were that one shape; a producer should be checked against
the other frontend kind's equivalent; a panel may legitimately be
wider than a PTY; and mutation testing cannot reach behaviour never
modelled, which is why 16/16 passed while five real defects stood;
* dired Stage 2 (#171) as an approved framing with no implementation,
carrying the order-independence decision and the reason a token was
rejected rather than built, the already-destructive refused kill,
path-derived buffer names, and the uncorrelated URI-store writers;
* the resource-op delete guard (#186) likewise, carrying the
four-phase ordering, why `find_buffer_for_path` is the wrong
lookup, that pmacs advertises no `workspace.workspaceEdit`
capability at all, and that `pmacs.fs.remove` is guarded by
neither lane.
The ledger then drops both framing lanes (335 lines) and rewrites the
bottom-panel lane to the remaining 2B-3 plan rather than deleting it,
since 2B-3 and Stage 3 are still ahead. Two stale claims elsewhere in
the file are corrected as a consequence: the intro's "#171's lane is
below", and the recovery block's instruction not to start 2B-3 until
#187 lands.
#188 deliberately gets no lane here. Its lane lives on its own branch
and arrives with it — writing one here for work that lands elsewhere is
what makes this file re-conflict on every merge.
No runtime code. No protocol change.
One conflict, in `docs/active-work.md`. Both sides inserted a new lane
block at the same point after the folding lane — this branch added the
#188 lane, main added the #186 lane — and main additionally replaced the
dired Stage 2 lane with revision 9's expanded version. Two independent
insertions at one anchor plus a rewrite of the block immediately below
them is not something the three-way merge can align.
Resolved by construction rather than by editing markers: main's file
taken whole, with this branch's #188 lane block inserted ahead of the
#186 lane. Lane order after the folding lane is now #188, #186, #171.
Verified against both parents instead of by inspection:
* the resolved file removes zero lines relative to main, so it is a
pure insertion and nothing landed by #186 or #171 was lost;
* the #188 block is byte-identical to this branch's;
* the dired lane is byte-identical to main's revision 9 text.
Ledger staleness carried over untouched, and it is now three entries
deep: the #186 and #171 lanes both still describe themselves as open
and unmerged, and this branch's own lane will join them when it lands.
Removing a landed lane is legal only once its facts reach
`docs/agent-handoff.md`, so none of the three is relabelled here. That
update wants its own carrier.
No framing content changed. No runtime code.
Advance the canonical landed-base anchors through the docs-only main
updates, give PR 187 a complete volatile lane, and stop calling Stage
2B-2 the next unstarted slice in the durable handoff.
Record both review rounds, the round-2 code checkpoint, verification,
and exact cross-machine recovery.
Close review round four by preserving applied edits through the
borrow-free fan-out, making the transaction criteria discriminating,
removing the unproven unlock capability, requiring the CRDT divergence
fault seam, and aligning Stage 2 with owner-local identity routing.
Update the active-work lane to carry revision 5 and its cross-lane
facts.
One conflict, in `docs/active-work.md`, and it is adjacency rather than
disagreement. PR #186 inserted its lane block directly above the dired
Stage 2 lane heading; revision 9 rewrote that heading from "OPEN, STALE,
DO NOT MERGE AS-IS" to "AT REVISION 9, AWAITING APPROVAL". Git saw a
rewrite abutting an insertion and could not tell they were independent.
Resolved as both, in order: #186's lane block kept intact, followed by
revision 9's heading. Verified in both directions rather than by
inspection — the resolved file removes zero lines relative to this
branch's tip (8f42826), so nothing of revision 9 was dropped, and
differs from main only in the dired lane, so nothing of #186's was.
Note for whoever refreshes the ledger next: the #186 lane block this
merge preserves still reads "OPEN, PROPOSED, DO NOT MERGE". That was
true when main recorded it and is not true now — #186 merged as
0f4e9e0. Removing a landed lane is legal only once its facts reach
`docs/agent-handoff.md`, so it is left alone here rather than
relabelled, per the ledger's own rule.
No framing content changed. No runtime code.
Retain panel statusline segments only for the exact side-window and
buffer presentation that published them. Clear that baseline whenever
the daemon publishes authoritative Absent.
Pin both transitions: same-window buffer replacement under NoMessage,
and Absent-to-Present under NoMessage.
Resolve review round 8 by making the withdrawn resource ordering
contract singular, assigning every revision-8 acceptance item to 2a,
and specifying one exact-pair LSP tombstone lifecycle.
Replace name/path equivalence with explicit buffer-name provenance,
pin both uncorrelated state creators and tombstone reclamation, correct
the independent-order test's bite, and refresh the active-work lane.
Answers review round 3 on 8e032d7 -- three P1, two P2. All five confirmed
against the tree; none re-litigated. Withdraws three of revision 3's own
decisions.
Why revision 3's exactness argument did not extend to CRDT: it proved that
`revision` bumps between the undo push and the on_edit broadcast, which
makes the predicate exact for the v0.1 stack -- and then treated that as a
proof about cleanup in general, when CRDT mutation happens upstream of
`revision` entirely (apply_to_crdt_then_normalize_bytes runs before the
rope edit). The argument was sound about the mechanism it examined and
silent about the one it did not, and stating it as a proof is what stopped
it being checked against the second.
P1-1, the cleanup predicate. Wrong in three directions, not two. Section
3.4 is rewritten around an explicit five-variant GeneratedOutcome reported
by the apply. Direction A is fixed by restating the invariant as a property
of the buffer -- a generated-locked buffer carries no history -- so NoOp
clears; that phrasing needs no reference to `revision` and is therefore
mode-independent, which is the actual repair. Direction B gets a Diverged
variant that clears nothing and surfaces, because clearing would destroy
the last local record of the pre-edit rope; the divergence itself is
pre-existing, reaches apply_edit today, and is deferred as its own lane
rather than pretended to be fixed here. Third direction, found while fixing
the other two and not in the review: the unconditional relock locked a
fresh buffer that was never successfully written -- a mid-codepoint
generated insert on a writable *scratch* returned Err and left it
read-only. Rejected now restores the entry lock state.
P1-2, vacuous criteria 15-16. Confirmed, and the contradiction was internal
-- section 3.4 argued pre-validation makes an invalid range cost nothing,
then the criteria used an invalid range to test post-apply cleanup, in
adjacent sections. Both rewritten around a valid write that fails at the
on_edit broadcast, staged with a Rust-side FailingView (view::View is pub,
attach_view is pub). Criterion 16 splits, because section 3.4 now answers
relock-on-failure and no-lock-on-refusal differently. New 16b and 16c cover
the two P1-1 directions; 16c carries an explicit caveat that its staging
recipe is not verified here, with a four-variant fallback rather than a
criterion that passes by never reaching its path. Rule adopted: every
criterion names the exit it drives the implementation to, and that exit
must be inside the mechanism under test.
P1-3, provenance. generated_lock withdrawn. The defect is not the rule's
details but that a derived fact must be maintained by every mutation of
what it derives from, and set_read_only is pub with callers this document
does not control. Replaced by identity_protected -- a property of what the
buffer is, set once by TerminalSession::open, never written by
set_read_only. The lift/upgrade/restore cycle at
terminal_copy_mode_acceptance.rs:578-584 is transparent to it, so Q#GB12
and Stage 2 criterion 4 need no change, which is the test that the rule is
right. Section 10 now names `--test terminal_copy_mode_acceptance --features
crdt` as its own gate step: acc16e is cfg-gated, so a default-feature run
never compiles it.
P2-4, the registration dependency. Confirmed: revision 3 replaced an
existing guarded optional dependency with an unguarded call, and the
configuration the guard defends is reachable -- LuaHost is pub and nine
test files already build one. Q#GB18 becomes symmetric guarded optionality:
each module answers for its own buffers and consults the other through the
guard shape already in the tree. That also removes the teardown obligation
revision 3's registry introduced. New criterion 21 pins the minimal
harness.
P2-5, the fold criterion. Confirmed. Split into 13a [main] for the
behaviour change and 13b [mutation] for the status string, whose pre-image
cannot be main because on main the call succeeds and sets no status.
Sweep D, rules spanning v0.1/CRDT or fresh-lock/existing-lock: 13 examined,
3 broken (the two the review named plus the relock one), 1 that looked at
risk and verified clean -- undo_crdt_mode does return NothingToUndo
(src/buffer.rs:1374-1376), so Stage 2 criterion 4's assertion is correct in
both modes.
Sweep E, every criterion added in revisions 3 and 4 audited for whether the
state it asserts is reachable by the operation it performs: 19 audited, 3
defects, all fixed here. Also records that S2-17 is the one criterion for
which the pre-validation exit IS the mechanism under test, so it stays as
written.
All counts re-measured at 7586905 with the command output pasted into the
revision-4 block; all unchanged.
Status stays PROPOSED. No runtime code, no protocol change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
Review round 4 accepted the core design and the two surfaced choices,
then found two remaining contract defects and one ledger-ownership
defect. Close all three without starting runtime implementation.
Narrow the last normative "always answers" consumer to Q#RD7's honest
"always attempts while the response channel remains live" contract.
Strengthen the audit procedure to search normalized prose and term
stems so Markdown emphasis cannot hide a consuming site from a literal
phrase sweep.
Replace Q#RD12's incomplete no-op/clear/conflict table with a total
no-op/clear/refuse verdict exposed through one named internal binding.
The shared Rust helper now has an explicit mapping for missing with and
without ignore, non-NotFound stat errors, modified buffers, and
editing-in-progress buffers. Specify how both the primitive and Lua
preflight consume the same result. Add acceptance 11c for known
NotFound before earlier batch mutation and 11d for fail-closed stat
uncertainty.
Stop rewriting #171's lane from #186. Restore the sibling block exactly
to main, retain only the stable cross-lane ownership split, and record
that #171 owns its volatile state. Sequence #186 before #171 because
the urgent data-loss guard is the dependency #171 adopts, not to make a
textual conflict cheaper.
Correct #186's own ahead count to include this revision commit and add
the previously omitted lsp_dispatch_seams_acceptance file to the lane's
touch list.
Still PROPOSED. No runtime code. Implementation begins only after
explicit user approval.
Review round 3 accepted the core --- pre-filesystem refusal, four-phase
ordering, the #171 split, Q#RD10 --- and raised four P1s and a P2. All
accepted.
The external-claim audit added in revision 3 certified this document as
clean while Q#RD3, the normative decision, still asserted the very LSP
claim the audit's own row 4 marked WITHDRAWN. It did so because it
checked each claim where it was DEFINED and never at the sites that
CONSUMED it, so a withdrawal recorded in one section coexisted with the
same claim doing load-bearing work three sections away. That is a defect
in the audit procedure rather than a missed line, and since the table is
now an artifact readers trust, section 1.15 records both the failure and
the corrected procedure: grep the whole document per row and check every
hit.
P1, lane state. Re-measured: `git rev-list --left-right --count
fd7ae37...7586905` returns `13 2`, so #171 is two commits behind, not
zero, and those two are exactly PR #189's COHERENCE change. Revision 3's
"0 behind" was taken against `ad41cf1` and reported in present tense
after main had moved. Both ledger lane entries are corrected in place
--- #186's revision and base, and #171's superseded `ab42a79` /
153-behind / merge-base-`c8ec8f3` ground truth --- rather than having a
correction layered above stale text, which is what made the ledger
self-contradicting. Every count now appears as pasted command output.
P1, withdrawn claim surviving. Q#RD3 called partial application
`FailureHandlingKind.Abort`, "the strategy the spec itself assigns to
any edit containing resource changes", and section 1.11 called `Abort`
the default "by omission". The spec establishes no default for a client
advertising no strategy. Both sites now say only that verified pmacs
behaviour resembles abort-style application, resting on the reproduction
in section 1.6.
P1, acceptance 15. `WorkspaceEditResponse::from_lsp_value` returns
`Self`, its doc says a shapeless result yields an empty response, and
the binding's only `?` is `lua_to_json` over a value that arrived
through `json_to_lua` --- so no server payload can make the parse fail
and the criterion could not fail. Decision, new Q#RD11: keep the wrap,
drive the test with an explicit throwing stub, label it defensive.
Q#RD7's promise narrows to "always attempts a response while the
response channel remains live", since `send_response` is itself under an
ignored `pcall`.
P1, absent-plus-ignore. `pmacs.fs.stat` dispatches async and
`canonicalize` resolves symlinks and returns nil for a dangling one, so
it disagrees with the primitive's `symlink_metadata` on exactly the
input this query turns on. New Q#RD12 specifies a structured Rust-backed
verdict --- no-op / clear / conflict --- evaluated with the same
`symlink_metadata` call, with an error contract that fails toward
refusal. New criteria 11a and 11b supply the missing opposite direction.
P2. Criterion 14's "fails in both directions" was false: with both
duplicates clean the setup cannot distinguish first-match validation
from full validation. The claim is fixed rather than the setup, because
criterion 6 already pins validation breadth. Section 8's touch table and
section 7's gate list are reconciled.
Sweep for corrections applied at one site while a dependent site kept
the old claim: count 4 --- the two `Abort` sites above, and the stale
#171 count in both the revision history and section 1.12. Nine other
withdrawn or revised claims were checked at every consuming site and
found clean.
Still PROPOSED. No runtime code. Implementation begins only after
explicit user approval.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
All seven citations re-verified against the tree; all seven hold, one is
understated. The split with #186 is untouched.
Revision 7's process read every mechanism it CITED but never read the
mechanisms it RELIED ON: it verified that tick exists and returns
settled ids without reading that the drain is a bare try_recv with no
execution token, and it verified that kill_buffer exists without reading
the order in which it mutates before it can refuse. Reading a symbol to
confirm it is there is not the same as reading it to confirm it does
what the surrounding argument needs.
F1 — reply order is not execution order. AsyncRuntime::tick
(src/async_runtime.rs:1003) drains the reply bus with no execution
token, so a worker can finish, be descheduled before sending, and have a
later reply arrive first. Asked the cheaper question first as directed:
reconciliation does NOT need to be order-dependent. Independent
mutations commute; interdependent ones cannot arise from any production
path (dired serializes, apply_resource_op is synchronous, the fs
primitives have zero production callers); and fs.lua:155-165 already
instructs callers needing ordering to serialize. So the guarantee is
withdrawn rather than engineered. A token under a mutation lock was
rejected: it serializes every fs mutation to close a hazard with no
production reachability and a documented caller-side remedy. No static
ordering rule is offered because none works — worked through in §6.
F2 — preflight, and the review understates it. kill_buffer clears
round_trip_buffers, closes side windows and redirects ordinary windows
before BufferRegistry::remove can refuse. Rev 7 said to treat the
refusal as "keep the buffer", which reads as though skipping phase 2
restored something; it does not. reconcile_delete now preflights
editing_in_progress — sound because phase 1 is pure EditorCore with no
Lua handle, so nothing can re-enter between check and removal.
Acceptance 53b is three separately-asserted properties, each with its
own named bite.
F3 — path-equivalence, not string equality. Names come from
path.display() as given while only file_path is normalized, so a
relative open leaves a short name a string rule mistakes for
user-chosen. Tested both directions; the custom-name arm is what stops
the fix becoming a name-clobberer.
F4 — bounded tombstone, and the census corrected this revision's own
first answer. A first pass checked only handle_notification and
concluded publishDiagnostics was the only uncorrelated writer. Wrong
lens boundary: mark_document_stale takes no LspServerId and creates URI
keys across three stores for every server. The full census also found
that diag_store has zero correlated writers and that
DiagnosticStore.by_uri is keyed by URI alone with no server component —
so the store most needing protection is the one a route purge cannot
help, and a (sid, uri) tombstone would not match it. Recorded as the
round's own defect class occurring inside the round.
F5 — three outcomes, because pmacs.minibuffer.read already has three.
Cancel does not fall through to on_no: for C, declining means copy the
non-colliding entries while cancelling means abandon, and conflating
them would make C-g perform a partial copy.
Sweep for the same defect class: three candidates, two cleared by
reading (run_all_must_succeed really does collect and not abort;
commit_to_refuses_an_await_and_restores really exists), one real and not
raised by review — the framing never said which HookKind the new hooks
register with, and short-circuit would let one subscriber silently stop
every later one reconciling. Both are now all-must-succeed.
P2: pmacs.lsp.forget_uri named with an error contract and pinned; the
ledger note rewritten — it claimed single-file scope while that same
revision edited the ledger — with both line counts measured at this tree
rather than quoted.
Status unchanged: PROPOSED, needs explicit user approval. No runtime
code. Do not merge.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
Levi Neuwirth