Skip to content

Conversation

@caozhiyuan
Copy link
Contributor

@caozhiyuan caozhiyuan commented Nov 3, 2025

This pull request introduces a new configuration system, structured logging, and support for the /v1/responses endpoint, along with improvements to model selection and request handling. The most important changes are grouped below:

Responses API Integration:

  • Added full support for the /v1/responses endpoint, including a new handler (src/routes/responses/handler.ts) that validates model support, streams or returns results, and logs all activity.
  • Enhanced src/routes/messages/handler.ts to route requests to the Responses API when supported by the selected model, including translation logic for payloads and results.
  • Updated the API documentation in README.md to include the new /v1/responses endpoint and clarify its purpose.

Configuration Management:

  • Added a new src/lib/config.ts module to provide persistent application configuration, including support for model-specific prompts, reasoning effort levels, and default model selection. Configuration is stored in a new config.json file in the app data directory, with automatic creation and safe permissions. [1] [2]

Logging Improvements:

  • Implemented a new buffered, file-based logging utility in src/lib/logger.ts for handler-level logging, with log rotation, retention, and structured output. Integrated this logger into key request handlers for better diagnostics. [1] [2] [3] [4] [5]

Token Counting Logic:

  • Refactored token counting in src/lib/tokenizer.ts to more accurately account for tool calls, array parameters, and model-specific behaviors (including GPT and Anthropic/Grok models). Added support for excluding certain schema keys and improved calculation for nested parameters. [1] [2] [3] [4] [5] [6] [7] [8]

…arsing and allign with vscode-copilot-chat extractThinkingData, otherwise it will cause miss cache occasionally
…ing small model if no tools are used

2.add bun idleTimeout = 0
3.feat: Compatible with Claude code JSONL file usage error scenarios, delay closeBlockIfOpen and map responses api to anthropic  support tool_use and fix spelling errors
4.feat: add configuration management with extra prompt handling and ensure config file creation
…just runServer to set verbose level correctly
@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Walkthrough

This PR introduces a new Responses endpoint (POST /v1/responses) with associated infrastructure including configuration management, per-handler logging, and bidirectional translation between Anthropic and internal Copilot response formats. Existing endpoints are updated to optionally route through the new API, with token counting logic enhanced for improved granularity.

Changes

Cohort / File(s) Summary
Documentation & Configuration
README.md, src/lib/config.ts, src/lib/paths.ts, src/lib/state.ts
Added config.json endpoint documentation; introduced CONFIG_PATH constant; exposed AppConfig interface with optional model-specific settings (extra prompts, reasoning effort, small model); added verbose flag to State.
Logging Infrastructure
src/lib/logger.ts
New per-handler logging module with daily log file rotation, buffer-based flushing, automatic cleanup of logs older than 7 days, and graceful shutdown handling.
Core Copilot Services
src/services/copilot/create-responses.ts, src/services/copilot/get-models.ts
Defined complete Responses API types (payload, streaming events, result structures); added createResponses function for HTTP integration; extended Model interface with vision, streaming, and structured_outputs support flags.
Response Translation Layer
src/routes/messages/responses-translation.ts, src/routes/messages/responses-stream-translation.ts
New bidirectional translators converting between Anthropic-format messages and internal Copilot payload/response structures; handles text, images, function calls, reasoning blocks, and metadata; includes streaming event state management.
Responses Endpoint
src/routes/responses/route.ts, src/routes/responses/handler.ts, src/routes/responses/utils.ts
New POST endpoint with rate limiting, model capability validation, manual approval workflow, and streaming support via Server-Sent Events; utility helpers to extract vision and initiator metadata.
Existing Endpoint Updates
src/routes/chat-completions/handler.ts, src/routes/messages/handler.ts, src/routes/messages/count-tokens-handler.ts, src/routes/messages/anthropic-types.ts
Integrated new logger (chat-completions); added conditional routing to Responses API with model warmup logic (messages); adjusted Grok token multiplier and tool-prompt gate logic; added signature field to AnthropicThinkingBlock.
Token Counting Enhancement
src/lib/tokenizer.ts
Refactored tool token calculation with model-specific differentiation (GPT vs. non-GPT); added array parameter support; enhanced properties field tokenization via new calculatePropertiesTokens helper; improved JSON serialization granularity.
Bootstrap & Server
src/start.ts, src/server.ts
Lazy-loaded server module; enabled verbose state assignment; added idle timeout configuration; registered Responses routes at /responses and /v1/responses.
Tests
tests/anthropic-request.test.ts, tests/responses-stream-translation.test.ts, tests/responses-translation.test.ts
Added signature fields to thinking block fixtures; new test suites for responses stream event translation and bidirectional payload translation with function call and reasoning block coverage.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Server
    participant ResponsesAPI as Copilot<br/>Responses API
    participant Translator
    participant Logger

    rect rgb(230, 245, 255)
    Note over Client,Server: Non-streaming Responses Request
    Client->>Server: POST /v1/responses
    Server->>Logger: debug: request received
    Server->>Server: validate payload & model capability
    Server->>Translator: translateAnthropicMessagesToResponsesPayload()
    Translator-->>Server: ResponsesPayload
    Server->>ResponsesAPI: POST /responses with payload
    ResponsesAPI-->>Server: ResponsesResult
    Server->>Translator: translateResponsesResultToAnthropic()
    Translator-->>Server: AnthropicResponse
    Server-->>Client: JSON response
    end

    rect rgb(245, 245, 230)
    Note over Client,Server: Streaming Responses Request
    Client->>Server: POST /v1/responses (stream=true)
    Server->>Logger: debug: streaming requested
    Server->>ResponsesAPI: fetch /responses (stream)
    ResponsesAPI-->>Server: EventSource (streamed events)
    loop for each ResponseStreamEvent
        Server->>Translator: translateResponsesStreamEvent()
        Translator->>Logger: debug: event processed
        Translator-->>Server: Array<AnthropicStreamEventData>
        Server-->>Client: SSE chunk
    end
    end

    rect rgb(245, 230, 245)
    Note over Server,Logger: Logging Path
    Server->>Logger: createHandlerLogger("handler-name")
    Logger->>Logger: ensure logs/ directory
    Logger->>Logger: append to daily log file
    Logger->>Logger: flush buffer periodically
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

  • src/lib/tokenizer.ts: Complex refactoring of token calculation with model-specific branching; impacts core accounting logic and requires validation against token counting guidelines.
  • src/routes/messages/responses-stream-translation.ts: Dense event translation with stateful block lifecycle management, function-call argument validation, and error recovery; multiple event types with subtle sequencing requirements.
  • src/routes/messages/responses-translation.ts: Comprehensive bidirectional translation layer with extensive helper logic for message aggregation, block construction, and metadata extraction; numerous edge cases for content block handling.
  • src/routes/messages/handler.ts: Conditional routing logic; ensure both legacy and new Responses API paths are correctly gated and don't introduce regressions in existing chat-completions flow.
  • src/lib/logger.ts: Resource management and cleanup on process signals; verify proper stream closure and buffer flushing to prevent data loss.

Poem

🐰 A new responses path opens wide,
With streaming dreams and translated pride!
From Anthropic to Copilot we fly,
While loggers hop and tokens multiply.
Config and state now dance as one,
The Responses API's quest begun! 🚀

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title broadly covers three major feature areas (responses API, configuration system, and structured logging) but lacks specificity about which is the primary change. Consider a more specific title that emphasizes the primary change, e.g., 'Add Responses API support with configuration system and structured logging' or focus on the most significant feature if one should dominate.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ea08fe and 01d4adb.

📒 Files selected for processing (22)
  • README.md (1 hunks)
  • src/lib/config.ts (1 hunks)
  • src/lib/logger.ts (1 hunks)
  • src/lib/paths.ts (1 hunks)
  • src/lib/state.ts (1 hunks)
  • src/lib/tokenizer.ts (8 hunks)
  • src/routes/chat-completions/handler.ts (4 hunks)
  • src/routes/messages/anthropic-types.ts (1 hunks)
  • src/routes/messages/count-tokens-handler.ts (1 hunks)
  • src/routes/messages/handler.ts (5 hunks)
  • src/routes/messages/responses-stream-translation.ts (1 hunks)
  • src/routes/messages/responses-translation.ts (1 hunks)
  • src/routes/responses/handler.ts (1 hunks)
  • src/routes/responses/route.ts (1 hunks)
  • src/routes/responses/utils.ts (1 hunks)
  • src/server.ts (2 hunks)
  • src/services/copilot/create-responses.ts (1 hunks)
  • src/services/copilot/get-models.ts (2 hunks)
  • src/start.ts (2 hunks)
  • tests/anthropic-request.test.ts (2 hunks)
  • tests/responses-stream-translation.test.ts (1 hunks)
  • tests/responses-translation.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (13)
src/server.ts (1)
src/routes/responses/route.ts (1)
  • responsesRoutes (7-7)
src/routes/responses/handler.ts (6)
src/lib/logger.ts (1)
  • createHandlerLogger (148-182)
src/lib/rate-limit.ts (1)
  • checkRateLimit (8-46)
src/lib/state.ts (1)
  • state (21-27)
src/services/copilot/create-responses.ts (3)
  • ResponsesPayload (8-26)
  • createResponses (328-355)
  • ResponsesResult (113-131)
src/routes/responses/utils.ts (1)
  • getResponsesRequestOptions (6-13)
src/lib/approval.ts (1)
  • awaitApproval (5-15)
src/lib/logger.ts (2)
src/lib/paths.ts (1)
  • PATHS (10-14)
src/lib/state.ts (1)
  • state (21-27)
tests/responses-stream-translation.test.ts (3)
src/services/copilot/create-responses.ts (3)
  • ResponseOutputItemAddedEvent (270-275)
  • ResponseFunctionCallArgumentsDeltaEvent (247-253)
  • ResponseFunctionCallArgumentsDoneEvent (255-262)
src/routes/messages/responses-stream-translation.ts (2)
  • createResponsesStreamState (74-82)
  • translateResponsesStreamEvent (84-142)
src/routes/messages/anthropic-types.ts (1)
  • AnthropicStreamEventData (185-193)
src/routes/responses/route.ts (2)
src/routes/responses/handler.ts (1)
  • handleResponses (21-73)
src/lib/error.ts (1)
  • forwardError (15-47)
src/routes/messages/responses-translation.ts (3)
src/routes/messages/anthropic-types.ts (13)
  • AnthropicMessagesPayload (3-26)
  • AnthropicMessage (82-82)
  • AnthropicUserMessage (72-75)
  • AnthropicAssistantMessage (77-80)
  • AnthropicUserContentBlock (62-65)
  • AnthropicAssistantContentBlock (67-70)
  • AnthropicImageBlock (33-40)
  • AnthropicThinkingBlock (56-60)
  • AnthropicToolUseBlock (49-54)
  • AnthropicToolResultBlock (42-47)
  • AnthropicTextBlock (28-31)
  • AnthropicTool (84-88)
  • AnthropicResponse (90-112)
src/services/copilot/create-responses.ts (20)
  • ResponsesPayload (8-26)
  • ResponseInputItem (89-94)
  • ResponseInputContent (96-99)
  • ResponseInputMessage (57-62)
  • ResponseInputText (101-104)
  • ResponseInputImage (106-111)
  • ResponseInputReasoning (79-87)
  • ResponseFunctionToolCallItem (64-70)
  • ResponseFunctionCallOutputItem (72-77)
  • Tool (35-35)
  • ToolChoiceOptions (28-28)
  • ToolChoiceFunction (30-33)
  • ResponsesResult (113-131)
  • ResponseOutputItem (143-146)
  • ResponseOutputContentBlock (178-181)
  • ResponseOutputReasoning (156-162)
  • ResponseReasoningBlock (164-167)
  • ResponseOutputFunctionCall (169-176)
  • ResponseOutputText (183-187)
  • ResponseOutputRefusal (189-192)
src/lib/config.ts (2)
  • getReasoningEffortForModel (92-97)
  • getExtraPromptForModel (82-85)
src/routes/messages/handler.ts (10)
src/lib/logger.ts (1)
  • createHandlerLogger (148-182)
src/lib/state.ts (1)
  • state (21-27)
src/routes/messages/anthropic-types.ts (1)
  • AnthropicMessagesPayload (3-26)
src/lib/config.ts (1)
  • getSmallModel (87-90)
src/routes/messages/non-stream-translation.ts (2)
  • translateToOpenAI (29-47)
  • translateToAnthropic (281-327)
src/services/copilot/create-chat-completions.ts (2)
  • createChatCompletions (8-47)
  • ChatCompletionResponse (95-110)
src/routes/messages/responses-translation.ts (2)
  • translateAnthropicMessagesToResponsesPayload (48-87)
  • translateResponsesResultToAnthropic (352-374)
src/routes/responses/utils.ts (1)
  • getResponsesRequestOptions (6-13)
src/services/copilot/create-responses.ts (3)
  • createResponses (328-355)
  • ResponseStreamEvent (206-219)
  • ResponsesResult (113-131)
src/routes/messages/responses-stream-translation.ts (3)
  • createResponsesStreamState (74-82)
  • translateResponsesStreamEvent (84-142)
  • buildErrorEvent (606-612)
src/routes/responses/utils.ts (1)
src/services/copilot/create-responses.ts (2)
  • ResponsesPayload (8-26)
  • ResponseInputItem (89-94)
src/lib/config.ts (1)
src/lib/paths.ts (1)
  • PATHS (10-14)
src/routes/messages/responses-stream-translation.ts (3)
src/services/copilot/create-responses.ts (15)
  • ResponseStreamEvent (206-219)
  • ResponseCreatedEvent (233-237)
  • ResponseOutputItemAddedEvent (270-275)
  • ResponseOutputItemDoneEvent (277-282)
  • ResponseFunctionCallArgumentsDeltaEvent (247-253)
  • ResponseFunctionCallArgumentsDoneEvent (255-262)
  • ResponseTextDeltaEvent (302-309)
  • ResponseReasoningSummaryTextDeltaEvent (284-291)
  • ResponseReasoningSummaryTextDoneEvent (293-300)
  • ResponseTextDoneEvent (311-318)
  • ResponseCompletedEvent (221-225)
  • ResponseIncompleteEvent (227-231)
  • ResponseFailedEvent (264-268)
  • ResponseErrorEvent (239-245)
  • ResponsesResult (113-131)
src/routes/messages/anthropic-types.ts (1)
  • AnthropicStreamEventData (185-193)
src/routes/messages/responses-translation.ts (1)
  • translateResponsesResultToAnthropic (352-374)
src/routes/chat-completions/handler.ts (2)
src/lib/logger.ts (1)
  • createHandlerLogger (148-182)
src/lib/state.ts (1)
  • state (21-27)
src/services/copilot/create-responses.ts (3)
src/lib/state.ts (1)
  • state (21-27)
src/lib/api-config.ts (2)
  • copilotHeaders (20-37)
  • copilotBaseUrl (16-19)
src/lib/error.ts (1)
  • HTTPError (6-13)
src/start.ts (2)
src/lib/state.ts (1)
  • state (21-27)
src/server.ts (1)
  • server (13-13)
🔇 Additional comments (16)
src/lib/paths.ts (2)

8-8: LGTM!

The CONFIG_PATH constant follows the established pattern and integrates cleanly with the existing paths system.


19-19: LGTM!

Ensuring the config file exists at startup is the correct approach for persistent configuration.

src/lib/state.ts (1)

18-18: LGTM!

The verbose flag integrates cleanly with the new logging system and follows the existing state pattern.

src/routes/responses/route.ts (1)

1-15: LGTM!

The route setup follows established patterns and includes proper error handling via forwardError.

src/start.ts (3)

34-34: LGTM!

Setting state.verbose early ensures the logging system is properly configured before server startup.


117-117: Clarify the rationale for dynamic server import.

The server module is now imported dynamically rather than at the top level. While this can be beneficial for deferring initialization until after setup, it also makes the dependency less obvious.

Could you clarify:

  1. What specific issue does the dynamic import solve?
  2. Are there side effects in the server module that need to be delayed?
  3. Does this affect error reporting during server initialization?

122-124: Verify the implications of disabling idle timeout.

Setting idleTimeout: 0 disables the idle connection timeout, which means connections can remain open indefinitely. This could potentially:

  • Prevent proper resource cleanup
  • Allow connections to accumulate if clients don't properly close them
  • Impact server resource usage under certain conditions

Please verify:

  1. Is there a specific issue with Bun's default idle timeout behavior?
  2. Have you tested this under load with long-lived connections?
  3. Are there application-level timeouts to prevent resource exhaustion?
src/server.ts (2)

9-9: LGTM!

The import follows the established pattern for route modules.


25-25: LGTM!

Mounting the responses route at both /responses and /v1/responses (line 31) provides consistent API versioning compatibility.

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

139-139: LGTM!

The test properly includes the new required signature field on thinking blocks, maintaining consistency with the updated AnthropicThinkingBlock interface.


172-172: LGTM!

The test data correctly includes the signature field for thinking blocks with tool calls.

src/routes/chat-completions/handler.ts (4)

6-6: LGTM!

The migration to per-handler logging improves log organization and aligns with the new structured logging system.


17-17: LGTM!

Creating a dedicated logger instance for the chat-completions handler enables better log filtering and rotation.


23-23: LGTM!

The logging calls are appropriately updated to use the handler-specific logger instance.


44-50: Good defensive coding with the isNullish check.

The isNullish guard properly handles cases where max_tokens is null or undefined, preventing potential runtime issues when accessing selectedModel?.capabilities.limits.max_output_tokens.

src/routes/messages/anthropic-types.ts (1)

56-60: Signature field is properly handled across all translation layers and streaming.

All code paths that construct AnthropicThinkingBlock objects correctly include the signature field:

  • In responses-translation.ts:389, thinking blocks are constructed with signature: (item.encrypted_content ?? "") + "@" + item.id
  • In responses-stream-translation.ts:199-207, signature deltas are properly emitted as stream events
  • In responses-translation.ts:250-251, the signature field is correctly parsed and consumed
  • Tests include the signature field with proper values

No issues found. The required field is consistently set during construction and properly propagated through all translation and streaming modules.

Comment on lines +50 to +52
if (!anthropicPayload.tools || anthropicPayload.tools.length === 0) {
anthropicPayload.model = getSmallModel()
}
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Don't override the requested model for normal chats

Line 50 resets anthropicPayload.model to getSmallModel() whenever tools is empty. Almost every Anthropic Messages request arrives without tools, so this branch fires for genuine user traffic, silently downgrading premium model selections and preventing us from ever hitting the Responses API for models that actually support it. Net effect: users who ask for gpt-4 (or any other advanced tier) will now be served the small model instead—a clear functional regression.

Please drop this unconditional override (or gate it behind a precise warmup detector) so real requests keep the model they asked for.

-  if (!anthropicPayload.tools || anthropicPayload.tools.length === 0) {
-    anthropicPayload.model = getSmallModel()
-  }
📝 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
if (!anthropicPayload.tools || anthropicPayload.tools.length === 0) {
anthropicPayload.model = getSmallModel()
}
🤖 Prompt for AI Agents
In src/routes/messages/handler.ts around lines 50 to 52, the code
unconditionally sets anthropicPayload.model = getSmallModel() whenever
anthropicPayload.tools is missing or empty, which overrides user-requested
premium models; remove this unconditional override so the payload preserves the
client-requested model (or only apply getSmallModel() under a specific warmup
detector condition). Ensure no change to other logic: delete the branch that
sets the small model for empty tools (or wrap it in an explicit warmup check) so
normal chats keep their requested model.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it fix claude code 2.0.28 warmup request consume premium request, forcing small model if no tools are used . anthropic message api used in claude code .

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caozhiyuan, thank you for the clarification! I understand now that this is intentionally addressing the Claude Code 2.0.28 warmup issue where those requests were consuming premium model tokens unnecessarily.

Just to confirm my understanding: this logic will apply the small model to any request without tools (not only warmup requests specifically), which means regular user chats without tool definitions will also use the small model. If that's the intended behavior for your use case, then this makes sense as a pragmatic solution.

I appreciate you explaining the context—I'll note this as intentional. 👍

(\__/)
(='.'=)  *noting for future*
(")_(")

✏️ Learnings added
Learnt from: caozhiyuan
Repo: ericc-ch/copilot-api PR: 142
File: src/routes/messages/handler.ts:50-52
Timestamp: 2025-11-11T04:33:30.513Z
Learning: In src/routes/messages/handler.ts, forcing anthropicPayload.model to getSmallModel() when no tools are present is intentional behavior to fix Claude Code 2.0.28 warmup requests consuming premium model tokens. This applies to all requests without tools, not just warmup requests, and is an accepted design decision.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +25 to +72
logger.debug("Responses request payload:", JSON.stringify(payload))

const selectedModel = state.models?.data.find(
(model) => model.id === payload.model,
)
const supportsResponses =
selectedModel?.supported_endpoints?.includes(RESPONSES_ENDPOINT) ?? false

if (!supportsResponses) {
return c.json(
{
error: {
message:
"This model does not support the responses endpoint. Please choose a different model.",
type: "invalid_request_error",
},
},
400,
)
}

const { vision, initiator } = getResponsesRequestOptions(payload)

if (state.manualApprove) {
await awaitApproval()
}

const response = await createResponses(payload, { vision, initiator })

if (isStreamingRequested(payload) && isAsyncIterable(response)) {
logger.debug("Forwarding native Responses stream")
return streamSSE(c, async (stream) => {
for await (const chunk of response) {
logger.debug("Responses stream chunk:", JSON.stringify(chunk))
await stream.writeSSE({
id: (chunk as { id?: string }).id,
event: (chunk as { event?: string }).event,
data: (chunk as { data?: string }).data ?? "",
})
}
})
}

logger.debug(
"Forwarding native Responses result:",
JSON.stringify(response).slice(-400),
)
return c.json(response as ResponsesResult)
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid logging full request/response payloads
These logger.debug calls write the entire request payload and every streamed chunk to disk via the new per-handler logger (retained for seven days by default). That captures user prompts, tool arguments, and model outputs verbatim, which is a high-risk compliance/PII retention issue unless you have explicit consent and data-governance controls in place. Please remove or heavily redact these logs (e.g., log only metadata like model id or event types, or gate them behind a short-lived in-memory logger) before shipping.

🤖 Prompt for AI Agents
In src/routes/responses/handler.ts around lines 25 to 72, remove or replace the
current logger.debug calls that serialize the full request payload and streamed
chunks; instead log only non-PII metadata (e.g., model id, whether
vision/initiator flags are set, event type, chunk id) and never log prompt or
chunk.data content verbatim. For the initial payload log drop
JSON.stringify(payload) and log payload.model or selectedModel.id and any
boolean flags; for streaming, log only chunk.id and chunk.event (no chunk.data)
or redact/mask it, and consider gating any full-content logging behind an
explicit opt-in short-lived in-memory flag or a secure, auditable
data-governance path before shipping. Ensure logs written here do not persist
sensitive user data and update comments/tests accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

user need run start -v to enable logger

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caozhiyuan, thank you for the clarification! I see that the logger.debug calls only write to disk when users explicitly run with the -v flag, which significantly reduces the exposure compared to always-on logging.

That said, even opt-in verbose logging carries some residual risk:

  1. User awareness: Users enabling -v for debugging may not realize it captures full prompts, tool arguments, and model outputs for 7 days.
  2. Compliance context: In some jurisdictions (GDPR, CCPA), even short-term retention of user content requires explicit consent and data-governance controls, even when opt-in.
  3. Production incidents: Someone might enable verbose mode to troubleshoot an issue and forget the retention implications.

Suggestions to further reduce risk:

  • Log metadata only in verbose mode: Even with -v, consider logging only payload.model, vision, initiator, chunk.id, and chunk.event (not chunk.data or full payloads).
  • Add a separate flag for full-content logging: Reserve a more explicit flag (e.g., --log-full-payloads) with a clear warning about PII retention.
  • Document the implications: Add a note in your README or startup message that -v logs request/response content for 7 days.

If verbose mode is specifically intended for debugging with full payloads and your users understand the implications, this design is reasonable. Just ensure it's documented and consider the suggestions above for production environments.


✏️ Learnings added
Learnt from: caozhiyuan
Repo: ericc-ch/copilot-api PR: 142
File: src/routes/responses/handler.ts:25-72
Timestamp: 2025-11-11T04:15:40.752Z
Learning: In the copilot-api repository, debug logging that includes full request/response payloads is intentionally gated behind the `-v` (verbose) flag to reduce PII exposure. Users must explicitly enable verbose mode to activate detailed logging.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +16 to +27
getPayloadItems(payload).some((item) => {
if (!("role" in item) || !item.role) {
return true
}
const role = typeof item.role === "string" ? item.role.toLowerCase() : ""
return role === "assistant"
})

export const hasVisionInput = (payload: ResponsesPayload): boolean => {
const values = getPayloadItems(payload)
return values.some((item) => containsVisionContent(item))
}
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix initiator detection to avoid misclassifying user-driven calls.

hasAgentInitiator currently returns true for any payload item lacking a role (e.g., tool call entries) or whenever an assistant_role message appears anywhere in history. After translation, almost every request includes prior assistant/tool items, so we now send X-Initiator: agent even when the user just typed the latest message. That misreports real user traffic to Copilot and can trigger agent-only throttling or different policy handling. Please determine the initiator from the most recent message that actually has a role, ignoring items without one.

-export const hasAgentInitiator = (payload: ResponsesPayload): boolean =>
-  getPayloadItems(payload).some((item) => {
-    if (!("role" in item) || !item.role) {
-      return true
-    }
-    const role = typeof item.role === "string" ? item.role.toLowerCase() : ""
-    return role === "assistant"
-  })
+export const hasAgentInitiator = (payload: ResponsesPayload): boolean => {
+  const items = getPayloadItems(payload)
+
+  for (let index = items.length - 1; index >= 0; index -= 1) {
+    const item = items[index]
+    if (!("role" in item)) continue
+
+    const role = (item as { role?: unknown }).role
+    if (typeof role === "string" && role.length > 0) {
+      return role.toLowerCase() === "assistant"
+    }
+  }
+
+  return false
+}
🤖 Prompt for AI Agents
In src/routes/responses/utils.ts around lines 16 to 27, the current logic treats
any item lacking a role as an agent initiator and also looks anywhere in history
for assistant messages; update it to determine the initiator from the most
recent payload item that actually has a role: iterate the payload items in
reverse order, skip items without a "role" or with an empty role, take the first
item that has a role, normalize it to lowercase, and return true only if that
role equals "assistant" (otherwise return false); ensure you handle
string/possibly-nonstring role types safely and keep type signatures unchanged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current logic is consistent with the create-chat-completions.ts isAgentCall logic: https://github.com/ericc-ch/copilot-api/blob/master/src/services/copilot/create-chat-completions.ts#L21

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the create-chat-completions.ts file to examine the isAgentCall logic
cat src/services/copilot/create-chat-completions.ts

Length of output: 4282


@caozhiyuan, thank you for pointing out the consistency with isAgentCall in create-chat-completions.ts. You're right that both implementations use .some() to detect agent involvement in the payload.

I can see that isAgentCall checks:

const isAgentCall = payload.messages.some((msg) =>
  ["assistant", "tool"].includes(msg.role),
)

While hasAgentInitiator has similar logic, there are a couple of differences:

  1. Missing role handling: hasAgentInitiator returns true for items without a role field, while isAgentCall doesn't encounter this case (all messages have roles)
  2. Tool role: isAgentCall checks for both "assistant" and "tool" roles, but hasAgentInitiator only checks for "assistant"

Questions to clarify the intended behavior:

  • Is the intent of X-Initiator to indicate "who sent the most recent message" or "is there any agent involvement in the conversation history"?
  • Should items without a role field (e.g., function outputs, tool call items in the Responses API structure) be treated as agent-initiated?
  • Should hasAgentInitiator also check for "tool" role like isAgentCall does for consistency?

If the current behavior (checking for any assistant/tool in the entire payload) is intentional for both endpoints, then the implementation is consistent as you noted, and the review comment's concern would apply to the broader design rather than just this specific function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Responses API function outputs, tool call items without a role field .

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the ResponseInputItem type definition and related types to understand which items have role fields
cat src/services/copilot/create-responses.ts | grep -A 30 "type ResponseInputItem"

Length of output: 696


🏁 Script executed:

#!/bin/bash
# Look for all the individual input item interfaces
ast-grep --pattern 'export interface Response$_Item {
  $$$
}'

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search for ResponseFunctionToolCallItem and ResponseFunctionCallOutputItem definitions
rg -nP -A 10 "interface Response(Function|FunctionCall)" src/services/copilot/create-responses.ts

Length of output: 1087


🏁 Script executed:

#!/bin/bash
# Get a broader view of the ResponseInputItem union and its constituents
rg -nP -B 2 -A 40 "^export type ResponseInputItem" src/services/copilot/create-responses.ts

Length of output: 1145


🏁 Script executed:

#!/bin/bash
# Check the ResponseInputMessage definition to confirm it has a role field
rg -nP -A 10 "^export interface ResponseInputMessage" src/services/copilot/create-responses.ts

Length of output: 320


@caozhiyuan, thank you for the clarification! I can now see from the type definitions that in the Responses API:

  • ResponseInputMessage has a role field (user/assistant/system/developer)
  • ResponseFunctionToolCallItem and ResponseFunctionCallOutputItem legitimately don't have a role field
  • ResponseInputReasoning also lacks a role field

So the current logic that returns true for items without a role field will catch function calls and outputs, treating them as agent-initiated. This makes sense because the presence of tool/function interaction typically indicates agent involvement in the workflow.

To clarify the intended semantics: Does the X-Initiator header indicate:

  1. Any agent involvement in the conversation history (current implementation via .some()), or
  2. Who sent the most recent conversational message (checking only the last message with a role)?

If the intent is #1 (detecting any agent involvement), then the current implementation is correct and consistent with isAgentCall. If it's #2 (identifying the initiator of the current turn), then we'd need to check only the most recent message-type item.

Could you confirm which semantic is intended for the Responses API?

🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current logic is consistent with the create-chat-completions.ts isAgentCall logic

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@caozhiyuan, understood! Since the logic is intentionally consistent with the existing isAgentCall implementation in create-chat-completions.ts, and you've confirmed this twice, I'll consider this design decision final. The current implementation maintains consistency across your codebase, which is a valid architectural choice.

Thank you for clarifying! 👍

@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the responses handler to see what payload it expects
echo "=== src/routes/responses/handler.ts ==="
cat src/routes/responses/handler.ts

echo -e "\n=== src/routes/messages/handler.ts (relevant sections) ==="
# Look for the translation logic in messages handler
rg -n -A 10 -B 2 "translateAnthropicMessagesToResponsesPayload" src/routes/messages/handler.ts

echo -e "\n=== Check handleWithResponsesApi function ==="
rg -n -A 30 "handleWithResponsesApi" src/routes/messages/handler.ts

Length of output: 5433


@caozhiyuan You're absolutely correct! I apologize for the error in the sequence diagram. After reviewing the actual implementation:

  • POST /v1/responses: Accepts ResponsesPayload directly (no Anthropic translation)
  • POST /v1/messages: Accepts Anthropic format and translates it via translateAnthropicMessagesToResponsesPayload() before calling the Responses API

Here's the corrected sequence diagram:

sequenceDiagram
    participant Client
    participant Server
    participant ResponsesAPI as Copilot<br/>Responses API
    participant Translator
    participant Logger

    rect rgb(230, 245, 255)
    Note over Client,Server: /v1/responses (Native ResponsesPayload)
    Client->>Server: POST /v1/responses<br/>(ResponsesPayload)
    Server->>Logger: debug: request received
    Server->>Server: validate payload & model capability
    Server->>ResponsesAPI: POST /responses with payload
    ResponsesAPI-->>Server: ResponsesResult
    Server-->>Client: JSON response (ResponsesResult)
    end

    rect rgb(255, 240, 245)
    Note over Client,Server: /v1/messages (Anthropic Format + Translation)
    Client->>Server: POST /v1/messages<br/>(AnthropicMessagesPayload)
    Server->>Server: shouldUseResponsesApi()?
    Server->>Translator: translateAnthropicMessagesToResponsesPayload()
    Translator-->>Server: ResponsesPayload
    Server->>ResponsesAPI: POST /responses with payload
    ResponsesAPI-->>Server: ResponsesResult
    Server->>Translator: translateResponsesResultToAnthropic()
    Translator-->>Server: AnthropicResponse
    Server-->>Client: JSON response (AnthropicResponse)
    end

    rect rgb(245, 245, 230)
    Note over Client,Server: Streaming (either endpoint)
    Client->>Server: POST with stream=true
    Server->>ResponsesAPI: fetch /responses (stream)
    ResponsesAPI-->>Server: EventSource (streamed events)
    loop for each ResponseStreamEvent
        alt /v1/responses
            Server-->>Client: SSE chunk (native format)
        else /v1/messages
            Server->>Translator: translateResponsesStreamEvent()
            Translator-->>Server: Array<AnthropicStreamEventData>
            Server-->>Client: SSE chunk (Anthropic format)
        end
    end
    end

    rect rgb(245, 230, 245)
    Note over Server,Logger: Logging Path
    Server->>Logger: createHandlerLogger("handler-name")
    Logger->>Logger: ensure logs/ directory
    Logger->>Logger: append to daily log file
    Logger->>Logger: flush buffer periodically
    end
Loading

Key differences:

  • /v1/responses is the native endpoint that accepts and returns Responses API format directly
  • /v1/messages is the Anthropic-compatible endpoint that performs bidirectional translation
  • Only the /v1/messages path uses the translation functions

Thank you for catching this!

🐰
(_(\
(='.'=)
(")_(")
learns from mistake

@caozhiyuan caozhiyuan changed the title feature about responses api feature about responses api, also introduces a new configuration system, structured logging Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants