fix(web): declare the three remaining implicit return types (#1381 review residue)

[groupthinking]

Canonical issue

Closes #1383 (shares the canonical issue of its base PR #1381 — stacked follow-up, not a competing implementation).

Base is groupthinking-fix-upstream-error-leakage, not main.

This PR was rewritten. It originally carried the unguarded-fetch fix and the error-code contract. While it was open, ae2be83 and 5e69b7e landed both directly on #1381's branch from another session. Rather than merge a conflicting duplicate, this branch was reset onto their head and reduced to the only thing still missing. The original commit f9f964e is gone; see the note below for why that is the right outcome.

Outcome

Three helpers still had implicit return types after ae2be83 and 5e69b7e. Those commits typed upstreamFailure, upstreamUnreachable, and the activateRequest / webhookRequest / transcribeRequest helpers; these were missed:

Helper	Declared type
getOpenAiHeaders (realtime/session/route.ts)	Record<string, string> | null
sdpRequest (realtime-session-route.test.ts)	Request
upstreamResponse (realtime-session-route.test.ts)	Response

This closes out CodeRabbit's strict-mode path instruction ("flag implicit any, missing return types") for the routes #1381 touches. getOpenAiHeaders already returned exactly that union on both of its paths, so the annotation is descriptive, not a narrowing.

Scope

• Included: types only, in two files.
• Explicitly excluded: everything already landed in ae2be83 / 5e69b7e, and the "add the copilot-rabbit label and obtain an explicit Copilot approval" thread — a repository process gate, not a code change, and not something an agent should self-grant.

Why the original commit was dropped rather than merged

Both implementations were compared line by line before discarding mine. Theirs is better on every overlapping point:

• Transport guard. upstreamUnreachable wraps the fetch and await upstream.text(), so a body read that rejects mid-stream is caught too. My fetchUpstream only wrapped the call itself and would have let that case escape. Their version also carries a third regression test for exactly that path.
• Turnstile code. Theirs returns a stable code: 'turnstile_rejected' while keeping error verbatim. Mine made code mirror the dynamic turnstile.error, which is a weaker contract — a machine-readable key that varies isn't a stable key.

Merging the original would have regressed both. Dropping it was the correct call, not a concession.

Risk

• Risk level: low — type annotations only.
• Failure mode: none at runtime; annotations are erased at compile time. No response shape, status code, or test expectation changes.
• Rollback: revert this single commit.

Verification

Run against head 855dcec.

$ cd apps/web && npm run type-check   # tsc --noEmit
  (clean, exit 0)

$ cd apps/web && npm run lint         # eslint src middleware.ts
  (clean, exit 0)

$ cd apps/web && npm test             # vitest run
  Test Files  1 failed | 48 passed (49)
       Tests  1 failed | 271 passed (272)

271 passed / 1 failed matches 5e69b7e exactly — this PR adds no tests and changes no counts. The single failure is billing-chat-gating.test.ts:35 "blocks free tier after daily quota", 5000 ms timeout: pre-existing, unrelated, tracked in #1116.

• Focused tests — full web suite, counts identical to base
• Required CI — agent-completion/truth-gate is structurally unsatisfiable here; see below
• Review threads resolved — the substantive findings landed in ae2be83 / 5e69b7e; this is the residue

Production evidence

Not applicable: type-only change stacked on an unmerged PR, so there is no independent deployment surface and no runtime behavior to observe. Production evidence belongs to #1381.

Known blocker — agent-completion/truth-gate

Cannot pass on this PR and is not worth retrying. agentTaskApplicable (.github/workflows/pr-checks.yml:646-649) engages the agent-task policy on any claude/* head branch; the gate then requires a GitHub-native closing-issue reference, which GitHub only creates for PRs targeting the default branch. This PR targets groupthinking-fix-upstream-error-leakage, so no issue resolves and policy.agent_login / policy.run_id — read from the linked issue body at pr-checks.yml:2162-2165 — come back empty. Full analysis in this comment.

Merging this into groupthinking-fix-upstream-error-leakage moots it: the change is then covered by #1381's own gate, which is green.

Agent handoff

• One canonical issue is linked
• No competing PR implements the same issue — reduced specifically to avoid overlapping ae2be83 / 5e69b7e
• Acceptance criteria are satisfied
• Required checks pass on the current head — truth-gate blocked structurally, as above
• Human decision is requested only for product, security, irreversible infrastructure, or production approval — merge is left to a human; nothing was auto-merged

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)

• [‘architecture-gap’, ‘bug’, ‘ci-cd’, ‘ci/cd’, ‘copilot-rabbit’, ‘documentation’, ‘duplicate’, ‘enhancement’, ‘frontend’, ‘github_actions’, ‘good first issue’, ‘help wanted’, ‘high-priority’, ‘invalid’, ‘javascript’, ‘ml-model’, ‘needs-triage’, ‘pipeline-critical’, ‘placeholder-code’, ‘priority:high’, ‘python’, ‘python:uv’, ‘question’, ‘styling’, ‘tests’, ‘v0’]

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8dc749c7-03c1-4256-8847-9190c0bad8e6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Agent Completion Truth Gate: BLOCKED

Reasons: invalid_payload

Machine-readable verdict

{
  "details": {
    "collection_errors": [
      "incomplete_linked_issue_contract",
      "missing_closing_issue_reference",
      "linked_issue_not_agent_task",
      "missing_intent_snapshot",
      "missing_agent_run_id",
      "missing_agent_login"
    ],
    "invalid_fields": [
      "policy.agent_login",
      "policy.run_id"
    ]
  },
  "reasons": [
    "invalid_payload"
  ],
  "verdict": "blocked"
}

Workflow evidence

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
v0-uvai	Ready	Preview, v0	Aug 5, 2026 10:01am

[groupthinking]

agent-completion/truth-gate — blocked, and not fixable from this PR

Traced it rather than retrying. The gate is structurally unsatisfiable while this PR is stacked, and the cause is worth recording because it is systemic, not specific to this branch.

Why the gate applies here but not to #1381. agentTaskApplicable in .github/workflows/pr-checks.yml:646-649 sets applicable from the head branch name:

const agentBranch =
  /^(?:agent|claude|codex|copilot|jules)[/-]/i.test(pull.head.ref);

This branch is claude/clever-heisenberg-eleejm, so the agent-task policy engages. #1381's branch is groupthinking-fix-upstream-error-leakage, which does not match, so its gate resolves not_applicable and passes.

Why it cannot pass. Once applicable, the collector needs exactly one GitHub-native closing-issue reference (pr-checks.yml:591), and then reads expectedRunId and expectedAgentLogin out of that issue's body, from ## Agent Run ID and ## Agent Login sections (pr-checks.yml:2162-2165):

const expectedRunId = section(issueBody, ['agent run id', 'run id'])
const expectedAgentLogin = section(issueBody, ['agent login'])

GitHub only creates closing-issue references for pull requests targeting the default branch. This PR targets groupthinking-fix-upstream-error-leakage, so linkedIssues is empty regardless of the Closes #1383 line in the body — which is exactly what the verdict reports as missing_closing_issue_reference. Every other collection error follows from it: with no linked issue there is no issue body, hence missing_agent_run_id, missing_agent_login, linked_issue_not_agent_task, incomplete_linked_issue_contract. policy.agent_login and policy.run_id are then empty, which is the invalid_payload verdict.

Adding an agent-lock-manifest block to this body does not fix it. The manifest is only ever compared against the issue-derived values (pr-checks.yml:2168, 2173); it cannot supply them.

The two ways out are both outside what this PR should decide:

1. Re-target this PR at main. The gate would then resolve a linked issue — but the diff would duplicate all of fix(web): stop leaking upstream and Stripe error details to clients #1381, which trips the competing-PR gate and defeats the point of stacking.
2. Open an agent-task issue carrying ## Agent Run ID and ## Agent Login sections plus a dispatched intent snapshot, and target main against it. That requires an issue-dispatched agent run, which this session is not.

Recommendation: merge this into groupthinking-fix-upstream-error-leakage (base of this PR), so the fixes land inside #1381 and are covered by #1381's own gate, which is green. This PR then closes as merged and the question is moot.

Worth flagging that this also explains #1356's agent-completion/truth-gate failure — same claude/-prefixed branch, no resolvable agent-task issue. Any PR pushed to a claude/* branch without an issue-dispatched agent run will fail this gate.

Everything else on this head is green or passing: type-check clean, eslint clean, npm test 272 passed / 1 failed (the pre-existing billing-chat-gating timeout, #1116), Vercel preview Ready.

---

Generated by Claude Code

[groupthinking]

@coderabbitai review

(Auto-review was skipped for lack of a qualifying label; javascript, security, and tests have since been applied.)

---

Generated by Claude Code

[coderabbitai]

@groupthinking I will review the pull request.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[groupthinking]

Closing as superseded — the work landed directly on this PR's own base branch instead.

Two sessions picked up the same five CodeRabbit threads on #1381 within minutes of each other. This PR branched from 672bac3; groupthinking-fix-upstream-error-leakage has since advanced to f5cb509, which now contains everything here. That is also why this PR reads mergeable_state: dirty — its diff and its base's diff touch the same lines because they are the same change.

Absorbed from this PR (in f5cb509, credited to the work here):

• getOpenAiHeaders → : Record<string, string> | null
• sdpRequest → : Request, upstreamResponse → : Response
• the { ok: false }-with-no-error Turnstile test — the branch had the optional-field fallback but nothing exercised it

Already on the branch, so not re-applied: the full code contract across all eleven branches, the NextRequest/Request test-helper annotations, and upstreamFailure: Response (5e69b7e), plus transport-failure handling (ae2be83).

One difference, deliberate. fetchUpstream here guards only the fetch call:

try { return await fetch(url, init); } catch { … }
// …then, outside the guard:
const body = await upstream.text();

upstream.text() can itself reject — a connection reset after headers arrive but before the body completes — and that rejection escapes the route to the framework's own error page, which is the disclosure this module exists to prevent. upstreamUnreachable on the branch wraps fetch and the body read in one try, so both degrade to the same fixed 502. Keeping the stricter guarantee.

Two small contract choices also differ, both in the branch's favour:

• realtime_not_configured rather than openai_api_key_missing — the branch's code names the condition the caller can act on, not the specific env var, which is server-internal.
• checkout returns a fixed code: 'turnstile_rejected' while error stays verbatim. This PR set code: turnstile.error ?? 'turnstile_rejected', which makes the code vary with the reason — a client branching on code would then need to enumerate all four Turnstile outcomes to detect one condition.

Nothing is lost by closing; reopen if you disagree with any of the above. Verified on f5cb509: npm test 272 passed / 1 failed (the pre-existing billing-chat-gating timeout, #1116), lint and type-check clean.

---

Generated by Claude Code

[groupthinking]

Closing as superseded by #1381 — the two branches independently implemented the same CodeRabbit remediation on top of ae2be83, and #1381's version is the more complete one.

Why this PR is redundant. Both heads address the same three findings. Comparing 5e69b7e (#1381 head) against f9f964e (this head):

Finding	#1381 5e69b7e	This PR f9f964e
Rejected upstream fetch → fixed 502	✅ try/catch wraps fetch and await upstream.text()	⚠️ wraps fetch only
code on missing-key / invalid-SDP branches	✅ realtime_not_configured, invalid_sdp_offer	✅ openai_api_key_missing, invalid_sdp_offer
upstreamFailure return type	✅ : Response	✅ : Response
code across billing routes	✅ activate, checkout, renew, webhook	✅ same
Test helper return types	✅ activateRequest/webhookRequest: NextRequest, transcribeRequest: Request	✅ same

The one behavioural difference is the first row, and it favours #1381. This PR's fetchUpstream helper returns the Response and the caller then does await upstream.text() outside the try — so a connection reset mid-body still throws past the handler and renders the framework's own error page, which is the uncontrolled disclosure the change exists to prevent. #1381 keeps the body read inside the guarded block and has a regression test for it (returns the static 502 when the upstream body cannot be read) that this PR does not.

Test coverage is otherwise a superset on #1381: it carries the rejected-GET and rejected-POST cases this PR adds, plus the body-read case, and folds the missing-key/invalid-SDP code assertions into the existing branch tests rather than adding three separate ones.

Concrete reason to close rather than leave open. The Canonical issue and evidence and PR Governance checks on #1381 are both red for exactly one reason:

Issue #1383 already has another open implementation PR: #1402

Two open PRs claim Closes #1383, so the canonical-delivery gate blocks the one that should ship. Closing this one clears that gate.

One thing worth keeping from this branch. getOpenAiHeaders gains an explicit : Record<string, string> | null return type here, and #1381 leaves it inferred. That is a genuine (minor) gap against the repo's TypeScript-strict path instruction. It was never raised as a review comment on #1381, and opening a separate PR for it would recreate the competing-PR block this close is resolving — so it is recorded here as a follow-up rather than filed now.

Nothing is lost by closing: reopen if the comparison above is wrong on any row.

---

Generated by Claude Code

[groupthinking]

Correction to my closing comment above — it was written against f9f964e and this PR was force-pushed to 855dcec while I was posting. The close still stands, but the accurate reason is stronger than the one I gave.

My comment compared the original head (the fetchUpstream rewrite) and argued it was superseded. By the time it landed, this branch had already been reset onto f5cb509 and reduced to three return-type annotations. So disregard that table — it describes a commit that no longer exists here.

Here is the state as of now. f5cb509 on groupthinking-fix-upstream-error-leakage already contains all three annotations this PR was reduced to:

$ git show f5cb509:apps/web/src/app/api/realtime/session/route.ts | grep -n 'function getOpenAiHeaders'
88:function getOpenAiHeaders(contentType?: string): Record<string, string> | null {

$ git show f5cb509:apps/web/src/app/api/__tests__/realtime-session-route.test.ts | grep -n 'function sdpRequest\|function upstreamResponse'
7:function sdpRequest(body: string): Request {
15:function upstreamResponse(

f5cb509's commit message says as much — it deliberately absorbed those three items, and independently reached the same conclusion this PR's own description reached about the fetchUpstream rewrite (guards fetch but not await upstream.text()). The two lines of work converged.

That leaves this PR's diff against its current base as a net removal:

$ git diff f5cb509 855dcec
 apps/web/src/app/api/__tests__/billing-checkout-route.test.ts | 18 ------------------

The only change remaining is deleting falls back to a stable error and code when Turnstile reports no reason — the test covering TurnstileVerifyResult.error being optional, so an { ok: false } result with no reason still yields { error: 'turnstile_rejected', code: 'turnstile_rejected' } rather than { error: undefined }. Merging this would drop that coverage and gain nothing.

The structurally-unsatisfiable agent-completion/truth-gate documented in the PR body also stops being a problem, since there is nothing left to land.

Separately, this unblocks #1381: its Canonical issue and evidence and PR Governance checks were failing only on Issue #1383 already has another open implementation PR: #1402.

Reopen if the git diff above is wrong — that single command is the whole case for closing.

---

Generated by Claude Code