iframe-proxy

mingqi · 2026-10-05T21:13:44Z

Summary

This PR fixes three problems in how the proxy handles requests to Copilot, and adds one small performance change.

1. Upstream requests are aborted when the client disconnects

Problem: If a client cancelled a streaming request (for example, pressed Esc in Claude Code), the proxy kept reading the Copilot stream until it finished. That wasted quota and kept connections open.

Fix:

createChatCompletions now accepts an optional AbortSignal and passes it to both fetch and events().
Both /v1/chat/completions and /v1/messages create an AbortController inside streamSSE. It is aborted from stream.onAbort() and in a finally block.
AbortError is no longer logged as an error. Other streaming errors are still logged.
Streaming and non-streaming paths are now separate branches. Each one checks that it got the response type it expected and throws if not.

2. HTTP connection pooling (Node only)

Problem: The default undici Agent was used without explicit limits.

Fix: Added initDirectConnectionPool(), which sets a global undici Agent with 16 connections per origin and pipelining: 1. It runs when --proxy-env is not set. The direct agent inside initProxyFromEnv uses the same settings. This is skipped under Bun, as before.

3. Assistant prefill / conversations that end on an assistant turn

Problem: Anthropic clients can send a conversation whose last message is from the assistant (a "prefill"). Copilot's OpenAI-style endpoint does not handle that, so the request either fails or returns an empty or unexpected completion.

Fix: When the translated conversation ends with an assistant message that has no tool calls, a { role: "user", content: "Continue." } message is added at the end.

4. Debug logging is cheaper

JSON.stringify of payloads, chunks, and responses now runs only when consola.level >= 4 (debug). Before this, every streamed chunk was serialized even when debug logging was off.

Testing

tests/anthropic-request.test.ts: added a test for assistant prefill, and updated the existing thinking-block test to expect the trailing Continue. message.
tests/create-chat-completions.test.ts: added a test that checks the abort signal is passed to the upstream fetch.
bun test, bun run lint

Notes

The prefill handling is a workaround. The model continues after a "Continue." prompt, not directly from the prefilled text, so the output may not join seamlessly onto the prefix.

🤖 Generated with Claude Code

…l continuation - Abort upstream Copilot requests when the client disconnects during streaming - Configure undici connection pool (16 connections per origin) - Skip debug JSON serialization unless debug logging is enabled - Append a 'Continue.' user message when the conversation ends with an assistant turn Co-Authored-By: Claude <noreply@anthropic.com>

coderabbitai · 2026-10-05T21:13:58Z

Walkthrough

Direct connections use a shared agent configuration when environment proxying is disabled. The chat-completions and messages handlers select streaming or non-streaming behavior from the request setting, pass abort signals for streamed requests, and gate debug logs by log level. Copilot requests retry once after a 401 or 403 response by refreshing the token. Message translation appends a user message containing "Continue." for qualifying assistant-ending conversations.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to a0447

Streaming requests that fail upstream, for example on an authentication or token problem, may now get a 200 streaming response instead of a clear HTTP error. The token-refresh retry path can also hide the real error. Resolve both before merging.

Security architecture risk: 🟡 Moderate · up to 1d2bc

Streaming cancellation improves normal request cleanup, but failed upstream requests bypass the existing error-body cleanup path. This creates an availability risk for the shared connection pool. Server-configured credentials, destinations, and explicit proxy selection remain preserved.

Retained concerns

Medium · security · inferred: Streaming acquisition failures bypass the previous error-body consumer and the new abort cleanup. Repeated upstream rejections with unread bodies could delay connection reclamation and impair other requests sharing the bounded direct pool. Normal streaming cleanup does not cover this acquisition-error terminal state.

Security review details

Security Blast Radius

inferred — The identified availability risk crosses request ownership within one server process and Copilot origin. Completion calls explicitly use the shared dispatcher; model retrieval uses the same state-derived origin and is expected to share the global transport under Node’s fetch contract. Deployment-wide or cross-tenant exposure is not established.

Security Findings and Attack Paths

inferred — A client able to reach a completion endpoint and pass configured gates can submit streaming requests that receive upstream non-OK responses. Those failures now leave no application-level body consumer or guaranteed controller abort. If unread bodies delay connection reclamation, repeated failures can consume shared pool capacity. Exhaustion timing and exploitability were not demonstrated; small or automatically completed error bodies are important limiting counterevidence.

Trust Boundaries and Controls

observed — The changed branches preserve rate-limit-before-approval ordering and perform upstream creation only after approval succeeds when enabled. Approval and rate limiting remain optional server configuration. Neither control establishes tenant authentication, and no new request-controlled destination, dispatcher, or bearer credential was introduced.

Resilience and Maintainability Implications

observed — Cancellation ownership begins inside the streaming callback after approval. Non-streaming requests still pass no signal, as before this PR. Approval-time interruption therefore remains a dependency-contract uncertainty rather than an established introduced regression; already-disconnected callback behavior could not be verified from available dependency source.

Hardening Proposals

proposed — Give upstream acquisition and stream processing one terminal-cleanup owner, explicitly dispose of failed response bodies, and preserve a protocol-visible failure outcome when an HTTP error can no longer be forwarded normally.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly summarizes the main changes: aborting upstream requests, pooling connections, and handling assistant prefill.
Description check		The description directly explains the implemented changes, affected behavior, testing, and known limitations.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/lib/token.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

src/services/copilot/create-chat-completions.ts

ESLint skipped: the matched ESLint configuration already failed (missing-dependency).

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

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

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 0dc4141b-7209-46b7-83f7-6f91c112e282

📥 Commits

Reviewing files that changed from the base of the PR and between 0ea08fe and 1d2bc7d.

📒 Files selected for processing (8)

src/lib/proxy.ts
src/routes/chat-completions/handler.ts
src/routes/messages/handler.ts
src/routes/messages/non-stream-translation.ts
src/services/copilot/create-chat-completions.ts
src/start.ts
tests/anthropic-request.test.ts
tests/create-chat-completions.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📜 Review details

🔇 Additional comments (6)

src/lib/proxy.ts (1)

5-21: LGTM!

Also applies to: 27-27

src/start.ts (1)

10-10: LGTM!

Also applies to: 33-34

src/routes/messages/non-stream-translation.ts (1)

71-78: LGTM!

tests/anthropic-request.test.ts (1)

157-161: LGTM!

Also applies to: 163-182

src/services/copilot/create-chat-completions.ts (1)

3-3: LGTM!

Also applies to: 11-11, 33-33, 37-43, 51-51

tests/create-chat-completions.test.ts (1)

58-69: LGTM!

coderabbitai · 2026-10-05T21:16:43Z

+          controller.abort()
+        })
+
+        const response = await createChatCompletions(payload, controller.signal)


🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Resolve upstream failures before returning an SSE response. Both routes now await the Copilot request inside the streamSSE callback. If the token is missing or Copilot rejects the request, Hono has already returned the SSE response. Hono then writes an error event instead of allowing an HTTP error response. (github.com)

src/routes/chat-completions/handler.ts#L62-L62: await the upstream connection before calling streamSSE; retain the controller for disconnect cancellation.

src/routes/messages/handler.ts#L58-L61: make the same change before starting the translated SSE response.

📍 Affects 2 files

src/routes/chat-completions/handler.ts#L62-L62 (this comment)

src/routes/messages/handler.ts#L58-L61

The Copilot token can be revoked server-side before its scheduled refresh, leaving the proxy returning 403 forbidden until restart. On 401/403 from chat/completions, fetch a fresh Copilot token (deduplicated across concurrent requests) and retry once. Also stop the periodic refresh from rethrowing inside setInterval. Co-Authored-By: Claude <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 7229c57d-9ad2-41d6-b1b5-1d81efa9aea1

📥 Commits

Reviewing files that changed from the base of the PR and between 1d2bc7d and a04477c.

📒 Files selected for processing: 2

src/lib/token.ts
src/services/copilot/create-chat-completions.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📜 Review details

🧰 Additional context used

🪛 ast-grep (0.45.3)

src/lib/token.ts

[warning] 53-53: Avoid logging sensitive data
Context: consola.info("Refreshed Copilot token:", token)
Note: [CWE-532] Insertion of Sensitive Information into Log File.

(log-sensitive-data-typescript)

🔇 Additional comments: 2

src/lib/token.ts: 1

31-37: LGTM!

Also applies to: 39-59

src/services/copilot/create-chat-completions.ts: 1

8-8: LGTM!

Also applies to: 28-44

coderabbitai · 2026-10-17T17:21:50Z

+    try {
+      await refreshCopilotToken()
+      await response.body?.cancel()
+      response = await send()
+    } catch (error) {
+      consola.error("Failed to refresh Copilot token:", error)
+    }


🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Limit the try/catch to refreshCopilotToken(). Let errors from the retried send() propagate.

The catch block also handles errors from response.body?.cancel() and from the retried send(). Here is the failure sequence:

The refresh succeeds.

Line 56 cancels the body of the first 401/403 response.

The retried send() rejects. This happens on a network error, or with an AbortError when the client disconnects during the refresh.

Line 59 logs "Failed to refresh Copilot token", which is wrong because the refresh succeeded.

response still points to the first 401/403 response, whose body is already cancelled.

Line 65 throws HTTPError with that response. A downstream error handler that reads error.response.text() or .json() then fails on the cancelled body.

The result is that the real error (network failure or abort) is lost. The client gets a misleading 401/403 or an error-handler failure. The route handlers check for error.name !== "AbortError" to skip logging aborts, so an AbortError swallowed here also gets logged as a token-refresh failure.

Catch only the refresh failure. If the refresh fails, keep the first response and do not cancel its body.

🐛 Proposed fix

--- "a/src/services/copilot/create-chat-completions.ts" +++ "b/src/services/copilot/create-chat-completions.ts" @@ -51,13 +51,17 @@ consola.warn( `Copilot returned ${response.status}, refreshing Copilot token and retrying`, ) - try { - await refreshCopilotToken() - await response.body?.cancel() - response = await send() - } catch (error) { - consola.error("Failed to refresh Copilot token:", error) - } + let refreshed = false + try { + await refreshCopilotToken() + refreshed = true + } catch (error) { + consola.error("Failed to refresh Copilot token:", error) + } + if (refreshed) { + await response.body?.cancel() + response = await send() + } } if (!response.ok) {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

try {

await refreshCopilotToken()

await response.body?.cancel()

response = await send()

} catch (error) {

consola.error("Failed to refresh Copilot token:", error)

}

let refreshed = false

try {

await refreshCopilotToken()

refreshed = true

} catch (error) {

consola.error("Failed to refresh Copilot token:", error)

}

if (refreshed) {

await response.body?.cancel()

response = await send()

}

mingqi changed the title ~~feat: stream abort handling, connection pooling, and assistant-prefil…~~ fix: abort upstream requests on client disconnect, pool connections, handle assistant prefill Oct 5, 2026

coderabbitai Bot requested changes Oct 5, 2026

View reviewed changes

coderabbitai Bot requested changes Oct 17, 2026

View reviewed changes

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

fix: abort upstream requests on client disconnect, pool connections, handle assistant prefill - #278

fix: abort upstream requests on client disconnect, pool connections, handle assistant prefill#278
mingqi wants to merge 2 commits into
ericc-ch:masterfrom
mingqi:master

mingqi commented Oct 5, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 5, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Oct 5, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Oct 17, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Sunbelt Computer Software

PL/B Language Development and Support

Conversation

mingqi commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

1. Upstream requests are aborted when the client disconnects

2. HTTP connection pooling (Node only)

3. Assistant prefill / conversations that end on an assistant turn

4. Debug logging is cheaper

Testing

Notes

Uh oh!

coderabbitai Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mingqi commented Oct 5, 2026 •

edited

Loading

coderabbitai Bot commented Oct 5, 2026 •

edited

Loading