Closes review round 2 — 1 blocking, 2 high, 1 medium — decides both
remaining open items, and re-integrates canonical `main` @ `ccf29e3`
(#172 + #157; documentation plus one `src/buffer.rs` regression test,
no protocol or Stage 2 source anchor moved).
**R2-1 (blocker) — the seam is three boundaries, not one.** Rev 2 asked
for a single document-bottom accessor. That is wrong: once a panel is
installed the present single value must DIVERGE, because several of its
consumers must not move at all. `text_area_bottom`
(`pmacs-gpu/src/main.rs:8490`) is today `status_band_top`,
`geometry_capacity_bottom`, and `document_text_bottom` at once. Rev 3
defines all three, classifies every one of its ~19 call sites as
status-owned / document-owned / geometry, and records that four sites
rev 2 named (`:3175`, `:3185`, `:6601`, `:6607`) consume a status-band
HEIGHT and no bottom coordinate at all, while the status background
`:5908` and status text `:7134`/`:7922` must stay at the physical
window bottom.
The acceptance is now a contrast assertion: installing a panel moves
every document-owned consumer WHILE the status band stays
pixel-identical. "Everything moved" alone is passed by a blanket
rewrite of the helper, which is exactly the wrong implementation.
**R2-2 (high) — epoch exactness.** `accept_frame_geometry` returns
`Advanced | Duplicate | Rejected` instead of a boolean that cannot
separate reconcile-needed from already-current from stale; if a boolean
is ever kept internally it must be named `advanced`, since `Duplicate`
is also accepted. Rev 2's exhaustion wording permitted retaining stale
geometry, which is not fail-closed — a real resize after exhaustion
would keep painting a panel sized to disowned geometry. The grid path
now clears `frame_geometry` to unknown and reconciles hidden, and the
frontend takes a terminal latch so a retained matching `Present` cannot
resurrect the band; only a fresh session clears it.
**R2-3 (high) — parent acceptance 52 splits.** 2A has no semantic panel
projection, so it can only prove the extracted painter honors an
explicit `None` map plus the `src/window.rs:562` comment fix. The real
contract is production-reachable only in 2B and is reasserted there
beside 42/43/44.
**R2-4 (medium) — touched gates named**: `statusline_segments_acceptance`,
`m11_5_semantic_acceptance`, `gpu_initial_target_acceptance`,
`gpu_font_acceptance`, beside the vterm, folding, and GPU suites.
Open items decided: `BASE_DIVIDER_HEIGHT = 4.0` at scale 1.0, scaled by
`FontMetrics::scale`, whole strip painted `ui.divider` and used as the
exact hover/drag hit rect; `TEXT_TOP` stays `16.0` unscaled, with
Q#BP15a's "all quantities use the frontend's current scale" narrowed to
font-derived metrics and the divider. Wholesale surface-inset/DPI
scaling is recorded as separate work, not smuggled in.
The ledger's bottom-panel lane keeps its census correction and gains
the three-boundary one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closes review round 1 — 2 blocking, 3 high, 3 revision points — and
rebases the ground truth onto `main` @ `d152120`.
Both blockers were rev 1 asserting something the parent framing
already decided otherwise:
- **R1-1.** Rev 1 said all 23 census reads route through
`primary_document_window`. Q#BP14 routes only the **Projection**
class that way; focus/input (#13-#15, #23), focus chrome and
surface-routed (#16-#19), and focus/session (#20) keep their own
authorities. Rev 1's rule would have broken remote-op validation and
application, `DispatchIdle`, presence, focused search/menu/completion
routing, and terminal bell ownership. §3.2 restores the four classes
as a table and the acceptance asserts each separately — the
focus-class assertions are the load-bearing half, since a test that
only proves "the document is used" passes with them wrongly
rerouted.
- **R1-2.** The three `src/statusline.rs` active reads have two
dispositions, not one. Only `:644` selects the wrong window; `:629`
and `:675` must keep tracking actual focus, because grid contexts
need a truthful `active`, revalidation must notice a focus change,
and parent acceptance 42 requires a document provider to be able to
observe `active = false` while the panel is focused.
The three high findings:
- Q#BP2S1 resolves to frontend-owned epochs (option 1) — a font or
scale transaction can need to invalidate an old `PanelFrame` while
the derived `CellSize` is identical, which daemon value dedup cannot
detect. Rev 2 adds the four-row transition table, splits grid
allocation from semantic acceptance into two APIs rather than one
ambiguous method, moves the grid allocator off `saturating_add` to
checked-with-fail-closed, and defines the initial epoch and both
exhaustion behaviors. Rev 1's "rejects a lower-or-equal epoch
carrying different data" was itself wrong: a lower epoch carrying
identical data is still stale.
- The `panel_capable` flip is narrowed to an authenticated semantic
session negotiated at **v21 or later**. Denying a v20 peer the new
events is insufficient if the daemon still places its window in a
side panel it cannot render — the gate is on placement.
- Parent acceptance criteria 37-55 are declared authoritative and
mapped to slices 2A/2B, with rev 1's eleven drafts demoted to
refinements. The painter-extraction criterion now pins cells, the
returned cursor, the focused window's `view_top` mutation, and
passive-window state.
All four scout obligations are closed (§5), and the pixel formula is
treated as contract work, not implementation detail:
- The shared/terminal-only validator boundary is named exactly.
- Four new outbox tail-coalescing tags beside the existing four.
- **`State::mono_advance` is unsafe to adopt**: absent a `FontFacts`
probe it samples the document's first shaped glyph, which would make
panel columns document-dependent. The declaration uses the existing
stable normal-face `probe_mono_advance` instead, and declares zero
usable geometry when it returns `None`.
- `BASE_DIVIDER_HEIGHT` does not exist. Rev 2 decides its scaling and
requires **one** document-bottom accessor routing every consumer
(caret, hits, minimap, terminal geometry, clipping, edge scrolling)
— a second unrouted seam is precisely the Stage 1 `Layout::compute`
two-caller defect. The concrete base value is left open for round 2.
Also: the coherence statement now names journey steps 7-10 instead of
claiming none, and drops rev 1's overclaim that this advances
background-work visibility — a panel gives output a placement but adds
no join key to COHERENCE §9's four disjoint activity planes.
The ledger's bottom-panel lane is updated from "no branch and no
framing yet" to the framing's real state, and carries an explicit
correction: that entry was itself the source of rev 1's census
mis-statement.
Factual corrections: `InitialTargetResult` is at `message.rs:1145`;
`primary_document_window` has four references and two production paths
(`daemon.rs:1639`, and `daemon.rs:2998` via `primary_document_buffer`,
which is census #22); fifteen PRs merged since the parent's last
re-scout, not eleven.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The re-framing `docs/bottom-panel-framing.md` rev 4 §2 requires before
Stage 2 (the GPU panel band) is implemented. Re-scouted against
canonical `main` @ `5aa9044`, protocol v20.
It does not restate the parent's decisions; it records what the
re-scout found. Every source anchor Stage 2 inherits had moved, but
none of the parent's mechanical model was falsified. Two facts held
and are load-bearing: protocol is still v20, so Q#BP9 resolves to
**v21** with no reservation needed, and both byte pins
(`InstanceMessage::InitialTargetResult`,
`FrontendEvent::TerminalPointer`) are still their enums' final
variants.
Four findings:
- **Q#BP2S1, new and open.** Stage 1 landed a daemon-side geometry
epoch allocator (`declare_frame_geometry`), but Q#BP15a specifies a
frontend-owned epoch echoed by every `Present`. The landed allocator
also dedups on value and uses `saturating_add`, which is neither
wrapping nor the fail-closed the framing asks for. Three resolutions
are stated with a recommendation.
- **The §1.3 census is essentially unrouted.** Stage 1 built the
`primary_document_window` seam but it has one production caller;
~80 direct `.active` reads remain. This is Stage 2's bulk, not its
tidy-up, and the stage plan sequences it first.
- **The statusline active read is three sites, not one.**
- **Four scout obligations are still open** and are named rather than
papered over, including the GPU-side pixel formula inputs.
Also carries the staged plan, draft acceptance criteria, the coherence
impact per `COHERENCE.md` §20 (§14 is the section it serves), and four
questions for the user.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stage 1 of the terminal config/copy-mode arc is in review; Stage 2 is
not started. Records the four decisions forced by scouted ground truth,
the four bites against four different wrong implementations, the two
reusable test instruments, and the gate results.
Stage 1 of the terminal config/copy-mode arc. The terminal had no
configuration surface at all: the command hardcoded $SHELL, scrollback
was a per-open argument only, and the escape chord was a literal in
Rust. No protocol change.
Profiles are a raw Lua table, pmacs.terminal.profiles, not a registry
setting: ConfigValue is four scalars with no table kind, so profiles
join pmacs.lsp.config and pmacs.pair.sets until table-valued settings
exist. The registry gains three scalars whose defaults reproduce the
previous behavior exactly.
Field resolution is explicit open argument, then profile field, then
scalar setting, then $SHELL. env MERGES, with explicit entries
overriding the profile's, because first-wins there would silently drop
half a user's environment. An explicitly named profile that does not
exist is an error even when terminal.default-profile is valid, so a typo
cannot silently fall back.
The two open-time settings resolve through the GLOBAL chain, because
they are read before the identity buffer exists and no caller could have
pinned a local override on a buffer that does not yet exist. Only
terminal.escape-key resolves per buffer, which makes a per-terminal
escape a supported feature.
The escape key is parsed at most once per (terminal, config epoch), and
the cache lives on TerminalSession so its lifetime is the terminal's,
with no purge hook to forget. The epoch alone is not a sufficient key:
it does not advance when focus moves between two terminals with
different buffer-local values, so an epoch-only cache serves one
terminal's chord to the other. An unparseable value falls back to C-c
and reports once per terminal per effective invalid value through the
status line, because a terminal with no escape chord cannot be escaped
to fix the setting that broke it.
Repeating the escape now sends THAT chord to the child through the
ordinary key encoder, rather than a hardcoded ETX. With an escape of
C-x, the previous code sent Ctrl-C and made literal Ctrl-X unreachable.
C-c t opens a terminal. COHERENCE Priority 1 names a terminal
keybinding, and section 2 step 8 grades the terminal works-but-
undiscoverable; C-c is already a live global prefix, so this is a new
leaf rather than a shadow. It is unreachable from inside a terminal,
where C-c is the escape.
Acceptance is tests/terminal_config_acceptance.rs, deliberately NOT
crdt-gated so CI actually runs it. Four bites, each against a different
plausible wrong implementation: a hardcoded ETX fails acc6/9; an
epoch-only cache key fails acc7; a single last-entry cache fails acc8's
parse count; removing the invalid-value fallback fails acc10.
Two test-instrument notes worth keeping. cat -v is the echo probe
because the screen rejects C0 controls before they reach cells, so a raw
echoed Ctrl-X would be invisible. And the probe counts occurrences
rather than testing presence, because a single-character probe collides
with the child's own banner text.
Three P1 lifecycle defects and two P2s. The focused suite was 20/20 with
every one of them live, which is the part worth keeping.
**1. The crashed primary respawned forever underneath the fallback.**
Round 2 skipped the retire call for terminal servers to avoid corrupting
them — but the crash had already armed `next_restart_at`, and
`maybe_restart` fires on every elapsed backoff with no attempt ceiling.
The broken command kept respawning under the live fallback.
The right call depends on the state, and each is wrong for the other:
`forget` REQUIRES a terminal state and removes the client outright,
which also drops the restart timer; `stop` is for a live one and
corrupts a terminal one (its not-initialized branch parks it in
`ShuttingDown` forever). `retire_server` now dispatches on state.
**2. Re-attachment targeted whatever buffer was active when the
asynchronous verdict landed.** `_attach_buffer` is an active-buffer-only
seam, and "some attachment now names a different server" is satisfied by
an unrelated Rust buffer — clearing the retry and leaving the Lean buffer
stale forever. The initiating buffer is now captured and the retry waits
for it.
**3. A failing fallback retried every tick forever, silently**,
contradicting acceptance 27's promise that a second failure surfaces.
"Waiting for the old server to go" and "attempting the replacement" are
now separate: once the old one is terminal or gone, the replacement is
attempted EXACTLY once, and a spawn failure is reported.
**4. The Lake version parser was being applied to arbitrary wrappers.**
`version_below_3_1` encodes lake's output contract; a working
`my-lean-wrapper` reporting "wrapper 1.0" would have been replaced
despite its server initializing fine. The version probe is now gated on
the command's basename being `lake`. The FAILURE latch stays
command-agnostic — that one keys on the server actually not starting,
which is true of any command.
**5. An unconfigured Lean server was reported as a failure** and latched,
poisoning the session so a later configuration could never take effect.
Absent config or command now means disabled; only a configured command
that produced no attachment is a failure.
**6. The ledger recorded pre-fix counts** after the fixes were pushed.
Now 25/25 and 3,214. That is the #161 fmt-blocker error in a slower
form: verification must describe the pushed tree.
Sign-offs requested in review: `M.fallback` is now `M._fallback`, an
underscored test seam, and its idempotence check compares args as well as
command — the same command with different arguments is not "already
applied". Dropping the `command ~= "lake"` guard stands for the failure
latch only.
Five regression tests added, and **three of them were too weak on first
write; only bite-testing found it**:
* asserting "no live non-fallback server" misses a respawn loop,
because a respawning server sits in `crashed` most of the time —
`attempt` is the observable that counts respawns;
* returning to a buffer with `find_or_open` re-fires
`buffer.after-load`, which repairs the attachment regardless of the
code under test — `switch_buffer` is the honest return;
* a MISSING command fails synchronously inside `after-load` where the
rebuild happens inline, so the async race cannot occur — only the
probe path exercises it.
Each of the five now fails against the exact round-2 mutation it targets.
Seven findings, two blocking. Every checkable claim was verified against
c8ec8f3 before being acted on; all seven held.
F1 (blocking) — the rename contract reached one path owner. Verified the
other four: Buffer::set_name documents itself as for "rename operations"
and set_buffer_path never calls it; rec.uri is cached per LSP attachment
and read at ~20 sites; dired's buffers are PATHLESS so no buffer-keyed
rebind can reach them; and the workspace-edit origin restore does not
fail gracefully — find_or_open on a renamed-away path hits
resolve_target_buffer's NotFound arm, which creates an empty path-backed
buffer, so it materializes a phantom at the obsolete path and selects it.
Rev 2 replaces the rebind with EditorCore::reconcile_rename — whole
registry, equality-or-path-component prefix, updates file_path AND name,
called by BOTH the async drain and apply_resource_op so the two cannot
drift — plus a new resource.renamed(old, new) hook so path-keyed Lua
consumers reconcile. lsp.lua recomputes rec.uri, issues didClose/didOpen,
and re-runs ensure_server because #161 keys affinity on project root, so
a cross-root move needs a different server. dired.lua follows its
handles. Verified the ordering the design needs already holds:
_async.tick calls _tick() before resuming any coroutine.
F2 (blocking) — deletion of visited paths had no policy. New §6 decides
all four cases. An unmodified visited buffer is killed; a MODIFIED one
refuses that entry, deliberately diverging from Emacs, because an
orphaned buffer is indistinguishable from a normal one and the next
C-x C-s silently resurrects the file. The check runs before the confirm
so the prompt states the skip. Adds a symmetric resource.deleted hook.
F3 (high) — the key table silently changed approved scope. The parent
lists `w` and contains no `M`. Restored `w` (Q#DR20); `M` is now an
explicit new-scope decision (Q#DR19) that REFUSES symlinks, since the
parent already ruled that the fixture's symlink-perms rejection "carries
over unchanged" and rev 1's warn-after-the-fact contradicted it.
F4 (high) — Q#DR13 contradicted the R contract. Narrowed to three
classes: set-based (D, M, C), flag-based (x), point-based (R, w).
F5 (high) — five falsifying acceptance items added, and the bite matrix
now names six mutations including "dispatch-all-then-await" and "add a
completion source to confirm".
F6 (medium) — take_settled_renames was underspecified. Took the
reviewer's preferred shape: tick returns a structured TickOutcome so
settle identity and rename metadata stay in one transaction.
F7 (2b) — defined the full C command flow with an up-front collision
scan and one confirm (declining copies the non-colliding entries), pinned
remove_dir_all's lstat safety at the primitive, and stated
dired.recursive-deletes as boolean/default false.
Not done, and said so: R is not widened to the marked set. Multi-file
rename needs a target-directory concept that does not exist, so R stays
point-based and is NAMED as a class rather than left an unstated
exception.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0126d2sikA6jZpFin3rtLCSK
Both conflicts were docs-only and resolved as unions, with one repair
taken from main: #156 fixed a pre-existing corrupted duplicate of the
"GPU initial target LANDED — #148" bullet in the handoff, whose tail ran
into the protocol-version text. This branch still carried the broken
copy, so the resolution keeps main's repaired `- Protocol **v20**` bullet
and drops the stub, along with main's now-superseded "Stage 1 IN REVIEW
as PR #165" sub-bullet.
Refreshed the canonical base to `d152120` and widened the stale-header
note from two lanes to three: #158 merged but its lane still reads
"PR #158 OPEN".
Brings the three required docs current after #158 merged, and discharges
the follow-up that framing named for itself.
COHERENCE.md section 16 audits the claim that the GPU frontend exceeds
the TUI "under real divergence pressure" without a privileged frontend
emerging. Inline math is the sharpest instance of that so far -- the GPU
typesets $...$ while the TUI shows LaTeX source, and the TUI fallback is
a named deferral. Section 25 makes that update ride the PR, so the
enumerated list gains the case along with what keeps it inside the rule:
the slice reserves no protocol version and adds no wire surface, so the
divergence is presentational and both frontends read the same model.
docs/inline-math-framing.md carried a licence error the slice framing
flagged in its own section 9 and deliberately did not fix in-branch,
since the parent is a merged document. Latin Modern Math is under the
GUST Font License, not the OFL; the row now says so and records the
~717 KiB bundled size.
docs/agent-handoff.md records the landing and re-anchors section 1 to
d152120. The bullet leads with the facts a fresh agent would otherwise
have to rediscover: the whole slice lives in pmacs-gpu because pmacs-gpu
depends only on pmacs-protocol and never on pmacs; the v0 subset is 34
Greek symbols, sub/superscript and \frac; an unsupported command fails
the WHOLE span back to source, so most inline spans in a real paper
still show LaTeX by design; and math is suppressed while the caret is
inside its span.
docs/active-work.md removes the merged lane per its own update protocol
and adds a Closed entry. Four things there are reusable beyond this arc:
a stale frontend binary is invisible from the source tree, so diagnose
with strings on the binary rather than by re-reading a checkout that is
already current; the dangerous integration was the one that did NOT
conflict, so decide from the shared-file set rather than from whether
git complained; integration is proved by predicting the other side's
test-count delta and checking it; and m4_5_basedpyright has no timeout,
hangs forever, and is intermittent, so an earlier clean sweep proves
nothing. It also corrects a claim I recorded on main: the branch's
missing CI was not an unidentified cause -- a conflicting PR builds no
merge ref, so no pull_request run is created.
Docs only; no code changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk
Round 1 review, four P1s. All real; the first two mean the fallback did
not work at all.
**1. The latch swapped the config but never spawned or re-attached.**
Nothing re-fires an attach on a config change and `attach_buffer`
early-returns for a live attachment, so the buffer stayed bound to the
server that had just been stopped. The user got a config edit and no
language server. `fire_latch` now rebuilds through a new
`pmacs.lsp._attach_buffer` export.
Two mechanics had to be right for that rebuild to happen at all:
* It is **retried on the tick**, because `pmacs.lsp.stop` leaves the
state `shutting-down`, which `server_is_live` counts as LIVE — an
inline re-attach early-returns the stale record and the swap is a
silent no-op.
* The latch **does not stop an already-terminal server**, and this is
a substrate bug worked around rather than a style choice.
`LspManager::stop` on a `Crashed` client takes its not-initialized
branch, terminates the dead process, and sets `ShuttingDown { ..
None }` on the premise that "the next exit observation cleans up" —
but the exit already happened, which is what made it `Crashed`. No
further event arrives, so the client is stuck in `ShuttingDown`
forever: `server_is_live` reads it as live so `attach_buffer` never
rebuilds, and `forget` refuses it for not being terminal. Stopping a
dead server is what makes it un-replaceable. Named in framing §6; the
fix belongs in `stop` and changes behavior for every language.
**2. A missing `lake` bypassed probe and latch entirely** — the single
most likely real failure. `ensure_server` swallows a synchronous ENOENT
and returns nil, so there was no attachment, and the hook keyed on
`active_attachment()` returned before arming anything. The hook now keys
on the buffer's LANGUAGE and treats a Lean buffer with no attachment as
the failure itself.
**3. `waitForDiagnostics` omitted `version`.** Lean's
`WaitForDiagnosticsParams` is `{ uri, version }` (v4.9.0,
`src/Lean/Data/Lsp/Extra.lean`); the request is how a client says which
revision it wants. It looked correct only because the fake server echoes
any payload — so the fake server now validates and returns InvalidParams
without it.
**4. The ledger stated the dangerous stacking order** in one sentence
and the correct rule in the next. Fixed to say BEFORE. A safety rule
written twice with opposite senses is worse than not written.
Also (P2): the probe/latch suite now drives the production path —
`buffer.after-load` -> ticks -> probe drain -> latch -> re-attach — with
real executable stubs, and asserts the originally opened buffer ends up
on a LIVE server. Round 1's acceptance 36 asserted every server was
terminal, i.e. pinned the ABSENCE of the fallback it claimed to test.
`M.fallback` is a table so the suite can point it at a working stand-in;
the probe now spawns `cfg.command --version` rather than a hardcoded
`lake`, which is also more correct for a user who configured a wrapper.
`swap_to_fallback`'s `command ~= "lake"` guard is gone: the latch fires
only when the configured server actually failed, one visible fallback
beats no server, and `probe.latched` is what keeps it to exactly one.
Three new bites, all against the committed tree: no re-attach after the
swap -> three latch tests fail; hook keyed on the attachment -> the
missing-`lake` case fails; `waitForDiagnostics` without `version` ->
acc37 fails with the server's InvalidParams.
Two stages, one arc, no protocol change. Stage 1 makes the terminal
configurable (profiles, scrollback, escape key) and binds the opening
command; Stage 2 adds copy mode and search over scrollback. They are
independently releasable and get separate branches and PRs.
Three scouted facts shaped the design, two of them ruling out the
obvious plan.
Profiles cannot be a config-registry setting: ConfigValue is four
scalars and there is no table kind, so profiles join pmacs.lsp.config
and pmacs.pair.sets as a raw Lua table while the registry holds only
scalars.
Search cannot reuse isearch in place: SearchStore addresses matches as
byte ranges into a buffer's rope, and a terminal identity buffer is
empty by construction.
An in-place copy mode would be the seventh dispatch shadow, which
COHERENCE section 6 grades weak and growing by one island per modal
feature, with no transient-keymap mechanism to migrate to.
Copy mode therefore materializes the retained rows into an ordinary
read-only buffer. isearch, motion, selection and the kill ring work
with no new substrate; the "keys must not reach the child" problem
dissolves because the snapshot is not a terminal; and describe-key
stays truthful because the bindings are buffer-local. The cost, stated
in the doc, is that the snapshot is point-in-time rather than a live
freeze.
Four review rounds produced the load-bearing parts: the escape-key
cache is owned by TerminalSession so its lifecycle is the terminal's,
with three acceptance pins that each fail a different wrong cache; the
snapshot needs set_round_trip_input because a Lua intercept does not
set Buffer::read_only and an optimistic CrdtOp would mutate both the
daemon buffer and the mirror; the double-escape must encode the
configured chord rather than a hardcoded ETX; and the two open-time
settings resolve through the global chain because they are read before
the terminal buffer exists.
No code changes in this commit.
The inline-math slice landed while this PR was open. Its own merge
removed its ledger lane, so the stale-header note above still names
exactly two; only the base anchor needed moving.
Continues docs/dired-framing.md, whose §§6-7 carry the approved shape of
marks and operations. This re-verifies every claim in them against
main @ c8ec8f3 — Stage 1 changed three of the files Stage 2 leans on
most — and adds what the parent did not decide: the batch-execution
contract, the confirmation surface, the staging cut, and acceptance.
Decisions continue the Q#DR scheme from Q#DR12.
Five corrections to the parent, one load-bearing:
- The rename rebind belongs in the drain (the parent's decision, kept)
but CANNOT be implemented in `AsyncRuntime::tick`: AsyncRuntime has no
buffer registry and no core. The seam is `pmacs._async._tick`, one
layer up, which already has `lua` in scope. AsyncRuntime harvests, the
binding rebinds.
- Line references drifted (tick 991 -> 1003, the FsUnit arm 1022 ->
1046, apply_resource_op's raw lookup 3248 -> 3249).
- The frozen fixture has NO mark-and-operate layer — eight commands, two
keys, and its "marks" are wdired text-position marks. So Stage 2 has
no in-repo reference implementation, which the parent's "45 tests pin
dired/wdired behavior" reads as implying it does.
- No y_or_n exists anywhere, and there is no runtime minibuffer.lua at
all — pmacs.minibuffer is Rust-only.
- `remove_blocking` already deletes files AND empty directories, so
`remove_dir_all` is needed only for non-empty ones. This is what makes
the staging cut possible.
Also carries a verified pre-existing defect, confirmed by probe rather
than inferred: a fire-and-forget non-stream job leaks its pending entry
forever (only stream eviction and take_result remove entries, and the
Lua handle has no __gc). Named as a deferral. The same probe establishes
that a settled job IS still readable at drain time, which is what makes
the rebind design sound.
Recommends splitting Stage 2 at the "needs a new Rust primitive" line:
2a is the mark layer plus d/x/D/R/M on the five ops that already exist,
plus the rename correctness fix; 2b adds mkdir/copy/remove_dir_all and
+/C and recursive delete. The reasoning is that 2a's only Rust is the
rename rebind, whose design is subtle enough to deserve a reviewer's
whole attention.
Coherence impact per COHERENCE.md §20 is stated in §0.5, including the
honest part: Stage 2 must add a `rename_paths` field to `PendingJob`,
which is a one-off where §9 wants a general owner/purpose — though a
side map, the alternative, is worse by §9's own diagnosis of the
parse-job link. 2b grows the closed JobKind enum 12 -> 15.
Touches only this file: docs/active-work.md and docs/agent-handoff.md
are held by the open docs PR #169.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0126d2sikA6jZpFin3rtLCSK
`main` moved through #158-#166 (Lean 4 Stage 2, COHERENCE.md, find-file,
the dired framing and Stage 1, the GPU terminal input fix) while this
documentation branch waited. Both required docs conflicted; neither
conflict was a code signal.
Resolution:
- `docs/active-work.md`: main's ledger is the base — every lane it has
gained since this branch was cut is kept verbatim. Only the
bottom-panel lane is replaced with this branch's "Stage 1 MERGED;
Stage 2 (GPU band) is next" section, and only the bottom-panel entry
is added to "Closed since the last snapshot".
- `docs/agent-handoff.md`: main's version is the base. This branch's §1
bottom-panel bullet, its §1 roadmap Arc 7 entry (which also records
that DAP is now unblocked), and its four §5 ops lessons are inserted
at their anchors.
One repair rides along. Main's `docs/agent-handoff.md` carried a
garbled fragment at §1: a duplicated, truncated "GPU initial target
LANDED — #148" bullet whose body was the tail of the old head-of-`main`
anchor bullet, leaving the `SUPPORTED=[6..=20]` protocol enumeration
orphaned mid-sentence. The fragment is removed and the enumeration is
restored as its own bullet.
No code changes; the merged tree's non-doc content is main's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Framing Q#LN7, Q#LN8, Q#LN16; acceptance 22–28, 24a/24b, 35, 36, 36a, 37.
Stacked on Stage 3a (#167), whose notification/response seams and
`pmacs.fs.canonicalize` this consumes. No protocol change; the only Rust
outside the test helper is one `include_str!` line.
**The Lake-aware root (Q#LN8).** `pmacs.project.detect` cannot express
this rule — it is innermost-wins by construction, and a Lake package's
`lean-toolchain` sits at the outermost level, so a file under
`<pkg>/.lake/packages/dep/` belongs to `<pkg>`'s server rather than
`dep`'s. The resolver walks up collecting markers and returns the
outermost, stopping at `pmacs.project.search_boundary()` so a stray
marker above a fixture cannot leak in.
Two things about the marker test are easy to get wrong in opposite
directions, and both are pinned. `io.open` **succeeds on a directory**,
so a truthiness check accepts a `lean-toolchain` directory; but
requiring a non-nil read rejects an **empty** `lean-toolchain`, which is
a legitimate marker — `locate-dominating-file` semantics are existence,
not content. The discriminator is `read`'s second return: decline only
on a non-nil error. Acceptance 24a and 24b each fail against the
implementation that satisfies only the other; both bites are recorded.
The root is canonicalized once up front, because a configured root
reaches `file_uri_for` verbatim and that URI is the affinity key (#161).
Canonicalizing the starting directory suffices — every ancestor of a
canonical path is canonical, since the walk only strips components.
**`lake serve` with a lazy probe and a one-shot latch (Q#LN7).** Nothing
runs at init: `pmacs.lsp.config` is declarative, and spawning a process
at startup for every user, Lean-using or not, is the cost rev 1 refused.
Both the probe and the server spawn are gated on a real Lean attachment.
The probe cannot gate the first attach — there is no blocking process
run, so its verdict arrives after `ensure_server` has already decided.
Hence the optimistic spawn, with the probe and latch correcting it. A
non-zero probe exit is deliberately NOT a trigger: §2.9's elan-shim case
makes `lake --version` fail on machines where `lake serve` still works,
and the server-failure latch covers that better. The probe answers only
the question failure detection would answer slowly — an old-but-working
lake that starts a useless server.
The latch stops the failing server **before** spawning the fallback, and
that ordering is load-bearing rather than defensive: the spec default is
`OnCrash`, the termination handler never consults the exit code, and
`maybe_restart` has no attempt ceiling, so a broken `lake` respawns
forever underneath the latch. `pmacs.lsp.stop` sets `restart = Never`,
which is what disarms it. Bitten: removing the stop fails acceptance 36.
The swap rewrites `command` and `args` only, so a user's `env`,
`settings`, `init_options` and `root` survive — a wholesale table
replacement would discard their `init.lua` at the moment they are least
likely to notice.
**`waitForDiagnostics` (Q#LN16)** resolves through Stage 3a's response
seam, with `M-x lean.wait-for-diagnostics` on top. `$/lean/fileProgress`
subscribes on the notification seam and is pinned end-to-end through a
new `leanprogress` mode on the fake server rather than by calling the
handler directly — the wiring is the only part that can break.
**Attribution (COHERENCE §9/§1.2).** The probe spawns as
`lean:lake-version-probe`, so a user wondering why their editor touched
`lake` finds an owner in `pmacs.process.list`. The latch reports through
`pmacs.editor.set_status` — the channel that exists — and acceptance 36a
observes that channel, so a report made only through the undefined
`pmacs.error` would fail it.
**Stage 1's acceptance 12 is updated, half superseded.** It asserted
`pmacs.lsp.config.lean4 == nil` to guard against a Stage-3 front-run;
Stage 3b is that stage, so keeping it would pin the opposite of the
intended behavior. The half that survives is the one about restraint,
and it matters more now: constructing an editor spawns nothing even
though the config exists and names `lake`, and opening a Lean buffer
with no server configured spawns nothing either. That is what holds
Q#LN7's "not at init" promise.
Bites recorded, all against the committed tree: bare `io.open` -> 24a
fails, 24b passes; require-non-nil-read -> 24b fails, 24a passes; no
canonicalization -> the symlink case spawns two servers; no stop before
fallback -> acceptance 36 fails.
#165's own commits could not update the handoff snapshot to name the
merge that contains them, so the protocol obligation lands here.
- `docs/agent-handoff.md`: absorb the dired lane into §1, replacing the
placeholder that promised exactly this. The bullet carries Stage 1's
durable substrate facts — why the tolerant `read_dir` had to be Rust,
why exposing the core normalizer beat mirroring it in Lua, the
fixed-width `_layout` contract Stage 3 reads offsets from, the
ambient-action buffer guard, treating a failure as the answer instead
of probing, the per-entry error cap, the first mode-scoped keymap and
the pre-existing test it broke, and the dedication a descent does not
carry. Refresh the head-of-`main` anchor and the last-updated line.
- `docs/agent-handoff.md` §5: two ops lessons that cost real time. A fix
must be committed before it is bitten, because `scripts/bite` restores
by `git checkout --` and reverts to HEAD; a CONFLICTING PR runs no CI
at all, because `pull_request` workflows build a merge ref GitHub does
not create while the branch conflicts, and nothing reports the absence.
- `docs/active-work.md`: remove the merged lane per update-protocol rule
4 and summarize it under "Closed since the last snapshot", keeping the
two forward items Stage 2 needs (the rename rebind is first-match-only
over a raw path, and Q#DR5's seam is the main-thread drain). Refresh
the canonical base. Flag the two lane headers that still call a merged
PR "IN REVIEW" — #161 and #166 — rather than editing lanes another
thread owns.
- `COHERENCE.md`: #165 is no longer a PR. Per §25 the audited claims this
work changed were updated when it landed; this corrects their tense in
seven places and the two prose lines that still asserted dired was in
flight.
- `docs/dired-framing.md`: status line to MERGED, and state plainly that
Stages 2 and 3 each still need their own framing.
Docs only; no code, no gate-relevant change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0126d2sikA6jZpFin3rtLCSK
Third merge of main into the lane, at b889873 (GPU terminal input #166).
Unlike the first two this one produced NO conflict -- and it is the case
that shows why a clean git merge-tree is not a reason to skip
integrating. #166 lands 41 lines in pmacs-gpu/src/main.rs, the same
heavily-rewritten file as the first integration; the two edits merged
silently only because they sit in different regions of it (#166 is
entirely in the headless probe, this lane rewrites the render path).
Merging the PR on that clean auto-merge would have shipped a combination
no gate had run.
Reconciliation, run against what #166 actually added rather than against
a pass/fail: it adds 3 library tests, 2 to vterm_stage3_acceptance, and
0 to pmacs-gpu. Predicted lib 1,826 -> 1,829, CRDT 2,003 -> 2,006, GPU
unchanged at 202; that is exactly what ran. Suite count 91 -> 92 is
#161's new lsp_multi_root_acceptance binary. All three sides' markers
verified live in the shared file.
Also records an ops trap that cost hours this session:
m4_5_basedpyright_initializes_and_negotiates_encoding does not time out,
it hangs forever, parking a --workspace sweep at 38 of 92 suites with a
live basedpyright langserver child. The per-suite M4 gate already skips
it; the workspace sweep needs the same flag.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk
The lane opened in the previous commit was scoped to the Vterm Stage 3
acceptance. Measuring it properly shows the problem is much larger and
not vterm-specific.
Comparing cargo test --list under CI's exact flags against the same
flags plus crdt: 3,024 versus 3,288. 264 tests are dark in CI, and the
single worst line is the library itself at 177 -- cargo test --lib
--features crdt is a required local gate that CI has never run. Ten
suites run zero or one test, including gpu_initial_target (#148's entire
acceptance, 1 of 14), gpu_invocation (#141's, 1 of 14), and a37, the
Stage 3 real-daemon/real-PTY/real-wgpu path that #135 built precisely
because a decoded-message fixture would prove none of the three fit
together.
The lane now carries the per-target table, the verified flag combination
for the fix, a two-part fix shape (a crdt leg on the test job, plus the
GPU-requiring suites onto the existing gpu-render job that already has
lavapipe), and an explicit instruction to sort deliberate exclusions
from accidental ones first -- some of the 264 are perf suites that are
ignored by default and belong to their own jobs, while m10_10_perf has
no ignore attribute and no job naming it.
docs/vterm-framing.md gains an as-framed audit section. The arc is
structurally complete and every test named in the Stage 2 verification
map exists, but criterion 22's "without thrash" clause was never pinned
anywhere -- the word appears nowhere in src or tests -- and that clause
describes exactly the defect #166 fixed. Of the nine Stage 3 tests, only
three drive a real daemon, so the six that construct EditorState
directly could never see a dispatcher-loop defect; a31 passes on the
broken tree for that reason. Four of the nine, including a37 and Stage 3
review round 1's own presence regression guard, do not run in CI at all.
The section also records what was not audited: section 11's blanket
claim about deferral safety covers roughly twenty items and none were
spot-checked.
docs/gpu-terminal-input-framing.md scores bet B2 true now that the
reporter has confirmed typing works, and retracts Q#GT5. The bash fixture
behind it does not reproduce in real use and was almost certainly
measuring its own timing rather than a product behaviour; it is marked
retracted rather than deleted so nobody re-derives it from an earlier
revision.
docs/agent-handoff.md section 5 gains the lesson the confirmation cost:
a daemon-side fix is not deployed until the daemon is restarted from a
tree containing it, and rebuilding a binary does nothing to a running
process.
No code changes.
CI round 1: both macOS jobs failed on the acceptance case added last
commit. APFS enforces valid UTF-8 in filenames, so `std::fs::write` with
a 0xFF byte in the name fails with EILSEQ ("Illegal byte sequence")
before `pmacs.fs.canonicalize` is ever called. The fixture cannot be
built there.
That is a filesystem refusing to represent the case, not a behavioral
difference: the subject — `to_str()` returning None for a non-UTF-8
resolution — is platform-independent Rust, and the Linux run pins it.
`#[cfg(unix)]` was the wrong granularity; review had asked for unix
gating on the symlink tests and I applied the same gate here without
checking whether the filesystem, rather than the API, was the
constraint.
Gated `#[cfg(target_os = "linux")]` with the reason in place, rather
than skipped at runtime, so a future failure here is a real failure and
not a silent no-op.
Ledger records both CI-round facts: this one, and that
`composition_overhead_under_ten_percent` is load-sensitive under a
parallel workspace sweep (it reported -4.6% realistic overhead in the
same run that tripped its 10% budget at 18.8%, which is noise, not work).
Six findings from review, one of them a real defect.
**`canonicalize` could emit a path that exists nowhere.**
`p.display().to_string()` substitutes U+FFFD for non-UTF-8 bytes, so a
resolution landing on such a path returned a plausible-looking string
that does not exist on disk — worse than nil, because this value becomes
a server-affinity key through `file_uri_for` and would silently fail to
round-trip, while the doc promised nil for anything unresolvable. Now
`.and_then(|p| p.to_str().map(str::to_owned))`: unrepresentable is a
decline, matching how the fs layer already treats non-UTF-8 symlink
targets.
Pinned by a new acceptance case that reaches a non-UTF-8 target through
an **ASCII** symlink, so the input is representable and only the
resolved output is not — the case a UTF-8 check on the argument would
miss. Bitten: restoring `display()` fails it.
The other five:
- `on_response`'s doc comment now warns that registering against a
server with no attached buffer is fire-on-death, not fire-on-reply,
because the drain visits only attached sids. It looks exactly like a
hung request while debugging, and 3b is the first caller likely to
hit it.
- `deliver_response`'s comment still carried the pre-correction
rationale ("removed BEFORE invocation ... must not be re-entered") —
the claim the bite disproved. It now says what is true: removal is
unconditional, before-vs-after is unobservable without a re-entrant
drain, and the reachable bug is gating removal on a clean return.
- Dropped `server_attempt`'s unused second return.
- Deleted a vacuous assertion in the no-attachment test (counting `_G`
entries to assert "lua globals are readable") — scaffolding that
pinned nothing, the exact shape the project's own lesson flags.
- `#[cfg(unix)]` on the three symlink-dependent tests.
Gates re-run in full. The sweep's first pass tripped
`composition_overhead_under_ten_percent` at 18.8% against a 10% budget;
it passes 3/3 in isolation here, passes in isolation on main, and the
same run reported the realistic-frame overhead as **-4.6%** — a negative
figure is measurement noise, not added work. Nothing in this diff is on
the render path. Rerun of the full sweep: 3,189 across 93 suites, zero
failures.
Main advanced twice inside one review round (#161, then #166), the
second landing while the first integration's sweep was still running.
The ledger now names both integrations, how each doc conflict was
resolved, and the verification numbers for the twice-merged tree -- plus
the lesson that a lane in review against a fast-moving main reruns its
gates per integration, not per push.
Main moved again while this lane was gating: the GPU terminal-input fix
merged as #166. One conflict, in COHERENCE.md's journey table, resolved
as the union -- this lane owns step 7's file half, #166 owns step 8's
GPU-terminal addendum.
The handoff snapshot and the active-work ledger both still described the
GPU terminal input work as in review. Per their own update protocols this
should have ridden #166; it did not, because the review that surfaced the
CI-coverage finding came after that PR was already green, and expanding
an approved PR to carry a new lane would have been the wrong trade.
docs/agent-handoff.md section 1 gains the #166 entry: the split into a
frontend-kind-neutral liveness half and a grid-only geometry half, the
extracted dispatcher loop body, the trap about the no-placement release
that reads like liveness and is not, and why the one-line guard was
rejected.
docs/active-work.md moves the lane to "Closed since the last snapshot"
and opens a new one: the Stage 3 real-path acceptance is dark in CI.
The workflow never enables the crdt feature, so every crdt-gated
acceptance test is not merely skipped but never compiled -- which covers
a37 (real daemon, real PTY, real wgpu) since #135 as well as the two
tests #166 added beside it. The fix is one step on the gpu-render job,
but it needs its own lane because it would run a37 under lavapipe for
the first time, and neither its timing budgets nor its wgpu path have
been exercised on that adapter. The lane also asks which other
crdt-gated suites are dark for the same reason.
Recorded alongside it: #166's three unit pins are not crdt-gated and do
run under CI's exact flags, including the controller-release pin whose
only job is catching the plausible wrong fix, so the regression
protection is live even though the real-daemon evidence is local-only.
No code changes.
The module doc said an uncaught raise inside a `pmacs.async` coroutine
"goes to *errors*, not the status line". #161's COHERENCE finding shows
that is wrong, and in the worse direction: `pmacs.error` is never
defined in production, so `step()`'s guarded report is dead and the raise
falls through to a bare `error()` inside `pmacs._async.tick()` -- whose
result `EditorState::tick_async` discards with `let _ =`. The failure
reaches nowhere at all, and dired would look like it silently did
nothing.
So the per-coroutine `pcall` plus `pmacs.editor.set_status` is
load-bearing, not tidy, and the doc now says which channel is dead, which
is live, and that the acceptance suite observes the live one -- the
corollary COHERENCE draws from that finding.
The ledger records the integration, the reruns on the merged tree, and
the ops lesson that cost three CI runs: a conflicting PR has no merge
ref, so GitHub creates no `pull_request` run and nothing reports the
absence.
Multi-root LSP affinity merged as #161 (`main` @ `46a1b8f`) while this
lane was in review, which made the PR conflict -- and a conflicting PR
has no merge ref, so GitHub silently stopped running CI on it after the
first push. Integrating rather than rebasing, per the #135/#137
precedent: the review anchors stay addressable and every gate is rerun
against the merged tree.
One conflict, in COHERENCE.md's in-flight list, resolved as the union of
both truths -- and #161 is now merged, which its own text still called a
PR.
The overlap to watch is `src/lua_bindings/mod.rs`: #161 widened the
`lsp.list()` row builder while this lane added `pmacs.path` and the
read_dir listing conversion. The merge was textually clean, which the
folding arc's lesson says is not the same as compiling, so the full gate
suite reruns from here.
Rev 5 said acceptance 34's second edge was a killed buffer. Implementing
it showed that is false: the Rust core fires exactly five hooks —
buffer.after-edit, buffer.after-load, buffer.after-switch,
frontend.detached, process.after-tick — and there is **no buffer-kill
hook**, so lsp.lua never tears an attachment down and the drain keeps
reaching that server. The premise (the drain builds its sid list from
`attachments`) was right; the inference needed attachments to be removed
on kill, and nothing removes them.
The reachable leak has the same root cause by a different path.
`attach_buffer` drops a sid from `attachments` the moment
`server_is_live` reports false and rebuilds against a fresh server — so
`crashed` / `stopped` is the event *least* likely to be drained, and an
event-driven purge leaks in exactly the case it exists for. The purge
therefore polls `pmacs.lsp.list()`, which enumerates the manager
directly. Acceptance 34's second half now exercises a server in **no**
attachment, which is the shape that discriminates: bitten, an
event-driven purge fails it while the attached case still passes.
§0.1 finding 6, Q#LN9, and acceptance 34 all updated; the wrong wording
is left visible with its correction rather than quietly replaced, since
the mistake is the useful part.
Ledger gains the Stage 3a lane: branch, worktree, what ships, both
corrected claims, the `install_async` load-order trap, the recorded
bites, the one knowingly unpinned guard, and gate results.
Two corrections found by bite-testing the suite rather than by reading
it.
**Acceptance 32 was mis-named.** It claimed to pin "the one-shot is
removed BEFORE invocation". Biting that — moving the removal after the
`pcall` — still passes, because `pcall` catches the raise either way and
the removal runs regardless. The before/after ordering is unobservable
unless a handler re-enters the drain, and nothing does. What the test
actually pins is that removal is **unconditional**: the bite that gates
it on `if ok then` fails 2 != 1, because the surviving registration is
invoked a second time by the purge. Renamed and re-commented to say so.
The implementation still removes before invoking, which is the right
defensive order; it is simply not what the assertion proves.
**The purge's generation check is defensive and untested**, now labelled
in place instead of reading as covered. Reaching it needs a crash and
its restart to both land in a gap with no `_async.tick`; the backoff is
500ms, so any tick in that window sees `crashed` and the
absent-or-terminal test fires first. Every attempt to stage it
deterministically exercised the `crashed` path instead. It stays as
insurance for a stalled editor, and says that about itself.
Bites recorded, all against the committed tree:
- removal gated on a clean return -> acc32 fails (2 != 1).
- purge driven by a `crashed`/`stopped` event seen in the drain, the
design the framing originally implied -> the no-attachment case fails
("never called"), while the attached case still passes. That is the
discrimination acceptance 34's second half exists for.
- a resolver without `pmacs.fs.canonicalize` -> two servers, pinned as
34b's own falsification.
Arc 8 Stage 3a (framing Q#LN9, Q#LN20). No Lean content: this changes
the event drain every LSP language runs through, and is split from the
Lean server work for the reason Stage 2 was.
**The seams.** `handle_server_requests` handled five `request` methods
and `initialized`, dropping every `notification` and `response` on the
floor. Dropping responses made `pmacs.lsp.send_request` a write-only API
from Lua — the reply was drained and discarded, so nothing outside
Rust's typed stores could consume one. Two new arms route to
`pmacs.lsp.on_notification(method, fn)` (persistent, method-keyed) and
`pmacs.lsp.on_response(sid, request_id, fn)` (one-shot). Both extend the
existing loop rather than opening a second `events_take` caller, which
would steal events from it.
A one-shot is removed **before** invocation, so a raising handler cannot
be re-entered. Every subscriber is `pcall`ed and a raise reports through
`pmacs.editor.set_status` per COHERENCE §1.2 — not `pmacs.error`, which
is defined nowhere in production. The notification list's length is
captured before the walk so a subscriber registering another cannot
extend the list being iterated.
**The purge is driven off `pmacs.lsp.list()`, not off a death event.**
The framing said acceptance 34's second edge was a killed buffer. That
was wrong, and scouting the implementation is what caught it: pmacs
fires exactly five hooks (`buffer.after-edit`, `buffer.after-load`,
`buffer.after-switch`, `frontend.detached`, `process.after-tick`) and
there is no buffer-kill hook at all, so `lsp.lua` never tears an
attachment down and the drain keeps reaching that server. No leak there.
The real leak is a different path with the same root cause. The drain
builds its sid list from `attachments`, and `attach_buffer` drops a sid
from that table the moment `server_is_live` reports false — rebuilding
against a fresh server. So the `crashed` / `stopped` event that should
trigger the purge is precisely the one most likely to go undrained. A
purge wired to that event leaks exactly when it matters.
`pmacs.lsp.list()` enumerates the manager directly and is unaffected by
attachment bookkeeping, so the purge polls it after each drain: a sid
that is absent, terminal, or running a **new generation** settles its
pending one-shots with an error. The generation check uses the `attempt`
field, because a crash-then-restart reuses the sid — without it a
one-shot would sit waiting on a reply the dead generation owed.
**`pmacs.fs.canonicalize`** (Q#LN20) is the one synchronous function on
`pmacs.fs`, and synchronous is the point: its consumer is a
function-valued `config.root` called from `ensure_server` <-
`attach_buffer` <- `buffer.after-load`, where there is no coroutine and
`pmacs.fs.stat`'s awaitable handle is unusable. It is installed from
`install_async` rather than `install_project` purely for load order —
`make_workspace` runs after `fs.lua` is evaluated, so a canonicalizer
placed there reads nil.
Acceptance: `tests/lsp_dispatch_seams_acceptance.rs`, 14 tests, driven
against `pmacs_fake_lsp` through rust so nothing needs a toolchain.
Dispatch integrity is exercised at real co-occurrence — the fake server
writes `workspace/applyEdit` and the `executeCommand` reply back to
back, so both land in one `events_take` batch. 34b asserts affinity
survives a symlinked open and is paired with its own falsification: the
same resolver minus the canonicalize call spawns two servers, so the
positive case cannot be vacuous.
COHERENCE.md section 25 and the handoff/ledger update protocols make
these ride the PR.
COHERENCE.md:
- Section 6 named one optimistic key classifier and attributed it to the
GPU. There are two, one per replica frontend:
crate::optimistic::classify_key belongs to the pmacs --attach TUI
replica, and pmacs-gpu has its own unrelated optimistic_insert_text /
optimistic_crdt_insert. The section's "kept honest by
dispatch_idle_for" claim is confirmed for both, which this
investigation verified rather than assumed.
- Section 16 graded per-frontend degradation strong on the evidence of
per-frontend fold projection. That grade stands, but the practice is
enforced by convention rather than structure, and this defect is the
counter-example; the note says so and points at what is now structural.
- Section 2 step 8 records that the terminal was broken outright on the
GPU frontend, not merely undiscoverable.
docs/agent-handoff.md section 5 gains four lessons: adjacency does not
make two operations alternatives (and two individually sound idempotence
guards can be jointly useless); bite against every pre-image the fix
could have taken, since the obvious guard here fixes the storm and
introduces a controller leak; a quiet child is an instrument, because a
frame storm hides inside a chatty fixture and a geometric readout is
satisfied by an oscillating geometry; and TerminalMode::Raw makes
sh-based input fixtures useless because there is no ICRNL.
docs/active-work.md gains the lane entry with the branch, the bite
matrix, the named out-of-scope items, and the gate results.
Framing rev 7 adds S1-10..S1-12 -- the three findings that changed
behavior, each stated as the durable lesson rather than as a diff:
painting takes a buffer and seating takes the world, so any post-await
cursor operation needs an active-buffer guard; the rendered columns are
a contract Stage 3 is planned against, so precision yields to width; and
`open_directory`'s changed-nothing-on-failure invariant is itself a
probe, which is why the symlink descent no longer lists the target
twice. Plus the tolerant-channel note: cancellation was never a backstop
for a dired listing, because nothing cancels one.
The ledger records the round, the updated counts (dired 25 + 25 CRDT,
sweep 3,189 across 92), and the process lesson that cost me the fixes
once: a mutation-bite helper restores with `git checkout --`, so a fix
must be committed before it is bitten.
The dispatcher loop applied BOTH terminal-layout syncs to EVERY attached
frontend. A semantic session satisfies both conditions, because it has a
term_sizes entry from AttachRequest and a semantic terminal declaration,
so its PTY was resized twice on every tick forever: the grid arm
installed the TUI placement size, the semantic arm installed the declared
content rectangle, and each arm's own idempotence guard only ever saw the
size the other had just written. The child took a SIGWINCH storm at tick
cadence and the screen reflowed continuously, which is what made typing
into a GPU terminal impossible while output kept flowing.
The grid arm is also the only per-tick controller-liveness release a
semantic frontend gets, so simply skipping it for those frontends trades
one defect for another: a GPU window that switches away from its terminal
would hold the controller forever, and no peer could resize that PTY
again. The semantic arm cannot take over that job, because the
buffer-follow snapshot clears the viewport declaration that would drive
it.
sync_terminal_layout is therefore split into a frontend-kind-neutral half
(panel reconciliation plus controller liveness, which read only views,
windows and the controller) and a grid-only geometry half (TUI placement
plus the resize). The dispatcher runs the neutral half for every attached
frontend once per tick, then exactly one geometry arm per frontend kind.
sync_terminal_layout survives as the composition of both halves, so the
in-process editor loop and LOCAL are unchanged.
The loop body is extracted into sync_terminal_layouts_for_tick, which
makes the grid/semantic exclusivity structural rather than two adjacent
ifs, and lets the tests drive the real loop body instead of a
re-implementation.
The release that fires when a window has no placement stays in the grid
half deliberately: a semantic frontend has no window_placements entry at
all, so moving it into the neutral half would release a GPU session's
controller on every tick.
Bite-verified against two pre-images, because one is not enough here --
the naive guard fixes the storm and introduces the controller leak, so a
single revert would score the fix complete when it is not:
pin main naive guard split
settle (acc 2+3) FAIL pass pass
controller release (acc 6) pass FAIL pass
grid still resizes (acc 5) pass pass pass
Real-path acceptance: a quiet child that counts SIGWINCH reports 144
frames in 4 s and WINCH 1..12 on screen against the pre-fix tree, versus
a settled screen with the fix. Acceptance 4 (input reaches the child and
returns) is a keep-working pin and passes on both sides -- key transport
was never the defect.
No protocol change; stays v20.
F1 (real, small-window misbehavior). `dired.revert`'s re-seat runs after
the read settles, and `pmacs.editor.move_to_line` is AMBIENT -- it moves
whatever window is active. A user who switched buffers (or hit `q`)
while the re-read was in flight had an unrelated buffer's cursor moved
to a line index that only means something in the dired listing. The
paint was already safe because it names its buffer; the seat now runs
only while dired is still the active buffer, and `seat_cursor`'s doc
says which callers are unconditionally in the right place and why.
Pinned by a test that starts the revert, switches to a six-line file
before the pump, and asserts that buffer's cursor never moved -- and
that the dired buffer still reverts when it IS active.
F2 (a trap set for Stage 3). `fmt_size` used `%10d`, so a size past ten
digits -- 10 GB and up, ordinary for VM images and core dumps -- widened
the field and shifted mtime and name right on that line alone. Cosmetic
today, but `_layout` is exported as a contract and Stage 3's
column-classifying intercept is planned against it. It now takes
`fmt_mtime`'s discipline: exact bytes while they fit, else a
fixed-width magnitude, so precision yields to the invariant rather than
the other way round. This is not the deferred human-readable column --
the exact count still shows right up to where it cannot. Pinned with a
sparse 12 GB fixture that skips if the filesystem refuses it.
F3 (honesty and a doubled read). The symlink arm claimed the probe cost
"one syscall"; it was a full `read_dir` -- opendir plus one lstat per
child -- and on success `open_directory` immediately read the same
directory again. Since `open_directory` reads before touching editor
state and raises having changed nothing (acceptance 15's invariant), its
failure IS the "not a directory" answer: the probe is gone, one read
remains, and the comment says what it actually does. New test pins both
arms -- a symlink to a directory descends under the path the user walked
(canonicalization is lexical, so the link is not resolved), and a
symlink to a file opens with the target's contents.
F4 (deliberate failure mode). A tolerant listing recorded readdir
iterator errors without bound, and `std::fs::ReadDir` need not terminate
after yielding one. Cancellation is NOT an adequate backstop here --
which is the reason for a constant rather than a comment saying it is: a
dired listing carries no supersede key, so nothing cancels it. A
directory whose iterator produces nothing but errors now fails with the
last error the way an unopenable directory does, after
READDIR_MAX_CONSECUTIVE_ENTRY_ERRORS; the counter resets on any entry
that materializes. Documented as untested and why: faking a failing
iterator needs the walk generic over it, a refactor with no other
consumer.
Smaller notes, all taken: READ_ONLY_LIMIT renamed NAME_VARIANT_LIMIT (it
caps `<2>`..`<99>`, nothing read-only); `fmt_perms`' omission of
setuid/setgid/sticky documented as a decision tied to the M8.3 fixture's
nine-bit parser; `format_outcome` binds the slice in the pattern instead
of re-traversing; and `pmacs.path.canonicalize`'s `to_string_lossy` is
noted as inside the existing non-UTF-8-path deferral rather than an
exception to it.
Process note, learned the hard way twice now: the round-1 dired.lua
fixes were briefly wiped because a mutation-bite helper restores with
`git checkout --`, which reverts to HEAD -- so a fix must be committed
BEFORE it is bitten, not after.
Two review findings, both revision edits.
**Q#LN8's marker test was wrong in the other direction.** Rev 5 fixed
the directory case by reading a byte and requiring a non-nil read — but
an **empty** `lean-toolchain` reads nil at EOF too, so that rule
declines a marker that exists, silently, falling through to
`pmacs.project.detect`. Marker semantics here are `lean4-mode`'s
`locate-dominating-file` semantics: existence, not content, and a
`lean-toolchain` can legitimately be empty.
The discriminator is `read`'s second return, probed on LuaJIT 2.1:
| Path | `io.open` | `f:read(1)` | Verdict |
|---|---|---|---|
| file with content | handle | `"l"`, no error | marker |
| empty file | handle | `nil`, no error | marker |
| directory | handle | `nil`, `"Is a directory"` | decline |
| missing | `nil` | — | decline |
So `local data, err = f:read(1)`, declining only on a non-nil `err`. The
rule needs no per-platform re-probe: both directory behaviors are
declines, since a platform whose `fopen` refuses a directory fails at
`io.open` and one that opens it fails at `read`. There is no platform on
which a directory both opens and yields a byte.
Acceptance gains **24b** (an empty `lean-toolchain` marks a root) beside
24a, with the obligation that each be shown to fail against the
implementation satisfying only the other. A suite carrying just one is
satisfied by a resolver silently wrong for the other case — which is
precisely how rev 5's first answer got written.
**Citation sweep.** Round 4 stated the `project_root_for` correction in
§0.1 without editing the citation in §2.5; the correction and the fix
are different acts, and noting one is not doing the other. Review caught
a second stale citation (`handle_server_requests` at :1448), which
prompted a sweep of every `file:line` from §2.4 onward. Four more were
stale. All six: `project_root_for` 513 → 592, `ensure_server` 527 → 610,
`handle_server_requests` 1448 → 1549, `take_typed_edit` 12798 → 12827,
`pair.lua` 213 → 229, `compile.lua` 264 → 266. Six others were verified
good and left alone, listed in §0.1 so the next sweep knows what has
already been checked.
Q#LN15's present-tense "the change is small and spans two files" now
reads as past tense with its PR number, since that stage landed. Its
pre-#161 line numbers stay as written — historical record, not
navigation.
Reported symptom: text input in a terminal does not work on the GPU
frontend while the TUI is fine.
Root cause, measured against a real daemon, a real PTY child, and the
real pmacs-gpu attach client: the dispatcher loop applies BOTH
terminal-layout syncs to EVERY attached frontend each tick. A semantic
session satisfies both conditions -- it has a term_sizes entry from
AttachRequest and a semantic terminal declaration -- so the PTY is
resized twice per tick forever, because the grid arm runs the TUI
placement helper that subtracts a modeline the GPU never draws. The
result is a SIGWINCH storm plus screen reflow at tick cadence, which
makes an interactive line editor unusable while child output keeps
flowing. Measured 730 terminal frames in 20 s for a static screen.
Revision 2 answers Q#GT4 from the code rather than deferring it, which
changes the fix from a one-line guard into a split of
sync_terminal_layout: the grid arm is also the only per-tick
controller-liveness release a GPU session gets, and the semantic arm
neither performs it nor can be made to, because the buffer-follow
snapshot clears the viewport declaration that would drive it.
The doc also records three falsified hypotheses, why the Stage 3
real-path acceptance passes on the broken tree, and a COHERENCE.md
section 6 citation error (two replica frontends have two different
optimistic classifiers; the audit names only one).
No code changes in this commit.
docs/dired-framing.md rev 6: §0 gains the Stage 1 implementation notes
(S1-1..S1-9) -- the normalizer is exposed rather than mirrored (so B2 is
false by one small binding, in the direction Q#DR2 preferred); R2-3's
dedication claim is falsified by the display policy; acceptance 3c
cannot pin the descent routing and now says so; dired is the first
builtin to bind a mode-scoped key, which one pre-existing lib test
assumed impossible; `C-x d` takes no completion source on purpose;
ownership is the handle table alone; the mark column ships blank; a
symlinked directory needs a probe; and interactive origin does not
survive an await.
COHERENCE.md, per its §25 (an audited claim this PR changes updates
here, riding the PR): §1.1's interactive-file-opening fact, §2's journey
step 7, §4's beginner-level `files`, §14's tree bullet (Stage 1 landed a
flat listing and did NOT invent a tree convention), and §15's Priority 1
list. Step 3 stays **Missing at the CLI** with the mechanism spelled
out: `pmacs .` still exits 1, and this arc deliberately does not claim
the CLI path -- it supplies the buffer a directory should resolve to.
docs/active-work.md: the dired lane rewritten for Stage 1, including why
the branch is a fresh cut rather than a rebase of `dired`, the durable
substrate facts, the bite results (one VACUOUS, recorded rather than
relabelled), and the verification. Its canonical-base line was four
merges stale and now names 8c86d34.
docs/agent-handoff.md: one forward pointer only. The handoff describes
merged state, so it absorbs the substance when this merges.
Stages 1 and 2 landed (#160, #161). Re-scouting Stage 3 against `main`
@ `46a1b8f` — six merged PRs past the rev-4 snapshot — produced three
findings that change the plan and four that confirm it. Two were
established by running Lua in a fresh `EditorState` rather than by grep,
and are marked *probed* in §0.1.
**Stage 3 violated this document's own splitting rule.** §4 says "no PR
in this arc mixes a cross-cutting substrate change with Lean feature
content" and "a reviewer looking at Stage 3 sees only Lean" — while §4's
own risk column for Stage 3 read "two `lsp.lua` generalizations". Those
cannot both be true. One generalization shipped as Stage 2; the other is
Q#LN9's dispatch seams, which modify `handle_server_requests` —
confirmed the only production drain of LSP events, since
`LspManager::take_all_events` has no non-test caller. By the test that
justified splitting Stage 2 out, that is cross-cutting substrate. Stage
3 is now 3a (seams + canonicalizer, no Lean) and 3b (the Lean server),
strictly sequential.
**The Lean resolver could not satisfy the contract Stage 2 documented.**
#161 established that a configured root reaches `file_uri_for` verbatim
and that the resulting URI is the affinity key. Probed:
`pmacs.editor.file_path()` is not canonical — opening
`<tmp>/linkpkg/sub/./../sub/a.lean` through a symlink yields
`<tmp>/linkpkg/sub/a.lean`, lexical collapse only. No canonicalize
binding is exposed to Lua, and `pmacs.project.detect` canonicalizes but
returns nil without a marker. So one Lake package opened by two
spellings would spawn two `lake serve` processes — the bug Stage 2 was
built to prevent, re-entered through Stage 3's door. New Q#LN20 adds a
synchronous `pmacs.fs.canonicalize`; it rides 3a, and it serves every
future function-valued root rather than only Lean's. Two alternatives
are recorded with why they were rejected — the `detect`-anchored walk in
particular is incorrect, not merely inelegant.
**`pmacs.fs.stat` is unusable in the resolver.** It is async and the
resolver runs synchronously inside `ensure_server` ← `attach_buffer` ←
`buffer.after-load`, with no coroutine to await on. Probed: `io` and
`os` are exposed in the sandbox, so the marker walk uses `io.open` — the
opposite of what a reader would assume, hence Q#LN8 now says so. One
edge, also probed: `io.open` succeeds on a directory, so the walk reads
a byte rather than testing for a handle, and acceptance 24a bites the
version that does not.
Confirmed rather than changed: Q#LN7's stop-before-respawn is necessary
(default policy is OnCrash, the termination handler never consults the
exit code, and `maybe_restart` has no attempt ceiling — a broken `lake`
respawns forever; `stop()` setting `restart = Never` is what disarms
it); the response seam works as specified, since `Response` events are
pushed unconditionally and `send_request` returns the keying id.
One confirmation narrowed the design. `handle_server_requests` builds
its sid list from `attachments` and `push_event` is uncapped, so
subscribers fire only for servers with a live attachment. That turns
acceptance 34 into a reachable leak: killing the buffer with a request
outstanding strands the registration behind a drain that no longer runs.
The purge is now driven from both edges and 34 exercises the buffer-kill
path, which is the one a user can reach.
Also: §9 states the lane's coherence impact per COHERENCE §20 (journey
steps, interaction islands, config registry, background attribution),
including the honest note that 3b makes §2's step-3 grade marginally
worse by adding one more instance of the silent-spawn-failure class.
Three items are named in §6 rather than paid: the uncapped event queue,
the dropped `cfg.restart`, and surfacing the spawn failure itself.
Acceptance keeps every rev-4 number. The two split sections are
bulleted with literal labels because a markdown ordered list renumbers
from its first item, and 3b's criteria are non-contiguous; round 3's
finding 4 was stale references surviving a renumber, and not renumbering
is the cheaper way to not repeat it. Stale cross-references from the
split were reconciled in the same pass, and `project_root_for`'s
citation was corrected from 513 to 592 per COHERENCE §25.
`describe_key_identifies_every_default_binding` iterated every binding
in the stack and asserted `pmacs.describe.key` resolves it context-free.
That held only because no builtin had ever bound a mode-scoped key:
dired is #129's first non-detection consumer, so its `n` / `p` / `g`
correctly resolved to nothing and the test went red on the feature
rather than on a defect.
It now sets the effective context per binding -- the mode for a
mode-scoped default, and explicitly NO mode for a global one, because a
leaked mode legitimately shadows a global chord of the same name
(dired's `RET` shadows `edit.newline-and-indent`, which is the point of
the mode) and would make the assertion compare the wrong pair. A floor
assertion keeps the new arm from going vacuous if the last mode-scoped
default is ever removed.
Also corrects a doc comment rather than leaving it to be believed:
acceptance 3c does not pin the descent ROUTING. Dired holds focus in its
own panel, so a raw `switch_buffer` lands in the same window and the
mutation is vacuous against that test; dedication is what distinguishes
the two paths, so the discriminating pin is the dedicated-panel test
next to it. Verified by mutation, not assumed.
Dired is the file surface, not a rider on one: before Stage 0 (#162)
pmacs had no way to open a file by path, and browsing is the half a
user reaches for when they do not already know the path. Stage 1 ships
the view.
builtin/runtime/dired.lua: one buffer per directory named by the
canonical path (Q#DR2) with an ownership check before any paint (F7);
read-only intercept plus round-trip input (Q#DR3); a `dired` major mode
carrying mode-scoped keys (Q#DR8) -- RET/f visit, ^ parent, n/p, g
revert, q quit, s sort; cursor re-seated by basename across every
wholesale repaint (Q#DR9); file visits through
`pmacs.window.display_file` and directory descent through dired's own
window (Q#DR10); `C-x d` / `C-x C-j`; and `dired.kill-when-opening`
through the config registry.
Two Rust changes, both narrow:
* `read_dir` grows per-entry tolerance behind an opt (Q#DR6). Five
per-entry conditions used to fail the entire listing, so a plain
refresh of a busy directory could just fail; the module doc's claim
that a tolerant wrapper was "the package's job" was false, because
the primitive hands Lua one structured error and no partial vec.
Per-entry readdir/lstat/readlink failures and non-UTF-8 symlink
targets now land in an `errors` channel; parent-level failures and
non-UTF-8 *names* stay fatal. The tolerance travels in the settled
payload, so the Lua boundary keeps the bare-array shape the frozen
M8.2 fixture consumes and never has to look the job back up. The read
ops' opts parsing now rejects unknown keys, so a typo'd `tolerant`
cannot silently degrade to the fatal contract.
* `normalize_buffer_path` is exposed as `pmacs.path.canonicalize`
rather than mirrored in Lua. Q#DR2 named exposure the preferred end
state; it needs no borrow plumbing, so dired's name-dedup and
`display_file`'s `find_buffer_for_path` dedup cannot fork, and the
mirror's Stage 2 removal is not owed.
tests/dired_acceptance.rs covers framing items 1-16 (22 tests), driven
through real key dispatch. Item 17 is the m8_1/m8_2/m8_3 gate.
One framing claim is corrected by the substrate: R2-3 expected a
dedicated dired panel to carry its dedication across a descent, but
`display_buffer` never replaces the buffer in a slot dedicated to
another one -- it discards every side-specific parameter and falls back
to the document window (Q#BP3 2.iii). Dired does not try to unpin the
user's panel; both arms are pinned.
Lean 4 Stage 2 (#161) landed while this branch's first-ever CI run was
in flight, which put the PR back to CONFLICTING at an unmoved head.
Merged rather than rebased, same as the 8c86d34 integration and for the
same reason: the PR is awaiting review rounds and a rebase would break
every review anchor.
The sole conflict was docs/active-work.md, as it was last time and for
the same structural reason -- every merge to main edits the lane ledger,
so a long-lived PR re-conflicts there and only there. Both sides' lanes
kept verbatim; main's updated Lean 4 heading taken over the stale one.
This integration is code-disjoint from the lane. Intersecting main's
changed files (COHERENCE.md, builtin/runtime/lsp.lua,
src/lua_bindings/mod.rs, tests/lsp_multi_root_acceptance.rs) against the
lane's own changed-file set leaves exactly docs/active-work.md, so none
of the first integration's pmacs-gpu/src/main.rs auto-merge risk recurs
here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HZjWMjwPXhPbt9upku9mCk
Separates the pre-integration numbers, which described a tree 28 commits
behind, from the ones that describe what the PR now proposes.
The GPU test count is the integration proof rather than merely a pass: it
went 199 to 202, and e547a90 added exactly three tests to pmacs-gpu,
which is the entire delta on main since the merge base. Both sides' tests
are therefore present and running, and neither was dropped by the
auto-merge. Confirmed structurally as well -- main's fix survives as the
deferred closure form rather than the eager one that panicked, with its
regression test, alongside this lane's math work in the same file.
The lane was 28 commits behind. Merged rather than rebased, per the
#135/#137 precedent: the PR is awaiting review rounds and a rebase would
break every review anchor.
The only conflict was docs/active-work.md, where both sides add lanes.
Kept both: main's lanes verbatim, with this lane leading since it is the
one in flight. The conflict was pre-existing rather than introduced by
the dired or Lean 4 ledger commits -- it already conflicted against main
at e745068.
The integration surface, derived from git diff merge-base..main rather
than from another PR's file list, is pmacs-gpu/src/main.rs: main gained
72 lines there from e547a90, the minimap all-blank-slab divide-by-zero
fix, and this lane rewrites large parts of the same file. Git auto-merged
it textually. A clean auto-merge is not evidence the tree compiles, so
the full gate suite is what discharges it; the ledger records the
post-integration numbers separately from the pre-integration ones, which
described a tree 28 commits behind.
Levi Neuwirth