diff --git a/context/code/clients.md b/context/code/clients.md index 24a347d9..6896ca3d 100644 --- a/context/code/clients.md +++ b/context/code/clients.md @@ -810,6 +810,22 @@ the keyboard disagreed. `the_formula_panel_has_no_enter_to_mirror_and_the_view_p asserts the binding's absence and names `click_formula_row` as what must change if it ever appears. +**The browser assertion that PR #60 dropped stays dropped, and improvise-qf57 +is the decision rather than the oversight.** `#grid input` `toHaveCount(0)` was +argued to be redundant with the `#mode` assertion on the grounds that +`toHaveText("FORMULAS")` is exact and `AppMode::FormulaEdit` labels as +`"FORMULA"` — a coincidence of two strings differing by one character, which +would be a thin reason to leave it out. Measuring it gives a better one: it is +not redundant, it is **unreachable**. `web/app.js` creates a grid `` in +exactly one place, the grid cell's own `mousedown`, gated on `cell.editable`; +no repaint creates one and no panel handler can reach it. And an invented +formula-row activation opens the **minibuffer**, not a grid editor. Injecting +`Command::ChangeMode(AppMode::formula_edit())` into `click_formula_row` and +running the spec: `#grid input` `toHaveCount(0)` **passes**, and the run fails +on the `#mode` assertion below it. Restoring it would add an assertion that +cannot fail — the same defect this file already records one line down, where an +explanatory `assert!` was removed for never being able to be the failing one. + **Rule 2 — a click is all of its intents or none of them** (improvise-hkr8). Each `click_*` returns `Option>` and `click` does the single `unwrap_or_default`. The case that matters is an activation the client cannot diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md index cce8fcc0..3c630857 100644 --- a/context/test-evidence/clients.md +++ b/context/test-evidence/clients.md @@ -537,9 +537,13 @@ file server — **deterministic where a closed port is not**: a refused port's `close` timing is a property of the host's network stack, and on the GitHub runner it did not arrive inside the window. The page cannot tell the two apart and neither can `web/app.js`: in both cases the socket never opened. -`HANDSHAKE_TIMEOUT_MS` restates `web/app.js`'s own constant, because two tests -have to outlast it and a number that silently disagreed with the page would make -them pass for the wrong reason. `logStatus` records **every distinct `#status`**, +`HANDSHAKE_TIMEOUT_MS` is **imported from `web/shell.js`** and restates nothing +(PR #65), and **one** test outlasts it — this sentence said the opposite of both +for a while, which is gotcha 48's third clause: a fact stated twice is changed by +a grep, not an edit. Since improvise-669b the spec also compares the *running +page's* window against the one it imported, because the single home is in the +TREE and the page loads `harness/serve/app/shell.js` — see +`outlastTheWatchdog` below. `logStatus` records **every distinct `#status`**, not just the last one, because several of these tests are about a message being *replaced*, which the final value cannot see. @@ -624,6 +628,25 @@ watchdog never given the chance — vacuous, and green. `outlastTheWatchdog` rea moved. Demonstrated both ways: with the install removed the suite passed 6/6, and with the guard in place the same omission fails by name. +**And it guards its second premise since improvise-669b: that the page runs the +constant the spec imported.** PR #65 single-homed the handshake window in +`web/shell.js`, which closed the *understated spec constant* — but the home is +single in the **tree**, not in what the page runs: the spec imports +`../../web/shell.js` and the page loads `harness/serve/app/shell.js`, the copy +`web/assemble.sh` put there. A stale `serve/` restores the exact vacuity, and +review demonstrated it — shrink the constant in source only, revert both socket +arms, 6/6 green. `createShell` now exposes `timeoutMs()` beside `timesArmed()` +and `outlastTheWatchdog` reads the running page's value and requires it to equal +the import. Injected with the ticket's own recipe (`8000` → `100` in +`web/shell.js`, no rebuild): the run goes to 5 passed / 1 failed, on +"the page's handshake window is not the one this spec imported", where the same +staleness was previously invisible. + +Note what this does and does not reach: it guards the tests that *call the +helper*. `"a peer that accepts and then says nothing trips the watchdog"` also +names `HANDSHAKE_TIMEOUT_MS`, but as a Playwright `timeout:` margin rather than +as a window to outlast — a stale value there costs patience, not evidence. + **The window is single-homed, and that is the second half of the same finding.** `HANDSHAKE_TIMEOUT_MS` lived twice — once in `web/app.js`, once in the spec — with a comment saying a silent disagreement "would pass for the diff --git a/harness/tests/contract.spec.js b/harness/tests/contract.spec.js index 06c87b03..fc4c5f70 100644 --- a/harness/tests/contract.spec.js +++ b/harness/tests/contract.spec.js @@ -214,6 +214,8 @@ test.describe("a pointer event reaches the client and its answer is painted", () // A formula row: navigation only, because FormulaPanel binds no Enter and // a click mirrors Enter. The Rust guard is on the KEYMAP — clients.md. + // `#grid input` toHaveCount(0) belongs here and is deliberately absent: + // it cannot fail, measured by injection (improvise-qf57, clients.md). await page.keyboard.press("Shift+F"); await expect(page.locator("#formulas li").first()).toBeVisible(); await page.keyboard.press("Escape"); diff --git a/harness/tests/shell.spec.js b/harness/tests/shell.spec.js index 0b51272f..435ba6c5 100644 --- a/harness/tests/shell.spec.js +++ b/harness/tests/shell.spec.js @@ -41,6 +41,12 @@ async function outlastTheWatchdog(page) { await page.evaluate(() => window.__improviseShell?.timesArmed() ?? 0), "the watchdog was never armed on this path: denying its message proves nothing", ).toBeGreaterThan(0); + // The import is the SOURCE tree's constant; the page runs the ASSEMBLED + // copy. A stale `serve/` makes them differ — improvise-669b, clients.md. + expect( + await page.evaluate(() => window.__improviseShell?.timeoutMs() ?? null), + "the page's handshake window is not the one this spec imported: re-run harness/build.sh", + ).toBe(HANDSHAKE_TIMEOUT_MS); const jump = HANDSHAKE_TIMEOUT_MS + 2_000; const before = await page.evaluate(() => Date.now()); await page.clock.runFor(jump); diff --git a/web/shell.js b/web/shell.js index 7100f234..90861beb 100644 --- a/web/shell.js +++ b/web/shell.js @@ -153,6 +153,9 @@ export function createShell({ onStatus, timeoutMs, setTimer = setTimeout, clearT this.setShell(text); }, armed: () => watchdog !== null, + /** The window this shell was built with. Exposed so a spec can compare the + * RUNNING page's value against the one it imported — improvise-669b. */ + timeoutMs: () => timeoutMs, /** How many times the watchdog has been armed. A test denying its message * needs this to be nonzero, or it is denying an unreachable event. */ timesArmed: () => arms,