test(oauth): cover credential-generation compare-and-set helpers

[KrasimirKralev]

What Problem This Solves

src/oauth-token-generation.ts exports eight helpers that stamp each OAuth credential save with an opaque generation marker, so the vault/persistence layer can do compare-and-set writes across multiple backing stores (oauth-vault.ts:167/173/214/308/320/363, oauth-persistence.ts). They decide which concurrent store write wins — yet shipped with no direct test coverage (they are only exercised transitively through save/load round-trips; a repo-wide search finds no test importing the module).

The contract is subtle and easy to regress silently:

• withOAuthTokenGeneration / withOAuthClientGeneration reuse an existing generation and only mint a new one (randomUUID) when none is present — so two independent stamps of the same value are distinct saves;
• withHiddenOAuthTokenGeneration / withHiddenOAuthClientGeneration make the marker non-enumerable (hidden from JSON/API values) while keeping it readable for recovery, and return the input unchanged when there is nothing to hide;
• sameOAuthTokenGeneration / sameOAuthClientGeneration require matching generation AND matching public value when a generation is present, return false for an absent current, and fall back to deep-equality only for two legacy (generation-less) values — so a generationed value never matches a legacy one;
• sameOAuthTokenValue / sameOAuthClientValue compare public values while ignoring only the internal marker.

A regression in any of these — dropping the reuse, leaking the marker into serialized values, or dropping the public-value check from the compare-and-set — would let a stale write win or a live write be rejected, and would not be caught before merge.

Why This Change Was Made

Coverage-only. This adds one new file, tests/oauth-token-generation.test.ts (+115, no production code touched), pinning all eight exported helpers' current main behavior. The tests drive the real exported functions directly (no stub), asserting enumerability via Object.keys/JSON round-trips and identity via reference checks, so they exercise the production compare-and-set path the persistence layer relies on. No new config, defaults, dependencies, or behavior — the diff is a single test file.

User Impact

No user-visible or runtime change. For maintainers, the credential-generation contract now regresses loudly instead of silently: a future edit that breaks generation reuse, the hidden-marker semantics, or the generation-plus-value comparison will fail this suite.

Evidence

Linux, Node 22.22, pnpm install --frozen-lockfile from source, branched off current main (7259c8c).

13/13 pass on clean main:

$ node_modules/.bin/vitest run tests/oauth-token-generation.test.ts
 Test Files  1 passed (1)
      Tests  13 passed (13)

Non-vacuous — the suite bites when the target is mutated (each mutation applied to oauth-token-generation.ts, suite re-run, then reverted byte-identical):

Mutation to oauth-token-generation.ts	Result
withOAuthTokenGeneration always mints (drop the reuse of an existing generation)	1 fail
sameOAuthTokenValue compares full objects (stop ignoring the generation)	4 fail
hidden marker made enumerable (enumerable: true)	1 fail
sameOAuthTokenGeneration drops the public-value check	1 fail
(reverted — control)	13 pass

Format / lint / types:

$ node_modules/.bin/oxfmt --check tests/oauth-token-generation.test.ts   # All matched files use the correct format.
$ node_modules/.bin/oxlint --type-aware --tsconfig tsconfig.json --deny-warnings tests/oauth-token-generation.test.ts   # exit 0
$ node_modules/.bin/tsc --project tsconfig.json --noEmit   # exit 0

Scope note: one new *.test.ts under tests/, +115 / -0, no production code touched. The randomness in randomUUID is only exercised through the deterministic distinct-vs-reuse contract (two stamps differ; a re-stamp preserves), so the suite is stable.

Related: none — coverage mined from the module itself; no linked issue.

---

AI-assisted contribution.

---

Generated by Claude Code

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 2, 2026, 12:40 PM ET / 16:40 UTC.

ClawSweeper review

What this changes

The branch adds a 15-case Vitest suite for OAuth token and client generation helpers, including hidden serialization and compare-and-set stale-write rejection behavior.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

This PR is still necessary: current main contains the OAuth compare-and-set helpers but not the direct regression suite. The focused test-only patch is correct, proven on its exact head, and has successful cross-platform builds; it remains open because it is still marked draft despite the collaborator’s stated disposition to land.

Likely related people: steipete is the high-confidence routing candidate from the helper’s introduction and the follow-up coverage commit.

Priority: P3
Reviewed head: ea0981fb6de7d8f7a628b38b0684f668ba427bef

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	Focused coverage, direct runtime proof, green cross-platform checks, and exact-head collaborator verification make this a high-confidence landing candidate.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (live_output): The PR body supplies a real targeted Vitest run and mutation checks, and the collaborator later reports 15 passing focused tests plus compiled-library stale-write proof on the exact head.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (live_output): The PR body supplies a real targeted Vitest run and mutation checks, and the collaborator later reports 15 passing focused tests plus compiled-library stale-write proof on the exact head.
Evidence reviewed	6 items	Current implementation contract: Current main implements generation reuse, hidden non-enumerable markers, and generation-plus-public-value comparison for both tokens and client registrations; the changed tests directly cover that established behavior. Focused PR coverage: The branch adds only tests/oauth-token-generation.test.ts; its client cases cover preserving an existing generation and rejecting a changed public client value carrying the same generation. Not implemented on current main: Current main does not contain the added direct OAuth-generation test file, so this coverage is not redundant or already shipped from this branch.
Findings	None	None.
Security	None	None.

How this fits together

OAuth credential persistence stamps token and dynamic-client records with opaque generations so concurrent writes across vault and persistence backends can use compare-and-set behavior. The helpers hide the internal marker from OAuth-facing values while preserving it for recovery and stale-write detection.

flowchart LR
  A[OAuth token or client] --> B[Generation helper]
  B --> C[Vault and persistence writes]
  C --> D[Compare generation and public value]
  D --> E[Accept current write]
  D --> F[Reject stale write]
  B --> G[Hidden marker for API values]

Loading

Before merge

• Complete next step (P2) - A collaborator explicitly requested landing after exact-head CI; the remaining action is to resolve the draft workflow state and merge, not an automated code repair.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Test scope	1 test file added; 123 additions, 0 deletions	The branch adds regression coverage only and does not change production behavior.
Exact-head CI	3 platform builds passed	macOS, Ubuntu, and Windows builds all report success for the submitted head.

Technical review

Best possible solution:

Clear the draft state and land the focused regression coverage after confirming the existing successful checks still apply to the exact head.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR adds direct regression coverage rather than repairing a reported runtime failure; the supplied live runs show the new tests execute against the real helpers.

Is this the best way to solve the issue?

Yes: direct unit coverage of the exported helpers is the narrowest maintainable way to lock down the established compare-and-set contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e1689c3dec7c.

Labels

Label justifications:

• P3: This is low-risk regression coverage with no runtime or user-visible behavior change.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body supplies a real targeted Vitest run and mutation checks, and the collaborator later reports 15 passing focused tests plus compiled-library stale-write proof on the exact head.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies a real targeted Vitest run and mutation checks, and the collaborator later reports 15 passing focused tests plus compiled-library stale-write proof on the exact head.

Evidence

What I checked:

• Current implementation contract: Current main implements generation reuse, hidden non-enumerable markers, and generation-plus-public-value comparison for both tokens and client registrations; the changed tests directly cover that established behavior. (src/oauth-token-generation.ts:39, 85f4c9e81508)
• Focused PR coverage: The branch adds only tests/oauth-token-generation.test.ts; its client cases cover preserving an existing generation and rejecting a changed public client value carrying the same generation. (tests/oauth-token-generation.test.ts:97, ea0981fb6de7)
• Not implemented on current main: Current main does not contain the added direct OAuth-generation test file, so this coverage is not redundant or already shipped from this branch. (tests/oauth-token-generation.test.ts, e1689c3dec7c)
• Feature-history provenance: A symbol-history search identifies the OAuth generation behavior as introduced by the OAuth rejected-refresh recovery work; blame attributes the helper lines to Peter Steinberger. (src/oauth-token-generation.ts:79, 85f4c9e81508)
• Exact-head verification and landing intent: A collaborator reported 15 focused tests, the full check and test gates, build, and a compiled-library stale-write proof on ea0981…, then stated to land once exact-head CI succeeds; the supplied checks show successful macOS, Ubuntu, and Windows builds. (tests/oauth-token-generation.test.ts:103, ea0981fb6de7)
• Documentation-index limitation: The required documentation-index wrapper could not run because Bun is unavailable in this read-only environment; source, diff, and history evidence above were collected successfully with read-only commands.

Likely related people:

• steipete: Git history attributes the OAuth generation helper to Peter Steinberger, and the same person added the missing client-side cases on this PR and documented the landing disposition. (role: feature introducer and recent area contributor; confidence: high; commits: 85f4c9e81508, ea0981fb6de7; files: src/oauth-token-generation.ts, tests/oauth-token-generation.test.ts)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (15 earlier review cycles; latest 8 shown)

• reviewed 2026-08-01T01:53:26.680Z sha 998fb1f :: needs maintainer review before merge. :: none
• reviewed 2026-08-01T05:08:56.872Z sha 998fb1f :: needs changes before merge. :: [P3] Remove the explanatory test preamble | [P3] Remove inline comments that restate assertions
• reviewed 2026-08-01T19:16:21.321Z sha 998fb1f :: needs changes before merge. :: [P2] Test client generation reuse | [P2] Reject stale client values with the same generation | [P3] Remove the explanatory test preamble | [P3] Remove assertion-restatement comments
• reviewed 2026-08-01T20:35:29.491Z sha 998fb1f :: needs changes before merge. :: [P2] Test client generation reuse | [P2] Reject stale client values with the same generation | [P3] Remove the explanatory test preamble | [P3] Remove assertion-restatement comments
• reviewed 2026-08-02T05:26:29.763Z sha ea0981f :: needs maintainer review before merge. :: none
• reviewed 2026-08-02T09:39:28.900Z sha ea0981f :: needs maintainer review before merge. :: none
• reviewed 2026-08-02T11:53:38.198Z sha ea0981f :: needs maintainer review before merge. :: none
• reviewed 2026-08-02T14:29:59.502Z sha ea0981f :: needs maintainer review before merge. :: none

[steipete]

Maintainer repair and verification are complete at ea0981fb6de7d8f7a628b38b0684f668ba427bef.

I added the two missing client-side compare-and-set cases from review: restamping preserves an existing client generation, and a changed public client value is rejected even when it carries the same generation. I also removed the redundant assertion-restatement comments. The contributor's original commit remains intact.

Proof on Node 26.1.0 / pnpm 10.33.2:

• pnpm exec vitest run tests/oauth-token-generation.test.ts — 15 passed.
• pnpm check — formatting, type-aware lint, and typecheck passed.
• pnpm test — 131 files passed, 1 skipped; 855 tests passed, 3 skipped.
• pnpm build — passed.
• Direct compiled-library run — {"reuseAccepted":true,"staleRejected":true}.
• AutoReview (Codex, branch versus origin/main) — clean, no accepted/actionable findings.
• Public model-identifier gate — PASS; this test-only diff has no model-bearing surfaces.

Maintainer disposition: LAND once exact-head CI completes successfully.

[steipete]

Landed as ced43e4.

Verification at exact head ea0981f before merge:

• pnpm exec vitest run tests/oauth-token-generation.test.ts — 15 passed (local, Node 24.18.0 / pnpm 10.33.2).
• Cross-platform CI builds (ubuntu / macos-15 / windows) all green on the head.
• Full-suite, build, and Codex AutoReview proof was already recorded in the maintainer verification comment above and the head has not changed since.

Test-only change, so no changelog entry. Thanks @KrasimirKralev!