diff --git a/COHERENCE.md b/COHERENCE.md index e7229f3..45a9683 100644 --- a/COHERENCE.md +++ b/COHERENCE.md @@ -95,7 +95,7 @@ remain open to them. | § | Concern | Grade | One-line state | |---|---|---|---| -| 2 | Golden product journey | **Broken at entry** | `pmacs .` exits 1; only "launch" and "edit" pass cleanly zero-config | +| 2 | Golden product journey | **Runs to step 5** | `pmacs .` opens the directory (Journey Stage 1a); thin from step 6 on | | 3 | Zero-configuration state | **Partial** | Defaults genuinely strong; missing-tool failure is silent, not graceful | | 4 | Progressive disclosure | **Inverted** | The advanced level is real; the beginner level is the missing one | | 5 | Unified discoverability | **Substrate without surface** | Best-in-class registration metadata; almost no way for a user to reach it | @@ -112,7 +112,7 @@ remain open to them. | 16 | Semantic frontend | **Strong** | v6..=v20 negotiated protocol; degradation practiced; TUI/GPU share the model | | 17 | Distribution | **Missing** | CI is test-only; no binaries, channels, checksums, or update path | | 18 | Onboarding | **Missing** | No welcome, no tutorial; `C-h` deletes a word; `M-x` is the only door in | -| 19 | Coherence acceptance tests | **Missing (culture ready)** | Superb per-arc acceptance discipline; zero cross-subsystem journey tests | +| 19 | Coherence acceptance tests | **Started** | `tests/journey_acceptance.rs` exists (steps 2, 3, 5); the other five scenarios are still unwritten | Three cross-cutting patterns explain most of the table; they are detailed in §1.1–§1.3: **substrate without surface**, **the silence @@ -1452,20 +1452,24 @@ subsystems, complementing (not replacing) subsystem tests: ### Ground truth -**Grade: missing — but the culture that would make them excellent is the -project's strongest process asset.** +**Grade: started — the first suite exists; five of the six scenarios +above do not.** -Zero cross-subsystem journey tests exist. Every acceptance suite in the -tree pins one subsystem's contract (superbly — bite-verified, -falsified-by-revert, vacuity-checked). Several of the scenarios above -are currently *untestable* because the behavior doesn't exist (install -in-session, disable, open a directory); the ones that are testable -(first launch, command discovery, worker cancellation, remote -attach/reconnect) could be written today and would immediately pin the -journey against regression. The first coherence acceptance suite should -be the §2 journey itself, growing a step at a time as steps become -real — that is how "the journey is a release gate" stops being -aspirational. +At audit time zero cross-subsystem journey tests existed. **Journey +Stage 1a created `tests/journey_acceptance.rs`**, the §2 journey itself, +seeded with steps 2 (launch unconfigured), 3 (open a real project), and +5 (edit immediately), and declared a ratchet: stages add rows, none +removes them. That is the "first launch" scenario, partially — missing +tools still have no actionable guidance to assert. + +The rest is unchanged. Every other acceptance suite in the tree pins one +subsystem's contract (superbly — bite-verified, falsified-by-revert, +vacuity-checked). Command discovery, workspace lifecycle, worker +ownership, package lifecycle, and remote execution have no +cross-subsystem suite; several remain *untestable* because the behavior +doesn't exist (install in-session, disable). Steps 6–12 join +`journey_acceptance.rs` as later stages make them real — that is how +"the journey is a release gate" stops being aspirational. (Related lesson already in the handoff: `compile_mode_acceptance` accidentally reads the real user config — an *unintentional* diff --git a/builtin/runtime/dired.lua b/builtin/runtime/dired.lua index adbc8f1..c8054fe 100644 --- a/builtin/runtime/dired.lua +++ b/builtin/runtime/dired.lua @@ -571,7 +571,15 @@ end -- deliberately so (Q#DR10): the next directory is the same kind of -- thing as the current one and belongs in the same slot, while a file -- is not a dired buffer and belongs in the document area. -local function display(handle, opts, departed) +-- +-- `captured` (Journey Stage 1a, Q#JR14) is the destination window a +-- background open must land in. It is NOT the same as "wherever the +-- scoped frontend is looking now": the scope fixes the *frontend*, and +-- within one frontend the selected window can still have moved to +-- another split while the listing was in flight. The preflight cannot +-- catch that -- the captured window is still live and still holds its +-- captured buffer -- so honoring it is this function's job. +local function display(handle, opts, departed, captured) local side = nil if departed ~= nil then -- Dired's own window, not the request's: walking a tree in a side @@ -587,6 +595,11 @@ local function display(handle, opts, departed) -- both the substrate's documented policy and Emacs's, so dired does -- not try to unpin the user's panel. pmacs.window.display(handle.buf, { side = side, select = true }) + elseif captured ~= nil then + -- `select = true` because the rest of the commit -- seat_cursor via + -- `pmacs.editor.move_to_line` -- acts on the frontend's ACTIVE + -- window, so the seat would land in the wrong window otherwise. + pmacs.window.display(handle.buf, { window = captured, select = true }) else pmacs.window.switch_buffer(handle.buf) end @@ -641,6 +654,13 @@ local function open_directory(path, opts, departed) -- inside it is refused (Q#JR14b), because a yield would restore the -- scope while this coroutine is still parked. local function commit() + -- The captured window, read once. Everything below that would + -- otherwise consult "the active window" must consult THIS instead: + -- the scope pins the frontend, not the selected window, and a split + -- or panel can take focus within that frontend while the listing is + -- in flight (Q#JR14). + local captured = opts.dest ~= nil and opts.dest:window() or nil + local handle = claim_handle(canonical) handle.entries = entries handle.errors = errors @@ -652,14 +672,19 @@ local function open_directory(path, opts, departed) if departed ~= nil then handle.prev = departed.prev else - local active = pmacs.window.buffer() + local active + if captured ~= nil then + active = pmacs.window.buffer(captured) + else + active = pmacs.window.buffer() + end if active ~= nil and handle_for_buffer(active) == nil then handle.prev = active end end paint(handle) - display(handle, opts, departed) + display(handle, opts, departed, captured) -- Seating happens after the display: `switch_buffer` zeroes the -- window cursor, so an earlier seat would be discarded. seat_cursor(handle, opts.select_name, 1) diff --git a/docs/active-work.md b/docs/active-work.md index 5a78b6d..919dc5d 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -331,10 +331,11 @@ If it does not, stop and repair the remote/fetch configuration. --check` clean. - Stage 4b (the input method) is NOT in this PR and not started. -## Journey Stage 1a — IMPLEMENTED on branch, gates run, PR pending +## Journey Stage 1a — PR #182 OPEN, review round 1 closed -- Framing `docs/journey-stage1a-framing.md` **rev 7** (four review - rounds, then two correction revisions found during implementation). +- Framing `docs/journey-stage1a-framing.md` **rev 8** (four review + rounds, two correction revisions found during implementation, one from + review round 1 of PR #182). Branch `journey-stage1a-directory-open`, rebased onto `githubsucks/main` @ `74301d1`. - Recovery: `git fetch githubsucks && git checkout @@ -355,6 +356,11 @@ If it does not, stop and repair the remote/fetch configuration. preflight moved after the callback → P1 + P2 fail, nothing else; drop the `ScopedFrontend` arm from `acting_frontend` → N4b fails, nothing else. That last mutation is why N4b exists — it left N4 green. + Round 1 of PR #182 added two more: dired's `display` back to + `switch_buffer`, and `prev` read from the ambient window → each fails + **N4c** alone. **The scope pins the frontend, not the window** — every + routing pin before N4c varied frontend identity and none varied the + selected window within one frontend, so 23 green pins missed it. - Ordering: PR #177 MERGED (2026-07-26), so 1a was unblocked. 1a lands before dired Stage 2. When 1a lands, Stage 2 must re-scout and revise its framing around the scoped `pmacs.window.commit_to` boundary before diff --git a/docs/journey-stage1a-framing.md b/docs/journey-stage1a-framing.md index ebb3cc9..ce5589f 100644 --- a/docs/journey-stage1a-framing.md +++ b/docs/journey-stage1a-framing.md @@ -1,7 +1,7 @@ # Journey Stage 1a — open a directory, on one path -**Status: framing, rev 6 — APPROVED at rev 5; rev 6 records -corrections found during implementation.** +**Status: framing, rev 8 — APPROVED at rev 5; revs 6–8 record +corrections found during implementation and review of PR #182.** **Serves `COHERENCE.md` §2 (the golden product journey), §19 (coherence acceptance tests), §20 Priority 1.** @@ -147,6 +147,30 @@ acceptance tests), §20 Priority 1.** else; mutation C (drop the `ScopedFrontend` arm) fails N4b and nothing else. +- rev 8 (2026-07-26) — **review of PR #182.** One implementation gap and + two stale claims: + - **dired did not honor the captured window.** §4.4 specified + `display{ window = dest:window() }`; the implementation still ended + in `pmacs.window.switch_buffer`, which targets whatever window the + *scoped frontend* has selected. The scope pins the frontend; it does + not pin the window. So a split or panel that took focus while + `read_dir` was pending received the listing, and `prev` was captured + from it too — with every preflight check passing, because the + captured window was still live and still held its captured buffer. + Fixed in both places (`display` and the `prev` read), and **N4c** + added. The suite's routing pins all varied *frontend* identity; + none varied the selected window within one frontend, which is why + 23 green pins missed it. + - **The §0 scorecard row still graded §2 "Broken at entry"** while §2's + own ground truth had been rewritten — the scorecard is a second copy + of the same claim and §25's update protocol covers both. §19's row + and ground truth were stale in the same way (this PR creates the + first cross-subsystem suite) and are corrected too. + - **P4 still said "leaves exactly one buffer"**, the exact claim rev 6 + corrected as false everywhere else. Restated to what it actually + pins — the file is in the *active window* — matching the test that + was already written correctly. + --- ## 0.5. Coherence impact (`COHERENCE.md` §20, required since #163) @@ -741,6 +765,15 @@ is **removed rather than recast**: it proved nothing N1 does not. - **N5** Bootstrap with a deliberately **non-scratch** LOCAL primary document buffer: the reply's `buffer_id` is that buffer, and after quiescence the window shows dired (Q#JR9, §4.5). +- **N4c — the captured *window*, not the captured frontend's selected + one (added rev 8).** One frontend, two windows: capture a destination, + then split and move focus to the other window and give it a buffer of + its own, then run dired's handler path with the captured destination. + The listing lands in the captured window, the focused window is + untouched, and `q` returns to the buffer the *captured* window showed. + Falsified independently by restoring `switch_buffer` in dired's + `display` and by reading `prev` from the ambient window — both were + verified to fail only this pin. - **N6 — `commit_to` scopes and restores, on every exit path.** Three cases, each asserting that **both** the scoped override and `core.active_frontend` return to their prior values: (a) `fn` returns @@ -826,10 +859,16 @@ entire claim. P3–P8 are preservation pins in the strict sense. that handle's `prev`, entries, and cursor untouched. *Mutation:* restore the ambient `handle.prev = pmacs.window.buffer()` outside the scope (§2.5 step 4). -- **P4 — the startup scratch is still dropped (Q#JR3).** - `EditorState::open` leaves exactly one buffer. +- **P4 — startup shows the file in the *active window* (Q#JR3, corrected + rev 6, restated rev 8).** `EditorState::open` displays the loaded + buffer in the active window and no window is left showing the startup + scratch. It does **not** assert a buffer count: `replace_active_buffer` + does not drop the scratch buffer, and rev 5's "leaves exactly one + buffer" wording — which survived rev 6's correction here by oversight, + caught in review of PR #182 — asserted a guarantee the editor does not + make. *Mutation:* replace `replace_active_buffer` with a bare - `install_buffer_in_window`. + `install_buffer_in_window` into some other window. - **P5 — the `NotFound` arm survives the refactor.** A nonexistent path yields an empty path-backed buffer with `[new file]` and fires no hook. *Mutation:* delete the `NotFound` arm from `resolve_target_buffer`. diff --git a/tests/journey_acceptance.rs b/tests/journey_acceptance.rs index d25cf4c..7ab36c0 100644 --- a/tests/journey_acceptance.rs +++ b/tests/journey_acceptance.rs @@ -570,6 +570,81 @@ fn commit_to_outranks_an_interactive_origin() { assert_eq!(local_window(&s), local_win); } +/// **N4c** — the commit lands in the *captured window*, not merely in +/// the captured frontend's currently selected one. +/// +/// Review finding on PR #182. Every other routing pin here varies +/// frontend identity; none varied the selected window *within* one +/// frontend, and dired's commit still ended in `switch_buffer`, which +/// targets whatever window the scoped frontend has active. The preflight +/// cannot catch this — the captured window is still live and still holds +/// its captured buffer — so a split that took focus while `read_dir` was +/// pending got the listing, and `prev` was captured from it too. +/// +/// Both halves are asserted: where the listing lands, and where `q` +/// goes. Falsified by restoring `pmacs.window.switch_buffer` in dired's +/// `display`, or by reading `prev` from the ambient window. +#[test] +fn a_background_open_uses_the_captured_window_not_the_selected_one() { + let td = project(); + let mut s = EditorState::new(); + exec(&s, "pmacs.lsp.config = {}"); + capture_dest(&mut s, td.path()); + + let target = local_window(&s); + let origin = buffer_in(&s, target).expect("the captured window's buffer"); + + // Split, move focus to the OTHER window, and give it a buffer of its + // own. The captured window is untouched, so every preflight check + // still passes -- which is exactly why this needs its own pin. + exec( + &s, + "local captured = dest:window() + pmacs.window.split_horizontal() + while pmacs.window.current() == captured do pmacs.window.focus_next() end + pmacs.window.switch_buffer(pmacs.buffer.create('*elsewhere*'))", + ); + let elsewhere = local_window(&s); + assert_ne!(elsewhere, target, "focus must have moved to another window"); + let elsewhere_buffer = buffer_in(&s, elsewhere); + + // dired's real handler path, with the captured destination. + exec( + &s, + &format!( + "pmacs.async(function() + pmacs.dired.open({:?}, {{ dest = dest }}) + end)", + canon(td.path()) + ), + ); + pump(&mut s); + + assert_eq!( + buffer_in(&s, elsewhere), + elsewhere_buffer, + "the window that took focus mid-listing must be untouched" + ); + assert_eq!( + local_window(&s), + target, + "the commit must select the captured window" + ); + assert!( + active_name(&s).starts_with("*dired:"), + "and the listing must be in it; got {:?}", + active_name(&s) + ); + + // `prev` came from the captured window too, not from `*elsewhere*`. + type_char(&mut s, 'q'); + assert_eq!( + buffer_in(&s, target), + Some(origin), + "`q` must return to the buffer the CAPTURED window showed" + ); +} + /// **N6a** — the scope is restored when the callback returns normally. /// /// Falsified by dropping the guard's restore, or by never swapping