Workflow Spec Review

Workflow — Spec Review (adversarial) — Persimmon Patterns

Step 2 of the Persimmon workflow: workflow-brainstorm → workflow-spec-review → workflow-plan. A spec can read perfectly and still contain production-grade landmines. This skill is the repeatable version of a senior engineer red-teaming the draft: it runs a checklist of the failure modes that don't surface in a happy-path read, so they're caught before the client approves and before code is planned — not by an angry customer at the counter.

It exists because these issues recur across builds. Catching them once is luck; catching them every time is a skill.

Trigger

• After workflow-brainstorm produces a spec, before workflow-plan
• "Review this spec" / "Is this spec ready?"
• Any spec touching payments, inventory, bookings, multi-channel notifications, tax, auth, or a migration off an incumbent system

How to run it

1. Read the spec end to end once for intent.
2. Run Pass 1 (completeness/RTM) then Pass 2 (adversarial checklist) below. For every hit, write a finding: what's wrong, why it bites in production, and the concrete fix — tagged with its bucket (engineering / domain-general / client-specific).
3. Apply fixes that are unambiguous directly to the spec (new acceptance criteria, new Risk-register rows). For anything the client must decide, log it to docs/client-context/open-questions.md.
4. Re-run the brainstorm self-review (placeholder/consistency/scope/ambiguity).
5. Only then hand off to workflow-plan.

Findings are not optional polish — a spec with an open concurrency or money-timing hole is not ready regardless of how complete it looks.

Pass 1 — completeness (traceability / RTM): nothing missing, nothing extra

Build a lightweight Requirements Traceability Matrix — left column = every requirement (each SOW item, each screen, each EARS criterion); right column = the spec elements (Prisma models, fields, components, Server Actions, flows) that satisfy it. Then check both directions:

• Forward — every requirement has a home. A SOW item / screen field / EARS line with nothing satisfying it → missing model/field/section. Does every SOW bullet map to a spec section? Anything in the spec beyond the SOW (added scope) → flag and client-confirm.
• Backward — every spec element earns its place. A model, field, or feature that no requirement needs → extra (cut it or justify it). An orphan is as much a finding as a gap.

Make the RTM mechanical, tiered to the project (workflow-traceability). Per the tiering matrix (project-type × project-stage), at T1/T2 every acceptance criterion carries a stable REQ-<PROJ>-<AREA>-<NNN> ID with a cited source (SOW§ | ADR-NNNN | constitution:), every screen's ID is its mockup filename (SCREEN-<slug>), and the registries live in docs/requirements/. Then run node <persimmon-skills>/scripts/traceability-audit.mjs <docs-root> — the mechanical half of this pass: it flags REQ-without-source, FLOW-without-persona/source, broken persona/screen/ADR links, and uncovered screens, and prints the RTM. But it only checks links among artifacts that exist — it cannot find a requirement nobody wrote. The forward pass above (every SOW item has a REQ) is still a human read; a green audit is not completeness (ADR-0008). A decision tracing to neither a skill nor a SOW/REQ line must become an ADR (meta-adr-authoring) — otherwise it's the orphan this pass exists to catch. (T0 projects skip IDs except on money/auth/data criteria.)

For the data model specifically, run the validation lenses in data-schema-design (replay every screen as Prisma queries, trace every entity lifecycle as enum members, snapshot check, cardinality/join-model check, reconcile vs the real business, write the reports as queries). The blanks in the matrix are the findings.

Classify every decision with the discriminator — would two competent teams, building independently, reach the same answer?

For client-owned decisions, apply the second axis — is a wrong guess cheap and reversible?

• Cheap/reversible (taste, defaults, copy) → decide provisionally, log as "assumed — pending client reaction." Don't add it to the question list.
• Costly/irreversible (money, law, live customer data, schema, auth) → ask before building → Client Questions log, tagged Fact (blocks building) or Policy (blocks acceptance). Legal/regulatory → always ask.

The decision log (sibling to the RTM). The RTM proves coverage; the decision log proves justification. One row per non-obvious decision: Decision | Owner (SOW §ref / ENG / IND / Client-Q#) | reversible|costly + fact|policy (if client) | Status | Rationale. Two checks: a row with no assignable owner = a decision made for no reason → cut or justify (the backward pass); a row owned by an open, costly Client-Q = a blocker.

The headline deliverable is the resulting short, high-value client-question list — only the costly/irreversible items, each tagged Fact/Policy — surfaced now to prevent a week-6 stall, kept short so the client actually answers it.

Pass 2 — adversarial (each row is a real failure mode)

Concurrency & finite resources

• Does anything finite (stock, pickup/time slots, tables, tickets) get claimed under concurrent checkout? Is there an atomic conditional claim + hold at checkout-start (Prisma guarded updateMany/raw UPDATE, or $transaction + FOR UPDATE/advisory lock), or can two users oversell the last unit? → backend-commerce-concurrency, data-booking-availability. Add an explicit acceptance criterion + Risk row.
• Is capacity claimed at checkout-start or only at webhook/payment success? Webhook-time claiming leaves the paid-but-no-record gap. Webhooks lag.
• Server Actions run concurrently per user/tab — is any read-then-write (counter, balance, slot) actually atomic in Postgres, or only "atomic" in TypeScript?

Money-state timing & integrity

• When is the order/booking row created? It must exist as PENDING/HELD at checkout-initiation (so the success page has something to show and a hold can be placed), with the webhook confirming — not the webhook creating it.
• Is the Stripe webhook idempotent? Webhooks fire more than once; is there an event-id table / unique constraint so a double-delivery doesn't double-charge/double-fulfill? → backend-webhook-handler.
• Is money computed in integer cents (never Float/Decimal math), and is tax/refund/adjustment handled? Are mutable values snapshotted onto historical rows? → data-schema-design.

Acceptance criteria that can't pass

• Does any acceptance criterion depend on a capability that won't be live at launch (e.g. SMS before A2P 10DLC clears, a custom domain before DNS/SSL propagates)? Reword as conditional ("…when the SMS channel is enabled") so verification can pass at launch and again later.
• Is every criterion verifiable as a user workflow, not a unit-test name or a vague aspiration?

Cutover & migration landmines

• Are there money-touching balances on the incumbent system (gift cards, store credit, deposits, subscriptions)? These are cutover blockers, not "open items" — promote to the Risk register with an explicit honor / parallel-run / hard-cut decision.
• Will any existing customer-facing URL/channel disappear at cutover without a redirect or comms?
• Railway/Next 16 build-time traps in the cutover plan: does any DB/auth()-reading page lack export const dynamic = "force-dynamic" (build prerenders → Railway build fails)? Is prisma migrate deploy vs db push chosen and consistent? Is the container targetPort aligned with PORT (8080)? → CLAUDE.md gotchas.
• NextAuth v5 behind Railway's proxy: is trustHost: true set, and do middleware redirects read x-forwarded-host? A miss bounces users to *.up.railway.app and breaks auth at cutover. → security-nextauth.

Jurisdiction, legal & compliance

• Is tax modeled per category (prepared food vs retail vs gift card), with the right local rules, or a single global rate? Gift cards taxed at purchase = wrong. For Brazilian-jurisdiction clients, confirm the correct regime (ISS vs ICMS, NF-e obligations) before assuming a flat rate.
• Any regulated flow modeled compliantly? (e.g. Google's review-gating prohibition — you may not route only happy guests to public reviews.)
• Payment data: does the design keep card data out of scope (Stripe-hosted fields / Elements, SAQ-A), never touching PANs? → backend-stripe / security.
• Regulated products — does any feature ship/sell something governed by law (alcohol, tobacco, supplements, firearms)? This can make a feature illegal, not just wrong. Flag as a costly/legal Client+legal question; never assume.
• Marketing consent / privacy — does a marketingOptIn imply email/SMS marketing? Then consent/unsubscribe + privacy-policy obligations apply (LGPD for BR clients, CAN-SPAM/GDPR as applicable). Policy + legal.
• PII handling — does the spec log document contents, names, or secrets anywhere? Persimmon rule: redact PII before logging; keys server-side only, never NEXT_PUBLIC_*. → security.
• Anything that could be illegal rather than merely wrong is a blocker, not an "open item."

Self-service (client autonomy)

• Does every integration the spec adds expose its credentials/config in the admin (editable by the client), or are keys dev-only in env? → backend-settings-admin.
• Is the content the client will change post-launch (team, images, hours, copy) admin-editable, or hard-coded in markup/JSX? → backend-content-management.

Scope, dependencies & operability

• Does any feature add operational burden the client must staff (in-house delivery, manual moderation)? Flag as a deliberate client decision, not a default.
• Are external dependencies with lead time (carrier registration, domain/SSL, third-party API approval, merchant onboarding) called out as human-blocked with the clock started?
• Is the spec scoped to a single coherent implementation, or should it split into phases?

Process integrity (was this applied, or improvised?)

• Does the spec carry a Skills applied: footer (which skills were read for which decisions) and a Decisions needing research: note (what was researched)? An empty footer on a non-trivial spec is itself a finding — the work can't be traced to applied skills. (Tiered: required at T1/T2.)
• Does every design/IA/architecture decision trace to a skill that was read, research that was run, or an ADR — not improvisation? Spot-check the nav/IA, the visual decisions, and any "best practice" claim.
• Were the aspects needing deep-research flagged and actually researched (industry IA, pricing, regulatory, unfamiliar APIs), with findings cited?

Data & failure handling

• What happens on the unhappy paths — payment failure, abandonment, partial cart, expired hold, declined card, webhook never arrives? Are they specified?
• Are empty/error/loading states specified for each screen (ties to the ## Screens section)?
• Does every Server Action input cross a Zod boundary, and does every action return the typed ActionResult union rather than throwing? → stack-zod-boundary, stack-server-actions.

Output

A short findings list appended to the spec (or as review comments), each with severity:

• Blocker — wrong money, oversell, compliance miss, unverifiable launch criteria, build-breaking Railway/Next config. Fix or log before plan.
• Risk — real but mitigable; ensure it's in the Risk register.
• Nit — clarity/consistency; fix inline.

Then either fixes-applied + handoff to workflow-plan, or a clear "blocked pending client answers (Q-NNN…)".

Anti-patterns banned

• Treating a clean happy-path read as "spec approved"
• Leaving money/legal/ops questions as quiet "open items" instead of Risk rows + Client Questions
• Writing acceptance criteria that are guaranteed to fail at verification time
• Skipping straight from brainstorm to plan on a commerce/payments/auth spec
• Approving a spec whose pages read the DB/auth() without force-dynamic (Railway build will fail)

Related

• workflow-brainstorm — produces the spec reviewed here (step 1)
• workflow-plan — runs only after findings are resolved/logged (step 3)
• data-schema-design — the data-model lenses + bucket classification used in Pass 1
• backend-commerce-concurrency, data-booking-availability, backend-stripe, backend-webhook-handler, backend-notifications, backend-settings-admin, backend-content-management — domain skills that fix findings
• stack-server-actions, stack-zod-boundary, security-nextauth, security — boundary, auth, and config checks referenced above