test(oauth): cover static client-information + client-secret resolver

[KrasimirKralev]

What Problem This Solves

src/oauth-client-info.ts exports the two helpers that assemble a static OAuth client information object and resolve its client secret, and the module shipped with no direct test coverage — no tests/oauth-client-info.test.ts exists on main. Both helpers carry contracts a plausible refactor could silently break:

• resolveOAuthClientSecret(definition, options) — when oauthClientSecretEnv is set it reads the env var and throws a labeled error if the value is missing (!value) or, under rejectBlank, blank; otherwise it returns the env value (whitespace preserved when rejectBlank is off). With no env ref it falls back to the inline oauthClientSecret, or undefined.
• buildStaticClientInformation(definition, options) — returns undefined with no oauthClientId; otherwise emits fixed grant_types/response_types, conditionally includes client_secret (via the resolver), redirect_uris (from a URL or string), and token_endpoint_auth_method, and propagates the resolver's throw.

A regression in any of these — dropping the !value guard, inverting a conditional-spread ternary, or losing the required-secret throw — would ship green today because nothing executes the module.

Why This Change Was Made

Coverage-only. This adds one new file, tests/oauth-client-info.test.ts (+112, no production code touched), pinning both helpers' current main behavior. The tests import the real exported functions and drive them directly — no stubs — so they exercise the production path callers actually hit. Both the primary and negative/omit paths are pinned for each helper: the resolver's env/inline/undefined returns plus its three throw paths, and the builder's undefined gate, each conditional field's include-and-omit pair, and the propagated throw. No new config, defaults, or dependencies.

User Impact

No user-visible or runtime change. For maintainers, the static-client-information contract now regresses loudly instead of silently: a future edit that drops the required-secret throw, breaks a conditional-field spread, or loses the env/inline resolver fallback will fail this suite.

Evidence

Linux, Node 22.22, pnpm install --frozen-lockfile from source. Branched off current main (base 4219927); the module under test, src/oauth-client-info.ts, is byte-identical to its state at the earlier-verified base 58986a7 (the only change between them is CHANGELOG.md), so the pinned behavior is current.

15/15 pass on current main:

$ ./node_modules/.bin/vitest run tests/oauth-client-info.test.ts
 RUN  v4.1.10 /home/user/mcporter
 Test Files  1 passed (1)
      Tests  15 passed (15)

Non-vacuous — the suite bites when the target is mutated (each mutation applied to src/oauth-client-info.ts, suite re-run, then reverted byte-identical). The representative M1 case was re-confirmed on this exact rebased head; the full matrix was validated against the byte-identical source:

Mutation to oauth-client-info.ts	Failing tests
drop the !value required-secret guard	3
drop the rejectBlank blank-value clause	1
env branch returns inline secret instead	3
drop the no-oauthClientId undefined gate	1
invert the client_secret conditional spread	2
invert the redirect_uris conditional spread	2
invert the token_endpoint_auth_method spread	1
break the fixed grant_types array	1
(reverted — control)	0 (15 pass)

$ # M1 re-confirmed on rebased head: drop `!value` guard
$ ./node_modules/.bin/vitest run tests/oauth-client-info.test.ts   # → 3 failed | 12 passed (15)
$ # source reverted byte-identical
$ ./node_modules/.bin/vitest run tests/oauth-client-info.test.ts   # → 15 passed (15)

Format / lint / types clean on the new file:

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

Scope note: one new *.test.ts file under tests/, +112 / -0, no production code touched. Direct sibling of the just-landed #246 (OAuth token-generation coverage) — same module family, mirror side.

_{Opened from a fork via the API; if GitHub's Allow edits by maintainers toggle isn't honored on this PR, a maintainer can still push to the branch or supersede-and-land.}

---

Generated by Claude Code

[clawsweeper]

Codex review: needs changes before merge. Reviewed August 7, 2026, 3:31 AM ET / 07:31 UTC.

ClawSweeper review

What this changes

The PR adds direct Vitest coverage for static OAuth client-information construction and OAuth client-secret resolution.

Merge readiness

⚠️ Needs maintainer review before merge - 3 items remain

Keep open for one small test-isolation repair: the new cleanup deletes an inherited environment value that later tests in the same worker may need.

Priority: P3
Reviewed head: 44e9275a34d51e706c6461b1d8fa15eb07552c56

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The direct coverage and mutation evidence are strong, with one small test-environment isolation defect remaining.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The PR body provides an after-fix direct Vitest transcript with 15 passing cases and a mutation matrix showing the suite catches representative regressions.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body provides an after-fix direct Vitest transcript with 15 passing cases and a mutation matrix showing the suite catches representative regressions.
Evidence reviewed	5 items	Focused patch: The reviewed head adds one 112-line OAuth test file; its cleanup deletes the environment key after every test. Current contract: Current main still resolves environment-backed OAuth secrets and conditionally includes them in static client information, so this suite targets a live behavior boundary. Repository test convention: An existing environment-sensitive test saves and restores its prior value, showing the applicable isolation pattern.
Findings	1 actionable finding	[P3] Restore the inherited test secret
Security	None	None.

How this fits together

The OAuth setup path turns a server definition plus an optional redirect URL into client metadata for an authorization server. It reads a secret from an environment reference or inline configuration and feeds the metadata into OAuth registration and token-refresh flows.

flowchart LR
  A[Server definition] --> B[OAuth client-info helper]
  C[Process environment] --> D[Secret resolver]
  D --> B
  E[Redirect URL] --> B
  B --> F[OAuth client metadata]
  F --> G[OAuth registration and refresh]
  H[New direct tests] --> B

Loading

Before merge

• Restore the inherited test secret (P3) - This cleanup always deletes the key, so a runner that supplied MCPORTER_TEST_CLIENT_INFO_SECRET loses it for later tests in the same worker. Save the original value, clear it before each test, and restore it afterward.
• Resolve merge risk (P1) - Until the test restores any inherited MCPORTER_TEST_CLIENT_INFO_SECRET, it can alter the runner environment seen by later tests in the same worker.
• Complete next step (P2) - A single mechanical test-isolation repair can resolve the remaining review finding without changing OAuth behavior.

Findings

• [P3] Restore the inherited test secret — tests/oauth-client-info.test.ts:16-18

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Test-only scope	1 file added, +112 / -0	The change is a narrowly scoped direct-coverage suite with no production code change.

Merge-risk options

Maintainer options:

1. Isolate the test environment (recommended)
   Save the inherited secret, clear it before each case, and restore it afterward before merging the focused test suite.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Preserve and restore MCPORTER_TEST_CLIENT_INFO_SECRET in the test setup; clear it before each test so unset cases remain deterministic.

Technical review

Best possible solution:

Preserve the inherited test-only secret, clear it before each case, and restore it afterward so coverage remains deterministic without leaking environment changes.

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

Yes. Source inspection shows every case mutates the process environment while cleanup unconditionally deletes the key, reproducing the inherited-value loss without a live OAuth server.

Is this the best way to solve the issue?

Yes, after the isolation repair. Direct coverage is the narrowest maintainable protection for these pure OAuth metadata helpers, but it must restore its test environment.

Full review comments:

• [P3] Restore the inherited test secret — tests/oauth-client-info.test.ts:16-18
  This cleanup always deletes the key, so a runner that supplied MCPORTER_TEST_CLIENT_INFO_SECRET loses it for later tests in the same worker. Save the original value, clear it before each test, and restore it afterward.
  Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

• P3: This is a focused regression-test improvement with only low-risk test-isolation follow-up.
• merge-risk: 🚨 automation: The cleanup can remove an inherited process environment value and affect later tests running in the same worker.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body provides an after-fix direct Vitest transcript with 15 passing cases and a mutation matrix showing the suite catches representative regressions.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides an after-fix direct Vitest transcript with 15 passing cases and a mutation matrix showing the suite catches representative regressions.

Evidence

Acceptance criteria:

• [P1] pnpm exec vitest run tests/oauth-client-info.test.ts.
• [P1] pnpm check.
• [P1] pnpm test.

What I checked:

• Focused patch: The reviewed head adds one 112-line OAuth test file; its cleanup deletes the environment key after every test. (tests/oauth-client-info.test.ts:16, 44e9275a34d5)
• Current contract: Current main still resolves environment-backed OAuth secrets and conditionally includes them in static client information, so this suite targets a live behavior boundary. (src/oauth-client-info.ts:4, 5f30f68d044b)
• Repository test convention: An existing environment-sensitive test saves and restores its prior value, showing the applicable isolation pattern. (tests/runtime-header-utils.test.ts:8, 5f30f68d044b)
• Re-review continuity: The current head is the same SHA previously reviewed, and the prior environment-restoration finding remains at the same changed lines. (tests/oauth-client-info.test.ts:16, 44e9275a34d5)
• Implementation provenance: History and blame show the current OAuth client-information module was introduced in this commit. (src/oauth-client-info.ts:1, 49dcd3e7fffd)

Likely related people:

• steipete: Blame attributes the current OAuth client-information helper to this commit. (role: current implementation introducer; confidence: high; commits: 49dcd3e7fffd; files: src/oauth-client-info.ts)
• KrasimirKralev: A previously merged PR added direct coverage for the adjacent OAuth token-generation helper module. (role: adjacent OAuth coverage contributor; confidence: high; commits: ced43e458a8b; files: tests/oauth-token-generation.test.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Restore the inherited environment value and rerun the focused Vitest file.

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 (7 earlier review cycles)

• reviewed 2026-08-06T10:30:33.427Z sha 44e9275 :: needs changes before merge. :: [P3] Restore the inherited test secret after each test
• reviewed 2026-08-06T11:39:31.781Z sha 44e9275 :: needs changes before merge. :: [P3] Isolate and restore the test environment variable
• reviewed 2026-08-06T13:17:11.932Z sha 44e9275 :: needs changes before merge. :: [P3] Preserve the inherited test environment value
• reviewed 2026-08-07T01:53:21.489Z sha 44e9275 :: needs changes before merge. :: [P3] Restore the inherited test secret
• reviewed 2026-08-07T03:54:04.815Z sha 44e9275 :: needs changes before merge. :: [P3] Restore the inherited environment value
• reviewed 2026-08-07T05:59:30.779Z sha 44e9275 :: needs changes before merge. :: [P3] Restore the inherited environment value
• reviewed 2026-08-07T06:11:37.724Z sha 44e9275 :: needs changes before merge. :: [P3] Restore the inherited test secret