fix(journey): honor the captured window, not the selected one
Review round 1 of PR #182. One implementation gap and two stale claims. **The scope pins the frontend; it does not pin the window.** Framing §4.4 specified `display{ window = dest:window() }`, but dired's commit still ended in `pmacs.window.switch_buffer`, which targets whatever window the scoped frontend has selected. A split or panel that took focus while `read_dir` was pending therefore 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. Both sites now read the captured window: `display` routes to it with `select = true` (the later `seat_cursor` acts on the active window), and the `prev` read asks it directly. N4c pins both halves. The suite's existing routing pins all varied *frontend* identity; none varied the selected window within one frontend, which is exactly why 23 green pins missed this. Bite: dired's `display` back to `switch_buffer` fails N4c alone; `prev` read from the ambient window fails N4c alone. Two stale documentation claims, both of which this PR was supposed to have already fixed: * **The §0 scorecard 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 protocol covers both. §19's row and ground truth were stale the same way — this PR creates the first cross-subsystem suite, which §19 says should exist and grades as missing — and are corrected too. * **P4 still read "leaves exactly one buffer"**, the exact claim rev 6 corrected as false everywhere else in the framing. Restated to what it actually pins: the file is in the *active window*. The test was already written correctly; only the framing lied. Framing rev 8. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
d9d1c0b423
commit
7741cf806a
34
COHERENCE.md
34
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*
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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`.
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue