diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 036c7dc9b0..fce97b69f0 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -127,6 +127,7 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-30 | claude/test-coverage-analysis-2vcd8a | 4f498b66a56b2a7eddde6c841a79621f23b59cc7 | PR #1398 babysit | BLOCKER CLEARED: CONFLICTING due to docs/outstanding-issues.md vs main (#115 band-adoption follow-up). Kept main #115 + next-id=116; preserved PR #109 single-branch/refspec update. Prior tip had no GitHub CI suite (only PR Policy/CircleCI) — push retriggered full CI. 0 review threads; 0 Bugbot findings. | verify:cheap PASS (432 files, 4467 passed \| 4 skipped); repo-hygiene 38/38; sweep:branch-ledger --no-fetch exit 0; format:changed PASS; Bugbot none; hosted CI re-triggered on tip | | 2026-07-30 | PR #1394 / `claude/top-search-design-mockups-w53znc` | `0d47141fc030684299dcb265e3d853c93b9e2a91` | CI/review closeout — merged | MERGED as squash `0d47141f`. Prior tip `4a001efa` had required CI green after prettier fix `61314887` (Static PR/CircleCI red on `#096` padding) and main sync. Layout/`/tools` false-positive fixed; `#115` deferred; review threads resolved. Post-merge ledger-only follow-up. | hosted Static/Unit/PR-required/CircleCI pass on pre-merge tip; vitest adoption 6/6; typecheck; Bugbot no open P0/P1; merge-tree clean | | 2026-07-30 | cursor/pr-1394-ledger-closeout-c2bf | f734dc4d4c8b19d5fec43bbd388c2a421e47668a | PR #1399 babysit / CI+Bugbot closeout | MERGE-READY. No failing CI, no unresolved review threads, merge-tree clean vs origin/main, Bugbot no bugs. Docs-only ledger append for merged #1394; no code fix required. | hosted PR required SUCCESS; Static PR SUCCESS (lint/typecheck/format/ledger); CircleCI verify SUCCESS; local check:branch-review-ledger PASS; prettier PASS; lint PASS; Bugbot pr-bugbot no findings | +| 2026-07-30 | claude/design-computed-style-proof | 228ffc8583dc0579cfba033bcbe0e7ef18be0317 | `#094` computed-style proof: unlayered-cascade check in check-design-system-contract.mjs plus ui-smoke computed-style assertions for the accent rail, forced-colors thickness and 44px tap targets; records ledger `#123` worktree-deregistration git redirection and `#124` issues:next-id collisions | PR `#1415` opened; auto-merge off | verify:pr-local 432/432 files 4460 tests; new Playwright case 1 passed; both gates negative-tested red on the injected `@layer` regression then reverted clean | | 2026-07-30 | PR #1396 / claude/latency-findings-impl-s8g01v | 6f75bba54684c104a9bd70b36c401f04ca4c57b5 | Babysit: sync main after Claude pre-paint fix | Synced origin/main (ledger-only #1399). MERGEABLE; merge-tree clean. Claude tip added pre-hydration overlay reserve fix. No unresolved threads. Bugbot still empty on prior tips. Contract 28/28. | header-scroll-hide-contract 28/28; merge-tree clean; prior verify:cheap/typecheck/lint retained | | 2026-07-30 | claude/test-coverage-analysis-2vcd8a | d5842e62238237ff5c47da0b32ef8d9f12819714 | PR #1398 babysit | COMPLETE for tip: cleared main conflict; fixed Codex P2 (reject refs/*→origin/* nesting); prior Codex P2 (destination check) already fixed in de594186 and resolved; 0 unresolved threads; merge-tree clean. Hosted CI re-running. | repo-hygiene 40/40; verify:cheap earlier PASS on pre-tip; format:changed PASS; Bugbot none; Codex P2 resolved | | 2026-07-30 | PR #1396 / claude/latency-findings-impl-s8g01v | 9d03b84f1a32a056f74727b4e6bdd5558c346bf0 | Babysit: resolve outstanding-issues after #1402/#1398 | FIXED CONFLICTING: took main open-table (widened cols + #109 refspec) and kept #116/#117 phone-chrome gaps (next-id=118). MERGEABLE expected. Codex P1s already fixed on tip and threads resolved. No Bugbot findings. | merge-tree clean; contract 28/28 earlier; verify:cheap on prior tip | @@ -144,6 +145,7 @@ 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 | 1397 | 82c834c8aa3223df3235fbc61d783deaf97f138c | pr-diff-review | reviewed clean as ledger-only; no tests/packages added — recommendations deferred by design; PR already merged | gh pr view + local diff: only docs/branch-review-ledger.md +1; no tests/ or package.json; no provider checks | | 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 | diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 32e6ca535a..11c4f8716e 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -153,6 +153,8 @@ removed after current-main verification; it is not missing recommended work. | #120 | P2 | issue | `verify:phone-chrome` exits 0 while reporting failed browser tests | **Outcome:** the phone-chrome gate cannot report success when no test executed. **Evidence 2026-07-30 (PR #1396):** `npm run verify:phone-chrome` finished with **exit code 0** while its own output ended `13 failed`. Every one of the 13 failed at browser launch (`browserType.launch: Executable doesn't exist ... chrome-headless-shell`), so zero assertions ran, yet the gate returned success. This is the green-when-broken case `AGENTS.md` warns about ("Exit code 0 alone is not proof") realised in a gate that is supposed to be the proof. **Next:** make the runner propagate the Playwright exit status, and fail loudly on a launch error rather than treating a zero-test run as a pass. Stop: do not paper over it by grepping output in the caller — the runner owns the status. | `scripts/verify-phone-chrome.mjs`; `scripts/run-playwright.mjs` | 2026-07-30 | | #121 | P3 | issue | Container Playwright browser build lags the pinned client | **Outcome:** browser gates run in remote sessions without hand-patching. **Evidence 2026-07-30:** the repo's Playwright client resolves headless-shell build `1234`; the container image provides `1194` at `/opt/pw-browsers`, so every browser test fails at launch. Worked around in-session by symlinking `chromium_headless_shell-1234/chrome-headless-shell-linux64/chrome-headless-shell` to the `1194` `headless_shell` binary plus its sibling resources — container-local, nothing committed, and it disappears with the session. `PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1` means the mismatch cannot self-heal. **Next:** decide whether the image pins the browser build or the repo pins a client matching the image; until then any remote session claiming browser proof must state which it used. | `docs/testing.md`; container `/opt/pw-browsers` | 2026-07-30 | | #122 | P2 | issue | `ci/circleci: verify` fails on every branch and its log needs operator access | **Outcome:** the CircleCI status is trustworthy signal again, or it stops reporting. **Evidence 2026-07-30:** `ci/circleci: verify` was `failure` on every open PR sampled — #1396, #1407, #1405, and #1400, which is a **docs-only** `AGENTS.md` change — plus #1403's head. It is sharply bounded in time: #1393's head **passed** at build 638 (03:57), and builds 645 (04:09) onward all failed. The job's entire contents were mirrored locally on PR #1396's exact tip and every part is green — `format:check` clean, `lint` exit 0, `typecheck` exit 0, `npm run test` `432 passed (432)` / `4473 passed \| 4 skipped`, and the PyMuPDF-gated `tests/pdf-extractor.test.ts` (the repo's only `process.env.CI`-gated tests) `6 passed (6)` under a locally built `PyMuPDF==1.28.0` venv with `PYTHON_BIN` set exactly as `.circleci/config.yml` does. So the failure is in the job's **environment**, not repo code. Around 40 builds fired in ~40 minutes across 8 open PRs in that window, so credit/quota exhaustion is the leading hypothesis — **explicitly unverified**: the CircleCI project is private and no CircleCI token is available to any agent session, and `api/v1.1/project/gh/BigSimmo/Database/` returns `Build not found` unauthenticated. **Next:** an operator opens one failing build and reads the failing step; if it is quota, either raise it or remove the CircleCI status so it stops masking real reds. **Stop:** do not chase this from a PR branch — it is not branch-specific, and no agent can read the log. Do not go looking for a CircleCI token. | `.circleci/config.yml`; PR #1396 session 2026-07-30 | 2026-07-30 | +| #123 | P2 | issue | A deregistered worktree silently redirects git at the shared primary checkout | **Outcome:** losing a worktree cannot put destructive commands on someone else's tree. **Detail:** on 2026-07-30 an in-flight session's worktree (`railway-token-secrets-setup-638ad6`) left `git worktree list` while its directory still existed but held no checkout. Every subsequent `git` call from that cwd resolved _upward_ to `C:/Dev/Apps/Database`, which was on another session's branch with 11 modified and 2 untracked files. A `git reset --hard origin/main` was issued from there; the primary reflog shows it did not land, so nothing was lost, but the same sequence would have destroyed that work. `git status` gave the only hint — paths printed as `../../../src/...`. **Next:** before any mutating git command, assert the cwd is still a registered worktree (compare `git rev-parse --show-toplevel` against the expected path, or check `git worktree list`), and treat a `../../` prefix in `git status` as fail-closed. The #077 primary-checkout write lease guards deliberate concurrent writes, not this accidental redirection. **Stop:** never run `reset --hard`, `clean -fd`, or a branch switch without that assertion. | session 2026-07-30; primary reflog vs worktree list | 2026-07-30 | +| #124 | P3 | issue | `issues:next-id` collides when agents work the same hour | **Outcome:** two sessions cannot mint the same ledger ID. **Detail:** a single task allocated `#096`/`#097`, found both taken by a Cursor Agent, moved to `#098`/`#099`, found those taken too, then `#108`/`#109`, and a later reconciliation renumbered them again to `#110`/`#111` — three collisions in ~24 hours. It resolved correctly each time only because the file carries a `union` merge driver and a human/agent fixed the numbering by hand, i.e. manual repair is absorbing a structural race. **Next:** either derive the ID from something non-colliding (date + short SHA, or the PR number) or have the append helper claim the marker and fail loudly on mismatch instead of trusting a read. **Stop:** do not reuse or renumber an ID that is already published in a commit message, PR body, or another row's cross-reference. | session 2026-07-29/30 across PRs #1375 and #1391 | 2026-07-30 | | #125 | P3 | issue | `ui-therapy-nav-scroll.spec.ts` cites a spec file that does not exist | **Outcome:** a reader following the comment finds the coverage it names, or the comment stops naming it. **Detail:** the spec's comment points at `mode-nav-bar-anchoring.spec.ts`, left behind when PR #1390 moved `ModeNav` into the universal header; no such file exists anywhere in the repo. Harmless at runtime, but it sends the next person looking for anchoring coverage to a file that is not there, and it is the kind of stale pointer that makes a reader distrust the surrounding comments. Either repoint it at the coverage that actually exists (`ui-mode-nav-density.spec.ts`, landed in #1405) or delete the reference. | Noticed and explicitly deferred in PR #1405's body to keep that diff scoped to `#113` | 2026-07-30 | | #126 | P3 | task | Quarterly branch-review ledger rotation reminder | **Outcome:** live ledger stays navigable after #1418 L4 bootstrap. **Next:** each UTC calendar-quarter start (or when the live table feels unwieldy), run `npm run ledger:rotate -- --dry-run`, then `npm run ledger:rotate` and commit live+archive. Lookup/sweep/check already read archives. **Stop:** do not hand-move rows; do not delete unique review content. | session 2026-07-30; follow-up to #1418 / L4 | 2026-07-30 | diff --git a/scripts/check-design-system-contract.mjs b/scripts/check-design-system-contract.mjs index 3910ddae52..dd58f9cf4a 100644 --- a/scripts/check-design-system-contract.mjs +++ b/scripts/check-design-system-contract.mjs @@ -22,6 +22,22 @@ const LITERAL_SHADOW_CLASS = /shadow-\[(?!var\()[^\]]+\]/g; const CUSTOM_CONTROL_CLASS_PROP = /(?:closeButtonClassName|sheetCloseButtonClassName|buttonClassName|triggerClassName)\s*=\s*(?:"([^"]*)"|`([^`]*)`)/g; +/** + * Component classes whose declared effect must beat a Tailwind utility on the + * same element, and which therefore MUST stay outside `@layer components`. + * + * Tailwind's utilities layer outranks `@layer components` regardless of selector + * specificity. `.search-band` sets `border-top: 2px solid var(--clinical-accent)` + * while the same element also carries `border-[color:var(--border)]`: inside a + * layer the utility wins and the accent rail silently renders as a 1px neutral + * border. That shipped in PR #1316 and a `toHaveClass("search-band")` assertion + * passed the whole time, because class presence is not effect. + * + * Add a selector here when losing the cascade would make it inert, not merely + * restyled. + */ +const UNLAYERED_EFFECT_SELECTORS = [".search-band"]; + const toPosix = (value) => value.split(path.sep).join("/"); function isPrototype(relativePath) { @@ -199,6 +215,49 @@ assert( const globals = textAt("src/app/globals.css"); assert(!/^\s*--space-\d+\s*:/m.test(globals), "unused --space-* tokens returned"); + +/** + * Every `@layer`/`@media`/`@supports` block enclosing `index`, outermost first. + * + * All ancestors matter, not just the innermost: `@layer components { @media (…) { + * .search-band { … } } }` still loses to Tailwind's utilities layer, so returning + * only the nearest at-rule (`@media`) would let that nesting pass. + */ +function enclosingAtRules(source, index) { + const stack = []; + for (let cursor = 0; cursor < index; cursor += 1) { + const character = source[cursor]; + if (character === "{") { + const head = source.slice(Math.max(0, source.lastIndexOf("}", cursor - 1) + 1), cursor); + const atRule = /@(layer|media|supports)[^{}]*$/.exec(head); + stack.push(atRule ? atRule[0].trim() : null); + continue; + } + if (character === "}") stack.pop(); + } + return stack.filter(Boolean); +} + +for (const selector of UNLAYERED_EFFECT_SELECTORS) { + // Only the base rule matters. The forced-colors overrides for the same + // selector are deliberately inside `@media` and must stay there. + // Allow leading indentation: a nested rule is still *found*, so the failure + // below reports the real problem (wrong layer) instead of "missing", and a + // purely cosmetic re-indent cannot masquerade as a deleted rule. + const pattern = new RegExp(`^[ \\t]*${selector.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\s*\\{`, "gm"); + const matches = [...globals.matchAll(pattern)]; + assert(matches.length > 0, `${selector} base rule is missing from globals.css`); + for (const match of matches) { + const enclosing = enclosingAtRules(globals, match.index); + const layers = enclosing.filter((atRule) => atRule.startsWith("`@layer`")); + assert( + layers.length === 0, + `${selector} must stay UNLAYERED — it is inside "${layers.join(" > ")}" (full ancestry: ` + + `${enclosing.join(" > ") || "top level"}), where Tailwind's utilities layer outranks it ` + + `regardless of specificity and its declared effect becomes inert`, + ); + } +} const primitives = textAt("src/components/ui-primitives.tsx"); assert( primitives.includes('export const chatComposerInput = "chat-composer-input"'), diff --git a/tests/ui-smoke.spec.ts b/tests/ui-smoke.spec.ts index aedb59cea6..6abfd769e5 100644 --- a/tests/ui-smoke.spec.ts +++ b/tests/ui-smoke.spec.ts @@ -3426,6 +3426,133 @@ test.describe("Clinical KB UI smoke coverage", () => { await expect(documentResults).toContainText("Best match"); }); + // Computed-style proof, not class presence. `.search-band` declares + // `border-top: 2px solid var(--clinical-accent)` while the same element also + // carries the `border-[color:var(--border)]` utility. Tailwind's utilities + // layer outranks `@layer components` regardless of specificity, so if the rule + // is ever moved into a layer the accent rail renders as a 1px neutral border — + // which shipped in PR #1316 while `toHaveClass("search-band")` passed + // throughout. Only the rendered value can catch that. + test("the search band's accent rail and forced-colors thickness survive the cascade", async ({ page }) => { + await page.setViewportSize({ width: 1280, height: 900 }); + await mockDemoApi(page); + await gotoApp(page, "/"); + await waitForDemoDashboardReady(page); + + await gotoApp(page, "/documents/search?mode=documents"); + // Not fillVisibleQuestionInput: that helper gates on the answer composer's + // "Generate source-backed answer" submit, which does not exist on this route + // (the documents submit is "Find matching documents"), so its precondition + // can never be satisfied here. Same hydration-safe shape, correct target. + const bandQuery = visibleQuestionInput(page); + await expect(bandQuery).toBeVisible(); + const documentsSubmit = page.getByRole("button", { name: "Find matching documents" }); + await expect(async () => { + await waitForReactEventHandler(bandQuery, "onChange"); + await bandQuery.fill("lithium monitoring"); + await expect(bandQuery).toHaveValue("lithium monitoring"); + await expect(documentsSubmit).toBeEnabled({ timeout: 2_000 }); + }).toPass({ timeout: 30_000 }); + await submitDocumentSearch(page); + + const band = page.getByTestId("search-query-ribbon").first(); + await expect(band).toBeVisible(); + + const rail = await band.evaluate((element) => { + const style = getComputedStyle(element); + // Resolve the token through a probe: getPropertyValue returns the + // *specified* value (`var(--primary-500)`), not a comparable colour. + const probe = document.createElement("span"); + probe.style.color = "var(--clinical-accent)"; + element.appendChild(probe); + const accent = getComputedStyle(probe).color; + probe.remove(); + return { + topWidth: style.borderTopWidth, + topStyle: style.borderTopStyle, + topColor: style.borderTopColor, + sideWidth: style.borderRightWidth, + sideColor: style.borderRightColor, + accent, + }; + }); + + expect(rail.topWidth, "the accent rail must render 2px, not the 1px utility border").toBe("2px"); + expect(rail.topStyle).toBe("solid"); + expect(rail.topColor, "the rail must paint --clinical-accent, not the neutral --border").toBe(rail.accent); + // The precise #1316 symptom: the rail collapsing into the other three edges. + expect(rail.topColor, "the rail is indistinguishable from the side borders").not.toBe(rail.sideColor); + expect(rail.topWidth).not.toBe(rail.sideWidth); + + // Under forced colors the rail cannot survive as hue — --clinical-accent + // resolves to LinkText and --border-strong to CanvasText — so it survives as + // thickness instead. That distinction is the clinical signal, and it was + // previously only asserted by text-searching globals.css. + await page.emulateMedia({ forcedColors: "active" }); + await expect + .poll(async () => band.evaluate((element) => getComputedStyle(element).borderTopWidth), { timeout: 5_000 }) + .toBe("3px"); + await page.emulateMedia({ forcedColors: null }); + + // Tap targets are a rendered contract too: --spacing-tap is 44px and + // min-h-tap must actually produce it. + // Select on the COMPUTED value, not the class name: a class-substring match + // also catches breakpoint variants that are inert at this width, and the + // point of this gate is the rendered result. `display: inline` is excluded + // because CSS genuinely does not apply min-height to inline boxes — such an + // element is reported separately rather than silently counted as a pass. + const tapAudit = await page.evaluate(() => { + const describe = (element: Element) => + `${element.tagName.toLowerCase()}.${(element.className || "").toString().split(/\s+/).slice(0, 3).join(".")}`; + // Derive the floor from the token rather than hard-coding 44: --spacing-tap + // is the source of truth, and the documented 48px sheet exception must also + // qualify. Anything at or above the token is held to its OWN declared + // min-height, so this stays correct if the token or a control moves. + const tapToken = getComputedStyle(document.documentElement).getPropertyValue("--spacing-tap").trim(); + const probe = document.createElement("div"); + probe.style.height = tapToken || "2.75rem"; + document.body.appendChild(probe); + const tapFloor = probe.getBoundingClientRect().height; + probe.remove(); + + const inlineCarriers: string[] = []; + const undersized: string[] = []; + let measuredCount = 0; + for (const element of document.querySelectorAll("*")) { + const style = getComputedStyle(element); + const declared = Number.parseFloat(style.minHeight); + if (!Number.isFinite(declared) || declared < tapFloor - 0.5) continue; + const rect = element.getBoundingClientRect(); + if (rect.width <= 0 || rect.height <= 0) continue; + measuredCount += 1; + // Validate EVERY match; only the diagnostic list is capped. + if (style.display === "inline") { + if (inlineCarriers.length < 10) inlineCarriers.push(`${describe(element)} (min-height ${style.minHeight})`); + continue; + } + if (rect.height < declared - 0.5) { + if (undersized.length < 10) { + undersized.push( + `${describe(element)} declared ${style.minHeight}, rendered ${Math.round(rect.height * 10) / 10}px`, + ); + } + } + } + return { tapFloor, measuredCount, inlineCarriers, undersized }; + }); + + expect(tapAudit.tapFloor, "--spacing-tap must resolve to a real pixel floor").toBeGreaterThanOrEqual(44); + expect( + tapAudit.measuredCount, + "expected at least one control declaring a tap-sized min-height (a vacuous pass otherwise)", + ).toBeGreaterThan(0); + expect( + tapAudit.inlineCarriers, + "a tap-sized min-height sits on an inline box, where CSS ignores it outright", + ).toEqual([]); + expect(tapAudit.undersized, "controls rendered below their own declared min-height").toEqual([]); + }); + test("dashboard defers source and administration requests until their surfaces open @critical", async ({ page }) => { await page.setViewportSize({ width: 1280, height: 900 }); await mockDemoApi(page);