Six items from claude's incremental review (`12:48:43Z`):
- monitoring/dashboard.json: Group the outcomes widget by both
`metric.label.outcome` and `metric.label.reason`. Previously all
failures collapsed into a single `turnstile_failed` series, which
contradicted the README claim that the `reason` label drives the
breakdown.
- monitoring/metrics/*.yaml: Narrow the metric filter to
`jsonPayload.event=("turnstile_siteverify" OR "pack_completed")`.
Without this anchor, any future code path attaching
`siteverifyDurationMs` to an unrelated log silently joins the
distribution and creates new metric label values.
- usePackRequest.ts: Mirror `progressMessage.value = null` alongside
the `progressStage.value = null` clear on token-acquisition aborted /
error branches. Prevents a future edit setting a verifying message
from leaking prior-run state.
- turnstile.test.ts: Add a focused `describe` block with five tests
asserting `siteverifyDurationMs` is attached to every post-siteverify
log (one success path + four reject branches). The metric YAML
filters on field presence, so a refactor that drops the field on any
branch would silently break the metric without other tests failing.
Uses the existing `vi.spyOn(logger, ...)` pattern; no clock injection
needed.
- monitoring/README.md: Note that the metric filter pins
`service_name="repomix-server-us"`, so future regions (`-eu`,
`-asia`) silently drop out until the filter is broadened or
per-region counterparts applied.
- monitoring/README.md: Add a `gcloud logging metrics describe` snippet
for verifying a YAML edit was actually applied (gcloud update is
silent on no-op vs effective change).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
decision(assertion-precision): switch the "no stray leading \`: : \`" guard from \`not.toContain(': : ')\` to an anchored regex \`/^Invalid request: : /\` — both catch the defect today, but the anchored form documents exactly which prefix shape we're guarding against and rules out any legitimate \`: : \` that might appear later in the message
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
intent(drift-guard): claude's follow-up review flagged that the hand-rolled schemaErrorWith fixture can't catch a change in valibot's internal PathItem shape — fixture-based tests would stay green while production fails, so add one real-valibot-issue case
decision(fixture-vs-real): keep the existing fixture-based tests as the bulk of coverage (cheap, focused on classifier logic) and layer a single v.safeParse-driven test on top — the fixtures stay fast, the new test catches shape drift
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
intent(test-ownership): move tests into website/server/tests/ so they collocate with the code under test and stop reaching up through three parents; reviewer follow-up wanted dedicated coverage and the root vs. website/server package boundary makes collocation the right long-term layout
decision(vitest-config): give website/server its own vitest.config.ts + `test` script; root's existing tests/**/*.test.ts include no longer catches server tests since they moved outside that tree, so the two test runs stay independent
decision(tsconfig-test): add tsconfig.test.json extending the build config and lift lint-tsc to `-p tsconfig.test.json` — the build tsconfig's rootDir: "./src" excludes tests/, so a single lint command wouldn't have type-checked them
learned(valibot-instanceof): with tests now resolving valibot from the same website/server/node_modules as validateRequest, the cause-check can go back to `instanceof v.ValiError` — the duck-type workaround was only needed when the root harness and server pulled different valibot copies
constraint(ci-website): added a `test-website-server` job that links the local repomix build the same way lint-website-server does; tests don't actually import repomix today, but colocation means they easily could later and the link step keeps parity