test(smoke): prod post-deploy Playwright smoke (5 flows) + rollback-on-red (#227) - #253
Conversation
…n-red (#227) Add a sanctioned ~5-test web Playwright smoke suite that runs against PROD immediately after each deploy, with Vercel Instant Rollback as the safety net. - playwright.config.ts: BASE_URL from SMOKE_BASE_URL, retries=0 (fix-or-delete, no retry-to-green), single chromium worker, setup/cleanup projects + storageState. - 5 specs: signup/login (OTP UI), create habit, log habit, Astra-creates-habit, open-paywall. - Setup signs in once via the real passwordless OTP UI behind a single auth seam, resets the smoke account, and marks onboarding complete; teardown resets it again so prod data is never polluted (POST /api/profile/reset). - smoke-prod.yml: on push to main, wait for the Vercel prod deployment of the commit to be READY, run the suite against prod, and on red roll back to the previous prod deployment (vercel rollback) and surface the failure. - Minimal data-testids added to load-bearing web elements (locale-proof selectors). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR 253 Code Review: APPROVE. No Critical/High findings. 3 Medium findings: (1) smokeEnv.baseURL in apps/web/e2e/support/env.ts:13 is exported but never used - playwright.config.ts reads SMOKE_BASE_URL directly; remove it. (2) .github/workflows/smoke-prod.yml has no paths: filter, so mobile-only pushes trigger unnecessary smoke runs that could cause spurious rollbacks; add paths filter for apps/web/** and packages/shared/**. (3) STORAGE_STATE_PATH in apps/web/e2e/support/env.ts:18 is exported but hardcoded again in playwright.config.ts:43,53 - import and reuse it. None of these block merge. Approved.
GET /api/habits returns PaginatedResponse<T> ({ items, page, pageSize,
totalCount, totalPages }) — the smoke helper read body.habits (always
undefined), so listHabitTitles() always returned [] and the
Astra-create-habit poll could never satisfy .toContain(title), timing out
at 60s every run. Matches the app's own consumer (use-habit-queries.ts
reads firstPage.items).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon
left a comment
thomasluizon
left a comment
There was a problem hiding this comment.
Code Review: PR #253 (orbit-ui-mobile)
Scope: PR #253 — prod post-deploy Playwright smoke (5 flows) + rollback-on-red (#227)
Recommendation: APPROVE (one High found during review, fixed in 24572a6)
Summary
Web-only Playwright smoke suite (the five sanctioned flows: auth, create habit, log
habit, Astra-creates-habit, paywall) + smoke-prod.yml with rollback-on-red, plus
zero-logic data-testid/id hooks added to app components for targeting. Walked the
full rubric; every gated subagent is N/A (web-only E2E; no shared types, DTOs, i18n, or
design-token changes). One High-severity correctness defect was found and fixed; one
automated selector finding was a false positive and is documented below so it is not
re-raised.
Findings
Critical
None.
High
[HIGH] listHabitTitles read the wrong response field → Astra smoke can never pass — FIXED in 24572a6
· dimension: Correctness (#1)
· location: apps/web/e2e/support/api.ts:23-24
· issue: GET /api/habits returns PaginatedResponse<T> ({ items, page, pageSize,
totalCount, totalPages }); the helper read body.habits, which is always undefined.
listHabitTitles() therefore always returned [].
· risk: The Astra-creates-habit poll (expect.poll(...).toContain(title), 60s) can never
become true → that flow fails on every run → one of five gating flows is permanently
red, wedging the prod-promotion gate (or inviting delete-to-green, against the
fix-or-delete-same-day rule).
· fix: Read body.items. Verified against the app's own consumer
(apps/web/hooks/use-habit-queries.ts:53,56 reads firstPage.items) and the API DTO
(Orbit.Application/Common/PaginatedResponse.cs). Applied in 24572a6.
· reference: CLAUDE.md "API contract — never invent fields the API doesn't return"; rubric #1.
Medium
None.
Low / Info
[INFO] Automated selector sweep false positive — no action
· An in-review selector-existence agent reported [data-testid="paywall-checkout"] as
MISSING and "dataTestId not a PillButton prop." False positive: the agent read the
main checkout (which lacks this PR's changes), not the PR branch. On the branch,
pill-button.tsx defines dataTestId?: string (:18), destructures it (:42), and renders
data-testid={dataTestId} (:55); upgrade/page.tsx:724 passes
dataTestId="paywall-checkout"; paywall.spec.ts:7 targets it. Selector exists.
· All other selectors verified present on the branch: data-habit-title,
habit-status-toggle, chat-input/chat-send, tour-fab-button, #habit-form-title,
#login-email, data-code-index, data-bottom-nav, habit-create-submit,
auth-send-code/verify-code, pending-op-confirm.
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | N/A (web-only E2E + zero-logic data-testid props; mobile has no E2E) |
| i18n-syncer | N/A (no user-facing strings / locale changes) |
| contract-aligner | N/A (no shared types / endpoints changed) |
| security-reviewer | N/A (no orbit-api code) |
Validation
| Check | Result |
|---|---|
| Lint | N/A (e2e config eslint-ignored per PR; fix is a type-safe field rename) |
| Type check | PASS (cast property rename; `body.items: HabitListItem[] |
| Tests | N/A (smoke runs post-deploy against prod, not in unit CI) |
| Build (api) | N/A |
What's good
- Suite is exactly the five sanctioned flows;
retries: 0,forbidOnlyon CI, ≤5 specs — compliant with the Testing policy. - DOM-assert flows (create/log habit) target stable
data-*hooks, not classes. - Selector hooks are zero-logic
data-testid/idadditions — no behavior change, correctly not requiring a mobile mirror.
Recommendation
APPROVE. The one High (wrong habits field) is fixed in 24572a6; a re-run is not
required — the fix is a verified field rename matching the live API shape. Backward-compat
guard N/A. Merge note: #227 auto-closes via this PR; ensure the paired orbit-api#217
(smoke-auth bypass) merges so the prod smoke account can authenticate.
There was a problem hiding this comment.
Code Review: PR #253
Scope: PR #253 — test(smoke): prod post-deploy Playwright smoke (5 flows) + rollback-on-red
Recommendation: APPROVE
Summary
This PR ships the sanctioned post-deploy Playwright smoke suite (5 flows) against prod, wired to Vercel Instant Rollback on failure. The implementation is well-structured: a clean support/ seam for auth, API helpers, and env config; correct workers: 1 + retries: 0 per policy; forbidOnly in CI; and a thorough CI workflow with secret validation, deploy-wait polling, rollback capture, and artifact upload. No logic changes to production code — all app touches are additive data-testid attributes used only by the test harness.
One dead-export finding (smokeEnv.baseURL) is flagged at Medium — it never causes a test failure but violates rule 2 and may mislead future authors. Everything else is clean.
Findings
Critical
None
High
None
Medium
[MEDIUM] smokeEnv.baseURL is exported but never consumed
· dimension: Dead / stale code (#2)
· location: orbit-ui-mobile/apps/web/e2e/support/env.ts:13
· issue: smokeEnv.baseURL = required('SMOKE_BASE_URL') is validated and exported, but no test
file or support module imports or uses smokeEnv.baseURL. The playwright.config.ts already
reads process.env.SMOKE_BASE_URL directly and throws if it is missing (line 4-8), so the
required() call in env.ts is a duplicate guard with no consumer.
· risk: Dead code — ships a speculative export that signals "the base URL is in smokeEnv" but
callers never use it. A future author might use smokeEnv.baseURL and bypass the
playwright.config.ts baseURL path, creating split-brain URL sourcing.
· fix: Remove the baseURL field from smokeEnv. The playwright.config.ts is the single source
of truth for the base URL and already throws on missing env. If tests ever need the URL
directly, they should read it from playwright's built-in baseURL fixture, not smokeEnv.
export const smokeEnv = {
testEmail: required('SMOKE_TEST_EMAIL'),
testCode: required('SMOKE_TEST_CODE'),
} as const
· reference: CLAUDE.md rule 2 (delete unused code immediately)
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PAIRED — all web changes are test harness only; no mobile mirror required (CLAUDE.md explicitly bans mobile E2E) |
| i18n-syncer | IN SYNC — no new user-facing strings introduced |
| contract-aligner | N/A — no packages/shared/src/types/* or endpoints.ts changes; no orbit-api changes |
| security-reviewer | N/A — no orbit-api code changed |
Validation
| Check | Result |
|---|---|
| Lint | N/A — author reports 0 errors locally; no working-tree changes to run against |
| Type check | N/A — author reports clean tsc --noEmit locally |
| Tests | N/A — smoke suite requires prod secrets and cannot run in this environment by design |
| Build (api) | N/A — no backend changes |
What's good
- Single-seam auth (
e2e/support/auth.ts) is the right pattern — all login mechanics in one function, specs never touch auth directly. - Unique smoke labels (
smokeLabel()) prevent collision between concurrent runs cleanly. - Global setup + teardown resets the smoke account before and after every run — no prod data residue.
- Rollback capture is deterministic — the previous-deployment URL is captured before the smoke run, not after failure, so a rollback always has a valid target.
- Secret validation step in the workflow catches misconfigured environments before hitting prod.
forbidOnly: isCIcorrectly prevents a stray.onlyfrom silently shrinking the suite in CI.cancel-in-progress: falseon the concurrency group is intentional and correct — back-to-back pushes each need to smoke their own deployment independently.retries: 0strictly enforces the fix-or-delete-on-flake policy.- The
actions/checkout@v5/actions/setup-node@v5versions are consistent with the rest of the repo's workflows. data-testidattributes are locale-proof selectors that survive copy/i18n edits — correct per the PR description.
Recommendation
One Medium finding (dead smokeEnv.baseURL export) — fix before or immediately after merge. The finding does not block correctness or security. Recommended action: approve and trim the dead export.
…moke # Conflicts: # apps/web/package.json # package-lock.json
…te path, scope triggers - Remove unused smokeEnv.baseURL (playwright.config.ts is the single source of truth for the base URL). - Import STORAGE_STATE_PATH into playwright.config.ts instead of hardcoding the auth-state path twice. - Add a paths filter to smoke-prod.yml (apps/web/**, packages/shared/**) so mobile-only pushes don't trigger a smoke run with no matching Vercel deploy (which would time out and could spuriously roll back). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The merge commit captured the pre-regeneration lockfile (main's, without the smoke suite's @playwright/test). npm ci requires lockfile<->package.json sync; regenerated via npm install --package-lock-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review: PR #253
Scope: PR #253 — test(smoke): prod post-deploy Playwright smoke (5 flows) + rollback-on-red (#227)
Recommendation: APPROVE
Summary
This PR adds the sanctioned 5-test post-deploy Playwright smoke suite (closes #227) with a GitHub Actions workflow that waits for the Vercel production deployment, runs the suite, and rolls back on failure. It also instruments the necessary production UI components with data-testid / dataTestId attributes. The implementation is clean, the test-isolation strategy (unique labels per run, dedicated smoke account with reset on both sides) is solid, and the rollback mechanism is deterministic. No Critical or High findings.
Findings
Critical
None
High
None
Medium
[MEDIUM] Teardown project will fail with 401 when setup fails partway through
· dimension: Correctness (#1)
· location: apps/web/playwright.config.ts:38-46 + apps/web/e2e/global.setup.ts:6-14
· issue: In global.setup.ts, storageState is persisted at line 13 — after resetSmokeAccount and the PUT /api/profile/onboarding call. If either of those calls throws (e.g. the reset endpoint returns 5xx), setup fails and storageState is never written. Playwright still fires the cleanup teardown project because it is registered as setup's teardown. global.teardown.ts loads storageState: STORAGE_STATE_PATH, but the file doesn't exist, causing the teardown itself to throw — masking the real setup failure in CI output.
· risk: CI reports a confusing double-failure (setup + teardown), making it harder to diagnose the root cause. The teardown also silently skips the account reset, leaving residue in prod.
· fix: Move page.context().storageState() to immediately after authenticate() so auth is always persisted before any post-auth API calls can fail.
· reference: CLAUDE.md rule 8
[MEDIUM] listHabitTitles uses an unchecked as cast on an external API response
· dimension: Type safety (#6)
· location: apps/web/e2e/support/api.ts:23
· issue: (await response.json()) as { items?: HabitListItem[] } asserts the shape of the /api/habits response without a Zod parse. If the BFF contract changes (e.g. key renamed), the test will silently return [] and pass because body.items will be undefined and the ?? [] fallback fires.
· risk: A contract change could make the Astra smoke test pass vacuously (habit list always empty), defeating the server-side assertion.
· fix: Import the shared habits-list Zod type from @orbit/shared/types and parse with .safeParse() — throw on failure so a shape change produces an explicit error rather than a silent vacuous pass.
· reference: CLAUDE.md rule 3
Low / Info
None
Subagents
| Agent | Verdict |
|---|---|
| parity-checker | PAIRED — data-testid additions are not behavioral changes; mobile has no E2E suite per CLAUDE.md policy |
| i18n-syncer | N/A — no user-facing strings added or changed |
| contract-aligner | N/A — no packages/shared/src/types/* or endpoints.ts changes |
| security-reviewer | N/A — no orbit-api changes |
Validation
| Check | Result |
|---|---|
| Lint | N/A (CI context; *.config.ts eslint-ignored; JSDoc/no-comments compliance confirmed by manual inspection) |
| Type check | N/A (no working-tree changes; PR body reports tsc --noEmit clean) |
| Tests | N/A (smoke suite requires prod creds; unit tests unaffected) |
| Build (api) | N/A (no backend changes) |
What's good
- Policy compliance is tight.
retries: 0,forbidOnly: isCI,workers: 1, and a hard cap of 5 specs are exactly what CLAUDE.md's Testing section requires. - Auth seam is well-designed.
e2e/support/auth.tsis the single place that knows how to log in; every spec inherits storage state; if auth strategy changes, only one function changes. - Rollback target captured before smoke runs. Step order (wait → capture previous → run smoke → rollback on failure) ensures the rollback target is always the deployment that was live before this push.
- No-pollution strategy is complete. Uniquely-labelled habit names prevent cross-run collisions; smoke account reset fires in both setup and teardown.
- Selector hygiene is correct.
data-testidanddata-habit-titleare used for load-bearing selectors; locale-dependent text is avoided. All selectors verified to exist in the production source. cancel-in-progress: falsemeans a queued smoke run is never cancelled by a subsequent push, which would leave a failed deployment un-rolled-back.vercel rollbackreceives the full deployment URL (constructed ashttps://${prev}), which is the format the CLI expects.- Secret validation at workflow start fails fast before any prod traffic is generated.
Recommendation
Two Mediums, no Critical or High. Merging as-is is acceptable with the two items tracked as immediate follow-ups:
- Teardown ordering (
global.setup.tsL13): movestorageStatesave to immediately afterauthenticate(). - Type-safe BFF assertion (
e2e/support/api.tsL23): replace theascast with a Zod parse of the shared habits-list type.
The main smoke mechanics are sound. Suite is ready to protect prod once orbit-api#217 merges and CI secrets are set.
|
There was a problem hiding this comment.
Code Review PR 253 - APPROVE. No Critical or High findings. The PR tip (2b4dbc5) is a package-lock sync; substantive work was already reviewed and approved in prior reviews. Confirmed: dead smokeEnv.baseURL export removed, STORAGE_STATE_PATH reused, workflow paths filter scoped, listHabitTitles field fix present. Two Mediums carried from prior review (teardown ordering in global.setup.ts and as-cast in api.ts) are not re-raised as blockers per rubric. Suite is policy-compliant: 5 specs, retries=0, forbidOnly in CI, unique run labels, account reset in setup+teardown, deterministic rollback target. Parity/i18n/contract/security all N/A (web-only E2E harness, no shared type or API changes). Approved.
1 participant
Closes #227
Sanctioned ~5-test web Playwright smoke suite that runs against prod immediately after each deploy, with Vercel Instant Rollback as the safety net (red → roll back the just-shipped prod deployment). Per the re-scoped issue (QA env #211 aborted): no QA→prod promotion gate; the gate is post-deploy + rollback.
The 5 smoke flows (
apps/web/e2e/*.spec.ts)auth.spec.ts)create-habit.spec.ts)log-habit.spec.ts)astra-create-habit.spec.ts)/upgraderenders the plan selector + checkout CTA. (paywall.spec.ts)Post-deploy workflow (
.github/workflows/smoke-prod.yml) — NEW fileOn push to
main(web deploys via Vercel Git integration on the same push):$GITHUB_SHAuntilREADY(fails fast on ERROR/CANCELED).SMOKE_BASE_URL(the live prod alias).vercel rollback <previous-prod-deployment>(verified flags:--non-interactive --timeout), then surface the failure. Captures the previous prod deployment up-front so the rollback target is deterministic.No prod pollution
POST /api/profile/resetwipes all user data) and marks onboarding complete; global teardown resets it again. Net: the account returns to a clean slate every run, reused indefinitely, with zero residue in real prod data.CI secrets an operator must add
SMOKE_BASE_URLhttps://app.useorbit.org).SMOKE_TEST_EMAILSMOKE_TEST_EMAIL).SMOKE_TEST_CODESMOKE_TEST_CODE).VERCEL_TOKENVERCEL_ORG_IDteamId).VERCEL_PROJECT_IDAuth (resolved → Option 1, paired PR orbit-api#217)
Prod login is passwordless email OTP with no prod test bypass on main. The suite's auth lives behind a single seam (
e2e/support/auth.ts) and uses the real verify-code UI with a fixed code accepted only for one pinned prod email behind a high-entropySMOKE_TEST_CODEsecret — wired by the paired backend PR. The suite will not pass against prod until orbit-api#217 merges andSMOKE_TEST_EMAIL/SMOKE_TEST_CODEare set in both prod (API) and CI.Policy compliance (CLAUDE.md “Testing”)
retries: 0— fix-or-delete on flake, never retry-to-green.forbidOnlyin CI so a stray.onlycan't silently shrink the suite.data-testids added to load-bearing web elements (locale-proof selectors that survive copy/i18n edits) — an allowed app-code touch.Validation
npx playwright test --list→ all 7 tests (5 smoke + setup + cleanup) discoverable.tsc --noEmit(apps/web) → clean.eslinton all new/changed files → 0 errors (specs honorlocal/no-comments; config file is eslint-ignored by project config).🤖 Generated with Claude Code