diff --git a/COHERENCE.md b/COHERENCE.md index 39b3f40..1af9981 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -397,7 +397,7 @@ Full verdict table: | 6 | Language intelligence | **Partial** | Rust grammar bundled and auto-attaches; rust-analyzer preconfigured (`builtin/runtime/lsp.lua`). **Journey Stage 1b-2 (PR open) ends the silence** for a server that fails to *start*: the status line names the command, language and errno once per `(language, root, command)`; the modeline reads `LSP:!` instead of nothing; and `M-x lsp.status` renders `*lsp*` over the `status_buffer_text()` renderer that had existed since M4.8 with no caller. **Still Partial**, and flips only on merge (§25): a server that starts and then *crashes* is still unsurfaced | | 7 | Find symbol / file | **File: fixed (open by path merged #162; browsing #165). Symbol: works but undiscoverable** | No find-file/dired/picker existed at audit. Now `C-x C-f` opens a known path and `C-x d` / `C-x C-j` browse (flat listing, `dired` mode keymap); `M-.`/`M-?`/`C-c o` still bound but advertised nowhere and server-gated; no workspace-symbol command; `pmacs.index.*` has no UI | | 8 | Open terminal | **Works** | Full PTY with scrollback + modeline segment, bound to `C-c t` and configurable through three registered settings (`terminal.default-profile`, `terminal.scrollback-rows`, `terminal.escape-key`) plus named `pmacs.terminal.profiles` (PR #173), and searchable through `M-x terminal.copy-mode` / `C-c C-t`, which materializes the retained scrollback into an ordinary read-only buffer (Stage 2). Named limitations: `C-c t` is unreachable from *inside* a terminal window, where `C-c` is consumed as the escape — `M-x terminal` still works there; and there is still **no close/kill command**, which is the remaining half of this step's discoverability gap. *Was broken outright on the GPU frontend until the double terminal-layout sync was fixed: the child took a `SIGWINCH` storm at tick cadence, so typing into it was impossible while output still flowed.* | -| 9 | Build / test | **Partial** | `M-x compile.run` works, defaults cwd to detected project root, parses Rust `-->` errors — but no keybinding, an **empty first prompt** (`initial = last and last.cmdline or ""`, `builtin/runtime/compile.lua:1134-1138`), and no `cargo build`/`cargo test` suggestion despite `ProjectKind::Cargo` existing (`src/project.rs:77`) | +| 9 | Build / test | **Works** | Journey Stage 1b-1 (#203): `C-c c` runs `compile.run`, and the first prompt is prefilled from the detected project kind (`pmacs.compile.defaults`, seeded `rust = "cargo build"`, extensible from `init.lua`) via `ProjectKind::Rust` — **not** `Cargo`, see §24. The prompt **captures** its directory rather than re-resolving at accept time, so the command it offers and the directory it runs in cannot drift while the minibuffer waits. Still defaults cwd to the detected project root and parses Rust `-->` errors. Named limitation: after `pmacs ` the active buffer is dired's and pathless, so the cwd falls back to the process cwd — §8's execution-location model owns that, and the degradation stays coherent (no suggestion is offered for a directory with no detected Cargo project) | | 10 | Inspect error | **Partial (good once reached)** | `E:n W:n` modeline counts, underlines, `M-g n/p` + ``C-x ` `` walking a unified compile/grep/diag source, message echo, `RET` visits. Gated entirely on step 6 or 9 succeeding first | | 11 | See background work | **Works but undiscoverable** | `*workers*` view via `M-x editor.list-workers`; `C-c C-k` cancel-at-point. No keybinding, no statusline spinner/progress indicator anywhere (§9) | | 12 | Close + restore | **Partial** | Per-file cursor+scroll (saveplace), recent files, minibuffer history, autosave recovery all restore zero-config. Open-buffer set and window layout do **not**: desktop-save is opt-in (`pmacs.session.desktop_mode(true)`) *and* a documented no-op under a daemon (`src/desktop.rs:323-326`, `:353-356`, Q#DS9) | @@ -407,12 +407,15 @@ A journey observation worth keeping verbatim from the audit: C-M-s` opens all folds, while opening a file, opening a terminal, and running a build have no bindings at all. -Two of that observation's three examples have since been answered — -opening a file by `C-x C-f` (#162) and opening a terminal by `C-c t` -(#173). **Running a build still has no binding**, and the underlying -inversion is a standing bias in how new work gets bound, not three -isolated omissions: the quote stays as written because it names the -pattern, and the pattern is not retired until step 9 is. +All three of that observation's examples have now been answered — +opening a file by `C-x C-f` (#162), opening a terminal by `C-c t` +(#173), and running a build by `C-c c` (Journey Stage 1b-1, #203). +**The quote stays as written**: it names a standing bias in how new work +gets bound, not three isolated omissions, and three fixes do not retire +a bias. What has changed is that the bias no longer has an uncontested +example in the golden journey — a new surface that ships without a +binding would be evidence the pattern is live again, and should be read +that way. --- @@ -1561,15 +1564,18 @@ blockers. **State: runs to step 5; thin from step 6 (§2). Mostly wiring, and unusually cheap:** directory-argument handling (**done**: Journey Stage 1a); a find-file surface (**done**: #162 open-by-path, #165 browsing); surfacing the LSP spawn failure with guidance (**in flight**: -Journey Stage 1b-2, §1.2); a compile keybinding + `cargo build`/`test` default from the existing -`ProjectKind::Cargo`; a terminal keybinding (**done**: `C-c t`, #173); a -welcome buffer. The journey acceptance suite (§19) is the ratchet that -keeps it fixed — it **exists now** (`tests/journey_acceptance.rs`, -Stage 1a), seeded with steps 2, 3, and 5. +Journey Stage 1b-2, §1.2); a compile keybinding + `cargo build`/`test` +default (**done**: Journey Stage 1b-1, #203, from the existing +`ProjectKind::Rust` — **not** `Cargo`, see §24); a terminal keybinding +(**done**: `C-c t`, #173); a welcome buffer. The journey acceptance +suite (§19) is the ratchet that keeps it fixed — it **exists now** +(`tests/journey_acceptance.rs`, Stage 1a), seeded with steps 2, 3 and 5, +and carrying step 9 since #203. -Journey Stage 1b is the named remainder, and it splits: **1b-1** (the -compile binding + project-kind defaults) and **1b-2** (LSP spawn -guidance) are both in flight; **1b-3**, the welcome buffer, remains. +Journey Stage 1b is the named remainder, and it splits: **1b-1 — the +compile binding + project-kind defaults — landed as #203**; **1b-2** +(LSP spawn guidance, step 6) is in flight; **1b-3**, the welcome buffer +(step 4), remains. ### Priority 2: Make workspace and location explicit @@ -1640,7 +1646,9 @@ implementation — this list is direction, not commitment): `resolve_target_buffer` unification, the destination-scope substrate, and the first journey acceptance suite. It routes `pmacs .` into #165's dired buffer rather than growing a second directory surface. - **Stage 1b-1 / 1b-2 — in flight**: compile defaults and bindings; + **Stage 1b-1 — landed (#203)**: the compile binding and project-kind + defaults, with the prompt capturing its directory rather than + re-resolving it at accept time. **Stage 1b-2 — in flight**: LSP-failure surfacing. **Stage 1b-3 — remaining**: welcome buffer. 2. **Discovery surface** (P4): the describe/list/where-is command family, M-x rich rows, help unification, help prefix. @@ -1738,6 +1746,17 @@ document is wired into CLAUDE.md/AGENTS.md as required reading: rate was once per file open, because a failed spawn leaves no record for the affinity loop to find. Corrected in place by Journey Stage 1b-2, along with three stale line citations in the same paragraph. +- **This document named a `ProjectKind` variant that does not exist**, + in two places: §2's step-9 row and §20 Priority 1 both said + "`ProjectKind::Cargo` existing (`src/project.rs:77`)". Line 77 is the + *doc comment*; the variant on line 78 is **`ProjectKind::Rust`**, + produced by the `Cargo.toml` marker. The audit read the comment and + named the comment. Corrected by Journey Stage 1b-1 — which also + establishes that the Lua side never sees the variant at all: + `pmacs.project.detect` returns the **tag string** `"rust"`. Kept here + rather than silently fixed, because a wrong type name in the document + work is evaluated against costs a scout a real detour. + - `docs/keybindings.md` — every `src/editor.rs` line citation in §3 is stale by ~250–1000 lines despite a "last verified @ `f8096ff` (2026-07-20)" stamp; its shadow list also omits the terminal `C-c` diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 544c1d7..e6e85d2 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -620,6 +620,60 @@ local function daemon_working_directory() return nil end +-- Where a run started right now would execute. Extracted from the +-- inline expression that used to live in `pmacs.compile.run`, because +-- the interactive command needs the SAME answer to build its prompt: +-- a suggestion computed from a different rule than the run obeys can +-- name a toolchain that is not at the directory the command executes +-- in (journey Stage 1b-1 framing §3.1). +local function resolve_cwd(explicit) + return explicit or project_root_of_active() or daemon_working_directory() +end + +--- Default compile command per detected project kind, keyed by the tag +--- `pmacs.project.detect` returns. Assign into this table from +--- `init.lua` to add or override one: +--- +--- pmacs.compile.defaults.go = "go build ./..." +--- +--- Only `rust` ships seeded, and that is a decision rather than an +--- omission: Rust has one answer, while npm/yarn/pnpm, make/cmake, and +--- `go build` versus `go test` do not. A wrong prefill costs more than +--- an empty one — the user must delete it before typing. +pmacs.compile.defaults = { rust = "cargo build" } + +-- Read `defaults[kind]` defensively. The table is public and +-- assignable, so a metatable with a throwing `__index`, a non-string +-- entry, or a non-table replacement all have to be survivable — the +-- same discipline `validated_rules` keeps for a hostile rule container +-- (round-2 finding 3). A broken `defaults` degrades to the pre-stage +-- empty prompt; it never prevents compiling. +local function default_command_for(kind) + if type(kind) ~= "string" then return nil end + local ok, value = pcall(function() return pmacs.compile.defaults[kind] end) + if ok and type(value) == "string" and #value > 0 then return value end + return nil +end + +--- Where the next compile would run, and what kind of project is +--- detected *from that directory*. Public getter (per API +--- conventions): `{ cwd = string|nil, kind = string|nil }`. +--- +--- `kind` is always `detect(cwd)`, one rule regardless of which branch +--- produced `cwd`. Note that is not the same as "`cwd` is a project +--- root": a Cargo workspace subdirectory contains no `Cargo.toml`, is +--- detected as `rust` by the ancestor walk, and is a perfectly good +--- place to run `cargo build`. +function pmacs.compile.context(explicit_cwd) + local cwd = resolve_cwd(explicit_cwd) + local kind = nil + if cwd then + local ok, proj = pcall(pmacs.project.detect, cwd) + if ok and proj and type(proj.kind) == "string" then kind = proj.kind end + end + return { cwd = cwd, kind = kind } +end + local function format_exit_marker(label, ev) if ev.kind == "exited" then return string.format("\n[%s exited with code %d]\n", label, ev.code or 0) @@ -762,7 +816,7 @@ local function start_run(slot, cmdline, opts) if cur and not pmacs.compile.is_generated_buffer(cur) then slot.prev = cur end - local cwd = opts.cwd or project_root_of_active() or daemon_working_directory() + local cwd = resolve_cwd(opts.cwd) -- Supersede (Q#CM9): terminate the old group and tombstone its -- pump entry; its terminal event still drives forget. @@ -1132,13 +1186,27 @@ pmacs.command.define { description = "Compile: run a command in a streaming *compilation* buffer (M-x compile).", fn = function() local last = pmacs.compile._last + -- Captured ONCE, here. `pmacs.minibuffer.read` is asynchronous and + -- nothing freezes the active window while a prompt is open, so + -- re-resolving inside `on_accept` would let the prompt offer + -- `cargo build` for A and execute in B. Sharing the resolver is + -- necessary and not sufficient; the resolution has to be captured. + -- This is Journey Stage 1a's `commit_to` discipline on a smaller + -- seam — the mechanism differs, the failure prevented is the same. + local ctx = pmacs.compile.context() pmacs.minibuffer.read { prompt = "Compile command: ", history = "compile", - initial = last and last.cmdline or "", + -- `_last` still wins: a user who ran `cargo test` once gets it + -- back rather than being reset to the project default. + initial = last and last.cmdline or default_command_for(ctx.kind) or "", on_accept = function(cmdline) if cmdline == nil or cmdline == "" then return end - pmacs.compile.run(cmdline) + -- `ctx.cwd` passes through verbatim, INCLUDING nil: a nil cwd + -- means every resolution step failed, and the header renders + -- "(unknown)" exactly as before. Re-resolving here would + -- reintroduce the drift for the case least able to tolerate it. + pmacs.compile.run(cmdline, { cwd = ctx.cwd }) end, } end, @@ -1186,3 +1254,16 @@ pmacs.keymap.bind { scope = "global", sequence = "M-g n", command = "error.next" pmacs.keymap.bind { scope = "global", sequence = "M-g p", command = "error.previous" } pmacs.keymap.bind { scope = "global", sequence = "C-x `", command = "error.next" } pmacs.keymap.bind { scope = "global", sequence = "M-!", command = "shell.command" } +-- Journey step 9 (COHERENCE §2): running a build had no binding at all, +-- while `C-c @ C-M-s` opened all folds. `C-c c` is free, sits under the +-- established `C-c` prefix, and does not collide with CUA copy (that is +-- `M-w`). Bound here rather than in `default.lua` because a runtime +-- module owns its own global keys — `terminal.lua` binds `C-c t`, +-- `lsp.lua` binds `C-c o`. +-- +-- Two inherited reachability limits, both pre-existing: inside a +-- terminal window `C-c` is consumed as the escape key, and the repl +-- package binds `C-c` at buffer scope. `M-x compile.run` still works in +-- both. `compile.recompile` gets no global chord — `g` in +-- `*compilation*` already covers rerun. +pmacs.keymap.bind { scope = "global", sequence = "C-c c", command = "compile.run" } diff --git a/docs/active-work.md b/docs/active-work.md index c3218b7..696ff0f 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -255,105 +255,63 @@ If it does not, stop and repair the remote/fetch configuration. never been enforced. Any CI job that compiles the `crdt` targets has to fix them first or it will be red on arrival. -## Journey Stage 1b-2 (P1) — IMPLEMENTED, PR OPEN +## Journey lane (P1) — 1a and 1b-1 MERGED; 1b-2 PR OPEN; 1b-3 REMAINS + +**Rewritten, not removed, at #203's merge.** Rule 4 removes a lane when +its ARC is done; the journey arc is not — 1b-2 is in flight and 1b-3 is +unframed. Stage 1a (#182/#183) and Stage 1b-1 (#203) are on `main` and +their durable facts are in `docs/agent-handoff.md` §1, which is rule 4's +precondition satisfied rather than deferred. + +**#203's merge obligation is DISCHARGED**: `COHERENCE.md` §2's step-9 +row now reads **Works**, §2's keybinding-inversion paragraph records all +three examples answered (the quote itself deliberately unchanged), §20 +Priority 1 and its arc list say "landed", and the handoff bullet says +LANDED. That flip rides *this* branch rather than a standalone docs PR, +because #204 already touches all three files and a separate PR would +re-conflict on every merge. - **Branch `journey-stage1b2-lsp-guidance`**, worktree - `../pmacs-journey-1b2`, based on `githubsucks/main` @ `fbcf235`. + `../pmacs-journey-1b2`, based on `githubsucks/main` @ `fbcf235`, + **integrated with `main` @ `1f290d5` (#203)**. `docs/journey-stage1b2-lsp-guidance-framing.md` revision 4, three review rounds closed (round 1: two blocking, three major, one minor; round 2: two blocking, two cleanups; round 3: one blocking; all - accepted). - Sibling of Stage 1b-1 (PR #203, step 9); this is step 6. -- **What it is.** `COHERENCE.md` §1.2's canonical silence: a - preconfigured-but-missing language server fails with no status - message, no record, and no modeline marker, while tree-sitter - highlighting keeps working and masks it. The stage reports the - failure with guidance, wires an `M-x lsp.status` surface, and gives - the modeline a way to say "failed" rather than nothing. -- **Half of it is already built and unwired.** - `LspManager::status_buffer_text()` renders "the `*lsp*` status buffer", - `last_error(sid)` exists, **both are exposed to Lua and tested** - (`pmacs.lsp.status_buffer_text`, `src/lua_bindings/mod.rs:10949`; - `tests/m4_acceptance.rs:2634`) — and there is **no production caller, - no `*lsp*` buffer, and no command**. Several `src/lsp.rs` and - `src/project.rs` doc comments refer to that buffer as if it exists. -- **The reporting pattern is already adopted twice in `lsp.lua` itself** - — root-resolver failures (`:570-585`) and subscriber failures - (`:1831-1836`), both `pcall(pmacs.editor.set_status, msg)` with the - `pmacs.error` arm riding along. The canonical case at `:658-674` was - simply never converted. This stage finishes an adoption; it does not - start one. -- **`COHERENCE.md` §1.2's frequency note is wrong, and it decides the - design.** It says the failure fires "once per project root". - `LspManager::spawn` returns early on failure *before* both - `status_tracker.ensure` and `clients.insert` (`src/lsp.rs:1287-1297`), - so a failed spawn leaves **no record at all**, `pmacs.lsp.list()` - cannot see it, and `ensure_server`'s affinity loop re-spawns. The real - rate is **once per file open**. Hence the rule: **memoize the report, - not the failure** — the spawn is still retried, so installing the - binary mid-session recovers with nothing to invalidate. + accepted). **Implemented; PR #204 open.** +- **What it is.** `COHERENCE.md` §1.2's canonical silence, journey step + 6: a preconfigured-but-missing language server now reports with + guidance, marks the modeline `LSP:!`, and appears in `M-x lsp.status`. +- **Half of it was already built and unwired.** + `LspManager::status_buffer_text()` and `last_error()` have existed + since M4.8, exposed to Lua and tested, with **no production caller** + and no `*lsp*` buffer, while several `src/lsp.rs` and `src/project.rs` + doc comments refer to that buffer as though it existed. +- **The reporting shape was already adopted twice in `lsp.lua` itself** + (root resolvers, notification subscribers). The canonical case was + silent because nobody had converted it — this finishes an adoption. +- **`COHERENCE.md` §1.2's frequency note was wrong, and it decided the + design.** `LspManager::spawn` returns early *before* both + `status_tracker.ensure` and `clients.insert`, so a failed spawn leaves + **no record**, `pmacs.lsp.list()` cannot see it, and the affinity loop + re-spawns: the real rate is **once per file open**, not once per + project root. Hence **memoize the report, not the failure**. - **The affinity key is `(language, key_uri)`, and `key_uri` is nil for - markerless files** — `ensure_server` sets it only when the root came - from config or a marker walk (`lsp.lua:644-648`), so loose files in - unrelated directories deliberately share one server per language. - Round 1 caught revision 1 keying the memo on the resolved *root*, - which would have split what the runtime shares and re-reported one - failure per directory. -- **Dedupe and current-failure state are two records, not one.** - `reported` is keyed by `(language, key_uri, command)` and never - cleared — the command is in the key so repointing config at a - different missing executable reports again. `failures` is keyed by - `(language, key_uri)` and **cleared when a spawn for that key - succeeds**, so `*lsp*` stops showing a failure the user has fixed. - A third, buffer-keyed projection feeds the modeline, because the - statusline provider is a **pure per-buffer lookup** and deriving an - affinity key inside it would run root resolvers and project detection - every frame, for every window. **Each projection carries its affinity - key and a success sweeps every projection holding it** — round 2 found - that clearing only the succeeding buffer leaves an earlier buffer - showing `LSP:!` while `*lsp*` reports nothing wrong, which is the - normal case for a project with more than one file. -- **`key_uri` is nil and Lua cannot index by nil** (`t[nil] = v` raises - `table index is nil`, checked under LuaJIT and 5.4), so the framing - prescribes **one** key encoding used by both tables: - `language .. "\0" .. (key_uri and ("u" .. key_uri) or "n")`. The - `u`/`n` discriminator is what makes markerless unambiguous against any - URI, and Lua strings are 8-bit clean so `\0` is a safe separator. -- **A new per-buffer table needs a removal path, and no existing one - would have supplied it.** Round 3 found the buffer projection had no - teardown, so killing a failed buffer leaked its entry for the session - and the sweep bound was false. The LSP resource reconciliation finds - work through `attachments_under` (`lsp.lua:2934-2944`), which iterates - `attachments` — and a failed buffer has **no attachment by - construction**, so it is invisible to every existing cleanup path. The - fix registers `pmacs.buffer.on_removed` once per projection, releases - the handle on the success sweep but **not** from inside the removal - callback (dispatch does `callbacks.take(id)` then iterates a local - vector, `src/lua_bindings/mod.rs:1949-1957`), and **clears** rather - than re-keys on `resource.renamed` / `resource.deleted` — re-keying - would assert a failure at a location where none was observed. -- **The durable surface cannot show the failure today.** - `status_buffer_text` renders from `self.clients`, which a failed spawn - never enters. The stage keeps its failure record in Lua and renders it - as a section above that output, rather than reshaping Rust's status - model before anyone has used the surface. Stated as a limitation, with - promotion named as follow-on. -- Its §1.7 records four stale `COHERENCE.md` §1.2 citations - (`:614-626` → `:658-674`; `:895-897` → `:1019-1021`; the frequency - note; and the now-false implication that no background failure is - reported anywhere). -- **ON MERGE, flip the step-6 grade.** `COHERENCE.md` §2's step-6 row - stays **Partial** while the PR is open, per §25's landed-evidence - rule, and says so in the row itself. §20 Priority 1 and the arc list - say "in flight". Same obligation shape as 1b-1's. -- **Bites, all directed.** Full revert fails 14 of 16 pins; removing the - modeline branch fails 6; removing the sweep fails **exactly one** — - the shared-affinity pin written for round 2's blocking finding, which - confirms no other pin covers it; removing the rename/delete - disposition fails exactly those two; removing the dedupe memo fails - the two dedupe pins; and keying the memo on the resolved root (the - revision-1 design) fails **only** the markerless pin, which is the - case where root and affinity key differ. + markerless files**, which deliberately share one server per language. + Lua cannot index by nil (`t[nil]` raises), so one encoding function + serves both tables with a `u`/`n` discriminator no URI can collide + with. +- **Three tables, three lifetimes**, plus a buffer-keyed projection for + the modeline — that provider runs for every window on every paint, so + deriving an affinity key inside it would invoke user root resolvers + during painting. **A success sweeps every projection sharing the key**, + and the projection has its own `pmacs.buffer.on_removed` teardown + because nothing existing reaches it (`attachments_under` iterates + `attachments`, and a failed buffer has none by construction). +- **ON MERGE of #204, flip the step-6 grade.** §2's step-6 row stays + **Partial** while the PR is open, per §25's landed-evidence rule, and + says so in the row. +- **Stage 1b-3 (welcome buffer, step 4) is unframed** — the last of the + 1b split. - Recovery from a clean checkout — **the two-argument form does not work** (`git worktree add ` fails with `fatal: invalid reference`): diff --git a/docs/agent-handoff.md b/docs/agent-handoff.md index 16cebc7..f7058d0 100644 --- a/docs/agent-handoff.md +++ b/docs/agent-handoff.md @@ -104,6 +104,49 @@ commands, read `docs/active-work.md` immediately after this file. interaction islands added, config-registry adoption, background-work attribution. Its §2 grades the golden journey; **Journey Stage 1a moved that grade off "broken at step 3"** — see the arc bullet below. +- **Journey arc (P1) — Stage 1b-1 LANDED (#203)** + (`docs/journey-stage1b1-compile-defaults-framing.md`). + Journey step 9 moves **Partial → Works**: `C-c c` runs `compile.run`, + and the first prompt is prefilled from the detected project kind via + `pmacs.compile.defaults` (seeded `rust = "cargo build"`, extensible + from `init.lua`). Lua, tests and docs; no Rust change, no protocol + change. The ratchet now carries steps 2, 3, 5 and **9**. + - **Sharing a resolver is not capturing one.** `pmacs.minibuffer.read` + is asynchronous and nothing freezes the active window while a prompt + is open, so having the prompt and `compile.run` call the *same* cwd + resolver still let them disagree: the user could be offered + `cargo build` for A and given a run in B by clicking away + mid-prompt. The interactive command captures `pmacs.compile.context()` + once and passes its `cwd` through to the run. **This is Stage 1a's + `commit_to` lesson on a smaller seam** — capture at request time, + never re-derive when the async work lands. Review found it; the + first framing had the weaker design and said it was sufficient. + - **A pin that never crosses the accept boundary cannot see that + class of bug.** Every originally-proposed pin compared values the + prompt and the resolver already agreed on, so the defect above + passed all of them. Two pins now accept the prompt and observe a + real process. + - **`ProjectKind::Cargo` does not exist** — `COHERENCE.md` named it + twice, citing `src/project.rs:77`, which is the *doc comment*; the + variant on line 78 is `ProjectKind::Rust`. Lua never sees the + variant anyway: `pmacs.project.detect` returns the tag string + `"rust"`. Corrected in both COHERENCE sites and recorded in its §24. + - **The compile fallback cwd is the process cwd, which under `cargo + test` is the pmacs repo — itself a Cargo project.** Any pin + asserting the *absence* of a Cargo suggestion that reaches the + fallback reports pmacs's own `Cargo.toml`. Bite D confirmed it: with + the kind derived from the process cwd, the plain-fixture prefill pin + still passed and only the nested-project pin caught it. + `set_search_boundary` is no defence — it clamps only a walk starting + *below* the boundary. + - **Named limitation, deliberately unfixed:** after `pmacs ` the + active buffer is dired's and **pathless** (`pmacs.buffer.create` + assigns no path; dired compensates through its own module-local + `handle_for_buffer`), so the cwd falls through to the process cwd. + Launched from elsewhere that is the wrong directory. The fix is + §8's execution-location model, not a reach into dired's private + table. What is guaranteed is that the failure stays *coherent*: the + suggestion always describes the directory the run will use. - **Journey arc (P1) — Stage 1a LANDED** (`docs/journey-stage1a-framing.md`). `pmacs .` opens a directory instead of exiting 1, on **one** path: `resolve_target_buffer` gained a diff --git a/docs/journey-stage1a-framing.md b/docs/journey-stage1a-framing.md index ce5589f..59691db 100644 --- a/docs/journey-stage1a-framing.md +++ b/docs/journey-stage1a-framing.md @@ -230,6 +230,12 @@ Not in 1a — Stage 1b: a compile keybinding and `cargo build`/`test` defaults from the existing `ProjectKind::Cargo`, LSP spawn-failure guidance (§1.2), a welcome buffer. +*(Correction, added by Stage 1b-1: there is no `ProjectKind::Cargo`. The +variant is `ProjectKind::Rust`; this line inherited the error from +`COHERENCE.md` §20, now recorded in its §24. Left in place rather than +rewritten — this document records what was planned — but flagged so the +wrong type name does not survive by being quoted.)* + --- ## 2. Ground truth (scouted 2026-07-26, `main` @ `d400f30`; re-verified rev 4) diff --git a/docs/journey-stage1b1-compile-defaults-framing.md b/docs/journey-stage1b1-compile-defaults-framing.md new file mode 100644 index 0000000..3a075d1 --- /dev/null +++ b/docs/journey-stage1b1-compile-defaults-framing.md @@ -0,0 +1,691 @@ +# Journey Stage 1b-1 — make building discoverable + +**Status: framing, rev 2 — awaiting review round 2.** +**Serves `COHERENCE.md` §2 (the golden product journey, step 9), §19 +(coherence acceptance tests), §20 Priority 1.** + +## 0. Revision history + +- rev 2 (2026-07-30) — review round 1. Two blocking, two major; all four + accepted, all four verified in the code first. + - **§3.1 did not actually prevent the drift it claimed to.** + `pmacs.minibuffer.read` is asynchronous: `on_accept` runs later, and + the active window can change while the prompt is open. `run` then + re-resolved the cwd from whatever was active *at accept time*, so the + prompt could say `cargo build` for A and execute in B. **Sharing one + resolver is not enough — the resolution has to be captured.** The + interactive `fn` now captures `context()` and passes its `cwd` + through to `run`. This is the Stage 1a lesson repeating: the + destination is captured at request time, not re-derived at commit + time (`commit_to`, §4.4 of that framing). + - **No pin accepted the prompt or observed a spawned run.** N1–N5 all + compared values that the prompt and the resolver had already agreed + on; a wrong cwd inside `on_accept` — exactly the defect above — + passed every one of them. Two pins now cross the accept boundary: + N3 (the captured directory survives a window switch) and N4 (accept + `cargo build` in a real Cargo fixture, assert the `Directory:` header + and a clean exit). + - **N3's named falsifier was not discriminating.** + `project_root_of_active()` already detects from the active file and + returns the *innermost* root, so re-detecting from that root yields + `node` again in `mixed_fixture/sub`. The mutation rev 1 named would + have left the pin green. N3 is repurposed to the prompt-to-run + handoff, and the mixed-fixture pin (now N5) states a falsifier it + actually catches. + - **§6.1 contradicted §3.1.** A Cargo *workspace subdirectory* + contains no `Cargo.toml` and correctly receives `cargo build`, so + "never offered `cargo build` for a directory with no `Cargo.toml`" + was false on the design's own terms. Reworded to *no detected Cargo + project*. +- rev 1 (2026-07-30) — first framing. Scouted against `githubsucks/main` + @ `22df6ab` (test-ambient isolation framing #201). + +## 1. What this stage is, and what it is not + +`COHERENCE.md` §20 Priority 1 names the remainder of the journey arc: + +> Journey Stage 1b is the named remainder: the compile binding + Cargo +> defaults, LSP spawn guidance, and the welcome buffer. + +Those three are unrelated in mechanism, in failure mode, and in cost. +This stage takes **only the first**: journey step 9, *build or test the +project*. LSP spawn guidance (step 6) and the welcome buffer (step 4) +get their own stages; §7 states the split and why. + +Nothing here is a new Rust primitive. The stage is Lua, tests, and docs. + +## 2. Ground truth + +Every claim below was read in the tree at `22df6ab`, not inferred from a +name. + +### 2.1 `compile.run` has no binding, and the docs say so + +`compile.lua` binds four global sequences (`:1183-1188`): `M-g n`, +`M-g p`, `C-x \``, `M-!`. `bind_slot_keys` (`:220-240`) binds +`RET`/`n`/`p`/`q`/`C-c C-k`/`g` at **buffer** scope inside a generated +buffer. `builtin/keymaps/default.lua` never mentions compile. There is +no third keymap file — `builtin/keymaps/` contains `default.lua` alone. + +`docs/keybindings.md:191-192` states it outright: + +> `compile.run` and `compile.recompile` are available through `M-x`; no +> global key is assigned to them. + +### 2.2 The first prompt is empty + +```lua +-- builtin/runtime/compile.lua:1131-1144 +name = "compile.run", +fn = function() + local last = pmacs.compile._last + pmacs.minibuffer.read { + prompt = "Compile command: ", + history = "compile", + initial = last and last.cmdline or "", +``` + +`_last` is set only by a completed `pmacs.compile.run` (`:1094`), so on a +fresh session `initial` is `""`. The user is asked what to build and +given nothing to build with. + +`initial` does reach the user: `Minibuffer::begin` calls +`replace_contents(&session.initial)` (`src/minibuffer.rs:106-107`), and +`contents()` (`:129`) reads it back. The prefill mechanism works; it is +handed an empty string. + +### 2.3 `ProjectKind::Cargo` does not exist — the variant is `Rust` + +**`COHERENCE.md` is wrong here, in two places.** Its §2 step-9 row and +its §20 Priority 1 paragraph both say "`ProjectKind::Cargo` existing +(`src/project.rs:77`)". Line 77 is a doc comment; the variant on line 78 +is `Rust`: + +```rust +// src/project.rs:77-78 +/// A Cargo workspace (`Cargo.toml`). +Rust, +``` + +The marker that produces it is `{ name: "Cargo.toml", kind: +ProjectKind::Rust, is_directory: false }` (`:148-150`). The audit read +the comment and named the comment. This framing corrects both COHERENCE +sites (§8). + +The correction is not cosmetic: it decides what the Lua side matches on. + +### 2.4 Lua already sees the project kind — as a tag string + +`pmacs.project.detect(path)` returns a table (`src/lua_bindings/mod.rs:11683-11694`): + +```rust +t.set("root", root.display().to_string())?; +t.set("kind", kind.tag())?; +t.set("language_id", kind.default_language_id())?; +``` + +`ProjectKind::Rust.tag()` is `"rust"` (`src/project.rs:101`). So the Lua +key is the string `"rust"`, and **no Rust change is needed to learn the +project kind**. + +`detect` accepts a directory as well as a file: `walk_for_marker` uses +`start` unchanged when it is not a file (`src/project.rs:225-229`). + +### 2.5 The cwd is resolved *inside* `run`, after the prompt has closed + +```lua +-- builtin/runtime/compile.lua:765 +local cwd = opts.cwd or project_root_of_active() or daemon_working_directory() +``` + +The interactive `fn` that builds the prompt cannot see this. Any +suggestion computed in the `fn` today would be computed from a different +rule than the one the run obeys — which is the failure this stage must +not ship. §3.1 is about closing that gap before adding the suggestion, +not after. + +### 2.6 The active buffer at the journey moment is **pathless** + +`project_root_of_active()` (`:600-609`) needs `buf:path()`. Directly +after `pmacs .` the active buffer is dired's, created by +`pmacs.buffer.create(name)` (`dired.lua:506`) — a name, never a path. +`buf:path()` returns `file_path()` mapped to a string +(`src/lua_bindings/mod.rs:1242-1248`), so it is `nil`. + +This is not a guess. dired itself compensates, and its own helper is the +evidence: + +```lua +-- builtin/runtime/dired.lua:205-217 +local function current_directory() + local buf = pmacs.window.buffer() + if buf ~= nil then + local ok, path = pcall(function() return buf:path() end) + if ok and path then ... end + local h = handle_for_buffer(buf) -- <-- the pathless case + if h then return h.path end + end + return canonicalize(".") +end +``` + +`handle_for_buffer` is a **module-local** table. Compile cannot reach it, +and reaching for it would make compile depend on dired — a new +interaction island for one directory string. §5 decides against it and +§6 states the residual gap. + +### 2.7 The last-resort cwd is the *process* cwd, evaluated at call time + +`daemon_working_directory()` reads +`pmacs.instance.identity().working_directory`, which is: + +```rust +// pmacs-protocol/src/message.rs:1703-1706 +working_directory: std::env::current_dir() + .ok() + .map(|p| p.to_string_lossy().into_owned()) + .unwrap_or_default(), +``` + +Two consequences, both load-bearing: + +- **In production**, `pmacs .` is launched from the project directory, so + the fallback happens to be right. `pmacs ~/code/proj` launched from + `~` resolves to `~`, and the fallback is wrong. That is a pre-existing + step-9 defect this stage does not fix (§6.1). +- **In tests**, it is the *test runner's* cwd — the pmacs repo root, + **which is a Cargo project**. `compile_mode_acceptance.rs:1721-1733` + already pins exactly this (`r1f8_inherited_cwd_resolves_to_the_daemon_working_directory`), + and `:1382` already warns that "fallback would search the test + process's cwd". Any pin that asserts *no* Cargo suggestion is at the + mercy of where `cargo test` was invoked unless it is designed to never + reach the fallback. §5.2 designs for that; it is the single largest + trap in this stage. + +### 2.8 `C-c c` is free, and `C-c` is not CUA copy + +Global `C-c` sequences in the tree: `C-c a`/`f`/`h`/`H`/`i`/`o`/`r`/`s`/`y` +(`lsp.lua`), `C-c t` (`terminal.lua:203`), `C-c @ …` (folding). +`C-c c` is unbound. Copy is `M-w` (`builtin/keymaps/default.lua:114`) — +the CUA trio was deliberately not taken — so binding under the `C-c` +prefix collides with nothing in the default map. + +`terminal.lua` binds its own `C-c t`; `lsp.lua` binds its own `C-c o`. +A runtime module owning its global binding is the established pattern, +and this stage follows it rather than editing `default.lua`. + +### 2.9 dired already decided the prompt shape + +`C-x d` prefills its prompt and **deliberately refuses a completion +source** (`dired.lua:749-756`): a candidate list makes RET-on-empty open +whatever sorts first, and a selected candidate shadows typed text. The +same reasoning applies to a compile command, so this stage prefills and +adds no completion source. + +## 3. Design + +### 3.1 One resolution, consumed twice + +Extract the `:765` expression into a helper and expose a read-only view +of it: + +```lua +local function resolve_cwd(explicit) + return explicit or project_root_of_active() or daemon_working_directory() +end + +--- Where the next compile would run, and what kind of project is +--- detected *from that directory*. Public getter (API conventions): +--- `{ cwd = string|nil, kind = string|nil }`. +function pmacs.compile.context(explicit_cwd) +``` + +`run` calls `resolve_cwd(opts.cwd)`; the interactive `fn` calls +`pmacs.compile.context()`. + +**Sharing the resolver is necessary and not sufficient.** +`pmacs.minibuffer.read` is asynchronous — `on_accept` runs an arbitrary +time later, and nothing freezes the active window while a prompt is +open. A mouse click, a second frontend, or a background open can change +what `project_root_of_active()` answers between the prompt and the RET. +Two calls to the same resolver at two different moments are still two +different answers, and the user is then shown a suggestion for A and +given a run in B. + +So the interactive command **captures** the resolution and hands it +through: + +```lua +fn = function() + local last = pmacs.compile._last + local ctx = pmacs.compile.context() -- captured once, here + pmacs.minibuffer.read { + prompt = "Compile command: ", + history = "compile", + initial = last and last.cmdline or default_for(ctx.kind) or "", + on_accept = function(cmdline) + if cmdline == nil or cmdline == "" then return end + pmacs.compile.run(cmdline, { cwd = ctx.cwd }) -- the same ctx + end, + } +end +``` + +This is Journey Stage 1a's `commit_to` lesson on a smaller seam: the +destination is captured when the request is made and revalidated at +commit, never re-derived from whatever happens to be ambient when the +async work lands. The mechanism differs — compile needs no scope +override, only the value — but the failure it prevents is the same one. + +`ctx.cwd` is passed through verbatim, **including `nil`**. A `nil` cwd +means every resolution step failed, and `run`'s header renders +`(unknown)` exactly as it does today; substituting a re-resolution there +would reintroduce the drift for the one case least able to tolerate it. + +**Only the interactive command captures.** `pmacs.compile.run(cmdline)` +called programmatically still resolves at call time, which is what a +caller with no prompt in between means by "here". + +With both halves in place: **the suggestion is a function of the +directory the command will execute in**, and the two cannot drift — +neither across the two resolutions nor across the wait for input. + +`kind` is `pmacs.project.detect(cwd).kind` — detection *from* the cwd, +which is not the same as "the cwd is the root": `opts.cwd = /proj/src` +in a Cargo workspace yields kind `rust` with root `/proj`. That is +correct (cargo works from a subdirectory) and is stated so a reader does +not read `kind` as "this directory is a project root". + +`project_root_of_active()` already calls `detect` once to get a root; +`context` then calls it again on the result. The second call is +redundant in that branch and is kept anyway, because the alternative is +two different rules for where `kind` comes from depending on which +branch produced the cwd. One rule, stated as: **`kind` is always +`detect(cwd)`.** + +### 3.2 The default table + +```lua +--- Default compile command per detected project kind, keyed by the +--- tag `pmacs.project.detect` returns. Assign into this table from +--- `init.lua` to add or override one. +pmacs.compile.defaults = { rust = "cargo build" } +``` + +**Only `rust` is seeded, and that is a decision rather than an +omission.** Rust has one answer. Node does not (`npm` / `yarn` / `pnpm` +/ a `scripts.build` that may not exist); Python does not; Go's build and +test are different commands with equal claim. A wrong prefill is worse +than an empty one — the user must first delete it, then type. The table +exists so a user or a package can add the answer *they* know. + +`cargo test` is reachable by editing the prefill. The prompt prefills +one string; offering both would need a candidate list, which §2.9 +already ruled out for this prompt. + +### 3.3 Precedence at the prompt + +``` +last.cmdline -- unchanged; a session that has compiled keeps its command + or defaults[kind] -- new + or "" -- unchanged +``` + +`last` winning is deliberate: a user who ran `cargo test` once should get +`cargo test` back, not be reset to `cargo build`. This is a preservation +pin (§5.4, P1), not an accident of ordering. + +### 3.4 The table is user-writable, so reading it is guarded + +`pmacs.compile.defaults` is public and assignable, which means a +metatable with a throwing `__index`, a non-string value, or a +non-table replacement all have to be survivable. The module already +holds this discipline for a hostile rule container (`validated_rules`, +round-2 finding 3: shell-command "must neither surface compile-rule +warnings nor fail on a hostile rule container"). + +The lookup therefore runs under `pcall` and accepts a value only when it +is a non-empty string. Anything else yields `""` — the pre-stage +behavior. **A broken `defaults` degrades to today's prompt; it never +prevents compiling.** + +### 3.5 The binding + +```lua +pmacs.keymap.bind { scope = "global", sequence = "C-c c", command = "compile.run" } +``` + +In `compile.lua`, beside the four existing global binds (§2.8). + +Two reachability limits it inherits, both pre-existing and both stated +rather than discovered later: + +- Inside a **terminal** window `C-c` is consumed as the escape key, so + `C-c c` does not arrive. `COHERENCE.md` §2 already records this for + `C-c t`. `M-x compile.run` still works there. +- The repl package binds `C-c` at **buffer** scope + (`builtin/packages/repl/init.lua:300`), which shadows the global + prefix in a repl buffer. Same escape hatch. + +`compile.recompile` gets no global binding: `g` in `*compilation*` +already covers rerun, and adding a second global chord for it is scope +this stage has no journey argument for. + +## 4. What this changes for the journey + +Walking `COHERENCE.md` §2 on a Rust project, unconfigured: + +| | before | after | +|---|---|---| +| launch | `pmacs .` lists the directory (Stage 1a) | unchanged | +| open a file | `RET` visits it (Stage 1a) | unchanged | +| build | *no key exists*; `M-x compile.run` → empty prompt | `C-c c` → `Compile command: cargo build` | +| accept | — | RET runs it in the detected root | +| errors | `M-g n` walks them | unchanged | + +Step 9's verdict row moves from **Partial** to **Works**; step 10 stops +being gated on the user already knowing `M-x compile.run`. + +## 5. Acceptance + +### 5.0 Two labels, as Stage 1a established + +**N** — new behavior, must fail on full revert. **P** — preservation, +legitimately green on the pre-image, falsified only by a named targeted +mutation. Stage 1a's §6.0 is the reason the distinction is kept: an +equivalence assertion between two implementations that already agree +proves nothing. + +Every pin below names the mutation that falsifies it. `scripts/bite` is +run against the suite before the PR opens. + +### 5.1 Where the pins live + +- **`tests/journey_acceptance.rs`** gains a step-9 section. This file is + the ratchet — *stages add rows, none removes them*. Its pins go + through the **real** entry points: `EditorState::open` on a directory, + a dispatched `RET`, a dispatched `C-c c`. Nothing calls + `pmacs.compile.context()` directly in this file. +- **`tests/compile_mode_acceptance.rs`** gains the module-contract pins: + `context()`'s shape, and the hostile-table guards. + +### 5.2 The fixture problem, and its only safe shape + +Per §2.7, the last-resort cwd is the test runner's cwd, and the test +runner's cwd is a Cargo project. **A pin that asserts "no `cargo build` +suggestion" and reaches the fallback will report the pmacs repo's own +`Cargo.toml` as the fixture's answer.** It would pass or fail on where +`cargo test` was invoked from. + +The negative pins are therefore built so the fallback is **never +consulted**: the fixture carries a *different* project marker, so +`project_root_of_active()` resolves inside the fixture and returns +before `daemon_working_directory()` is reached. + +``` +rust_fixture/ Cargo.toml, main.rs +mixed_fixture/ Cargo.toml, main.rs + sub/ package.json, index.js +``` + +`mixed_fixture` is what makes N5 discriminating: the file opened is +`sub/index.js`, the nearest marker is `package.json` (kind `node`, no +default), and the *outer* marker is Cargo. A suggestion computed from +anything other than the resolved cwd — the outermost marker, the launch +directory, the process cwd — produces `cargo build` here. The correct +implementation produces `""`. + +`pmacs.project.set_search_boundary` is set to the fixture root in each +test that detects, so a stray marker above the tempdir (a developer's +`/tmp/.git`) cannot leak in. Note what it does **not** do: it clamps the +upward walk from a start *below* the boundary, so it is no protection at +all for the fallback path, whose start is the repo root. That is why the +fixture shape above, not the boundary call, is the actual defense. + +### 5.3 New-behavior pins + +**N1 — the chord reaches the command.** +Launch on `rust_fixture`, `RET` on `main.rs`, dispatch `C-c c`; assert +`pmacs.minibuffer.is_active()`. +*Falsifier:* remove the `keymap.bind` line — the chord is unbound, no +session opens. +*Why it is separate from N2:* a prefill assertion alone would stay green +if the binding were removed and the prompt were opened some other way. +The binding is the thing COHERENCE says is missing; it gets its own pin. + +**N2 — the prompt is prefilled from the project kind.** +Same walk; assert `pmacs.minibuffer.contents() == "cargo build"`. +*Falsifier:* drop the `defaults[kind]` term from the precedence chain — +contents become `""`. + +**N3 — the prompt's directory survives a window switch.** +Launch on `rust_fixture`, `RET` on `main.rs`, dispatch `C-c c` — the +prompt is now open and has captured A. **Then open a file in a second, +unrelated directory B**, so the active buffer's project is no longer A. +Replace the minibuffer contents with `pwd` and dispatch `RET`. +Assert `*compilation*` contains `Directory: ` **and** that `pwd`'s own +output is `` — two independent readings of the same claim, one from +the header pmacs writes and one from the shell that actually ran. +*Falsifier:* drop `{ cwd = ctx.cwd }` from `on_accept`, i.e. rev 1's +design. `run` re-resolves at accept time, finds B, and both readings say +B. +*Why `pwd` and not `cargo build`:* the subject here is the directory, and +a cheap command keeps the pin's failure message about the directory. N4 +is the pin that runs the real thing. +*Why the contents are set rather than typed:* the prompt is opened +through the real chord; only the editing of an already-open prompt is +short-circuited, which is the same split `find_file_acceptance` documents +at its head. + +**N4 — the offered command runs, in the offered directory.** +A real Cargo fixture: `Cargo.toml`, `src/main.rs` with an empty `main`. +Launch on it, `RET` on `main.rs`, dispatch `C-c c`, dispatch `RET` +**without editing** — accepting exactly what was offered. Assert +`*compilation*` contains `Directory: ` and the clean-exit marker +`[compilation exited with code 0]`. +*Falsifier:* the same `{ cwd = ... }` removal as N3, and independently +any prefill that is not a runnable command. +*Why this pin has to exist:* every other pin in this stage compares +values that the prompt and the resolver already agree on. A wrong +directory inside `on_accept` — the exact defect rev 1 shipped — passes +all of them. This one crosses the accept boundary and observes a real +process. +*Operational detail:* the fixture builds into its own tempdir `target/`, +so it takes no lock the enclosing `cargo test` holds and leaves nothing +behind. The pin runs when `cargo` resolves on PATH and skips with a +message otherwise; **`PMACS_REQUIRE_CARGO_BUILD` only tolerates +absence** — presence of the binary decides execution, and the variable +makes a missing binary fatal in CI. That is the `PMACS_REQUIRE_BASH` +arming shape from #200, and it is stated here because getting the +polarity backwards is how a required pin becomes a silent skip. +*Named risk:* this is the only pin that depends on a working toolchain in +the fixture directory. If it proves flaky in CI, the fallback is to keep +the accept-and-observe shape and substitute a command with no toolchain +dependency — the prompt-to-run handoff is the claim, and `cargo build` is +the most faithful witness of it, not the only possible one. + +**N5 — the suggestion follows the directory the run will use.** +Launch on `mixed_fixture`, `RET` into `sub/index.js`, dispatch `C-c c`; +assert contents are `""` and `context().kind == "node"`. +*Falsifier:* a rule that derives the kind from anything other than the +resolved cwd. The plausible one is the **launch directory** — Stage 1a +made it prominent, and `mixed_fixture` was launched on the Cargo root, so +that rule yields `cargo build` here. So does deriving it from the process +cwd. +*What it does not catch, stated because rev 1 claimed otherwise:* +re-detecting from `project_root_of_active()`'s answer. That helper +already returns the **innermost** root, so detecting from it yields +`node` again and this pin stays green. Rev 1 named that mutation as the +falsifier; it is not one. + +**N6 — `context()` is total in a launched session.** +Property, not a constant, because the value is environment-dependent +(§2.7): after a launch, `context().cwd` is non-nil, and `context().kind` +equals `pmacs.project.detect(context().cwd)`'s kind (both nil, or both +the same string). Asserted with a dired buffer active — the pathless +case — so the fallback branch is the one under test. +*Falsifier:* make `resolve_cwd` return `nil` when the active buffer has +no path, i.e. drop the `daemon_working_directory()` term. +*What it deliberately does not assert:* which directory. Pinning that +would pin the test runner's cwd. + +### 5.4 Preservation pins + +**P1 — `_last` still outranks the kind default.** +Run a compile with an explicit cmdline that is not `cargo build`, then +open the prompt in the Rust fixture; assert the contents are the last +cmdline. +*Targeted mutation:* reorder the precedence chain to put `defaults[kind]` +first. Green on the pre-image (there was no default), red under the +mutation. + +**P2 — a hostile `defaults` cannot break compiling.** +Three cases in `compile_mode_acceptance`: `defaults` replaced by a +non-table; a `__index` metatable that raises; a non-string entry for +`rust`. In all three the prompt opens with `""` and `compile.run` still +executes a typed command. +*Targeted mutation:* remove the `pcall` / type guard — the raising case +propagates out of the command and no prompt opens. + +**P3 — the existing compile bindings are unchanged.** +`M-g n`, `M-g p`, `C-x \``, `M-!` still dispatch to their commands, and +`compile.run` is still reachable through `M-x`. +*Targeted mutation:* the new `bind` call written as an `unbind`+`bind` +pair over the wrong sequence. + +### 5.5 Gates + +The full suite from `CLAUDE.md`, plus `compile_mode_acceptance`, +`journey_acceptance`, `dired_acceptance`, and `find_file_acceptance` as +the touched suites. Local runs must control all five bootstrap-storage +variables (`XDG_CONFIG_HOME`, `XDG_DATA_HOME`, `XDG_STATE_HOME`, +`XDG_CACHE_HOME`, `PMACS_STATE_HOME`) — the ambient-root isolation lane +(#201) is **framing only**, so the workaround is still required and +`compile_mode_acceptance` is one of the suites that goes red without it. + +## 6. Named limitations — stated, not discovered later + +### 6.1 `pmacs ` from elsewhere still resolves the wrong cwd + +Launched as `pmacs ~/code/proj` from `~`, the active buffer is dired and +pathless (§2.6), so the cwd falls through to the process cwd `~` +(§2.7). Compile would then run in `~`, and — consistently, since §3.1 +ties them — suggest nothing. + +**This stage does not fix it.** The fix needs a notion of "the directory +this session is working in" that is not any one module's private table, +which is `COHERENCE.md` §8 (First-Class Execution Locations) — a model +gap, not wiring. Reaching into dired's `handle_for_buffer` would make +compile depend on dired for one string and add exactly the kind of +interaction island §6 of COHERENCE is about. + +What this stage does guarantee is that the failure is **coherent**: the +suggestion describes the directory the command will run in, whatever +that directory turns out to be. The user is never offered `cargo build` +for a directory with **no detected Cargo project**. + +That wording is load-bearing and rev 1 got it wrong. "No `Cargo.toml`" +would have been false on this design's own terms: a Cargo *workspace +subdirectory* contains no `Cargo.toml`, is correctly detected as `rust` +by the ancestor walk, and correctly receives `cargo build` — which cargo +itself runs happily from a subdirectory. The predicate is detection, not +the presence of a file in that one directory (§3.1). + +N6 pins the property; it deliberately does not pin the value. + +### 6.2 The default is per-kind, not configurable through the registry + +`pmacs.compile.defaults` is a plain Lua table, not a registered setting. +It cannot be one: `ConfigValue` is four scalars, so a kind→command map +is not expressible. A scalar `compile.default-command` that overrides +the table *is* expressible and is **deferred, not skipped** — it is one +more precedence step and a registry entry, and it belongs with the +config-adoption work (§20 Priority 6) rather than bolted on here. + +### 6.3 One binding does not retire the inversion + +`COHERENCE.md` §2 keeps a standing observation verbatim: keybinding +coverage is inverted relative to frequency, with `C-c @ C-M-s` bound +while opening a file, opening a terminal, and running a build were not. +Two of the three have been answered (#162, #173); this stage answers the +third, and §8 updates the paragraph accordingly. The *quote* stays as +written, because it names a bias in how new work gets bound rather than +three omissions. + +## 7. Staging — why 1b is split + +`COHERENCE.md` §20 bundles three items under "Stage 1b". They share a +priority and nothing else: + +| | subsystem | shape | risk | +|---|---|---|---| +| **1b-1** (this) | compile + project | wiring, Lua only | low | +| 1b-2 | LSP lifecycle | a failure that is currently *silent* (§1.2) must become visible without becoming noise | medium | +| 1b-3 | startup buffer | new content, plus §18's `C-h`-deletes-a-word problem | low, but touches the default keymap | + +One feature, one branch, one PR. 1b-2 is the hard one — the silence +asymmetry is a design question about *when* to speak, not a wiring +question — and bundling it with a keybinding would hold the cheapest +journey fix in the tree behind the most contested one. + +## 8. Coherence impact + +Per `CLAUDE.md` and `COHERENCE.md` §20's standing process change. + +- **Journey steps touched:** 9 directly (Partial → Works); 10 indirectly + — it was "gated entirely on step 6 or 9 succeeding first". +- **Interaction islands:** none added. The prompt is the existing + minibuffer; the binding joins the existing `C-c` prefix; the kind + comes from the existing detector. `pmacs.compile.defaults` is an + extensible table, not a new modal surface. +- **Config registry adoption:** none, deliberately — §6.2 gives the + mechanism reason and names the deferred scalar. +- **Background-work attribution:** unchanged. Compile already spawns + through the process-group machinery; this stage changes what is typed + into the prompt, not what is spawned or how it is tracked. +- **Doc updates riding this PR** (§25 requires it): + - `COHERENCE.md` §2 step-9 verdict row → Works, and the + `ProjectKind::Cargo` → `ProjectKind::Rust` correction **in both + places** (§2 row and §20 Priority 1). §24 gains the drift entry. + - `COHERENCE.md` §2's post-table paragraph: "Running a build still has + no binding" → answered, with the quote itself left intact (§6.3). + - `docs/keybindings.md`: the `C-c c` row, and the removal of the + "no global key is assigned to them" sentence at `:191`. + - `docs/agent-handoff.md` §1: the journey arc bullet gains Stage 1b-1. + +## 9. Open questions for review + +- **Q#J1 — is `C-c c` the right chord?** It is free, it is under the + established `C-c` prefix, and it matches what most Emacs distributions + bind compile to. The alternative worth naming is `C-c C-c`, which is + more finger-friendly but is the chord many major modes claim + buffer-locally, so a global one would be shadowed unpredictably later. +- **Q#J2 — should `rust` be the only seeded default?** §3.2 argues yes + on the grounds that a wrong prefill costs more than an empty one. The + counter-argument is that `go build` and `make` are about as + unambiguous as `cargo build`, and seeding them would make the table + read as a real registry rather than a Rust special case. +- **Q#J3 — should the prefill be selected, so typing replaces it?** + Emacs leaves the prefill unselected and the point at the end. dired's + prefill does the same. Matching them means "accept" is RET and + "replace" is a kill-line first. Changing it is a minibuffer-wide + behavior change and out of scope, but it is the ergonomic difference a + user will notice first. + +## 10. Ledger + +Branch `journey-stage1b1-compile-defaults`, worktree +`../pmacs-journey-1b1`, based on `githubsucks/main` @ `22df6ab`. +Framing only; no code, no PR yet. + +Recovery from a clean checkout — the two-argument form of +`git worktree add` does not work for a remote-only branch (it fails with +`fatal: invalid reference`, because after a bare fetch no local branch +exists): + +```sh +git fetch githubsucks +git worktree add ../pmacs-journey-1b1 \ + -b journey-stage1b1-compile-defaults \ + githubsucks/journey-stage1b1-compile-defaults +``` diff --git a/docs/keybindings.md b/docs/keybindings.md index b94bd63..922692f 100644 --- a/docs/keybindings.md +++ b/docs/keybindings.md @@ -187,9 +187,13 @@ Source: `builtin/runtime/compile.lua`. | `M-g n` / `M-g p` | `error.next` / `error.previous` — compile/grep errors when that source has claimed navigation, otherwise LSP diagnostics | | `` C-x ` `` | `error.next` | | `M-!` | `shell.command` — asynchronous output in `*shell-command*` | +| `C-c c` | `compile.run` — prompts, prefilled from the detected project kind | -`compile.run` and `compile.recompile` are available through `M-x`; no -global key is assigned to them. +`compile.recompile` is available through `M-x`, and through `g` inside +`*compilation*`; no global key is assigned to it. `C-c c` is unreachable +from inside a terminal window (`C-c` is consumed as the escape key) and +inside a repl buffer (which binds `C-c` at buffer scope); `M-x +compile.run` still works in both. ## 2. Buffer-local panel keymaps diff --git a/tests/compile_mode_acceptance.rs b/tests/compile_mode_acceptance.rs index 377b094..e7f21e0 100644 --- a/tests/compile_mode_acceptance.rs +++ b/tests/compile_mode_acceptance.rs @@ -2750,3 +2750,106 @@ fn r5f3_tracked_line_start_matches_the_scan_across_transitions() { "every rewind lands at the tracked line start; buffer:\n{text:?}" ); } + +// --------------------------------------------------------------------------- +// Journey Stage 1b-1 — `pmacs.compile.defaults` is public and assignable +// +// The table is user-writable, so a metatable with a throwing `__index`, a +// non-string entry, and a non-table replacement all have to be +// survivable — the same discipline `validated_rules` keeps for a hostile +// rule container. A broken `defaults` must degrade to the pre-stage empty +// prompt; it must never prevent compiling. +// --------------------------------------------------------------------------- + +/// Open the compile prompt and return what it offered. +fn compile_prompt_initial(s: &EditorState) -> String { + exec(s, "pmacs.command.invoke('compile.run')"); + assert!( + eval::(s, "return pmacs.minibuffer.is_active()"), + "compile.run must open a prompt even with a hostile defaults table" + ); + eval(s, "return pmacs.minibuffer.contents()") +} + +#[test] +fn j1b1_a_non_table_defaults_degrades_to_an_empty_prompt() { + let s = editor(); + exec(&s, "pmacs.compile.defaults = 42"); + assert_eq!(compile_prompt_initial(&s), ""); +} + +#[test] +fn j1b1_a_raising_defaults_index_degrades_to_an_empty_prompt() { + let s = editor(); + exec( + &s, + "pmacs.compile.defaults = setmetatable({}, { + __index = function() error('hostile') end, + })", + ); + assert_eq!(compile_prompt_initial(&s), ""); +} + +#[test] +fn j1b1_a_non_string_default_is_ignored() { + let s = editor(); + exec(&s, "pmacs.compile.defaults = { rust = {}, }"); + assert_eq!(compile_prompt_initial(&s), ""); +} + +#[test] +fn j1b1_a_hostile_defaults_still_lets_a_typed_command_run() { + // The consequence that matters: degradation must not cost the user + // the ability to compile. Asserts output produced, not a property + // preserved. + let mut s = editor(); + exec( + &s, + "pmacs.compile.defaults = setmetatable({}, { + __index = function() error('hostile') end, + })", + ); + // Through the INTERACTIVE command, so the hostile lookup actually + // happens. Calling `pmacs.compile.run` directly never consults + // `defaults` at all, which made an earlier draft of this pin vacuous + // — it passed with the guard removed. + exec(&s, "pmacs.command.invoke('compile.run')"); + assert!( + eval::(&s, "return pmacs.minibuffer.is_active()"), + "the prompt must still open" + ); + exec(&s, "pmacs.minibuffer.set_contents('echo still-compiles')"); + exec(&s, "pmacs.minibuffer.accept()"); + assert!( + pump_until(&mut s, 10_000, |s| { + compilation_text(s).contains("still-compiles") + }), + "a broken defaults table must not prevent compiling; got:\n{}", + compilation_text(&s) + ); +} + +#[test] +fn j1b1_context_reports_the_cwd_a_run_would_use() { + // `pmacs.compile.context` and `pmacs.compile.run` must answer from + // one resolution — an explicit cwd is honoured by both, and the kind + // is detected from that same directory. + let s = editor(); + let dir = tempfile::tempdir().expect("tempdir"); + std::fs::write(dir.path().join("Cargo.toml"), b"[package]\nname=\"x\"\n").expect("write"); + exec( + &s, + &format!( + "pmacs.project.set_search_boundary({:?})", + dir.path().display().to_string() + ), + ); + let kind: String = eval( + &s, + &format!( + "return pmacs.compile.context({:?}).kind", + dir.path().display().to_string() + ), + ); + assert_eq!(kind, "rust", "the kind is detected from the given cwd"); +} diff --git a/tests/journey_acceptance.rs b/tests/journey_acceptance.rs index 416d276..7ce2457 100644 --- a/tests/journey_acceptance.rs +++ b/tests/journey_acceptance.rs @@ -1298,6 +1298,451 @@ fn preservation_display_file_still_refuses_a_directory() { ); } +// --------------------------------------------------------------------------- +// Step 9 — build or test the project (Journey Stage 1b-1) +// +// `COHERENCE.md` §2 graded this **Partial**: `M-x compile.run` worked but +// had no keybinding, an empty first prompt, and no `cargo build` +// suggestion. These rows are the ratchet for the three. +// +// The trap these fixtures are built around: compile's last-resort cwd is +// `std::env::current_dir()` evaluated at call time, which under `cargo +// test` is the **pmacs repo root — itself a Cargo project** +// (`compile_mode_acceptance.rs` pins exactly that). So any pin asserting +// the *absence* of a Cargo suggestion must be shaped so the fallback is +// never consulted: each fixture carries its own marker, and +// `project_root_of_active()` therefore answers from inside the fixture. +// `set_search_boundary` is not the defence — it clamps only a walk that +// starts below the boundary, and the fallback's walk starts at the repo. +// --------------------------------------------------------------------------- + +fn ctrl(s: &mut EditorState, c: char) { + s.dispatch_key( + FrontendId::LOCAL, + key(KeyCode::Char(c), KeyModifiers::CONTROL), + ); +} + +/// Open the compile prompt through the real `C-c c` binding. +fn press_compile_chord(s: &mut EditorState) { + ctrl(s, 'c'); + type_char(s, 'c'); +} + +/// A Cargo project a journey can plausibly be run against. +fn cargo_project() -> TempDir { + let td = tempfile::tempdir().expect("tempdir"); + std::fs::write( + td.path().join("Cargo.toml"), + b"[package]\nname = \"journey-fixture\"\nversion = \"0.1.0\"\nedition = \"2021\"\n", + ) + .expect("write Cargo.toml"); + std::fs::create_dir_all(td.path().join("src")).expect("mkdir src"); + std::fs::write(td.path().join("src/main.rs"), b"fn main() {}\n").expect("write main.rs"); + td +} + +/// Clamp detection to the fixture so a stray marker above the tempdir +/// (a developer's `/tmp/.git`) cannot leak in. +fn bound_detection_to(s: &EditorState, dir: &Path) { + exec( + s, + &format!( + "pmacs.project.set_search_boundary({:?})", + dir.display().to_string() + ), + ); +} + +/// The project root as **detection** reports it. +/// +/// `pmacs.project.detect` canonicalizes before walking +/// (`canonicalize_or_passthrough`, `src/project.rs:509-511`), so a +/// `/var/folders/...` tempdir on macOS comes back as +/// `/private/var/folders/...`. `canon()` is *lexical* — it never +/// resolves symlinks — so it is the wrong expectation for any value +/// that has passed through detection, which is exactly what the compile +/// cwd is. Using it here failed both macOS CI legs while Ubuntu (where +/// `/tmp` is not a symlink) stayed green. +fn detected_root(path: &Path) -> String { + std::fs::canonicalize(path) + .unwrap_or_else(|_| path.to_path_buf()) + .to_string_lossy() + .into_owned() +} + +fn minibuffer_active(s: &EditorState) -> bool { + eval(s, "return pmacs.minibuffer.is_active()") +} + +fn minibuffer_contents(s: &EditorState) -> String { + eval(s, "return pmacs.minibuffer.contents()") +} + +fn named_text(s: &EditorState, name: &str) -> String { + eval( + s, + &format!( + r#" + for _, id in ipairs(pmacs.buffer.list()) do + if pmacs.describe.buffer(id).name == {name:?} then + return id:slice(0, id:len()) + end + end + return "" + "# + ), + ) +} + +/// Drive frames until `pred` holds, pumping the process supervisor as +/// well as the async runtime — a compile run is a spawned child. +fn pump_processes_until( + s: &mut EditorState, + timeout_ms: u64, + mut pred: impl FnMut(&EditorState) -> bool, +) -> bool { + let stop = Instant::now() + Duration::from_millis(timeout_ms); + loop { + if pred(s) { + return true; + } + if Instant::now() >= stop { + return false; + } + s.tick_processes(); + s.tick_async(); + std::thread::sleep(Duration::from_millis(5)); + } +} + +/// Walk the journey to the point where a build would be run: launch on +/// the directory, then `RET` on `name` to open a file. +fn walk_to_open_file(dir: &Path, name: &str) -> EditorState { + let mut s = launch(dir); + bound_detection_to(&s, dir); + let line = line_of(&s, name); + exec(&s, &format!("pmacs.editor.move_to_line({line})")); + press(&mut s, KeyCode::Enter); + pump(&mut s); + assert_eq!( + active_name(&s), + dir.join(name).display().to_string(), + "precondition: the walk must actually open the file, or every pin \ + below is really testing the dired buffer" + ); + s +} + +/// **N** — `C-c c` reaches `compile.run`. Separate from N2 because a +/// prefill assertion alone stays green if the binding is removed and the +/// prompt is opened another way — and the binding is the thing +/// `COHERENCE.md` says is missing. +#[test] +fn journey_step9_the_compile_chord_opens_the_prompt() { + let td = cargo_project(); + let mut s = walk_to_open_file(td.path(), "Cargo.toml"); + press_compile_chord(&mut s); + assert!( + minibuffer_active(&s), + "C-c c must open the compile prompt; active buffer is {}", + active_name(&s) + ); +} + +/// **N** — the prompt is prefilled from the detected project kind. +#[test] +fn journey_step9_the_prompt_is_prefilled_for_a_cargo_project() { + let td = cargo_project(); + let mut s = walk_to_open_file(td.path(), "Cargo.toml"); + press_compile_chord(&mut s); + assert_eq!( + minibuffer_contents(&s), + "cargo build", + "a Rust project's first compile prompt must offer its build command" + ); +} + +/// **N** — the directory the prompt captured survives a window switch. +/// +/// The prompt is opened against A, the active buffer then moves to an +/// unrelated directory B, and only then is the command accepted. Both +/// readings — the header pmacs writes and the shell's own `pwd` — must +/// say A. Falsified by dropping `{ cwd = ctx.cwd }` from `on_accept`, +/// which is what re-resolving at accept time looks like. +#[test] +fn journey_step9_the_prompt_runs_in_the_directory_it_captured() { + let a = cargo_project(); + let b = project(); + let mut s = walk_to_open_file(a.path(), "Cargo.toml"); + press_compile_chord(&mut s); + assert!(minibuffer_active(&s), "prompt must be open against A"); + + // Move the active buffer to B while the prompt waits for input. + exec( + &s, + &format!( + "pmacs.window.display_file({:?})", + b.path().join("alpha.txt").display().to_string() + ), + ); + pump(&mut s); + + // Accept a cheap command: the subject is the directory, so a fast + // one keeps the failure message about the directory. Only the + // editing of an already-open prompt is short-circuited; the prompt + // itself was opened through the real chord. + exec(&s, "pmacs.minibuffer.set_contents('pwd')"); + press(&mut s, KeyCode::Enter); + let finished = pump_processes_until(&mut s, 10_000, |s| { + named_text(s, "*compilation*").contains("exited") + }); + assert!(finished, "the accepted run must finish"); + + let text = named_text(&s, "*compilation*"); + // Detection-canonical, not lexical: the compile cwd came from + // `pmacs.project.detect`, and `pwd` reports the physical directory. + let a_path = detected_root(a.path()); + assert!( + text.contains(&format!("Directory: {a_path}")), + "the header must name the directory the prompt captured, not the newly active one;\n{text}" + ); + assert!( + text.contains(&a_path), + "and `pwd` must agree with the header;\n{text}" + ); +} + +/// **N** — the offered command runs, in the offered directory. +/// +/// Every other pin here compares values the prompt and the resolver +/// already agree on. A wrong directory inside `on_accept` passes all of +/// them. This one accepts what was offered, unedited, and observes a +/// real process. +#[test] +fn journey_step9_the_offered_command_builds_the_project() { + if !binary_available("cargo") { + assert!( + std::env::var_os("PMACS_REQUIRE_CARGO_BUILD").is_none(), + "PMACS_REQUIRE_CARGO_BUILD is set but `cargo` is not on PATH" + ); + eprintln!("skipping: `cargo` is not on PATH"); + return; + } + let td = cargo_project(); + let mut s = walk_to_open_file(td.path(), "Cargo.toml"); + press_compile_chord(&mut s); + assert_eq!( + minibuffer_contents(&s), + "cargo build", + "precondition: the prompt offers the build command" + ); + + // Accept exactly what was offered — no editing. + press(&mut s, KeyCode::Enter); + let finished = pump_processes_until(&mut s, 120_000, |s| { + named_text(s, "*compilation*").contains("exited") + }); + assert!(finished, "the offered build must finish"); + + let text = named_text(&s, "*compilation*"); + assert!( + text.contains(&format!("Directory: {}", detected_root(td.path()))), + "the build runs in the detected project root;\n{text}" + ); + assert!( + text.contains("[compile exited with code 0]"), + "and the command that was offered is one that actually runs;\n{text}" + ); + // Positive control on the run itself. A clean exit alone does not + // prove cargo compiled anything — this names the fixture crate, so + // the pin fails if the offered command ever stops being a build. + assert!( + text.contains("journey-fixture"), + "cargo must actually have built the fixture crate;\n{text}" + ); +} + +/// **N** — the suggestion follows the directory the run will use. +/// +/// A Node project nested inside a Cargo one, opened at the *inner* file. +/// Falsified by deriving the kind from the launch directory (the Cargo +/// root here) or from the process cwd — both yield `cargo build`. +/// +/// It does **not** catch re-detecting from `project_root_of_active()`'s +/// answer: that helper already returns the innermost root, so detecting +/// from it yields `node` again and this pin stays green. +#[test] +fn journey_step9_a_nested_project_gets_its_own_kind_not_the_outer_one() { + let outer = cargo_project(); + let sub = outer.path().join("sub"); + std::fs::create_dir_all(&sub).expect("mkdir sub"); + std::fs::write(sub.join("package.json"), b"{ \"name\": \"inner\" }\n").expect("write pkg"); + std::fs::write(sub.join("index.js"), b"console.log(1)\n").expect("write index.js"); + + let mut s = walk_to_open_file(&sub, "index.js"); + press_compile_chord(&mut s); + // `minibuffer_contents` is "" both for an empty prefill and for no + // minibuffer at all, so the emptiness assertion below is vacuous + // without this. Bite A caught it: with the binding removed this pin + // stayed green while six others failed. + assert!( + minibuffer_active(&s), + "precondition: the prompt is actually open" + ); + assert_eq!( + eval::(&s, "return pmacs.compile.context().kind"), + "node", + "the kind is detected from the resolved cwd, not the launch directory" + ); + assert_eq!( + minibuffer_contents(&s), + "", + "and `node` has no seeded default, so nothing is offered" + ); +} + +/// **N** — `context()` is total in a launched session. +/// +/// Asserted with the pathless dired buffer active, so the fallback +/// branch is the one under test. It pins a *property*, deliberately not +/// a value: which directory the fallback resolves to is the test +/// runner's cwd and pinning it would pin the environment. +#[test] +fn journey_step9_the_compile_context_is_total_even_with_no_file_open() { + let td = cargo_project(); + let s = launch(td.path()); + assert!( + active_name(&s).starts_with("*dired:"), + "precondition: the pathless dired buffer is active" + ); + assert!( + eval::(&s, "return pmacs.compile.context().cwd ~= nil"), + "a launched session always has somewhere to run a build" + ); + assert!( + eval::( + &s, + "local c = pmacs.compile.context() + local ok, p = pcall(pmacs.project.detect, c.cwd) + local detected = (ok and p) and p.kind or nil + return c.kind == detected" + ), + "and its kind is exactly what detection answers for that cwd" + ); +} + +/// **P** — `_last` still outranks the kind default. +/// +/// Green on the pre-image (there was no default). The targeted mutation +/// is reordering the precedence chain to put `defaults[kind]` first. +#[test] +fn journey_step9_preservation_the_last_command_outranks_the_default() { + let td = cargo_project(); + let mut s = walk_to_open_file(td.path(), "Cargo.toml"); + exec(&s, "pmacs.compile.run('true')"); + let finished = pump_processes_until(&mut s, 10_000, |s| { + named_text(s, "*compilation*").contains("exited") + }); + assert!(finished, "precondition: a run completed and set _last"); + + press_compile_chord(&mut s); + assert_eq!( + minibuffer_contents(&s), + "true", + "a session that has compiled keeps its own command, not the project default" + ); +} + +/// **P** — the compile keys that already existed still dispatch. +/// +/// Targeted mutation: writing the new binding as an unbind+bind pair +/// over one of these sequences. +#[test] +fn journey_step9_preservation_the_existing_compile_bindings_survive() { + let td = cargo_project(); + let s = walk_to_open_file(td.path(), "Cargo.toml"); + for (sequence, command) in [ + ("M-g n", "error.next"), + ("M-g p", "error.previous"), + ("C-x `", "error.next"), + ("M-!", "shell.command"), + ("C-c c", "compile.run"), + ] { + let bound: String = eval( + &s, + &format!("return pmacs.keymap.lookup({sequence:?}).command"), + ); + assert_eq!( + bound, command, + "{sequence} must still dispatch to {command}" + ); + } +} + +/// True when `name` resolves on PATH — the fixture-dependency gate the +/// process suite uses, so a missing binary skips rather than fails. +fn binary_available(name: &str) -> bool { + std::process::Command::new("which") + .arg(name) + .output() + .is_ok_and(|o| o.status.success()) +} + +/// **N** — the compile directory is *detection*-canonical, not lexical. +/// +/// This pin exists because its absence broke both macOS CI legs while +/// Ubuntu stayed green: `/var` is a symlink to `/private/var` there, so +/// a tempdir's lexical and canonical paths differ, and the original +/// assertions used the lexical one. +/// +/// Reproducing it on Linux needs an explicit symlink — which is also a +/// real configuration `Workspace::detect`'s own doc comment names +/// ("`/tmp/sandbox/foo` symlinked to `/home/user/code/foo`"). Launching +/// through the link makes the two paths disagree on every platform, so +/// the regression can no longer hide behind a filesystem that happens +/// not to use symlinks. +#[cfg(unix)] +#[test] +fn journey_step9_the_compile_directory_is_detection_canonical() { + let parent = tempfile::tempdir().expect("tempdir"); + let real = parent.path().join("real"); + std::fs::create_dir_all(real.join("src")).expect("mkdir real"); + std::fs::write( + real.join("Cargo.toml"), + b"[package]\nname = \"journey-fixture\"\nversion = \"0.1.0\"\nedition = \"2021\"\n", + ) + .expect("write Cargo.toml"); + std::fs::write(real.join("src/main.rs"), b"fn main() {}\n").expect("write main.rs"); + + let link = parent.path().join("link"); + std::os::unix::fs::symlink(&real, &link).expect("symlink"); + + // Precondition, or the pin is vacuous: the two spellings must + // actually differ, which is the whole condition macOS creates for + // free and Linux does not. + assert_ne!( + canon(&link), + detected_root(&link), + "the fixture must make lexical and canonical paths disagree" + ); + + let mut s = walk_to_open_file(&link, "Cargo.toml"); + press_compile_chord(&mut s); + exec(&s, "pmacs.minibuffer.set_contents('pwd')"); + press(&mut s, KeyCode::Enter); + let finished = pump_processes_until(&mut s, 10_000, |s| { + named_text(s, "*compilation*").contains("exited") + }); + assert!(finished, "the accepted run must finish"); + + let text = named_text(&s, "*compilation*"); + assert!( + text.contains(&format!("Directory: {}", detected_root(&link))), + "the header must name the directory detection resolved to;\n{text}" + ); +} + // --------------------------------------------------------------------------- // Step 6 — receive language intelligence (Journey Stage 1b-2) //