Skip to content

NWP-201: issue virtual cards from the console - #173

Open
sachanta wants to merge 3 commits into
JJFromTenex:mainfrom
sachanta:NWP-201-issue-cards
Open

NWP-201: issue virtual cards from the console#173
sachanta wants to merge 3 commits into
JJFromTenex:mainfrom
sachanta:NWP-201-issue-cards

Conversation

@sachanta

Copy link
Copy Markdown

Ticket

Closes NWP-201

What changed

Ops can issue a virtual card from the console instead of messaging the platform team and waiting hours. The issue form takes a nickname, merchant, spend limit and currency, and the card exists the moment it is submitted — with its limit set, so the Slack-thread-and-hope path that produced two wrong limits last month is gone.

There is a /cards list, a detail page showing spend against the limit, and freeze/unfreeze from either place. Card numbers are generated on the server on the 4242 test BIN with a valid Luhn check digit, shown once on the success screen, and masked as •••• 1234 everywhere afterwards.

How I verified it

npm test52 passing, 24 of them new in src/lib/cards.test.ts, covering Luhn validity across 200 generated numbers, the 4242 BIN prefix, masking, all nine status transitions, and every validation branch.

npx tsc --noEmit clean. npm run lint clean. npm run build passes with /cards, /cards/[id], /api/cards and /api/cards/[id] in the route table.

Against a dev server:

  • Reveal once — created a card via POST /api/cards, got 4242 6177 8944 9882. Grepped for that number in GET /api/cards, GET /api/cards/:id, the /cards HTML and the /cards/:id HTML: absent from all four. In the browser, issued a card, saw the number on the success screen, clicked Done, and confirmed the full number was gone from the DOM while •••• 9882 remained.
  • Validation — eight curl calls, each returning 400 with a readable message: missing merchant, unknown merchant, zero limit, negative limit, limit of 5,000,001, currency JPY, a non-integer limit of 250.5, and a whitespace-only nickname. A limit of exactly 5,000,000 returns 201, so the boundary is inclusive.
  • State machineactive → frozen → active → cancelled all 200. cancelled → active and cancelled → frozen both 409. An unknown status is 400; an unknown card is 404.
  • In the browser — issued "Vendor subscriptions — Q4" at 1250.50, which stored as 125050 minor units and rendered $1,250.50. Card appeared at the top of the list without a reload. Froze a card from the detail page: badge flipped to Frozen and the button became Unfreeze in place, no page reload. Opened a card at 86% spend and confirmed the bar renders amber.
  • No regression from seedingGET /api/payments?status=disputed still returns pay_001610 first with 33 total, identical to before this branch.

Acceptance criteria

Core:

  • Issue a card. Dialog takes nickname, merchant, spend limit, currency; card appears in the list on submit.
  • Card list at /cards with nickname, merchant, masked number, spend limit, status and created date.
  • Card detail showing the full record and spend against the limit.
  • Generated card numbers — server-side, 4242 BIN, valid Luhn.
  • Reveal once, mask forever — full number only in the creation response and the success screen.
  • Server-side validation — missing merchant, zero/negative limit, limit above 5,000,000, currency outside USD/EUR/GBP.

Stretch:

  • Freeze and unfreeze without a full page reload (router.refresh()).
  • Spend progress on the detail page, amber past 80%.
  • Merchant category lock, chosen at issue time and shown on the card.
  • Tests on the Luhn generator and the status transitions.
  • Empty and error states written, not default.

Bugs fixed along the way

None fixed, one found and deliberately left alone:

src/data/metrics.ts:31,34 and src/data/analytics.ts:54,73 accumulate money as floats. Each does payment.amount / 100 and adds the result into a running total, so the overview charts and metrics are summing dollars as floating point rather than integer minor units. That is ORG-STANDARDS #1 ("integer minor units") and #2 ("format once, at the edge" — the division is a display concern happening inside aggregation). Root cause is that the chart components want major units and the conversion was pushed up into the data layer instead of down into the formatter.

It is untouched by this ticket and fixing it would have put an unrelated change to the overview in a cards PR, so I left it. It wants its own ticket.

Notes for the reviewer

The seed generator has a trap in it. src/data/generate.ts uses one module-level mulberry32 PRNG shared by every record it produces. Drawing from it for cards anywhere before the existing payment loop shifts every subsequent draw, silently rewriting every payment, dispute and payout in the app. Card generation is therefore appended after generatePayouts(), and there is a comment saying so. The verification above includes a before/after check on payment ids for exactly this reason.

Why the full number cannot leak. The Card type has no field to hold it. issueCard() returns { card, cardNumber } as two values, so the number never touches the record — a future GET cannot accidentally serialize something that was never stored. The record keeps last4 plus an opaque reference.

Spend is seeded, not derived. Nothing in the store links a payment to a card, so inventing that relationship was out of scope for the clock. Seeded cards carry a spend value at fixed ratios (one at 86%, one at 94%) so the amber threshold is demonstrable; newly issued cards start at zero.

Category vocabulary is invented. The ticket asks for a category lock but names no categories, so MerchantCategory is a fixed five-value union in types.ts, allowlisted server-side. If there is a real vocabulary somewhere, that union is the one place to change.

StatusBadge was extended rather than duplicated. Its three Record<AnyStatus, …> maps are exhaustive, so active/frozen/cancelled had to be added to all three — a miss is a build failure rather than a runtime surprise.

Not done: no test covers the route handlers themselves, only the pure functions beneath them. The validation and transition logic is fully covered in cards.test.ts and the routes are thin wrappers over it, but a request-level test would be the next thing I wrote.

🤖 Generated with Claude Code

sachanta and others added 2 commits September 10, 2026 11:33
Plans the build against the code that exists: the shared PRNG in
generate.ts that card seeding must not disturb, StatusBadge's exhaustive
status maps, and the money and date helpers to reuse rather than rewrite.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ops issued cards by messaging the platform team, hours per card and twelve
to twenty a week, with the limit living in a Slack thread. Issuing now
happens in the console with the limit set at creation.

Numbers are generated server-side on the 4242 test BIN with a valid Luhn
digit, returned exactly once in the creation response and never stored: the
Card record holds the last four and an opaque reference, so no later payload
has a field to leak. Status transitions are guarded in the query layer, not
the UI, and cancelled is terminal.

Seed cards are generated after generatePayouts() on purpose. generate.ts
shares one PRNG across every record, so drawing earlier would have silently
rewritten every payment, dispute and payout in the app.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JJFromTenex

JJFromTenex commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Claude Code 101 — Repo Rescue

🏆 Build Battle Score: 92 / 100

One-line verdict: A genuinely shippable card-issuing flow — server-side validation, honest masking, and a real state machine all check out — let down only by missing Tier 2 stretch items and a light-touch planning trail.

Core criteria — 100 / 100 (35%)

  1. Issue a card: ✅ — issue-dialog.tsx posts nickname/merchant/limit/currency; card appears via router.refresh().
  2. Card list: ✅ — /cards/page.tsx shows nickname, merchant, masked number, limit, status, created date.
  3. Card detail: ✅ — full record plus spend-vs-limit progress bar in cards/[id]/page.tsx.
  4. Generated numbers: ✅ — generateCardNumber builds on TEST_BIN with a computed luhnCheckDigit, server-side only.
  5. Reveal once: ✅ — full number lives only in dialog-local issued state, dropped on close (issue-dialog.tsx:reset); Card type has no field for it.
  6. Server-side validation: ✅ — validateIssueInput runs inside the POST route handler, not just the client.

Correctness rules — 100 / 100 (20%)

  • Minor units: ✅ — spendLimit/spend are integers throughout; formatting only at formatMoney call sites.
  • Luhn on 4242 BIN: ✅ — real generator (src/lib/cards.ts), no hardcoded number, 200-iteration test.
  • Masking: ✅ — not stored on Card, not in list/detail payloads, cleared from client state on drawer close.
  • State machine: ✅ — canTransition enforces active⇄frozen, either→cancelled, cancelled terminal, checked server-side in setCardStatus.
  • Server-side validation: ✅ — confirmed in POST /api/cards/route.ts.

Context and planning — 70 / 100 (10%)

No spec file in docs/specs/ or docs/epics/ is present in the diff. The PR description substitutes for it with real detail — naming actual files (src/data/generate.ts, StatusBadge.tsx, types.ts) and reasoning about the seed-PRNG ordering trap — and the delivered code matches that reasoning closely. That's a considered plan, just not a standing spec document, which is exactly the 0.7 tier.

Code quality — 90 / 100 (15%)

Tests are substantial (24 new, covering Luhn, transitions, every validation branch) and sit beside the code they exercise — this reads as honest coverage, not padding. No console.log/TODOs, no DB/migration added, seed data untouched in spirit (extended via generate.ts, not hand-edited JSON), StatusBadge extended rather than duplicated. Minor ding: CATEGORY_LABELS is duplicated verbatim in both page.tsx and issue-dialog.tsx rather than shared. Reported bug (float accumulation in metrics.ts) was correctly identified but explicitly not fixed, so no quality credit applies either way — that's consistent, not a violation.

PR description — 100 / 100 (5%)

Thorough and honest: states what was built, what was verified (with concrete curl/grep evidence), what stretch was hit, and explicitly flags what's not done (route-level tests) rather than hiding it.

Stretch goals — 75 / 100 (15%)

Tier 1: ✅ Freeze/unfreeze without reload · ✅ Amber progress bar · ✅ Category lock · ✅ Luhn/transition tests · ✅ Written empty/error states — all five, cap 0.50.
Tier 2: ✅ Currency matches merchant — src/lib/cards.ts validateIssueInput rejects currency !== settlesIn, and the form derives+locks currency from the chosen merchant in issue-dialog.tsx. ❌ Idempotent issue — submit is only disabled client-side via isLoading, no server-side idempotency key or guard. ❌ Spend honesty — new cards start at 0 and stay honest, but the seeded cards in generate.ts carry invented fixed ratios (0.86, 0.94, etc.) rather than 0 or a derivation from real store data, which the rubric's rule speaks against. ❌ Cancel from UI — card-actions.tsx only toggles active/frozen; no cancel action exists anywhere in the diff despite the state machine supporting it. ❌ Audit trail — no transition history array anywhere on Card or in the store.


Breakdown: Core (100 × 0.35) + Rules (100 × 0.20) + Context (70 × 0.10) + Quality (90 × 0.15) + PR (100 × 0.05) + Stretch (75 × 0.15) = 92 / 100

One thing to do differently next time: Spend the last few minutes on Tier 2, not more Tier 1 polish — a cancel button with confirm and a server-side idempotency guard were both cheap relative to what was already built, and would have pulled this into the high-90s.

The diff was too large to review in full, so only the first part was graded.


Powered by Anthropic and Tenex

The form defaulted the currency to the merchant on select but left the
dropdown editable, and the server never checked the pairing, so a client
could issue a GBP card against a USD merchant. Validated server-side now,
and the form locks the field once a merchant is chosen.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants