Commit Graph

5 Commits

Author SHA1 Message Date
Levi Neuwirth fb14dc9ec3 test(isolation): migrate the corpus off the ambient roots
The mechanical half, riding on the census in the previous commit.

* 342 in-process construction sites in 65 files now take
  `new_with_roots` / `open_with_roots` with `iso::roots()`. The isolated
  base is a pure function of `CARGO_TARGET_TMPDIR` — no counter, no
  `OnceLock` — so two copies of the module in one binary agree instead of
  racing, and the tree lives somewhere `cargo clean` owns rather than
  leaking into `/tmp` once per run. It is shared deliberately:
  materialization is content-gated and idempotent, so a per-test
  directory would repeat it ~330 times per run for a byte-identical
  result.

* `journey_acceptance` keeps the ambient `EditorState::open`, because
  proving the production entry point has a caller is the whole of what
  that ratchet is for. Rev 2's "isolated by the environment its binary is
  launched with" was not a mechanism — cargo launches each test binary
  with the caller's environment, and a binary cannot re-point its own
  roots before its tests run. Each test is now a thin parent that
  re-execs this binary for its own name with controlled roots, and the
  child runs the body against production's call. Two pins guard it: the
  child asserts all four roots resolve inside the controlled base, and
  the suite asserts against its own source that it has not quietly taken
  the seam. The parent also asserts the child ran `1 passed` — a stale
  `--exact` filter would otherwise hollow the whole thing out silently.

* The shared spawners take all five storage variables.
  `spawn_daemon_process_with_env` set `HOME` and `XDG_CONFIG_HOME` only;
  `HOME` is a FALLBACK, so it isolates a root only while the matching
  `XDG_*` is unset — the harness's apparent adequacy was a property of
  one developer's environment. The PTY spawner backfills whichever of the
  five its caller did not pin. The 10 direct `Command::new` daemon and
  attach spawns get the same treatment.

Three suites had `mod common;` behind `#[cfg(feature = "crdt")]`;
`common::iso` is needed in every build, so those are ungated. Files that
already pull in `common` reach `iso` through a `use` rather than a second
`#[path]` declaration — loading one file as two modules is
`clippy::duplicate_mod`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
2026-07-31 18:48:45 -04:00
Levi Neuwirth b70393762e test(lsp): gate the non-UTF-8 fixture on linux, not merely unix
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).
2026-07-25 17:03:10 -04:00
Levi Neuwirth a9ef257930 fix(lsp): decline a non-UTF-8 canonicalization; round 1 review
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.
2026-07-25 16:53:27 -04:00
Levi Neuwirth 027c7d18e0 test(lsp): name acc32 for what it pins; label the unpinned guard
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.
2026-07-25 15:58:34 -04:00
Levi Neuwirth 12236b265d feat(lsp): notification/response dispatch seams and fs.canonicalize
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.
2026-07-25 15:37:48 -04:00