test(nova): address Nexus review of QA.4 PII → decision-log spec (GOV-593)#15
Merged
Shaivpidadi merged 2 commits intodevfrom Apr 24, 2026
Merged
Conversation
…-593)
Three fixes from the Nexus review on GOV-593:
1. Require x-correlation-id on the chat response and match strictly on it
(`pii-decision-log.spec.ts`). The previous `hasCorr = correlationId ?
... : true` made the match predicate reduce to "any redact/transform
decision in the log" when no correlation id was present — a stale
row from a prior run would satisfy it. Now the test fails loudly if
the header is missing, and the find predicate requires an exact
match on correlationId.
2. Remove the duplicate QA.4-2 dashboard-assertion test from
`governed-chat.spec.ts`. The PII prompt → /api/v1/decisions flow is
now owned only by `pii-decision-log.spec.ts`; the QA.4-2 describe
block keeps just the chat-UI side (Redact badge + Redacted tile) so
the two specs don't duplicate the same assertion.
3. Bump the warmup loop in `.github/workflows/e2e.yml` from 10 to 20
attempts (20 × 15s = 300s), so the Render free-tier chat / keycloak
/ precheck services have more headroom to exit cold-start before
Playwright starts. Recent dev runs were failing with
"chat failed to wake up after 10 attempts" well before the spec
itself ran.
Also addressed the two non-blocking notes while touching the file:
- Use `expect.poll` on `/api/v1/decisions` so the assertion tolerates
ingestion lag (decision may not be in the read model yet when the
chat response returns).
- Scope the dashboard decision-row assertion to `getByRole('row')` so
it can't match filter labels or legend text.
GOV-593
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
20 × 15s = 300s was still failing on dev runs — the failure mode was
not cold-start latency but curl -f flagging a non-2xx/3xx response
from the chat app. Using -f made warmup flap whenever the container
was warm but /login returned, say, 4xx (e.g. middleware redirect back
to an error state) or the Render edge served a branded error page.
Switch to capturing the HTTP status with curl -w "%{http_code}" and
treating anything except connection failure (000) or 5xx (502/503/504,
Render's cold-start error codes) as "service is awake". This matches
what we actually care about in warmup — container has exited cold-start
— and defers "is the app serving the expected page?" to the Playwright
suite, which has the right assertions for it.
Also switch chat warmup to "/" (always served by the app root) instead
of "/login" so a route-level regression in the login page doesn't fail
warmup.
Refs: GOV-593 review (Nexus). Prior run 24874040921 failed with 20x
"chat waiting..." against /login; this unblocks the spec from actually
running against staging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Addresses the three blocking items in Nexus's review of GOV-593:
pii-decision-log.spec.tsnow fails loudly if/api/chatdoesn't carryx-correlation-id/x-request-id, and thefindpredicate matches strictly on that id. No more "any redact/transform row in the log" fallback.QA.4-2second test ingoverned-chat.spec.tsthat asserted the same PII → dashboard-decision flow.pii-decision-log.spec.tsis now the sole owner; the QA.4-2 describe keeps only the chat-UI assertion (Redact badge + Redacted tile)..github/workflows/e2e.ymlwarmup loop from 10 to 20 attempts (300s cap per service) so Render free-tier cold-start worst case fits.Also handled the two non-blocking notes:
/api/v1/decisionslookup inexpect.pollto tolerate async ingestion lag.getByRole('row')so it doesn't match filter labels / legend chips.Test plan
pnpm tsc --noEmitclean (done locally)pii-decision-log.spec.tsruns and asserts the dashboard row by correlation idRefs: GOV-593, GOV-592 (warmup change builds on PR #12)