test(e2e): route-affordance walk doc precision, negated-matcher findings and parked mount reads (#2682 follow-ups)

[Chris0Jeky]

Test-only. The five follow-ups from the fresh-context review of PR #2679, over the three files PR #2673 created. No product code touched.

Refs #2682

Summary

1. Header pointer made true, not dropped. route-affordances.spec.ts says which defence covers each walked response row is "stated per row in the inventory". Four rows carried no such note, so they have one now:

• home.quick-capture-submit — PaperHomeView's onMounted fetches the home summary and nothing else (PaperHomeView.vue:381-384), so only this submit can raise the POST.
• inbox.archive — PaperInboxView's onMounted (PaperInboxView.vue:504-508) issues no request at all, and the pattern is anchored to a POST on /archive.
• review.approve — PaperReviewView's onMounted (PaperReviewView.vue:2546-2558) loads board options and proposals and starts a queue poll, all GETs.
• calendar.next-month — gains the spent-mount-read half beside the exact-string post-condition its comment already explained.

2. Token-substitution doc now states the real surface. The old text said tokens are substituted in url and response pattern strings and listed {openingMonthLabel} among them. Read from fillTokens's call sites rather than from the issue: it is applied at five consequence sites — url.pathPattern, response.urlPattern, value.value, text.text, textChangedFrom.from — plus a role selector's name. It is not applied to attribute's selector/attribute/value, the enabled selector, node/focus test ids, or any css/testId selector value, so a token written into one of those is matched literally and the row silently fails to find its node. {openingMonthLabel} appears in exactly two places (calendar.previous-month's textChangedFrom.from and calendar.next-month's text.text) and in no pattern at all. fillTokens is unchanged.

While there: RouteEntry.entryPath claimed {boardId} "is substituted". Nothing reads that field — the walk navigates with literal page.goto calls and the guard compares route names — so it is now documented as the read-nothing field it is.

3. assertEnabled reads documented as dead, both kept. The guarded status variant declares assertEnabled: true as a required literal, so the guard's !== true throw and the walk's if (affordance.status.assertEnabled) cannot take their false branch. Both reads stay as belt-and-braces and now say so at their own sites and in the assertion-8 docblock: the type is the enforcement, and widening the field to boolean is what would make them live.

That also answers the review's "unlike the response branch, this has no canary". The asymmetry is the point: a response consequence on a guarded row type-checks — assertion 9 builds exactly that row with no cast — so its canary tests a shape someone can write. The assertEnabled shape is not reachable without a cast, so a canary there would only prove the checker against something the type already makes unconstructible. Considered and declined; recorded in the docblock rather than left implicit.

4. textChangedFrom — hypothesis refuted, so nothing changed. The review expected expect(locator).not.toHaveText(x) to pass on a zero-element locator, which would have made the kind vacuous and required a toHaveCount(1) companion. It does not pass. Confirmed two ways on Playwright 1.62.1 (@playwright/test and playwright-core both 1.62.1):

• Code. node_modules/playwright/lib/matchers/expect.js — toHaveText with a string sends the to.have.text expression (not .array). node_modules/playwright-core/lib/coreBundle.js — in Frame._expectInternal, a selector resolving to zero elements special-cases only to.be.visible, to.be.hidden, to.be.attached, to.be.detached, to.be.in.viewport and the .array/to.have.count expressions; everything else falls through to matches = options.isNot; missingReceived = true. Frame.expect then treats matches === options.isNot as "keep polling", so the assertion runs to its timeout.
• Behaviour. A chromium probe against a selector matching nothing: FAILED after 3019ms — Error: element(s) not found. The control cases behaved as expected (present node with differing text passed in 12 ms; present node with matching text failed).

So no toHaveCount pairing was added, expectConsequence is unchanged apart from the comment, and calendar.previous-month proves its own move rather than leaning on calendar.next-month one row later. The finding is recorded on the textChangedFrom kind, at the expectConsequence case, and in the assertion-8 docblock.

5. All four mount reads parked. The calendar, metrics, notifications and home mount reads each armed a page.waitForResponse before page.goto and awaited it only on the following line, so a goto that threw left the wait unawaited and its 30 s rejection landed on the worker with no test to own it. New readOnMount helper parks the outcome the moment the wait is armed — the pattern activate already uses for row waits — and all four go through it. Success-path semantics are identical: armed before, awaited after, asserted 2xx.

Test plan

All commands from frontend/taskdeck-web unless noted.

Command	Result
npx vitest --run --maxWorkers=2 src/tests/guards/routeAffordanceCoverage.spec.ts	Test Files 1 passed (1), Tests 9 passed (9) — one path named, one file run
npm run typecheck (vue-tsc -b)	clean
npx eslint on the three changed files	clean, exit 0
git diff --check	clean
E2E — npx playwright test tests/e2e/route-affordances.spec.ts --reporter=line	4 passed (19.0s) — all four walk blocks green against a locally started stack (API :5000 + Vite :5173, brought up by the config's own webServer; both ports confirmed free first). Run with the four Llm__Gemini__* vars unset, which this repo rejects by design.

The E2E spec is covered by no tsconfig project — tsconfig.app.json and tsconfig.vitest.json both scope to src/, which is what its own header says and why its expectConsequence switch throws on default rather than relying on never. Since this PR changes executable code there (readOnMount, the type Response import, four call sites), it was additionally checked out-of-band with npx tsc --noEmit --ignoreConfig --strict --skipLibCheck --target ES2022 --module esnext --moduleResolution bundler --types node tests/e2e/route-affordances.spec.ts — clean. That is a best-effort check, not a repo gate.

The inventory module is type-checked, as a dependency of the guard spec under tsconfig.vitest.json.

Red-first

Item 5 is the only behavioural change, so it was proved defective before it was fixed. A scratch probe (kept outside the tree) drove both shapes against page.goto on a refusing port, with process.on('unhandledRejection') recording leaks and a 4 s tail so a leaked rejection had time to surface:

SHIPPED shape (page.waitForResponse then await page.goto)
  navigation error reported: page.goto: net::ERR_UNSAFE_PORT ...
  unhandled rejections leaked: 1
    - page.waitForResponse: Timeout 2000ms exceeded while waiting for event "response"
PARKED shape (readOnMount)
  navigation error reported: page.goto: net::ERR_UNSAFE_PORT ...
  unhandled rejections leaked: 0

Both shapes report the same navigation error; only the shipped one trails an orphan rejection.

Item 4 needed no red-first: the hypothesis was refuted, so no code changed.

Not verified

• CI's E2E Smoke is the second proof. The local run used the deterministic Mock LLM (resolvePlaywrightBackendLlmEnv forces Llm__Provider=Mock unless a demo/live-LLM flag is set; none was) on chromium at 2 local workers, not CI's single-worker shared-budget lane.
• The four per-row "no mount read can settle this" claims are read from each view's onMounted, not observed on the wire. They restate at row level what the spec header already asserted for those rows; a request raised from somewhere other than onMounted (a watcher firing on first paint, say) would not be caught by that reading. The rows' own E2E assertions are what would fail if the claim were wrong.
• No canary was added for the assertEnabled branch, so the throw remains unexercised at runtime. That is the documented, deliberate outcome of item 3, not an oversight — see the Summary.

Boundaries

Only the three files PR #2673 created are touched:

• frontend/taskdeck-web/tests/e2e/route-affordances.spec.ts
• frontend/taskdeck-web/tests/e2e/support/routeAffordanceInventory.ts
• frontend/taskdeck-web/src/tests/guards/routeAffordanceCoverage.spec.ts

Not touched: any product code under src/ (a doc-precision PR must not change behaviour), the other E2E specs, playwright.config.ts, and docs/**. docs/STATUS.md and OUTSTANDING_TASKS.md are deliberately untouched — the coordinator records this PR.

The issue's informational note stays as recorded and was not acted on: a postCondition: { kind: 'response' } on any row passes typecheck and the vitest guard and throws only inside E2E Smoke, via expectConsequence's response case. That is the documented design and the written-down limit of the cheap gate.

No scratch artifacts were left in the tree; the two probes live under the session scratchpad (.../scratchpad/worker-2682/) and are not needed to reproduce anything — the code references in item 4 are enough.

Round 2 (head 4b1a20003)

Review verdict was SHIP with three findings against the new prose — in a doc-precision PR that is the defect class, so all three are fixed in one wording-only commit. No executable change and no timeout bound added.

1. The "30 s" claim was false, in both the header and readOnMount. These calls pass no timeout and this config supplies no default, so the wait has no event timeout of its own. Verified in the installed tree: playwright/lib/index.js:259 defaults the actionTimeout fixture to 0; playwright.config.ts never sets actionTimeout/navigationTimeout (only timeout: 45_000 at :111 and expect.timeout: 8_000 at :113) and nothing under tests/ calls setDefaultTimeout; playwright/lib/index.js:349 assigns _defaultContextTimeout = actionTimeout || 0; coreBundle.js:57160-57169 returns that 0 rather than falling through to the library default (0 !== void 0, so the chain stops); and coreBundle.js:58412 arms a timer only if (timeout), which 0 is not. Page.waitForResponse reaches that path via coreBundle.js:61252-61260 → _waitForEvent at :61268-61275. So a mount read whose response never arrives hangs to the test timeout — 90 s in the board-seeded walk, 45 s in the Home/Inbox block — and reports "Test timeout exceeded". Both sentences now say that. Parking still earns its place for a different reason, also now stated: an unawaited wait is rejected at page close with a TargetClosedError (coreBundle.js:61278-61279 and :61265-61267), by which time no test is left to own it. The text also records that activate caps its row waits at { timeout: 15_000 } while these mount reads take no cap, and that bounding them is tracked on [Testing][E2E] Route-affordance walk: doc precision and dead-branch follow-ups from the #2679 review #2682.

   Note on the round-1 red-first probe above: it passed its own explicit { timeout: 2000 } to make the leak observable in seconds. That is what produced the Timeout 2000ms exceeded rejection in the transcript. It demonstrates the parking mechanism correctly, but it is not the spec's own bound — under this config the unparked failure mode is hang-to-test-timeout, or TargetClosedError at page close.

2. "{boardId} … and nowhere else" was false. It also appears in the workspace-board entry's entryPath, which this same PR documents as never substituted. The census now names that occurrence and points at the entryPath docblock.

3. The verbatim-usage list was incomplete. It omitted value.selector, text.selector and textChangedFrom.selector — all used verbatim by expectConsequence. Added, and the list is now labelled complete rather than illustrative, with the distinction made explicit: those kinds fill their value/text/from payload but not the node they read it from.

Proof at 4b1a20003: npx vitest --run --maxWorkers=2 src/tests/guards/routeAffordanceCoverage.spec.ts → Test Files 1 passed (1), Tests 9 passed (9); npm run typecheck clean; npx eslint on the changed files clean (exit 0); git diff --check clean.

No E2E rerun is owed for this round — the commit changes only comment text, so the 4-passing-blocks run recorded above still stands at this head.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

[Chris0Jeky]

Review record (alpha product-trust lane; test-only, one fresh-context read-only reviewer subagent since Codex credits are exhausted, SC-9; round 1 at head b458e1cb0, inputs: the clean worktree at the head and the merge-base diff, 3 files).

Verdict at round 1: SHIP on the merge-blocking bar. Because this PR's purpose is doc precision, its three findings, all sentences in the new text, get one wording-only commit in round 2 rather than a tracked issue:

• MEDIUM, fixed in round 2 as wording: the header (~58) and the readOnMount docblock (~511) assert a "30 s" waitForResponse bound that does not exist under this config: actionTimeout defaults to 0, playwright.config.ts sets none, nothing calls setDefaultTimeout, and playwright-core arms no rejection for 0, so a mount read that never gets a response hangs to the test timeout (90 s or 45 s) and reports "Test timeout exceeded". The parking itself is correct (an unparked promise rejects at page close with no owning test). Bounding the four mount reads the way activate bounds its waits (15 s) is a behaviour change beyond the finding and is recorded on #2682 as a follow-up, not bundled here.
• LOW, fixed in round 2 as wording: "{boardId} in three url.pathPatterns and nowhere else" omits its unsubstituted entryPath occurrence (~336), which the same PR's entryPath docblock documents.
• LOW, fixed in round 2 as wording: the verbatim-use enumeration (~80-84) omits value.selector, text.selector and textChangedFrom.selector.

Clean lenses corroborated by the reviewer against the tree at the head: readOnMount attaches its rejection handler synchronously before any await, arms before navigate() as before, awaits once and rethrows the original error, with a byte-for-byte identical success path; the Playwright zero-element not.toHaveText reading is right (the zero-element special cases in coreBundle.js are count, array, hidden, visible, detached, attached and in-viewport; everything else keeps polling and surfaces "element(s) not found"); each per-row note checked against its view's onMounted (PaperHomeView summary only, PaperInboxView no request, PaperReviewView GETs only with one production approve call site and the bulk endpoint unable to match the row's anchor, CalendarView's spent-read chain); the token census (six fillTokens call sites, {openingMonthLabel} in exactly two text fields and no pattern, entryPath read by nothing repo-wide); the assertEnabled dead-branch claims (required literal, no cast-free canary, the contrast with assertion 9 real); the vitest guard changed only in comments and still cannot pass vacuously; no walked row's assertion strength changed.

Unverified by the read-only reviewer, run by the worker at the head: guard spec 9/9 (Test Files 1), typecheck, eslint, git diff --check, and the E2E walk 4 passed locally against the config's own web server; CI's E2E Smoke lane is the second proof.

Round count: 2 (wording only, so no further review pass is owed). Merge gate remaining: the fix head's CI green and the three-minute age.

[Chris0Jeky]

Round 2 at 4b1a20003 (comments only, 2 files): both bound sentences now say the mount reads have no event timeout of their own under this config (actionTimeout defaults to 0, _timeout() returns 0 because 0 is not undefined, rejectOnTimeout arms nothing for 0) and are bounded only by the test timeout, that an unawaited wait is rejected with TargetClosedError at page close, and that activate caps its own waits at 15 s while the mount reads do not (follow-up on #2682); the {boardId} census names the unsubstituted entryPath occurrence; the verbatim-use list is complete with the three selector fields. The worker also notes its round-1 probe passed its own { timeout: 2000 }, which is why it saw a timeout rejection: the parking it proved is real, the bound was the probe's. Guard spec 1 file / 9 passed, typecheck, eslint and git diff --check clean; no E2E rerun owed for comment text. Merge gate: CI green at 4b1a20003 plus the three-minute age.