diff --git a/.claude/skills/gates/SKILL.md b/.claude/skills/gates/SKILL.md index 615e079bd7..37f961dde8 100644 --- a/.claude/skills/gates/SKILL.md +++ b/.claude/skills/gates/SKILL.md @@ -24,7 +24,7 @@ Check these before believing any result. for exactly this reason — if installed packages do not match `package-lock.json`, treat any test, lint, or typecheck result as void until `npm ci` has run. Its own failure message says as much. - **`verify:cheap` stops at the first failing check.** Everything after that point never ran. Do not - describe the change as broadly verified when the gate died at check 2 of 26. + describe the change as broadly verified when the gate died at check 2 of 30. - **`format:check` is required in CI but is not part of `verify:cheap`.** A locally green `verify:cheap` can still fail CI on formatting. Run `npx prettier --write ` before pushing — scoped to your files, never `prettier --write .`, which sweeps the whole tree. diff --git a/.github/actions/setup-ui-e2e/action.yml b/.github/actions/setup-ui-e2e/action.yml index 9179601d15..ec4735dccb 100644 --- a/.github/actions/setup-ui-e2e/action.yml +++ b/.github/actions/setup-ui-e2e/action.yml @@ -16,6 +16,11 @@ runs: with: path: ~/.cache/ms-playwright key: playwright-chromium-${{ runner.os }}-${{ hashFiles('package-lock.json') }} + # Without a prefix fallback, any lockfile bump forces a cold browser download in + # every UI job at once. A stale archive is harmless: `playwright install` below + # still fetches whatever the new version needs. + restore-keys: | + playwright-chromium-${{ runner.os }}- # The browser archive is cached, but apt libraries are runner-local and must be # installed even on a browser-cache hit. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4c5ccc3895..ad52fe1c7f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -422,6 +422,9 @@ jobs: # Fail-fast @critical Chromium smoke on PRs/merge_group before the full # production suite. Skipped on main/schedule — those run the full job only. # Keeps merge safety: pr-required still demands the full Production UI job. + # + # Deliberately NOT sharded: it is already the small subset, and sharding it + # would spend runners without shortening the critical path. ui-critical-fast: name: Production UI critical needs: changes @@ -456,6 +459,19 @@ jobs: playwright-report/ if-no-files-found: ignore + # Sharded across runners, NOT across workers. Playwright stays `workers: 1` / + # `fullyParallel: false` / `retries: 0` inside each shard, so determinism is + # unchanged and per-runner load goes DOWN rather than up — which matters because + # the duplicate-page-root strict-mode failures (#093) are load-dependent. + # + # Measured 2026-07-30 (before `ui-critical-fast` landed): this job ran 15m26 of + # an 18m36 run — 83% of wall clock — while every other job finished by minute 4. + # Playwright itself reported `339 passed (13.5m)`; the balance is the isolated + # production build. + # + # Branch protection requires only the `pr-required` aggregate, and `needs` on a + # matrix job yields the roll-up of all shards, so the aggregate below needs no + # change and the per-shard job names are free to differ. ui-critical: name: Production UI needs: [changes, ui-critical-fast] @@ -468,6 +484,37 @@ jobs: (needs.ui-critical-fast.result == 'success' || needs.ui-critical-fast.result == 'skipped') runs-on: ubuntu-24.04 timeout-minutes: 45 + strategy: + # One shard's failure must not cancel the others: a cancelled sibling would + # report as `cancelled` and re-create exactly the ambiguity #095 removed. + fail-fast: false + matrix: + # THREE, and the count is measured rather than chosen. `fullyParallel: false` + # makes a spec file the indivisible unit, so shard sizes are lumpy and more + # shards is not monotonically faster. Re-measured 2026-07-30 against this + # merged tree — 342 non-quarantine/non-mockup chromium tests + # (`--list --shard=i/N`): + # N=3 -> 121/111/110 largest 121 (35%) + # N=4 -> 121/106/98/17 largest 121 (35%) — same bound, one more runner + # Earlier on a 340-test tree, N=5 -> 121/106/0/96/17 and N=8 gave two empty + # shards. N=4 buys nothing over N=3 because one 121-test spec group bounds + # both, and any N with an empty shard would go RED: `test:e2e:pr` + # deliberately omits `--pass-with-no-tests` (only the advisory lane has it). + # Re-measure before changing this number, and keep every shard non-empty: + # npm run ensure + # PLAYWRIGHT_BASE_URL= npx playwright test --project=chromium \ + # --grep-invert "@quarantine|@mockup" --shard=i/N --list + # + # MEASURED on the first real sharded run (CI 30530618838, 2026-07-30): + # shard 1 -> 121 tests, 9m36 + # shard 2 -> 111 tests, 6m54 + # shard 3 -> 110 tests, 6m20 + # Per-test cost is NOT uniform — 111 tests took 6m54 while 121 took 9m36 — + # so counting tests UNDERSTATES the largest shard. A count-balanced split is + # the best `--shard` can do; balancing by duration would mean splitting the + # slow spec files themselves. Predicting from test count alone was wrong by + # ~40% here, so trust a measured run over the arithmetic. + shard: [1, 2, 3] steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -478,7 +525,7 @@ jobs: uses: ./.github/actions/setup-ui-e2e - name: Chromium production journeys - run: npm run test:e2e:pr + run: npm run test:e2e:pr -- --shard=${{ matrix.shard }}/3 - name: Classify exact failed test identities if: failure() @@ -488,7 +535,9 @@ jobs: if: failure() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: - name: production-ui-diagnostics-${{ github.run_id }} + # Shard-scoped: upload-artifact v7 runs with `overwrite: false`, so a + # shared name would make the second failing shard fail on upload. + name: production-ui-diagnostics-${{ github.run_id }}-shard${{ matrix.shard }} path: | test-results/ playwright-report/ @@ -877,6 +926,10 @@ jobs: with: path: ~/.cache/ms-playwright key: playwright-${{ runner.os }}-${{ hashFiles('package-lock.json') }} + # Prefix fallback so a lockfile bump does not force a cold download of all + # four browsers; `playwright install` below still fetches any missing version. + restore-keys: | + playwright-${{ runner.os }}- - name: Install Playwright browsers run: | diff --git a/.github/workflows/codex-autofix-review-comments.yml b/.github/workflows/codex-autofix-review-comments.yml index 958ff83286..fb11624867 100644 --- a/.github/workflows/codex-autofix-review-comments.yml +++ b/.github/workflows/codex-autofix-review-comments.yml @@ -13,6 +13,10 @@ jobs: request-codex-autoresolve: name: Request Codex auto-resolve runs-on: ubuntu-24.04 + # Every other workflow in this repo bounds its jobs; without this the GitHub + # default of 360 minutes applies to a job that only reads metadata and posts a + # comment. + timeout-minutes: 10 # Only run after Codex submits a completed review, never on the first inline # comment mid-review. Approvals/dismissals are handled below (no findings). if: > @@ -293,6 +297,7 @@ jobs: resolve-codex-thread: name: Resolve Codex review thread runs-on: ubuntu-24.04 + timeout-minutes: 10 # Marker-driven thread closure only. This is the narrow job that carries # pull-requests: write, and it acts solely on trusted disposition replies. if: > diff --git a/CLAUDE.md b/CLAUDE.md index aa3ad0e38b..75a10d88a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,7 +134,7 @@ Verification pyramid — run the **smallest gate that covers the change**, then | Gate | What it is | | ----------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `npm run test:focused -- --files ` | Source-only iteration. Fails closed for deleted files and test infrastructure — then run `npm run test`. | -| `npm run verify:cheap` | The broad local gate: 24 static/consistency gates + `lint` + `typecheck` + full offline unit suite | +| `npm run verify:cheap` | The broad local gate: 27 static/consistency gates + `lint` + `typecheck` + full offline unit suite | | `npm run verify:pr-local` | Closest local mirror of the PR gate; adds format and conditional build / client-bundle scan / RAG fixture validation. `-- --dry-run --files ` shows selection without running. | | `npm run verify:ui` | Chromium production journeys. Run `npm run ensure` first. | | `npm run verify:phone-chrome` | Phone-chrome changes; selects affected owners/journeys before escalating to `verify:ui` | diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 9c2d0731e3..3cd1a6d0de 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -144,6 +144,19 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-30 | PR #1396 / claude/latency-findings-impl-s8g01v | 70e810b66881e17aa9f58126fdad970986bda911 | User ask: resolve comments + Production UI phone-scroll + main sync | FIXED: synced main (DIRTY was staleness); removed union ledger dup; adapted phone-scroll asserts for Answer strategy-overlay + overlay/reserve-only calculator budget + focus pre-scroll inside 8px reveal band. Codex P1s already on tip; 0 unresolved threads. Focused Chromium phone-scroll 9/9 green (system Chrome). | phone-scroll focused 9/9; check:branch-review-ledger PASS; merge-tree clean; prior Codex P1s retained | | 2026-07-30 | HEAD | 13c16cf07c854b50daa35a2ef2a2ea76d5e059e1 | ci-testing-approach | findings: UI-load flake #093 dominates PR reds; schedule full-sentinel blocks release-browser via audit; UI scope overfires on src/app/api; ~40% PR runs cancelled wasting ~12 UI-hrs; CI_TRIAGE inert; eval:rag:offline claimed-in-CI but only fixtures run | gh-ci-500-runs,ci.yml,ci-change-scope,testing.md,process-hardening,outstanding-issues-093-095-097-023,flake-ledger-empty | | 2026-07-30 | cursor/ci-testing-review-1bf5 | 13c16cf07c854b50daa35a2ef2a2ea76d5e059e1 | ci-testing-approach | Corrects the ref cell from the unresolved placeholder "HEAD" to the actual branch name, so ledger:lookup can match this review by branch (Codex P2 finding on PR #1406). | node scripts/branch-review-ledger.mjs lookup cursor/ci-testing-review-1bf5 --scope ci-testing-approach | +| 2026-07-30 | PR #1427 / claude/ci-testing-review-2l8klp | db8209be707b79142d1d228d8c4e04120f9cdeaa | ci-testing-review | Measured PR CI from the Actions API: Production UI is 15m26-16m31 of a 16.8-18.6min run (83-89% of wall clock; Playwright itself 339 passed (13.5m)), every other job done by minute 4; 25/60 completed runs in an 83min window were cancelled (42%). FIXED: sharded ui-critical across 3 runners (count measured - N=4 gives the same 121-test critical path, N=5/N=8 give empty shards which would go red without --pass-with-no-tests); root-caused the real red (ui-phone-scroll dragScrollBy clamped silently and returned nothing, so a 720px request could deliver a fraction and the correct assertion failed 10s later) and made the drag prove its delivery with assertions byte-identical; browser-cache restore-keys; codex-autofix job timeouts; visual config serialised; gate-count guard added and mutation-proven. DEFERRED as #125-#129: ui_changed over-firing on src/app/api, cold Next cache in the Playwright build, advisory-UI cost vs zero quarantine tests, inert CI_TRIAGE, dead changes outputs. | verify:cheap PASS (431 files / 4493 passed, 4 skipped); verify:pr-local PASS (same); prettier --check . PASS; check-gate-manifest PASS + mutation-proven red at stale count; shard balance measured via playwright --list; verify:ui NOT RUN - container cannot launch Chromium (issue #121, build 1234 vs 1194) so the phone-scroll fix and the sharded job are unexecuted, PR CI is first execution | +| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | ad9da6a6f8ba3884b389fa78e678bb88ee72d9d1 | ci-hygiene-gates | implemented matrix unblock, scope narrow, cancelled≠failure, pinned gitleaks, critical-first UI, eval:rag:offline; skipped #093; verify:cheap 4471 pass | verify:cheap,check:ci-scope,check:gitleaks-pinned,check:gate-manifest,eval:rag:offline | +| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | b660dbc5a10d7ca3da03541028017f0abc6b5bd3 | ci-hygiene-gates merge-readiness | findings | check:ci-scope;check:gitleaks-pinned;scope-classify PR files ui_changed=false;sim cancelled-as-neutral | +| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | 8f3283d00da274dee507a1b8e9b611321d1f35be | pr-1413-merge-readiness | READY after main sync + cancel-to-green fix; draft until tip CI green; deferred #093 + CI_TRIAGE_ENABLED confirm | verify:cheap:4481-pass;format:outstanding-issues;merge-tree:clean;cancelled:!cancelled();hosted:awaiting-tip | +| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | 0d70de480f370fec3e7f3774f13d906318a09b3c | pr-1413-merge-readiness | READY: synced with main/#1409; tip CI success incl PR required; draft; deferred #093 + CI_TRIAGE_ENABLED | merge-tree:clean;ci-cache-safety:13/13;hosted:30520195863:success;PR-required:pass | +| 2026-07-30 | origin/main | 3569e7888bba5d11f143f27c11eb9bfa58800e4f | dependency installation and CI reproducibility | no P0-P2 findings; corrected stale setup-ui-e2e cache description | manifest-lock parity; Actions pins; merge-marker scan; merged PR 1360 diff | +| 2026-07-30 | cursor/process-anti-conflict-speed-1edf / PR #1416 | 2a31fcee0ef2e330a4901481c0810103d89c96cf | process anti-conflict merge readiness | NOT READY. CI green on stale head, but merge-tree vs current main is CONFLICTING in ci.yml, package.json, and add/add on check-outstanding-issues.mjs after #1410 landed a stronger #112 gate. Keep unique value: AGENTS anti-conflict procedure, #116 PR mergeability workflow, merge=union on outstanding-issues (explicitly still open after #1410). Drop duplicate weaker outstanding-issues checker; re-verify after sync. | ledger:lookup NOT REVIEWED; merge-tree dirty vs origin/main; ManagePullRequest CI SUCCESS (15 ok / 0 fail, Production UI skipped as non-UI); local verify:pr-local earlier on pre-conflict head 4467 passed | +| 2026-07-30 | cursor/process-anti-conflict-speed-1edf / PR #1416 | 387ffd07887f1160fca8fe98c1c4809e852531ae | process anti-conflict merge readiness | READY after sync with main. Kept #1410 structural outstanding-issues gate; added merge=union + runtime attr check; retained #116 PR mergeability workflow and AGENTS anti-conflict playbook; dropped duplicate weaker checker/test. merge-tree clean vs origin/main. | merge-tree clean; check:outstanding-issues pass; check:pr-mergeability pass; check:gate-manifest pass; verify:pr-local pass | +| 2026-07-30 | cursor/process-anti-conflict-speed-1edf / PR #1416 | 38ae07b989e6414026235debad0e429ba64cf462 | process anti-conflict merge readiness | READY at final tip. Same as prior READY plus this ledger append only; merge-tree still clean vs origin/main. | merge-tree clean; verify:pr-local on 387ffd07 parent (4480 passed); tip is ledger-only follow-up | +| 2026-07-30 | codex/chat-dependency-pr-review-dependency-pr-review-20260730 | 2aee8c74e64bd09954fe1b474c73b25805063de0 | open PR changed-scope review | APPROVE: setup action description now matches clean npm install with npm-download caching; no behavior change. | check:github-actions PASS; diff review; no unresolved threads | +| 2026-07-30 | PR #1427 / claude/ci-testing-review-2l8klp | b9de34d40d2dc5ab164bb1eb582db1cfcd1009c3 | ci-testing-review | SUPERSEDES the 2026-07-30 db8209be record, which asserted a root cause now REFUTED. That record claimed dragScrollBy clamping made the ui-phone-scroll red; main's #127 carries trace evidence (PR #1404 run 30521269873) that the drag delivered in full (scrollTop 1272 = 552+720) with ~1300px runway spare and a 10s non-flip is a latched state. The change is a diagnostic and guard, NOT a fix, and is now labelled so in the docstring, commit, PR body and #127. Remaining candidates: scrollHidden false vs sharedChromePinned latched; they are indistinguishable from the DOM because only the composite data-scroll-hidden is exposed. Prime suspect in source: composerFocusPinsChrome has a still-the-active-owner guard, headerFocusPinsChrome has none (master-search-header.tsx:397-398). ALSO: this PR ran zero pull_request workflows for ~2h (no CI/Gitleaks/Semgrep, only pull_request_target) because a real conflict blocked refs/pull/1427/merge - issue #116, caught by main's new PR mergeability check. Merging main fixed it and CI ran green first try. | CI run 30530618838 SUCCESS (13m39). MEASURED shard result, correcting the ~7min prediction: Production UI (1) 121 tests 9m36, (2) 111 tests 6m54, (3) 110 tests 6m20 - per-test cost is NOT uniform, shard 1 holds the slow specs, so the largest shard is 9m36 not the predicted 6.8min. ui-critical-fast 3m14. PR required SUCCESS. verify:cheap on merged tree PASS (434 files / 4563 passed, 4 skipped); prettier --check . PASS; ui-phone-scroll ran locally 1x via PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH: 56 passed (5.3m) - three-run protocol NOT completed and not applicable, since this is not a flake fix. | +| 2026-07-30 | PR #1400 | e869cb9d7ab20939361b277d1c6fcc07bbb6ca45 | pr-1400-guard-push-band-adoption | MERGED — 17 review findings across guard-push.mjs and the band-adoption gate, all fixed and thread-resolved; each fix confirmed by reverting it and watching the guard fail. Guard now checks the pushed SHA in a git worktree (config, dynamic config, policy escalation incl. removal, lockfile Prettier parity); adoption gate replaced presence-matching with per-module reachability, closing six false greens of one root cause plus type-position edges found in self-review. Merge verified: all 8 commits ancestors of main, 4 changed files byte-identical. | verify:cheap 432 files / 4470 passed 4 skipped; format:check clean; PR required success; services+tools page gutting reports orphans; 6 push-guard scratch-repo cases with real exit codes; every fixture mutation-verified | +| 2026-07-30 | claude/top-search-design-mockups-w53znc | 939d5799b9999f3f63928e1b2c95d097f07eff90 | open PR changed-scope review | APPROVE: PR 1400 closeout and issue IDs 131-134 are unique, internally consistent, and preserve the append-only ledgers. | check:branch-review-ledger PASS; check:outstanding-issues PASS; diff review; no unresolved threads | | 2026-07-30 | origin/circleci-project-setup | 9a55990053e26c02703b1ec9f2523a7c85e21e14 | branch-cleanup | REJECTED and deleted remote. Unique tip only changed trailing newline on obsolete .circleci hello-world config; CircleCI removed from main in PR #1412. No open PR. | fetch --prune; three-dot + tip inspect; gh pr list open=0; main has no .circleci; GitHub reads explicitly authorized; no non-GitHub provider checks. | | 2026-07-30 | origin/execute-audit-code-remediation | 3470279fba23ad442d59d34552eb576e87f24141 | branch-cleanup | REJECTED and deleted remote. PR #1162 already merged; sole unique commit was a ledger CI-green row already present on main (edcd17a1…). No open PR; no unique product content. | fetch --prune; cherry-pick log; tip-to-tip/three-dot; grep ledger for edcd17a1; gh pr 1162 MERGED; open=0; GitHub reads explicitly authorized; no non-GitHub provider checks. | | 2026-07-30 | origin/apply-audit-remediation-protocol | 046cb38ad45411c5f539636d4c976b25195f7bbd | branch-cleanup | RETAIN. Closed PR #1338; tip still has unique files main lacks (motion-tokens.ts, use-overlay-presence.ts) plus stale sheet/globals diffs. Not empty vs main; do not delete. | ledger lookup; cherry-pick+three-dot; blob existence on main; gh PR #1338 CLOSED; GitHub reads explicitly authorized; no non-GitHub provider checks. | @@ -154,17 +167,7 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-30 | origin/execute-audit-remediation-tasks | bdcf8d5c1f14c927bf5b71aaacd34d006856da4f | branch-cleanup | RETAIN. Closed PR #1347; tip adds check-answer-quality-thresholds.ts and check-cost-cap-preflight.ts that main lacks, plus other diffs. Keep. | ledger lookup; cherry-pick; MAIN_LACKS path check; gh #1347 CLOSED; GitHub reads explicitly authorized; no non-GitHub provider checks. | | 2026-07-30 | origin/execute-typography-audit-fixes | dd641579f4cf54f82de89ef268ac8aa6acb439b5 | branch-cleanup | RETAIN. Closed PR #1185 (clean successor #1294 merged); tip blobs still differ from main on globals.css and mockup typography tweaks. Not empty; keep. | ledger lookup; cherry-pick; blob equality; gh #1185 CLOSED #1294 MERGED; GitHub reads explicitly authorized; no non-GitHub provider checks. | | 2026-07-30 | origin/implement-audit-design-fixes | dda4a42baa34e28f12d1e676fcabdbdfaef820c8 | branch-cleanup | RETAIN. Closed PR #1263; tip still carries unique not-found/error route files and a large three-dot diff vs main. Keep. | ledger lookup; cherry-pick; path existence; gh #1263 CLOSED; GitHub reads explicitly authorized; no non-GitHub provider checks. | -| 2026-07-30 | PR #1400 | e869cb9d7ab20939361b277d1c6fcc07bbb6ca45 | pr-1400-guard-push-band-adoption | MERGED — 17 review findings across guard-push.mjs and the band-adoption gate, all fixed and thread-resolved; each fix confirmed by reverting it and watching the guard fail. Guard now checks the pushed SHA in a git worktree (config, dynamic config, policy escalation incl. removal, lockfile Prettier parity); adoption gate replaced presence-matching with per-module reachability, closing six false greens of one root cause plus type-position edges found in self-review. Merge verified: all 8 commits ancestors of main, 4 changed files byte-identical. | verify:cheap 432 files / 4470 passed 4 skipped; format:check clean; PR required success; services+tools page gutting reports orphans; 6 push-guard scratch-repo cases with real exit codes; every fixture mutation-verified | -| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | ad9da6a6f8ba3884b389fa78e678bb88ee72d9d1 | ci-hygiene-gates | implemented matrix unblock, scope narrow, cancelled≠failure, pinned gitleaks, critical-first UI, eval:rag:offline; skipped #093; verify:cheap 4471 pass | verify:cheap,check:ci-scope,check:gitleaks-pinned,check:gate-manifest,eval:rag:offline | -| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | b660dbc5a10d7ca3da03541028017f0abc6b5bd3 | ci-hygiene-gates merge-readiness | findings | check:ci-scope;check:gitleaks-pinned;scope-classify PR files ui_changed=false;sim cancelled-as-neutral | -| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | 8f3283d00da274dee507a1b8e9b611321d1f35be | pr-1413-merge-readiness | READY after main sync + cancel-to-green fix; draft until tip CI green; deferred #093 + CI_TRIAGE_ENABLED confirm | verify:cheap:4481-pass;format:outstanding-issues;merge-tree:clean;cancelled:!cancelled();hosted:awaiting-tip | -| 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | 0d70de480f370fec3e7f3774f13d906318a09b3c | pr-1413-merge-readiness | READY: synced with main/#1409; tip CI success incl PR required; draft; deferred #093 + CI_TRIAGE_ENABLED | merge-tree:clean;ci-cache-safety:13/13;hosted:30520195863:success;PR-required:pass | -| 2026-07-30 | cursor/process-anti-conflict-speed-1edf / PR #1416 | 2a31fcee0ef2e330a4901481c0810103d89c96cf | process anti-conflict merge readiness | NOT READY. CI green on stale head, but merge-tree vs current main is CONFLICTING in ci.yml, package.json, and add/add on check-outstanding-issues.mjs after #1410 landed a stronger #112 gate. Keep unique value: AGENTS anti-conflict procedure, #116 PR mergeability workflow, merge=union on outstanding-issues (explicitly still open after #1410). Drop duplicate weaker outstanding-issues checker; re-verify after sync. | ledger:lookup NOT REVIEWED; merge-tree dirty vs origin/main; ManagePullRequest CI SUCCESS (15 ok / 0 fail, Production UI skipped as non-UI); local verify:pr-local earlier on pre-conflict head 4467 passed | -| 2026-07-30 | cursor/process-anti-conflict-speed-1edf / PR #1416 | 387ffd07887f1160fca8fe98c1c4809e852531ae | process anti-conflict merge readiness | READY after sync with main. Kept #1410 structural outstanding-issues gate; added merge=union + runtime attr check; retained #116 PR mergeability workflow and AGENTS anti-conflict playbook; dropped duplicate weaker checker/test. merge-tree clean vs origin/main. | merge-tree clean; check:outstanding-issues pass; check:pr-mergeability pass; check:gate-manifest pass; verify:pr-local pass | -| 2026-07-30 | cursor/process-anti-conflict-speed-1edf / PR #1416 | 38ae07b989e6414026235debad0e429ba64cf462 | process anti-conflict merge readiness | READY at final tip. Same as prior READY plus this ledger append only; merge-tree still clean vs origin/main. | merge-tree clean; verify:pr-local on 387ffd07 parent (4480 passed); tip is ledger-only follow-up | -| 2026-07-30 | origin/main | 3569e7888bba5d11f143f27c11eb9bfa58800e4f | dependency installation and CI reproducibility | no P0-P2 findings; corrected stale setup-ui-e2e cache description | manifest-lock parity; Actions pins; merge-marker scan; merged PR 1360 diff | -| 2026-07-30 | codex/chat-dependency-pr-review-dependency-pr-review-20260730 | 2aee8c74e64bd09954fe1b474c73b25805063de0 | open PR changed-scope review | APPROVE: setup action description now matches clean npm install with npm-download caching; no behavior change. | check:github-actions PASS; diff review; no unresolved threads | -| 2026-07-30 | claude/top-search-design-mockups-w53znc | 939d5799b9999f3f63928e1b2c95d097f07eff90 | open PR changed-scope review | APPROVE: PR 1400 closeout and issue IDs 131-134 are unique, internally consistent, and preserve the append-only ledgers. | check:branch-review-ledger PASS; check:outstanding-issues PASS; diff review; no unresolved threads | | 2026-07-30 | cursor/safe-branch-cleanup-78a8 | c5190f38834ef2e928edc4876d971aa9d5d69fe7 | open PR changed-scope review | APPROVE after fixes: cleanup refs are discoverable, provider evidence is accurate, and issue 108 closure is preserved against current main. | check:branch-review-ledger PASS; check:outstanding-issues PASS; two review threads resolved | | 2026-07-30 | claude/outstanding-issues-triage-24c8ow | 8d2710fd6cbdc84e8c50a6c9bc0a1e1a0cd612c8 | open PR changed-scope review | APPROVE: completed items 095, 096, 104, 109, and 115 move to archive with no deletion, duplicate ID, or stale next-id. | check:outstanding-issues PASS; check:branch-review-ledger PASS; diff review; no unresolved threads | | 2026-07-30 | claude/latency-findings-impl-s8g01v | e7ff5e933ba1f34d5adbd46dd77c38aced11ed44 | open PR changed-scope review | APPROVE: ordering-risk documentation is accurate and the near-bottom refusal guard now proves its geometry is non-vacuous before asserting no hide. | diff check PASS; focused test review; no unresolved threads; exact-head Production UI required | +| 2026-07-30 | claude/ci-testing-review-2l8klp | 2e2160bc8b9d2d824209c217c67cb9cac1be3a8d | open PR changed-scope review | APPROVE: three-way UI sharding, critical-first gating, measured drag travel, and gate-manifest updates preserve required-check aggregation and deterministic Playwright settings. | check:github-actions PASS; check:ci-scope PASS; check:gate-manifest PASS; ledger guards PASS; exact-head sharded Production UI required | diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 5ddaa6d1a7..a8684d1ae3 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -82,7 +82,7 @@ removed after current-main verification; it is not missing recommended work. | 34 | `#105` | Optional | High — browser/UI verification | When the heavy-run lock is free | 20–40 minutes | Run `verify:ui` over the ten `LoadingPanel` fallbacks and confirm the Supabase `preconnect` reaches `` on a live page. Implementation already shipped; this row is the outstanding verification only. | | 35 | `#126` | Optional | Standard — repository hygiene | Once per UTC calendar quarter, or when the live ledger grows large | 5–15 minutes | Run `npm run ledger:rotate -- --dry-run`, then `npm run ledger:rotate` if the preview looks right; commit the live+archive diff. Stop if dry-run shows unexpected mass moves or archive path collisions. | - + ## Open items @@ -159,6 +159,11 @@ removed after current-main verification; it is not missing recommended work. | #132 | P3 | issue | Both client-side push guards are inert for agent pushes | **Outcome:** the format and auto-merge guards protect every push, or their blind spot is explicit. **Detail:** `scripts/guard-push.mjs` printed `auto-merge: gh not available — auto-merge check skipped (fail-open)` for pushes from a remote agent environment, so the auto-merge race sentinel never evaluated; and `core.hooksPath` is set only by a local `npm install`, so an agent pushing from its own checkout bypasses `.githooks/pre-push` entirely. Both guards therefore protect exactly the environment least likely to break the rule, which is why the AGENTS.md format-before-push instruction is still load-bearing even though the tooling now exists. Observed directly on PR #1400: a push landed while auto-merge was armed with nothing to stop it. **Next:** provide `gh` (or a token-based equivalent) in agent environments so the sentinel can evaluate. **Do not move the format check into `pull_request_target`** — that context carries secrets and a write token, and a format check must execute PR-head code including this repo's now-loadable dynamic `prettier.config.*`, which is the classic privileged-context vector; `.github/workflows/pr-policy.yml` deliberately checks out only `github.workflow_sha` for exactly this reason. Formatting is already enforced server-side by `Static PR checks` running `format:check` on ordinary `pull_request` CI, so the guard's only unique value is failing fast before the push — nothing to duplicate. The auto-merge sentinel reads PR metadata only and could safely live in `pull_request_target` if it is ever worth moving. | PR #1400; session 2026-07-30 | 2026-07-30 | | #133 | P3 | rec | `docs/outstanding-issues.md` conflicts on nearly every `main` advance | **Outcome:** two agents editing different rows of this ledger do not conflict. **Detail:** the table is padded to fixed column widths, so a single row's edit re-pads all 59 open rows and git sees the whole table as one changed hunk. On 2026-07-30 this file conflicted twice within an hour on PR #1400, and each conflict silently stopped **all** CI on that PR (`#116`) — so the cost is not the merge itself but the invisible loss of every check while it lasts. Both conflicts were mechanical: only two rows differed semantically out of 58. **Next:** stop padding this table (Prettier will still render it readably, and one-row edits become one-line diffs), or split the open items into per-row files. **Do not** apply a `merge=union` driver — tested 2026-07-30 and it is worse: union concatenates conflicting hunks, so two sides each bumping the marker produce two `next-id` lines, corrupting the file silently where a conflict would fail loudly. `#112`'s new `check:outstanding-issues` gate catches the corrupted result but does not prevent the conflict. | PR #1400; session 2026-07-30 | 2026-07-30 | | #134 | P2 | issue | Ledger union-merge driver is absent wherever `npm install` was skipped | **Outcome:** the ledger's union-merge protection is present wherever a merge happens, or its absence is loud. **Detail:** `.gitattributes` declares `docs/branch-review-ledger.md merge=ledger`, but the driver itself lives in git _config_, installed by `postinstall` -> `scripts/install-git-hooks.mjs`. A container that skips `npm install` (this repo's remote agent sessions do — the session hook reports "node_modules matches the lockfile, skipping install") therefore has the attribute without the driver, and git silently falls back to an ordinary merge. On 2026-07-30 a `git merge origin/main` on PR #1424 produced **conflict markers inside the append-only ledger** at three lines; `git merge` itself did not name the file, so only `npm run check:branch-review-ledger` caught it. Committing that would have corrupted the file the guard exists to protect. **Next:** make the absence loud — have `check:branch-review-ledger` (already in `verify:cheap` and `static-pr`) fail when `.gitattributes` declares `merge=ledger` but `git config merge.ledger.driver` is unset, so the environment is caught before a merge rather than after. `npm run hooks:install` is the one-line fix once detected. **Stop:** never trust a `merge=union`-style attribute to be active just because `.gitattributes` declares it; the driver is per-checkout config. | PR #1424; session 2026-07-30 | 2026-07-30 | +| #135 | P2 | rec | UI scope overfires: `ui_changed` matches every `src/app` path | **Outcome:** a change that cannot alter a rendered journey stops paying the longest job in CI. **Detail:** `uiPatterns` in `scripts/ci-change-scope.mjs` matches all of `src/app`, so an edit confined to `src/app/api/**` sets `ui_changed` and runs the full Chromium gate — measured 2026-07-30 at 15m26 of an 18m36 run, the entire critical path. **Next:** decide whether API-only diffs can be excluded. **Not done blind, deliberately:** the journeys exercise a production build that serves those routes, so a naive exclusion can hide a real regression; this needs a decision plus a compensating check, not a quieter filter. Sharding `ui-critical` (PR #1427) cut the cost of over-firing but did not remove it. | `scripts/ci-change-scope.mjs`; CI runs 30520443076 / 30519912667; session 2026-07-30 | 2026-07-30 | +| #136 | P2 | rec | Playwright's isolated production build cannot reuse Next's build cache | **Outcome:** the fixed ~2 min build stops being repaid on every UI job and every shard. **Detail:** `scripts/run-playwright.mjs` builds into `.next-playwright/${pid}-${Date.now()}/dist`, so Next's webpack filesystem cache (which lives under `distDir`) is cold every run — ~1m56 of the 15m26 `Chromium production journeys` step measured 2026-07-30, and a larger share of the sharded critical path. The `build` job already caches `.next/cache` correctly; this path simply cannot hit it. **Next:** allow the run root to be pinned via an env var (defaulting to today's behaviour) and cache it in `ui-critical`. **Blocker:** the runner removes the run root on every exit path, which `docs/testing.md` states as a contract, so caching needs an explicit keep flag and must not ship without executing the runner. | `scripts/run-playwright.mjs`; `docs/testing.md`; session 2026-07-30 | 2026-07-30 | +| #137 | P3 | rec | Advisory UI spends ~3 min per UI PR on five mockup tests | **Outcome:** the advisory lane costs what its signal is worth. **Detail:** `ui-advisory` runs on every UI PR (3m14 measured 2026-07-30) to cover `@quarantine` plus `@mockup` journeys — but `tests/flake-ledger.json` is empty and there are **zero** `@quarantine` tests in the suite, so it executes 5 `@mockup` tests. It is `continue-on-error` and outside `pr-required`, so it can also rot unnoticed. **Next:** gate it on mockup-file scope, or accept the cost as the price of keeping the lane warm for future quarantines. Either is defensible; the current state is just unmeasured. | `.github/workflows/ci.yml` `ui-advisory`; `tests/flake-ledger.json`; session 2026-07-30 | 2026-07-30 | +| #138 | P3 | task | CI Triage ships inert pending a repo variable | **Outcome:** a PR red that is really a main-side regression is labelled as such instead of costing an author a debugging session. **Detail:** `.github/workflows/ci-triage.yml` is complete and self-tested (`check:ci-triage` runs in `verify:cheap` and `static-pr`) but every run short-circuits on `vars.CI_TRIAGE_ENABLED == 'true'`, which is unset. Its purpose is the failure mode this doc records repeatedly: CI merges the PR branch with current `main`, so a main regression surfaces on every open PR. **Next:** operator sets the repository variable, then confirm one triage comment posts. Reads job metadata from a trusted default-branch checkout only; never runs PR code. | `.github/workflows/ci-triage.yml`; session 2026-07-30 | 2026-07-30 | +| #139 | P3 | rec | `changes` job computes outputs nothing consumes, and over-triggers coverage | **Outcome:** the change-scope contract says what it means. **Detail:** `changes` exports `source_changed`, `workflow_changed`, `changed_files` and `rag_eval_changed` and **no job reads any of them** — only `rag_eval_changed` is documented as intentionally advisory (`ci-change-scope.mjs`); the other three read as live wiring. Separately `coverage_changed` is derived as _any non-doc file_, so a workflow-only edit runs the ~4 min coverage job. **Next:** delete or document the dead outputs; decide whether coverage should narrow. Low value alone — bundle with the next `ci.yml` change rather than minting a PR. | `.github/workflows/ci.yml`; `scripts/ci-change-scope.mjs`; session 2026-07-30 | 2026-07-30 | ## Resolved / archive diff --git a/docs/process-hardening.md b/docs/process-hardening.md index 527b8f5180..5cf6e45c8d 100644 --- a/docs/process-hardening.md +++ b/docs/process-hardening.md @@ -97,6 +97,57 @@ artifact before release; see unchanged open-PR head. Only that sweep receives job-scoped `checks: write`; neither path checks out PR code or updates branches. Behind-but-clean heads remain an operator `sync:pr-branches` concern. Contract: `npm run check:pr-mergeability`. +- **Confirmed in the field the same day (PR #1427):** that PR pushed, opened, and was marked + ready for review while producing **zero** `pull_request` runs — no `CI`, no `Gitleaks`, no + `Semgrep` — with only `pull_request_target` (`PR policy`) firing. `git merge-tree` showed + real conflicts against a base 35 commits ahead. The new `PR mergeability` check caught it + and named the cause. Read a missing check list as a conflict signal, never as a pass. + +## CI shape and cost, measured per job (2026-07-30) + +The PR #1406 sample above counts runs. This is where the time inside one goes — job-level +timings from two full UI-scope PR runs (`30520443076`, `30519912667`), read from the Actions +API rather than estimated: + +| Job | Duration | On the critical path? | +| ----------------- | --------------- | ------------------------ | +| Change scope | 13 s | yes | +| Static PR checks | 3m03 | no | +| Safety and config | 47 s | no | +| Unit coverage | 3m57 | no | +| Advisory UI | 3m14 | no | +| **Production UI** | **15m26–16m31** | **yes — 83–89% of wall** | +| PR required | 5 s | yes | +| **Whole run** | **16.8–18.6 m** | | + +- **Every other job finishes by minute 4 and then waits ~12 minutes for Production UI.** + Inside it, Playwright reported `339 passed (13.5m)`; the remaining ~2 min is the isolated + production build (see outstanding-issues `#136`). Docs-only PRs run 3–5.5 min. +- Because the 42% cancellation rate is dominated by pushes that supersede a run mid-Production-UI, + almost all of the wasted runner time is this one job. Shortening it cuts churn cost directly. +- **`ui-critical` is therefore sharded across runners** (`ci.yml`), not parallelised within one: + `workers: 1` / `fullyParallel: false` / `retries: 0` are unchanged inside each shard, so + determinism is identical and per-runner load falls — which matters because `#093`'s duplicate + page root is load-dependent. +- **The shard count is measured, not chosen.** With `fullyParallel: false` a spec file is + indivisible, so shard sizes are lumpy. Against the 342 required chromium tests on the + post-merge tree, N=3 gives 121/111/110 and N=4 gives 121/106/98/17 — the same 121-test + bound for an extra runner. On the 340-test tree just before, N=5 and N=8 produced **empty** + shards, which would go red because `test:e2e:pr` deliberately omits `--pass-with-no-tests`. + Re-measure with + `npx playwright test --project=chromium --grep-invert "@quarantine|@mockup" --shard=i/N --list` + (needs a running server via `npm run ensure` and `PLAYWRIGHT_BASE_URL`) before changing N, + and keep every shard non-empty. +- **These timings predate `ui-critical-fast`.** The `@critical` fail-fast job (15 tests) now + runs before the full suite, so the UI critical path is that job plus the slowest shard. +- **Measured on the first real sharded run** (CI `30530618838`, 2026-07-30, all green): + `ui-critical-fast` 3m14, then shards of 9m36 / 6m54 / 6m20; whole run **13m39** against a + 16.8–18.6 min unsharded baseline. **The prediction from test counts was wrong by ~40%:** + 6.8 min was expected for the largest shard, 9m36 happened. Per-test cost is not uniform — + 111 tests took 6m54 while 121 took 9m36 — so a count-balanced split understates the slowest + shard whenever the slow specs land together. `--shard` can only balance by count; balancing + by duration would require splitting the slow spec files themselves. Prefer a measured run + over the arithmetic when judging any further shard change. ## Phase 1 - Active now diff --git a/scripts/check-gate-manifest.mjs b/scripts/check-gate-manifest.mjs index abbfb2a40e..42eb71a3c0 100644 --- a/scripts/check-gate-manifest.mjs +++ b/scripts/check-gate-manifest.mjs @@ -83,6 +83,47 @@ for (const gate of localGates) { } } +// Prose that states a gate COUNT drifts silently, because nothing derives it. Both +// numbers below were wrong when found: CLAUDE.md said 24 static gates against an actual +// 25 (`check:assets` landed before the line was written), and the gates skill said "check +// 2 of 26" against an actual 28. A stale count is not cosmetic — an agent that believes +// the chain is 26 long cannot tell how much of it a mid-chain failure skipped. These +// assertions fail closed: if the anchor phrase disappears, the guard reports the lost +// anchor rather than passing on a document it no longer checks. +const HEAVY_GATES = new Set(["lint", "typecheck", "test"]); +const staticGateCount = localGates.filter((gate) => !HEAVY_GATES.has(gate)).length; + +const documentedCounts = [ + { + file: "CLAUDE.md", + pattern: /(\d+) static\/consistency gates/, + expected: staticGateCount, + describes: "static/consistency gates in the verify:cheap chain (excludes lint/typecheck/test)", + }, + { + file: ".claude/skills/gates/SKILL.md", + pattern: /check \d+ of (\d+)/, + expected: localGates.length, + describes: "total gates in the verify:cheap chain (includes lint/typecheck/test)", + }, +]; + +for (const { file, pattern, expected, describes } of documentedCounts) { + const text = readFileSync(file, "utf8"); + const match = text.match(pattern); + if (!match) { + failures.push( + `${file} no longer matches ${pattern} — the gate-count guard lost its anchor. Restore the phrasing or update the pattern in scripts/check-gate-manifest.mjs.`, + ); + continue; + } + if (Number(match[1]) !== expected) { + failures.push( + `${file} says ${match[1]} where the chain has ${expected} (${describes}). Update the document, or the chain, so they agree.`, + ); + } +} + if (failures.length > 0) { console.error("Gate-manifest drift — a local verify:cheap gate is not enforced in CI:"); for (const failure of failures) console.error(`- ${failure}`); @@ -90,5 +131,6 @@ if (failures.length > 0) { } console.log( - `Gate-manifest OK: all ${localGates.length} verify:cheap gates are enforced in CI (static-pr + mapped jobs).`, + `Gate-manifest OK: all ${localGates.length} verify:cheap gates are enforced in CI (static-pr + mapped jobs), ` + + `and the ${staticGateCount} static gates are documented consistently.`, ); diff --git a/tests/ui-phone-scroll.spec.ts b/tests/ui-phone-scroll.spec.ts index b9eb28fe0a..9455fe69e5 100644 --- a/tests/ui-phone-scroll.spec.ts +++ b/tests/ui-phone-scroll.spec.ts @@ -196,7 +196,14 @@ async function addPhoneScrollRunway(page: Page) { filler.style.pointerEvents = "none"; main.append(filler); }); - await page.waitForTimeout(50); + // Wait for the runway to exist rather than for 50ms to pass. The sleep was a bet + // that layout lands within 50ms of the append; on a loaded CI runner it does not + // have to, and the caller then drags against a scroll range that is still short. + await expect + .poll(async () => (await readGeometry(page)).maxOffset, { + message: "the appended 1600px runway must reach layout before the caller scrolls against it", + }) + .toBeGreaterThanOrEqual(minimumHideTravelPx); } interface ScrollGeometry { @@ -267,29 +274,97 @@ function readFlipCount(page: Page): Promise { * Drags whichever phone scroller is active in deliberate steps (one per * frame) so the scroll state machine sees real directional intent. Browser * tabs move the document; installed/bounded layouts can still move main. + * + * Returns the distance actually travelled. `scrollTop +=` clamps silently at + * either end of the range, so the requested distance is a ceiling, not a + * promise — callers that need the drag to cross a threshold must check it + * (see `dragScrollUntilHidden`). */ -async function dragScrollBy(page: Page, totalPx: number, stepPx: number) { - await page.evaluate( +async function dragScrollBy(page: Page, totalPx: number, stepPx: number): Promise { + return page.evaluate( async ({ total, step }) => { - const main = document.getElementById("main-content"); - if (!main) return; - const mainOverflowY = getComputedStyle(main).overflowY; - const mainOwnsScroll = - /^(?:auto|scroll|overlay)$/.test(mainOverflowY) && main.scrollHeight > main.clientHeight + 1; - const documentScroller = document.scrollingElement ?? document.documentElement; - const scrollOwner = mainOwnsScroll ? main : documentScroller; + // Re-resolved every step: releasing the chrome changes the runway mid-drag + // and can hand ownership between main and the document. Resolving once up + // front, as this helper used to, keeps pushing an element that has stopped + // scrolling and reports nothing unusual when it does. + const resolveOwner = () => { + const main = document.getElementById("main-content"); + const mainOverflowY = main ? getComputedStyle(main).overflowY : ""; + const mainOwnsScroll = Boolean( + main && /^(?:auto|scroll|overlay)$/.test(mainOverflowY) && main.scrollHeight > main.clientHeight + 1, + ); + const documentScroller = document.scrollingElement ?? document.documentElement; + return { + element: mainOwnsScroll && main ? main : documentScroller, + eventTarget: mainOwnsScroll && main ? (main as EventTarget) : (window as EventTarget), + }; + }; + if (!document.getElementById("main-content")) return 0; const steps = Math.max(1, Math.ceil(Math.abs(total) / step)); const direction = total < 0 ? -1 : 1; + let travelled = 0; for (let i = 0; i < steps; i += 1) { - scrollOwner.scrollTop += direction * step; - (mainOwnsScroll ? main : window).dispatchEvent(new Event("scroll", { bubbles: true })); + const { element, eventTarget } = resolveOwner(); + const before = element.scrollTop; + element.scrollTop += direction * step; + travelled += element.scrollTop - before; + eventTarget.dispatchEvent(new Event("scroll", { bubbles: true })); await new Promise((resolve) => requestAnimationFrame(() => resolve(undefined))); } + return travelled; }, { total: totalPx, step: stepPx }, ); } +/** + * Minimum downward travel that can legitimately hide the phone chrome. The + * document-detail contract below asserts `scrollTop > 120` in the hidden state, + * so a drag that delivers less than this cannot hide anything and the chrome is + * right to stay visible. + */ +const minimumHideTravelPx = 160; + +/** + * Drags down to cross the scroll-hide threshold, and proves the page had the + * runway and that the gesture actually travelled. + * + * This is a DIAGNOSTIC AND A GUARD, NOT A FIX for the `#127` flake. That issue — + * "document-detail hide sticks visible under CI load" — carries trace evidence + * from PR #1404 (run `30521269873`) that on its occurrence the drag delivered in + * full (`documentElement.scrollTop` = 1272, exactly the 552 + 720 asked for) with + * ~1300 px of runway to spare, and that a 10 s non-flip is a latched state rather + * than a race. So under-delivery was NOT the cause there, and this helper would + * have passed its checks and failed on the same assertion. + * + * What it does buy: `scrollTop +=` clamps silently, and the old helper returned + * nothing, so "the drag was short" could never be ruled out from a CI log alone. + * Now it is ruled out by construction — a future failure here proves the gesture + * landed, which narrows `#127` to its remaining candidates (`scrollHidden` false + * vs `sharedChromePinned` latched). Separating THOSE two still needs the pin + * state exposed in the DOM; today only the composite `data-scroll-hidden` + * (`scrollHidden && !sharedChromePinned`) is observable, so both look identical. + * + * The call-site assertions are untouched: a genuinely stuck header fails exactly + * as before, once the drag is proven to have happened. + */ +async function dragScrollUntilHidden(page: Page, totalPx: number, stepPx: number) { + await expect + .poll( + async () => { + const geometry = await readGeometry(page); + return geometry.maxOffset - geometry.scrollTop; + }, + { message: "the page must have enough remaining downward runway to cross the scroll-hide threshold" }, + ) + .toBeGreaterThanOrEqual(minimumHideTravelPx); + const travelled = await dragScrollBy(page, totalPx, stepPx); + expect( + travelled, + `a ${totalPx}px drag delivered only ${travelled}px, so the chrome was never asked to hide`, + ).toBeGreaterThanOrEqual(minimumHideTravelPx); +} + interface PageOwnedFooterGeometry { footerOpacity: number; footerPosition: string; @@ -482,7 +557,7 @@ for (const phoneOwner of ["browser document", "standalone PWA main"] as const) { ); await sectionTrigger.evaluate((element) => element.blur()); - await dragScrollBy(page, 720, 24); + await dragScrollUntilHidden(page, 720, 24); await expect(collapse).toHaveAttribute("data-scroll-hidden", "true"); await expect(overlayStack).toHaveAttribute("data-scroll-hidden", "true"); await expect(composer).toHaveAttribute("data-scroll-hidden", "true"); @@ -582,7 +657,7 @@ for (const phoneOwner of ["browser document", "standalone PWA main"] as const) { await page.emulateMedia({ reducedMotion: "reduce" }); await dragScrollBy(page, -480, 16); await expect(collapse).not.toHaveAttribute("data-scroll-hidden", "true"); - await dragScrollBy(page, 720, 24); + await dragScrollUntilHidden(page, 720, 24); await expect(collapse).toHaveAttribute("data-scroll-hidden", "true"); const reducedHidden = await readPrimaryScrollAndDomGeometry(page, { stack: '.phone-sticky-header-stack[data-phone-motion="overlay"]', @@ -647,7 +722,7 @@ test("compiled standalone PWA rules bind full-height footer chrome to the inner expect(initial.footerBackdropPosition, "the PWA footer scrim must share its shell-owned edge").toBe("absolute"); expect(initial.documentRunway, "the PWA document must stay bounded while main owns scrolling").toBeLessThanOrEqual(1); - await dragScrollBy(page, 720, 24); + await dragScrollUntilHidden(page, 720, 24); await expect(page.getByTestId("universal-header-collapse")).toHaveAttribute("data-scroll-hidden", "true"); await expect(page.locator("form.answer-footer-search-dock")).toHaveAttribute("data-scroll-hidden", "true"); const hidden = await page.evaluate(() => ({ @@ -1170,7 +1245,7 @@ test("calculator dock clears its focus pin after a focused submit opens and clos await expect(input).not.toBeFocused(); await addPhoneScrollRunway(page); - await dragScrollBy(page, 900, 24); + await dragScrollUntilHidden(page, 900, 24); await expect(dock).toHaveAttribute("data-scroll-hidden", "true"); });