golf/rounds: preview harness (draft, stacked on #1837) - #1846
golf/rounds: preview harness (draft, stacked on #1837)#1846njrini99-code wants to merge 23 commits into
Conversation
…lip is not an outage Three fixes from the 72h incident export. Only the first is user-facing; the other two are the alerting lying about the other 24 incidents. 1. QUALIFIER LEADERBOARD WENT STALE ON A REALTIME RECONNECT (user-facing). `use-qualifier-realtime.ts` passed no `onStatus`, so it never refetched when the socket came back. postgres_changes delivers nothing that happened while the transport was down and resumes from "now", so a dropped socket — mobile handover, a backgrounded tab, a Realtime blip — froze the leaderboard on pre-drop scores for the rest of the session. A coach reads that surface WHILE the round is played, and a stale leaderboard is indistinguishable from a correct one where nobody has posted. This is the same defect #1822 locked down for the calendar; the qualifier hook never got the same treatment. Verified the new test fails against the unfixed hook before wiring it. 2. ~95 FALSE "Cron failure" OUTAGE EVENTS IN 19 HOURS, from a job that never failed once. `Sentry.captureCheckIn` only BUFFERS; on Vercel the invocation returns immediately afterwards and the instance freezes with the envelope unsent. The `in_progress` check-in survives that because the job's own work runs behind it; the TERMINAL check-in is by construction the last thing emitted, so nothing carries it. Measured on api-cron-db-health-sampler (the only cron frequent enough to make it visible, every 5 min): ok=1-4 vs timeout=8-11 per hour, error=0, missed=0, traces finishing under a second. `finishCronCheckIn` now flushes — via a new `flushTelemetryNow`, NOT `scheduleTelemetryFlush`, because that one drops a request while a flush is in flight and on the failure path `recordJobRun` has just awaited a logServerEvent write. A flush already draining cannot carry an envelope not yet buffered. 3. CHANNEL_ERROR REPORTED AS AN ERROR-LEVEL ISSUE THE INSTANT IT HAPPENED, asserting an impact `observeRealtimeChannel` cannot observe (#1824's rule). realtime-js reconnects on its own and the call sites recover — the calendar refetches on re-SUBSCRIBED, and after (1) so does the qualifier. The capture is now deferred 30s and cancelled if the channel returns to SUBSCRIBED (recovered) or CLOSED (torn down). A feed still down after that window is a real outage and still pages. The RATE signal is untouched: metric, breadcrumb and warn log still fire per occurrence, so a spike of self-healing blips stays visible in Bridge. Verified: npm run typecheck clean; eslint --max-warnings 0 clean on all nine files; 1153 tests pass across src/lib/observability, src/hooks/golf/__tests__ and src/test/observability. NOT verified: (2) and (3) are delivery/severity behaviour that only production exercises — the monitor's timeout:ok ratio and the RJ issue going quiet are the real confirmations, and both need a promote. Also triaged, no code needed: incidents 10-12/24-25 (roster "not-a-real-uuid-12345") were fixed by 44f4ce1 35 minutes after they fired, and incident 15 (helm_debug_list_agent_runs) by a61161d's ignoreErrors entry — both events predate the deploy now serving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
Incidents 7-9 and 22 of the 72h export: `savePartialRound` answered "Player profile not found", and the client retried it at 5s, 15s and 30s, then opened a circuit breaker that probes every 60 seconds for the rest of the round. Every attempt re-failed identically and wrote another `severity:error` server event plus another `'high'` client one, so one condition the player could not possibly fix became an open-ended stream of incidents from a single session. The ladder in `use-shot-state-machine.ts` is failure-BLIND — it retries any rejected save. That is exactly right for an outage and exactly wrong for a refusal whose cause cannot change while the player keeps playing. The codebase already knew this: `hole_invalid` was excluded from the breaker on precisely this reasoning (B5, "would keep retrying a failure retrying can never clear"). The auth and player-profile refusals belong in the same set and were simply never added. So: `isUnrecoverableRoundWriteFailure` in round-missing-recovery.ts, the module that already owns round-write failure vocabulary, and both round screens branch on it where they previously hard-coded `hole_invalid`. CODES, NOT SENTENCES. `error` carries player-facing prose that can be reworded at any time, and a client matching on "Player profile not found" breaks silently the day someone improves the wording. `ActionResult` has always had an optional `code`, so the two refusals now carry `auth_required` / `player_missing` and the classifier reads those. `hole_invalid` still matches via `error` too, because that one has always travelled as a bare key there. Default stays RETRYABLE: a transient failure missing from the set is retried as it should be. Mis-classifying a network blip as terminal would abandon a round that the next tick would have saved — a far worse trade than one extra retry, so the set is closed and explicit. What the player is told changed too. "Player profile not found" is accurate and useless to someone standing on a fairway: it does not answer the only question they have, which is whether the last two hours are gone. Both sentences now lead with the shots being safe on the device, and the pair where saving has actually STOPPED (as opposed to hole_invalid, which is fixable in place) interrupts once with a toast — `showAutoSaveWarning`'s "sync may be delayed" would have been untrue. Once per round, via a ref, because the auto-save effect re-runs on every shot entered afterwards. Recovery is deliberately preserved for the auth case: saving is not disabled, so a player who signs back in has their next auto-save succeed. What is gone is the machinery that retried on its own forever. Verified: npm run typecheck clean; eslint --max-warnings 0 clean on all nine files (exit code checked directly, not through a pipe — the first run DID surface two real react-hooks/exhaustive-deps warnings for the new showToast dependency, now fixed); 2367 tests pass across src/lib/golf, both rounds screens, src/app/golf/actions/__tests__, src/hooks/golf and src/test/golf. Three existing assertions were updated for the added `code`, none weakened. NOT verified: no production evidence yet that the incident stream stops — that needs a promote, which is the owner's call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
…ork kept rewriting Audit pass over the always-loaded rules. Each correction below was verified by running the thing the prose describes, not by reading it. 1. autonomy.md claimed `permissions.deny` "covers the Supabase CLI migration path AND the account-wide Supabase MCP mutations". The CLI half is solid. The MCP half is not: `docs/CONTROL_PLANE_ENFORCEMENT.md` — which is GENERATED from live config — records that the display-name spelling `mcp__claude_ai_Supabase__*` "match[es] nothing the session can call today", and that the UUID spelling is CONFIGURED but NOT observed to remove the tools, with id stability UNVERIFIED under the registered gap MCP_DENY_RULES_KEYED_ON_ROTATABLE_CONNECTOR_IDS. Reconfirmed from a live session inventory: no `mcp__claude_ai_*` name exists. This is the third time this exact paragraph has carried a false safety claim, and the placement is what makes it serious: it is the paragraph that tells an agent it is safe to act without asking. Rules keyed on a name nothing exposes are not enforcement, and are now described as what they are. 2. tsconfig.json said `.next/types/**` and `.next/dev/types/**` were "deliberately gone (2026-08-26)". They were not gone. `next build` and `next dev` rewrite this file and add both globs back on every run, so the comment described a state that ended at the next build — and left the canonical checkout permanently dirty in a tracked config file nobody had edited. Removing them was unwinnable. The original reason has also expired. On Next 16.3.4 the offending `.next/types/validator.ts` is no longer generated; the globs now pull in exactly `routes.d.ts` and `root-params.d.ts`. Measured on this tree: `npm run typecheck` with both globs and a populated `.next/` is exit 0, 0 errors, 2 generated files in the program. CI is unaffected — it runs on a fresh checkout where the globs match nothing. So they stay, and the comment now records why rather than asserting a state. 3. AGENTS.md gains the `nextjs-agent-rules` block, for the same reason: `next dev` writes it (node_modules/next/dist/server/lib/generate-agent-files.js) and re-adds it on removal. Its claim was checked — node_modules/next/dist/docs exists. Committing it is what keeps the tree clean; deleting it is a loop. Verified: npm run typecheck exit 0; npm run docs:check exit 0 (all five gates, 0 new drift, every path resolves, enforcement inventory and tool-authority matrix both match live configuration). Not fixed here, and still failing: `npm run control-plane:verify` exits 2 on `mutation-budget 25/1`. That needs worktrees released, which AGENTS.md makes a human act. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
… one laundered into a green check
`worktree-lifecycle.mjs` is supposed to refuse to present a report as a
finding when every PR lookup failed — AGENTS.md states that contract, and it
exists because a blackout is indistinguishable from a genuinely clean
repository. It did not hold. Two independent bypasses, both reproduced.
1. THE DENOMINATOR COUNTED ROWS THAT NEVER LOOKED ANYTHING UP. A worktree
with no branch (detached HEAD) is given a SYNTHETIC `{ lookup: 'OK' }` at
the row-building site. The guard reconstructed "did we try?" from
`row.prLookup` afterwards, so those synthetic successes landed in the
denominator and `failed === attempted` was false. Two detached-HEAD
worktrees out of seventy-four were enough. Measured in the canonical
checkout with every `gh` call failing: exit 0, 43 failed lookups, and a
summary reading `0 branches deletable` — precisely the outcome the guard
is written to make impossible.
The synthetic 'OK' cannot just be renamed: both classifiers in
`scripts/lib/worktree-lifecycle.mjs` branch on `prLookup !== 'OK'`, so any
other value silently converts those rows to UNKNOWN. Counting at the call
site inside `prFor()` keeps every verdict byte-identical and cannot drift
from the row shape.
2. `--json` NEVER REACHED THE GUARD AT ALL. It printed and `process.exit(0)`
before it. `control-plane-verify.mjs` is a --json consumer, so a total
blackout surfaced there as `PASS unclassified-branches ... 0
NO_UPSTREAM_UNIQUE_WORK` — a green control-plane check standing on
evidence that did not exist. The guard is now computed before the JSON
exit and carried by the exit code; the array shape is unchanged so
existing consumers keep parsing, and the consumer short-circuits to
UNKNOWN on status 2 rather than reading the rows.
VERIFIED BY REPRODUCTION, both directions:
in-sandbox (gh cannot reach GitHub): --json exit 0 -> 2, human exit 0 -> 2
and INFRASTRUCTURE_FAILURE now prints
outside the sandbox (gh works): --json exit 0, 74 rows, lifecycle checks
still PASS; 75/75 tests in src/test/scripts/worktree-lifecycle.test.ts pass
What the fix immediately exposed, on the same repo minutes apart: the
blacked-out run reported `0 NO_UPSTREAM_UNIQUE_WORK` and UNKNOWN for
open-pr-residue; the real-evidence run reports 10 branches whose commits
exist nowhere else, and open-pr-residue as a FAIL with 12 undispositioned
open PRs. Every control-plane run made from a sandboxed shell before this
commit was reading the rosier picture.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
REPO_MAP.md is the file AGENTS.md tells you to read before adding a route, an
action wrapper, a toast call or a design-token consumer — "so you don't have
to re-derive these conventions by grepping from scratch". Fifteen of its
anchors had drifted, and every one of them still resolved to SOMETHING, which
is the worst version of this bug: an agent following the cite does not get an
error, it gets plausible unrelated code and copies the wrong idiom.
withBaseballAction with-baseball-action.ts:248 -> :339
(:248 is a line inside an unrelated option type;
the wrapper is 91 lines further down)
toast.error sites crm/page.tsx:839,843,886,985,996
-> :586,921,972,1062,1067
(all five cited lines were comments or an effect
guard; not one was a toast call)
tabs[0]! trap nav-registry.ts:322,328,335,341,348,389,427,430
-> :358,364,371,377,384,425,476,479 (8/8 wrong)
goals[0]! trap progress-drivers.ts:145 -> :161
(:145 was a comment terminator)
`strokes-gained.ts:122` was checked and is exact — left alone.
Each replacement was verified by re-reading the new line and confirming the
named symbol is on it: `:339` is the `export function withBaseballAction`
signature, the five crm lines are four `toast.error` and one `toast.loading`,
all eight nav-registry lines contain `tabs[0]!`, and `:161` contains
`goals[0]!`.
The doc's own banner already says to treat anchors as hints rather than
facts. That is honest but does not help — a self-labelled hint still gets
followed, and nothing checks these. `docs:path-drift` validates that a PATH
resolves, never that a LINE still holds its symbol, which is why all fifteen
sat here silently.
Verified: npm run docs:check exit 0 (all five gates).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
…ives a thumb, a menu that closes, a message you can copy #1830 replaced the floating kebab with press-and-hold and made `conversation-kind.ts` the one group derivation. Both were right; both were left one step short, and the gaps land on the affordance that PR made load-bearing. **Own messages became uncopyable on a desktop.** `select-none` went onto own bubbles unscoped, and the menu carrying Copy is `lg:hidden` while the hover row that survives at `lg` has only Edit and Delete. So from `lg` up the PR removed text selection at exactly the widths where nothing gave it back — the PR's own reasoning for the opt-out ("so iOS cannot race its menu against ours") never reached that far. Scoped to `max-lg:`, which is precisely where Copy exists. The callout suppression stays unscoped; it is inert off iOS Safari. **The press cancelled on the tremor of holding still.** `onPointerMove: cancelLongPress` fired on sub-pixel jitter, and a resting thumb is never perfectly still. Since the kebab is gone this gesture is the ONLY route to Copy, Edit and Delete on a phone, so an unreliable press is an unreachable menu. 10px of slop: above finger jitter, far below what a real scroll covers in 450ms. `exceedsLongPressSlop` is exported because the threshold IS the behaviour and the values either side of it are what a test can pin — synthesising pointer coordinates through the rendered component would be testing jsdom. **The menu had one way out, and it was the X.** Long-press opens it without moving the page, so it floated on beside a bubble that had scrolled away. It now closes on the next press anywhere else and when the thread scrolls under it. Its own listener, not a branch inside the stick-to-bottom handler: that one is about scroll POSITION and is torn down per conversation; this one is about a menu being open. **State outlived the thread it belonged to.** Edit mode, its draft, the delete prompt and the open menu all survived a conversation switch, so coming back re-entered a bubble mid-edit holding text from before and re-asked a delete the user had walked away from. Nothing could be MISDELIVERED — Save and Confirm only render inside the matching bubble — but this is the hazard the keyed composer is documented against, for the four pieces of state that were not keyed with it. **One consumer never got the one derivation.** `FairwayMessages` still asked raw `is_group` before fetching group participants, so a flagged two-person DM fired three extra queries to build a map the pane then never reads — its `isGroup` is the participant-count derivation, so it resolves the sender from `other_participant`. The rail (144) and the pane (812) were converted; this was missed. Line 227 is deliberately NOT converted: that is find-or-create, not presentation, and changing it would make a flagged DM newly eligible to match. Also: an empty thread showed nothing while the other person typed — the indicator lived inside the has-messages branch — and "say hello below" is the wrong thing to say to someone watching a reply being written. And each conversation now gets its own one-shot attachment-retry budget; the bounding set was never cleared, so an attachment that lost the commit-order race once stayed on its retry chip for the rest of the session, and leaving the thread and coming back could not earn it another attempt. Verified: typecheck, `lint --max-warnings 0`, `npm test` (1563 files, 15229 passed / 10 skipped) and `next build` all green, and `max-lg:select-none` is present in the emitted CSS bundle — a variant that silently failed to compile would have looked identical in the diff. NOT device-verified, and the dev server could not show it either: the page redirects to login and this session has no credentials, so the browser evidence here is that the route boots and builds, not that the gesture feels right on glass. Long-press timing and the 10px slop are exactly the class of thing that reasons correctly and can still be wrong under a thumb. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019euu9jtJM6WvMVj17coqZ1
… against the machinery Second pass from the parallel audit. Every claim below was verified by reading the workflow, script or source it describes — not by reading prose about it. WOULD HAVE CAUSED A WRONG ACTION - shipping.md said `new-worktree.sh` gives "isolated dependencies". It does not install dependencies at all; the script itself prints `not installed — run: node scripts/ensure-worktree-deps.mjs <dir>`, and AGENTS.md — which outranks this file — says so outright. An agent that made a worktree on this wording and ran `npm test` got a bare checkout. - quality-gates.md said `playwright.yml` runs `smoke` on PRs. That file has no `smoke` job and no `pull_request` trigger: `on: workflow_dispatch:` is its only trigger, and its own header records the 2026-09-02 deletion. PR e2e is a different workflow, `pr-smoke.yml`. Fixed in the bullet AND in the table row above it, which carried the same claim. - quality-gates.md said "Nothing references `node --test` — not one npm script, not one workflow". package.json has eight, and four run in GitHub Actions. Worse, it is a live counterexample to the bullet's own conclusion: `flags:check` names `scripts/__tests__/check-feature-flags.test.mjs` on a `node --test` line, so a file there executes in CI without appearing in `vitest.config.ts`. Rewritten as two mechanisms, not one. - REPO_MAP.md cited the aggregate check as `Review Gate / all`. That is the retired name `code-review-tooling.md` exists to warn about — it "posts NOTHING… the phantom-check trap that made every PR unsatisfiable". The constitution warned about it in one file and handed it to you in another. - REPO_MAP.md described `withAdminObserved`'s logging as "fire-and-forget via resolveObservedUser()". The source says the opposite in its own header: "It is no longer fire-and-forget: the Bridge write is SCHEDULED past the response (scheduleBridgeWrite)". An agent trusting the doc would remove or bypass that scheduling. Its three anchors were also wrong (:56 -> :73, comment 21-25 -> 27-38, guard :28 -> :39). STALE / ABSENT - code-review-tooling.md omitted the CircleCI `android` workflow entirely (three workflows exist: weekly, ios, android), omitted `promptfoo-evals` from the weekly list (seven jobs scheduled, six named), and omitted the `env-secrets` check from the analyzer list — which is a real gate carrying an `id`, so the aggregate reads its outcome. AGENTS.md documents all three and names THIS file as the authority to keep in step. - quality-gates.md routed the reader to ci.yml's `control-plane` and `feature-knowledge` jobs. Neither exists. ci.yml's jobs are detect-changes, static-checks, typecheck, lint, unit-tests, unit-tests-timezone, next-build, supabase, baseball-auth-smoke, all — the `docs:check` members are steps of `Static checks`. - REPO_MAP.md documented `helm-website-ui/` as a second Next.js app. It was deleted in 761bea0 (#810). Also removed `/soreness-preview` (no such route; only a `lifting/actions/soreness.ts` server action), fixed the `admin-nav.test.ts` path to `__tests__/`, and repointed two dead cross-references into CLAUDE.md — it has neither a "context routing table" nor a "color-family list". REPORTED, NOT FIXED — an owner action, not an agent one shipping.md asserted that project scope owns `autoMemoryEnabled` and the value is false. Measured: `.claude/settings.json` = false, `~/.claude/settings.json` = TRUE. The invariant is violated right now, and which scope wins for this key is UNVERIFIED — the deny-over-allow precedence proven in §4 is the permissions resolver and does not generalise. The rule now records the measured state and says clearing the user-scope key is the owner's call. USER-SCOPE SETTINGS WERE NOT EDITED: they affect every project and any concurrent session. Verified: npm run docs:check exit 0 (all five gates). Each corrected file:line re-read to confirm the named symbol is on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
The positive assertion already pins the fix: if `max-lg:` is ever dropped from the own-bubble class, `toContain` fails. The negative one restated that by string prefix and would additionally fail on a reformat of the `cn()` call — reporting the absence of a string rather than the loss of desktop copyability, which is a tripwire pointing at the wrong thing. (Landed separately because an `--amend` raced another session's commit onto this shared checkout and folded the change into `docs(rules): eleven claims…`. That commit is restored verbatim as this one's parent; nothing of it was rewritten.) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019euu9jtJM6WvMVj17coqZ1
…rrent session's file MessageThreadPane.longPress.test.ts belongs to the messages work committed in b8e82f3 by a concurrent session. My previous commit removed one line from it: - expect(source).not.toContain("isOwn && 'select-none"); I never opened that file. The cause is the shared-index hazard AGENTS.md and autonomy.md both warn about, in a form neither of them spells out. Both say to use `git add <explicit paths>` and never `git add -A`. I did exactly that, and it was not enough: `git commit` writes the ENTIRE INDEX, not the paths you just added. The other session had a stale copy of this file staged in the index we share, so it rode along and its newer line read as a deletion. `git add <paths>` constrains what you STAGE. Only `git commit --only -- <paths>` constrains what you COMMIT. In a shared checkout the second is the one that matters, and the rules currently only teach the first. Restored verbatim from b8e82f3; `git diff b8e82f3 HEAD -- <path>` is empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
… a concurrent session's file" This reverts 768447a. The assertion I "restored" was deleted ON PURPOSE by the session that owns this file, in ef9bb47, with a better reason than my restoration had: The positive assertion already pins the fix: if `max-lg:` is ever dropped from the own-bubble class, `toContain` fails. The negative one restated that by string prefix and would additionally fail on a reformat of the `cn()` call — reporting the absence of a string rather than the loss of desktop copyability, which is a tripwire pointing at the wrong thing. I read a one-line deletion in a file I had not opened, concluded it was collateral damage from a shared index, and put the line back. It was not damage. Their own commit message also corrects my diagnosis: the line moved into my commit because THEIR `--amend` raced my commit onto this shared checkout, not because my `git add` over-scoped. Both of my conclusions were reasonable from what I could see and both were wrong, which is the actual lesson: in a shared checkout, a diff you did not author is not evidence of a mistake, and the owner of the file is the only one who can say. Ask before restoring. File restored to ef9bb47 verbatim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
…taler than the doc
Third and final pass from the parallel audit. Verified against the machinery,
not against prose about it.
CLAUDE.md listed `src/lib/golf/surface-registry.ts` among "generated artifacts
[that] outrank prose", in a paragraph ending "never hand-edit". Nothing
generates it: `grep -rn surface-registry scripts/ package.json` is empty, there
is no npm script, and the file carries no stamp — its only "generated" mentions
are about generated nav UI. So the canonical registry of every golf surface
name and href was marked off-limits to the agents who need to add to it, and
hand-editing is the ONLY way it can ever change.
This is `shipping.md` §1 inverted. That rule warns a regeneration stamp is not
evidence of correctness and says to verify the generator. Here the generator was
asserted without one existing at all. `database.ts` genuinely is generated and
now says so, with its regen command and its `db:types:check` guard named.
CLAUDE.md also described `docs:check` as "regen + both drift gates". It is five
gates, and it is NON-mutating — the `docs:regen && git diff` shape it names was
deliberately replaced, so the comment told an agent the command would rewrite
files when it will not.
REPO_MAP.md's staleness banner is the best joke in the repo: it warned that
counts rot, using a count that had rotted. It claimed 192 commits to `src/**`
since the verify point. The audit measured 408; I measured 412 twenty minutes
later. It now carries the command instead of a number, and says why.
Also: `createAdminClient()` cited `admin.ts:4`, an import line — the function
is at `:24`. And code-review-tooling.md omitted `agent/fix-circleci-ios-*` from
the CircleCI iOS branch filter, which is precisely the opt-in an agent fixing
iOS CI needs; its `verified:` stamp also predated content dated twelve days
after it.
NOT APPLIED, deliberately. The audit measured eleven more wrong counts in
REPO_MAP's idiom table (98 -> 148 sites, 200 -> 421, 31 -> 9, and so on) and
found the Helm Bridge route atlas documents 22 of 37 routes. Those need the
table re-derived rather than eleven numbers bumped to values that will be wrong
again next month; the ordering claims they carry ("most-used of the three")
still verify. Two anchor corrections were also dropped because I could not
confirm them independently.
Verified: npm run docs:check exit 0; npm run repo:doctor PASS.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
…e path that has two doors now Last pass from the parallel audit. code-review-tooling.md listed the per-PR fast path as five things. ci.yml runs eight jobs plus a path-detect gate and an aggregate. One of the three it omitted — `Static checks` — is named correctly two paragraphs earlier in the same file, so the file disagreed with itself. Now quotes the display names verbatim, which is what a required-context list is matched against. Its `.gitleaks.toml` note said the rotated dev password is allowlisted "only in audit docs". The `[allowlist]` `paths` are global across every rule and include a source file — the Supabase error-envelope privacy test, which needs a real-shaped JWT to prove redaction works. The old wording implied a source file could never be allowlisted, which would make a legitimate entry look like a mistake to remove. autonomy.md's frontmatter said "working-style guidance, not code claims — nothing here to grep". The file names three scripts, three npm scripts, a .gitignore line number, a git config key, settings.json hook shape, and a commit SHA; three drift scanners already read it (check-doc-path-drift, check-doc-schema-drift, control-plane-verify). That stamp discouraged exactly the audit that found this pass's Supabase MCP overstatement. And its worktree section said there is "exactly one supported way" to make a worktree. There are two doors now: the harness offers `isolation: "worktree"` and `EnterWorktree`, which give you a checkout and none of the script's four guarantees — no `--no-track` (so the `agent/foo -> origin/main` trap is live again), no `.helm/workspace.json` (so the lifecycle tool returns KEEP_WORKSPACE_INTENT_REQUIRED), no budget accounting. The section also never mentioned that `new-worktree.sh` can REFUSE — one mutation workspace at a time plus a disk reserve — which is why dispatching three parallel agents with a worktree each fails on the second, by design rather than by fault. Verified: npm run docs:check exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
…hat rot
THE ATLAS. `src/app/admin/**` was documented as "22 `page.tsx`, flat — no
route groups" with 22 routes enumerated. There are 37, and fifteen were
undocumented: /billing, /database, /engineering, the whole /lenses/ tree
(6 leaves), /lifting, /qualifiers, /releases, /reliability, /self-heal, /slo,
/teams index, /thread/[entity]/[id], /traces, /utilization, /work-log.
"Flat" was the more misleading half. It is still literally true of route
GROUPS, but `lenses/` and `thread/[entity]/[id]` are nested trees, and
`lenses/` is where per-product lenses now go — an agent placing a new Bridge
panel would not have learned that from this doc. Re-derived in full, heading
corrected, and the enumeration now ends with the command that measures it.
THE COUNTS. Every usage figure in the idiom tables was wrong:
withBaseballAction 98 -> 148 createAdminClient 200 -> 421
withLiftingAction 23 -> 30 fetchAllRows* 69 -> 123
withAdminObserved 129 -> 177 direct sonner 31 -> 9
Rather than replace six wrong numbers with six that will be wrong again, the
numbers are gone. `shipping.md` §1 forbids a count in prose for exactly this
reason ("counts rot within weeks and a stale number reads as current
forever"), and REPO_MAP was the largest violator in the repo. A note above the
table records what they were, why they went, and the one-line grep that
measures any of them. The ordering claims those cells carried — "most-used of
the three", "most-used data-access primitive" — were re-checked and still
hold, so they stay.
The sonner figure is worth keeping visible: it is the only count that FELL,
31 -> 9. The doc was overstating a legacy pattern that is being retired, which
would have pushed an agent to treat a nearly-finished migration as a live
mess.
DISPOSITIONS. `config/open-pr-dispositions.json` carried rows for #1725 and
#1738, both merged, both with their transitional grace ENDED — the
control-plane verifier names them and says to delete them in the PR you are
already opening. Done; that half of the `open-pr-residue` failure is now gone
from its output.
Still failing, and not mine to invent: 12 OPEN PRs have no recorded
disposition. Each needs a `worktree_policy` decision (KEEP or
PARK_IF_REPRODUCIBLE) from whoever owns it.
Verified: npm run docs:check exit 0; open-pr-residue no longer reports stale
rows; control-plane-verify run with real GitHub evidence, not from inside the
sandbox.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
… confirm first time Completes the anchor pass. Each replacement was verified by reading the new line and confirming the named symbol sits on it: --fw-dur-*/--fw-ease- block globals.css 1767-1845 -> 1933-2011 notice-error-ink join UI baseball/join/[code]/page.tsx 170-174 -> 212-216 BASEBALL_NAV_REGISTRY baseball/nav-registry.ts 330 -> 348 GOLF_COACH_HUBS golf/nav-registry.ts 201 -> 225 GOLF_PLAYER_HUBS golf/nav-registry.ts 231 -> 255 buildCoach/PlayerRail+Bottom golf/nav-registry.ts 304-460 -> 340-487 WCAG P422 darkened token design-tokens.css 87 -> 142 Two of these I reported as unconfirmable in the previous pass and left alone. That was my error, not the audit's: I had truncated a grep at `head -2` and concluded `BASEBALL_NAV_REGISTRY` was not at :348 when it is, and I checked the wrong end of the join-page range. Re-checked properly here. Declining to act on unverified evidence was right; the verification itself was sloppy. The old golf-builder range is the sharpest of the seven: `304-460` EXCLUDED `buildPlayerBottomNavItems`, which is at :487 and is one of the four functions the cell names — the range pointed away from a symbol in its own sentence. Also dropped two more counts, `(1366 lines)` and `(579 lines)`. The first was exactly right and the second was wrong by six, which is the argument against both: a reader cannot tell which kind they are looking at, and neither earns its place next to a path that already resolves. Verified: npm run docs:check exit 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
…re, and two greens that verified nothing
**`team_communications` required a baseball spec for a golf feature.**
`required_checks` named `npm run test:e2e -- e2e/messages.spec.ts`. Every one
of that spec's nine `page.goto` calls targets `/baseball/dashboard/messages`,
and this feature's `routes:` are `src/app/golf/(dashboard)/dashboard/messages`.
So the one required check for the feature could not execute a line of it, and
running it to green read as coverage. Re-pointed at
`e2e/golf-critical-paths.spec.ts`, which actually loads
`/golf/dashboard/messages`. The baseball spec stays under `code.tests` —
annotated for what it is — because it is still the messaging spec; it is just
not evidence about this feature.
**And that spec self-skips on credentials Playwright itself injected.**
`hasGolfCoachAuth` is a module-level read of `GOLFHELM_COACH_EMAIL` /
`GOLFHELM_COACH_PASSWORD`. Both are in `.env.local`, and Playwright prints
`injected env (80) from .env.local` at startup — yet the constant is false,
because that injection does not reach the spec module's top-level read. Same
command, one change, measured today:
npx playwright test ... -g "messages loads a conversation" 1 skipped, exit 0
set -a; . ./.env.local; set +a; <same command> 1 passed, exit 0
Both green; the first verified nothing. That is `quality-gates.md` §2's
category exactly — a gate that cannot fail — so it is recorded there, and the
export is written into the registry entry beside the check so the next reader
does not have to rediscover it. With the credentials exported, all 11 golf
critical-path tests pass against a real browser.
**`npm run dev` inside the Bash sandbox fails while reporting success.**
An EMFILE watcher flood (with `ulimit -n` at 1048576, so it is the sandbox's
limit, not the shell's) and a `.next/dev was deleted → Restarting` loop. The
log prints `✓ Ready in 133ms`; `curl` gets `Empty reply from server` and then
refuses to connect. The obvious diagnosis — a second dev server fighting over
`.next/` — was wrong: `lsof -a -p <pid> -d cwd` put the other two `next`
processes in a worktree with their own `.next`. Outside the sandbox it is ready
in 158ms and answers 200. Recorded in `shipping.md` §3 with the instruction the
episode actually earns: curl the server before reporting it as running.
Verified: `docs:check` (all five gates), `check-registry-globs` (0 dead of 635).
`markdown:ratchet` fails +4 on this branch, and it fails identically with these
three files stashed — the drift is pre-existing from concurrent work here, not
from this change. Baseline deliberately NOT raised.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019euu9jtJM6WvMVj17coqZ1
… the bubbles were under their own floor Captured the real logged-in surface at 390px and 1440px rather than reasoning about it, and five things were wrong that reading the code did not reveal. **The thread header was invisible, and it was not missing — it was occluded.** Measured on a 390px viewport, `section[aria-label="Conversation"] > header` renders at top 51 with height 75, carrying Back and the other person's name. The sticky hub sub-nav paints over it, so all that reached the screen was a ~10px sliver: no name, no way back. #1830 hid the shell top bar and the tab bar so this header could be THE header, and then left the one bar that covers it. It also explains a 39px document overflow — `docScrollHeight` 883 against an 844 viewport — because `FairwayMessages`'s `mobileShowChat` branch already sizes itself with no term for that bar. Hiding it under `data-fw-immersive` makes arithmetic that was already written correct, and it is the same argument the rule makes for the other two: a switcher between sibling destinations does not belong pinned over a surface that has claimed the screen. **Every group's last message sat off the axis of its own siblings.** The timestamp was a flex sibling of the bubble, so it shortened the line it was on: three bubbles ran flush to the edge and the fourth — always the one that shows a time — was pushed inboard by the width of "Read 4:06 PM". Moved into the bubble column, which already carries items-end/items-start. Structural, not a spacing value. **The bubbles were below their own floor.** The well was `surface` (L 0.984) and incoming bubbles were `surface-sunken` (L 0.963) — 0.021 apart, when design-tokens.css records 0.978-against-0.984 as "near-identical" and set canvas 0.03 under surface precisely so "cards now clearly LIFT off it". The tier was inverted: messages sank into their container. The well is now `canvas` and both bubbles lift off it, which is the pairing the token file was tuned around. **"Read" three times in one screen.** The receipt is a fact about the conversation — how far the other person has got — not about each utterance. Once, against the newest thing you sent. **Day separators drew the same two rules as the New marker,** whose own comment says it rules itself because "this line means something the day separators do not". Only hue carried that. The day label is now quiet and centred, so the claim is true and the New marker is the one ruled thing in the thread. Message text goes from `text-body-sm` to `text-body`: caption treatment made the content of the product read as metadata about itself. Verified: typecheck, lint --max-warnings 0, npm test (1563 files, 15229 passed / 10 skipped), and re-captured against the live surface after each change. The dark circle over the composer in those captures is `NEXTJS-PORTAL`, the dev indicator — confirmed by `elementFromPoint`, not a product element. Recording it because it reads exactly like an orphaned nav avatar and I misread it as one. Still open, both visible in the captures and neither touched here: the DM resolves to "Unknown User" when the other participant is in neither golf_coaches nor golf_players (use-golf-messages.ts:1049 leaves `otherParticipant` undefined), and on desktop both panels stop short of the viewport with dead canvas beneath. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019euu9jtJM6WvMVj17coqZ1
…end the autoMemory split-brain Owner-authorised cleanup. 26 worktrees -> 7; control-plane 19 pass/2 fail -> 20 pass/1 fail. WORKTREES. Eighteen were parked and one detached checkout removed. Every branch was KEPT — parking removes the disposable checkout, never the commits. The tool refused half of them at first with `UNKNOWN_REMOTE — no upstream, commits here may exist nowhere else`, which was correct and worth obeying: sixteen branches held work that existed ONLY on this disk. Four of them carried genuinely unique commits (7, 10, 10 and 1 — 28 in total reachable from no other ref at all). So they were pushed FIRST, then parked. That inverts the risk: the commits are now on the remote instead of one laptop, and the checkouts became reproducible, which is what `PARK_IF_REPRODUCIBLE` actually means. Pushing cost nothing — `ci.yml` triggers on `push` only for `main`, so ten agent branches landed with zero workflow runs. Pushing also let the classifier PROVE what it previously could not. Four branches resolved to `DELETE_MERGED_EXACT` (PR MERGED and tip === PR head OID, never ancestry, because this repo squash-merges) and were deleted, along with three merged remote branches that `delete_branch_on_merge` had left behind. Two detached checkouts were removed by hand after checking each: `deploy-main` was an ancestor of origin/main, and `review-1792`'s HEAD is reachable from agent/supabase-observability-p2. Neither held anything. The six that remain are the six that should: five have uncommitted work and one is a live session's checkout with its cwd inside it. `mutation-budget` therefore still fails at 6/1, and that failure is now HONEST rather than an artefact of never cleaning up. The budget is not being raised to make it green. OPEN PRs. `open-pr-residue` now PASSES. Twelve open PRs had no recorded disposition; all twelve have one. Ten are `KEEP` — the conservative value, never parked automatically — with the reason stated per row (active worktree, or no local checkout, or dependabot). Two (#1831, #1832) are `PARK_IF_REPRODUCIBLE` because their checkouts were clean and pushed with the tip matching the PR head; parking those removed the CHECKOUT only and left both PRs and branches untouched. AUTOMEMORY. `~/.claude/settings.json` set `autoMemoryEnabled: true` while `.claude/settings.json` set it to `false`, and nothing on disk settles which wins — the deny-over-allow precedence proven for permissions does not generalise to arbitrary keys. The user-scope key was REMOVED rather than set to `false`, so project scope governs and there is no second value to reconcile. Exactly one line changed; backup at `~/.claude/settings.json.bak-2026-09-04`. shipping.md §1b now records the resolution and says to remove the key again if it reappears. Verified: npm run docs:check exit 0; control-plane-verify run with real GitHub evidence (not from inside the sandbox, where every PR lookup fails). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
…oes not carry Seven-artboard design canvas plus the constraints an implementer needs and cannot get from pictures. Pins the two things review caught the design getting wrong, so the build does not inherit them: the Fairway card recipe (rounded-card is 20px, and elevation is border+shadow-card OR borderless shadow-soft — never both, which surface.tsx calls the cheap-UI tell; nested rows are Insets, not cards inside cards), and the interaction model (selecting avatars overlays PLAYER SCHEDULES for the 'find common free time' job named at FairwayCalendar.tsx:300 and audit P237 — not 'who is free for this event', which does not exist). Names the load-bearing constraint: PLAYER_COLORS has 8 entries and rosters run 10-15, so the view switches encoding at the cap rather than degrading — per player colour under 8, counts and ranked free windows for the whole roster. Names what must be BUILT: the common-free-time intersection. getPlayerAvailability returns busy intervals; nothing computes free windows from them today. Specified as a pure tested function in src/lib/golf/, not in a component and not in SQL. Five slices, one PR each, with acceptance gates and the realtime/loading/ hydration traps that would otherwise be rediscovered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
The inventory was one line stale (a doc added on this branch was not counted), which turns knowledge:check red locally and the Static checks job red on the PR. Regenerated with npm run knowledge:doc-inventory. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Four rounds of mobile mockups for the course picker, new-round and
shot-tracking screens were rejected, and the diagnosis was exact: "it's
not even my components." They were not. They were hand-written
lookalikes built from token values and doc headers, because every one of
these screens sits behind auth AND behind a multi-step state machine, so
there was no way to see the shipped component without playing a round.
This removes the approximation step.
`/fairway-preview/rounds?screen=...` mounts the REAL components at phone
width with fixture props — FairwayShotTracking (live state machine),
FairwayNewRoundEntry, CourseCard shelves and FairwayTeeCard. It is a
sibling of the existing `/fairway-preview`, which does the same job for
the Wave-1 primitives: not linked in nav, imports no route module, every
callback a no-op.
`scripts/design/` turns any dev route into a readable PNG through the
app's own compiled CSS. The README records why each step exists, because
each one was a wrong conclusion before it was a fix — most of them
producing something that looked like a design defect and was not:
- CSS `zoom` does not change media-query evaluation, so a 390px body
was still matching `lg:` and painting the DESKTOP two-column layout.
- Stripping scripts leaves framer-motion's `opacity:0` initial style
un-animated, so the page renders blank.
- React serialises the attribute as `srcSet`; a case-sensitive strip
left it in place, WebKit preferred an unreachable candidate from it
over the inlined src, and every course photo became a broken-image
glyph on an image-forward surface.
- Turbopack dev generates a route's CSS chunk on demand, so the first
response for a new route can link a sheet missing that route's own
classes — measured, `w-[390px]` in the markup and absent from the
CSS. The snapshotter now warms the route and reports any class that
resolves to no rule at all, which is the silent-zero this repo's
quality-gates rule exists to catch.
Verified: `tsc --noEmit` clean, `eslint --max-warnings 0` clean on the
new files, and all five screens rendered and read. `.claude/settings.json`
carries an unrelated concurrent edit and is deliberately not in this
commit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
…bels mid-word
Both of these were found by rendering the shipped components at 390px
through `scripts/design/`, not by reading them. Measured, not estimated:
the sticky element is painted a flat colour and its height read off the
render.
**Round chrome: 405 -> 332 CSS px, 48% -> 39% of a 390x844 screen.**
nav row 86 (unchanged)
hole strip 172 -> 100
shot track 142 (unchanged)
Each phone column of the scorecard stacked FIVE lines — number, "Par N",
"N yds", score, and a check — at a 72px minimum width. Two consequences,
both bad on the one screen a player uses eighty times a round, one-handed,
outdoors:
- Nearly half the viewport was chrome before the hole hero began, and
the club/result controls started below the fold.
- Only five columns fitted across 390px, so on hole 7 THE HOLE BEING
PLAYED was not on screen at rest. It is now (seven fit at 52px).
Par and yardage are not lost. For the hole being played they are in the
hole hero immediately below — "Hole 7 · PAR 5 · 561 YDS TO PIN" — and
every column keeps them in its aria-label, so the screen-reader reading
is byte-identical. Desktop keeps all five lines at `lg:`.
Deliberately NOT touched: the shot-progress track, which is already the
result of a density pass with its own recorded rationale (dot / chip /
ring instead of six 44px pills), and the Prev/Exit/Next row, whose
behaviour is pinned by UI-10 in mobile-audit-2026-09-02.
**`truncate` and `flex` cannot sit on the same element.**
The Select trigger carried `'truncate flex items-center gap-2'`. With
`display:flex` the label text becomes an anonymous flex item, and
`text-overflow: ellipsis` — which only applies to a block container's own
inline content — silently does nothing. A long option was cut mid-word
with no ellipsis and no other sign it had been cut:
before Fall Travel Qualifier (1/3 rounds comple
after Fall Travel Qualifier (1/3 rounds comp...
which reads as a data error, not as truncation. The text now gets its own
truncating block inside the flex row, with `min-w-0` so it can shrink
below its content width. This is every Select in the product with an
option label longer than its trigger, not just the qualifier one.
Verified: `tsc --noEmit` exit 0; `eslint --max-warnings 0` clean on both
files; mobile-audit-2026-09-02 9/9; both screens re-rendered and read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
…t say so The media-query collapse is what makes the 390px render trustworthy, and it is also what makes a single render incapable of checking a responsive change: at 390 every `lg:` rule is deleted, so `hidden lg:block` and `min-w-[52px] lg:min-w-[72px]` render only their phone branch. That is not hypothetical — the scorecard change in 9826be4 is entirely `lg:` work and was verified at 390 alone. Re-rendered at 1280 afterwards it is correct (five lines per column, 72px, OUT column, two-column layout), but "correct" was luck rather than evidence until that second render existed. The silent-zero guard does not cover it either: it skips responsive and state prefixes, because after the collapse those rules are legitimately absent and it cannot distinguish "collapsed away" from "never compiled". The README now says both things, and gives the raw-sheet grep that can tell them apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YbV4yCGHRkv7T51KchT8HP
The one uncommitted edit left in the canonical checkout by the rounds preview harness session; committed so the branch can be pushed intact. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
…king copy (A2b) (#1848) * chore(settings): carry the 21 live deny rules the constitution left behind Track B applied these on the machine on 2026-09-05; the constitution PR committed the project settings from its own worktree and, lacking confirmation of some connector ids, omitted 21 of them: whole-server denies for Google Drive, Google Calendar, three job-search connectors and v0; Gmail send/reply/forward/trash; Notion create/update/move; Apollo emailer and task creation; Zapier write actions. Every id here is present in this session's own tool inventory, which is the confirmation the constitution was waiting for. Without this commit the canonical checkout's working copy was the only place those rules existed. Enforcement inventory regenerated from the merged file. Verified: control-plane:verify:static exit 0, docs:check exit 0, src/test/scripts + mcp-access-contract 234 passed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * chore(control-plane): record dispositions for the four PRs opened today open-pr-residue flags every open PR without a row; #1845, #1846, #1847 and #1848 now carry one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * chore(control-plane): drop the row for #1834, closed without merging Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Draft. Preserves the rounds preview harness work that lived only in the canonical checkout: the real round components rendered in the design preview, the phone-width fix, and the one-snapshot-per-breakpoint guard note, plus one memory edit. Stacked on #1837, so nineteen of the commits here are that PR's.
Opened by the control-plane reset so the canonical checkout could return to main without losing work. Not for merge as-is: rebase onto main once #1837 lands, or close if superseded.
🤖 Generated with Claude Code