# v7.5.0 — implementation field notes (claude-opus-4-7[1m])

## Wall-clock estimate

~38 minutes from spec read to passing build + commit (no retries on
verification). Most of the time was rote: 10 handler files each needed
the same 7-line insertion, and re-reading the file (mandatory under
the harness's "Edit requires Read" rule) added latency that would have
been ~0 in a batch-edit world.

## Deviations from the spec as written

1. **Helper-level response builder.** The spec said high-sensitivity
   handlers should call `assertActiveMembershipForOrg()` and let the
   caller handle the response. Implementing it that way meant pasting
   ~7 lines of try/catch + NextResponse.json into every handler. I
   added `respondToMembershipError(err)` next to the helper as a
   convenience response builder, so each handler is 5 lines of
   try/catch instead. Strictly an LOC-reduction; preserves the
   helper-throws-typed-error contract.

2. **HEAD + OPTIONS treated as GET.** The classifier's "non-GET on
   /api/dashboard/* → high" rule would have made HEAD/OPTIONS high
   too. CORS preflights (OPTIONS) firing the RPC on every request felt
   wrong, and HEAD is by definition non-mutating. I narrowed the
   non-GET rule to "NOT in (GET, HEAD, OPTIONS)" and added a test that
   pins this. The spec didn't say either way; this seemed less
   surprising.

3. **High-sensitivity routes don't mint a cookie on success either.**
   The spec said "skip cookie cache" on high-sensitivity reads but
   didn't say whether to mint a fresh cookie on success. Minting one
   would let a subsequent low-sensitivity request hit the cache —
   harmless but muddies the policy. I made high-sensitivity an
   absolute "no cookie ops" path. Added a comment in the middleware
   explaining the choice.

4. **API key mint route NOT given the helper.** The spec said "wire
   `assertActiveMembershipForOrg()` into handlers that touch a
   `[orgId]` path." `api-keys/mint` and `api-keys/revoke` do not have
   an orgId in the path — they're scoped per-user. The middleware
   already covers them via `HIGH_SENSITIVITY_PATTERNS` (always-RPC).
   No handler-level call needed.

5. **Membership status `invite_pending` collapsed into `member_inactive`.**
   The helper exposes 4 error codes (`member_disabled`,
   `member_inactive`, `no_membership`, `check_failed`). The
   `check_membership_status` RPC distinguishes `inactive` and
   `invite_pending` but the user-visible code is the same. Matches the
   existing `statusToReason` mapping in the v7.0 middleware.

## Tests that surfaced real-world findings (not synthetic)

1. **Existing `disabled-user-jwt.test.ts` had 4 cases asserting old
   per-route error codes** (`not_owner`, `not_admin`, `not_found`).
   The v7.5.0 helper now short-circuits these uniformly to
   `member_disabled` — which is strictly an upgrade (uniform error
   surface, blocks earlier, doesn't depend on each RPC's per-route
   guard). Updated the assertions and left a comment explaining the
   shift. This was a real-package shape — a tightening test that fired
   because the new behavior is actually more secure.

2. **`org-update.test.ts` test 26b ("non-member → 403 not_owner")**.
   Same shape. Pre-v7.5.0 a non-member fell through to the RPC's
   NULL-role guard and got `not_owner`. Post-v7.5.0 the helper returns
   `no_membership` first. Both responses are 403 + non-enumerating, so
   the security property is preserved; the body code is now uniform.
   Updated the test.

3. **`members-list.test.ts` test 2 ("non-member → 404")** and
   `audit.test.ts` test 3 ("non-member → 404"). The original 404s were
   anti-enumeration. The new 403 `no_membership` is also non-
   enumerating (same body for any (org, user) the user doesn't belong
   to). Updated to expect 403.

4. **TS strict-null on regex capture group.** `parseOrgIdFromPath`
   needed an explicit `typeof candidate !== 'string'` guard because
   `RegExp.exec()` returns `(string | undefined)[]` under strict null
   checks. Caught by `tsc --noEmit`, not by runtime tests. Standard
   gotcha; reinforces that the typecheck-as-test ratchet pays off.

## High-sensitivity routes added beyond the spec list

- **`/api/dashboard/api-keys/*` GET** — codex W4 explicitly flagged
  this. Listing API key prefixes for a user is admin-class even on
  GET, so the pattern includes `/api/dashboard/api-keys` (with
  terminator). The spec text mentioned this in the codex notes; I
  added it to the pattern list.
- **`/api/dashboard/orgs/:id/cost.csv`** — the spec listed `cost*`
  glob; I split it into an anchored regex that catches both `cost`
  and `cost.csv` but rejects `costume`. Boundary test added.
- **`/api/dashboard/orgs/:id/billing/**`** — spec listed billing as
  high; added with anchored regex.

## What didn't quite map

- The spec's middleware pseudocode says "Continue WITHOUT minting a
  new cookie" on high-sensitivity success. The actual middleware
  shape is "mutations applied to the final response." So the
  implementation returns a `RevocationDecision` with no mutations
  (rather than literally not minting). Same semantics, different
  control flow. Worth noting if future refactors compare to the spec.

- The spec mentioned a v7.6+ `withActiveMembershipRequired()` wrapper
  decorator. I considered prototyping it for shared code reduction
  but the explicit-call shape is what the spec actually scoped, and
  the wrapper introduces typing complexity that I'd rather get right
  in a focused refactor. Deferred as noted in the CHANGELOG.

## Caveats

- I did NOT run `next build` from the repo root (workspace build was
  done from `apps/web/`). The root `npm test` still passed all 1606
  CLI tests, and the web build was clean from `apps/web/`.

- `npm install` flagged 6 audit vulnerabilities (5 moderate + 1
  high). These pre-existed; no PR-introduced deps. Out of scope for
  this PR.
