[codex] add Codex WebSocket transport and compat fixes#86
Merged
Conversation
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds WebSocket transport support (with auto-fallback and cooldown) to the OpenAI Codex backend, a WS continuation store for delta replay, Codex payload/tool-schema strict-compatibility handling, route-prefix-based routing overrides, token file permission hardening, gzip request body support, and debug instrumentation, plus extensive test coverage and model renaming to gpt-5.3-codex-spark. ChangesOpenAI Codex Backend & Routing Infrastructure
Estimated code review effort: 5 (Critical) | ~150 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 6 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (6 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
internal/plugins/frontends/anthropic/handler.go (1)
148-149: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRedundant
EnsureCallDiagcall dropsTrimSpaceused earlier.Line 149 sets diag context with
strings.TrimSpace(call.Session.ALegID); the newly added line 181 re-establishes it with the raw, untrimmedcall.Session.ALegID. IfALegIDever carries surrounding whitespace, the trace/session correlation value used for the remainder of the request (streaming/non-streaming encode, error logging) will diverge from the one used duringh.execute, producing inconsistent diagnostics/correlation IDs pre- vs post-execute.🐛 Proposed fix
- ctx = diag.EnsureCallDiag(ctx, traceID, call.Session.ALegID) + ctx = diag.EnsureCallDiag(ctx, traceID, strings.TrimSpace(call.Session.ALegID))Also applies to: 180-182
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/plugins/frontends/anthropic/handler.go` around lines 148 - 149, The diagnostics context in anthropic/handler.go is being reset with an untrimmed session ID, which overrides the earlier trimmed value and can cause inconsistent trace/session correlation. Update the later EnsureCallDiag call in the handler flow so it uses the same strings.TrimSpace(call.Session.ALegID) value as the initial setup, keeping the trace context consistent across h.execute, streaming/non-streaming encoding, and error logging.internal/plugins/backends/openaicodex/plugin_test.go (1)
1225-1240: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExact-order native-ID assertion is tightly coupled to inventory ordering.
slices.Equal(got, want)requires the built-in Codex model list to stay in this exact order. If a future change appends/reorders models for legitimate reasons (e.g. alphabetizing, adding a model in the middle), this test breaks even though behavior is correct. Considerslices.Contains-based checks per model, or sort both slices before comparing, unless the exact order is itself a guaranteed contract worth locking down.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/plugins/backends/openaicodex/plugin_test.go` around lines 1225 - 1240, The test in TestModelInventory_builtinWhenNoneConfigured is over-constraining the built-in Codex model inventory by asserting exact slice order with slices.Equal. Update the assertion to verify the expected native IDs are present without depending on ordering, or sort the collected IDs and expected list before comparing, using the existing snap.Models and NativeID values to keep the test stable across legitimate inventory reordering.internal/plugins/frontends/openailegacy/decode.go (1)
155-167: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUndocumented behavior change:
DecodeChatRequestnow silently drops previously-rejected empty assistant messages.Empty assistant turns (no content/tool_calls/function_call) used to fail decode; they're now silently skipped via
errEmptyAssistantMessage. This is a legitimate compatibility fix, but there's no doc comment onDecodeChatRequest/parseMessagesor on the sentinel error explaining that decode now silently drops such messages instead of erroring, which matters for callers relying on the previous strict behavior. As per coding guidelines, "warn if the PR changes exported types, function signatures, error behavior... without clearly explaining the compatibility impact."📝 Suggested doc addition
+// errEmptyAssistantMessage marks an assistant turn with no content, tool_calls, +// or function_call. parseMessages silently drops such messages instead of +// failing decode, to tolerate OpenAI-compatible clients (e.g. OpenCode) that +// emit empty assistant history entries after compaction. var ( errEmptyAssistantMessage = errors.New("empty assistant message") errEmptyChatContent = errors.New("message content string is empty") )Also applies to: 266-276
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/plugins/frontends/openailegacy/decode.go` around lines 155 - 167, Add documentation for the changed decode behavior in `DecodeChatRequest` and `parseMessages`, and explain the `errEmptyAssistantMessage` sentinel in place. The issue is that empty assistant messages are now skipped instead of causing decode to fail, so update the relevant doc comments to state this compatibility change and its effect on callers that depended on strict rejection of empty assistant turns.Source: Coding guidelines
internal/plugins/backends/openaicodex/managed_oauth_files.go (1)
21-29: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftAvoid path-based reads here; the symlink guard still leaves a race window.
ReadDironly filters the initial snapshot.checkTokenFilePermissionsandos.ReadFileboth resolve the path again, so a file swapped to a symlink after the directory scan can still be followed. Open the file without following symlinks (or use a pre-opened fd) instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/plugins/backends/openaicodex/managed_oauth_files.go` around lines 21 - 29, The symlink check in managed OAuth file loading still leaves a TOCTOU race because the later permission check and read re-resolve the path. Update the file access flow in managed_oauth_files.go so the JSON file is opened without following symlinks, or use a pre-opened file descriptor throughout both checkTokenFilePermissions and the read logic, instead of calling path-based os.ReadFile after ReadDir.internal/plugins/frontends/gemini/handler.go (1)
128-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent nil-logger handling between adjacent error paths.
The new decode-failure branch (128-133) always logs via
h.Logor aslog.Default()fallback, but the very next error branch —call.Validate()failure (138-144, unchanged) — still only logsif h.Log != nil. With the new pattern established right above it, validation failures silently go unlogged wheneverh.Logis nil, which is now an inconsistency in the same handler.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/plugins/frontends/gemini/handler.go` around lines 128 - 144, The error handling in handler.go is inconsistent between the decode failure path and the adjacent call.Validate() failure path. Update the validate-call branch in the Gemini handler to use the same logger fallback pattern as the decode branch: assign h.Log to a local log variable, fall back to slog.Default() when nil, and pass that logger to diag.LogError so validation failures are still logged even when h.Log is absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/core/diag/debug_summary.go`:
- Around line 33-48: Replace the manual insertion sort in StableCounts with a
standard library string sort by using sort.Strings on the keys slice, and update
imports accordingly. Also check streamdebug.go’s stableStrings helper and remove
the duplicated sorting logic there by reusing this shared helper or applying the
same sort.Strings approach so the two call sites stay consistent.
In `@internal/pluginreg/backends_openaicodex_test.go`:
- Around line 339-363: The openai-codex backend currently accepts an invalid
transport value from YAML and only fails later in Open, so move validation into
the configuration/build path instead. Update the backend factory path used by
NewRegistry/InstallStandardBackendsOn and BuildBackend so transport is checked
when constructing the backend, and return an error for unsupported values like
quic before the backend is usable. Keep Open as a fallback only if needed, but
prefer failing fast in the build/config parsing flow for
OpenAICodexBackendFactory.
In `@internal/plugins/backends/openaicodex/authjson.go`:
- Around line 138-144: In authjson.go, the token-path validation in the code
that uses os.Stat and then reads the file only checks permissions, so a 0600
FIFO or device can still be accepted. Update the validation logic in the
token-loading path to reject any non-regular file by checking
info.Mode().IsRegular() before allowing os.ReadFile to proceed, and return a
clear error from the existing token-file check path.
In `@internal/plugins/backends/openaicodex/managed_oauth_files.go`:
- Around line 25-29: The managed OAuth file scan is treating symlink-only
directories as having “no managed oauth account files” because skipped symlinks
are not included in the count. Update the logic in the managed oauth loader that
walks `.json` entries and enforces the final error message to distinguish
between zero entries and zero usable entries. When every entry is skipped by the
symlink check, report “no usable managed oauth accounts” instead of the generic
missing-files message, and apply the same fix to the related handling path
referenced by the same scan logic.
In `@internal/plugins/backends/openaicodex/payload_test.go`:
- Around line 195-220: The no-tools payload test in
TestPayloadForCall_noToolsOmitsToolChoice only checks for tool_choice and misses
the parallel_tool_calls field emitted by PayloadForCall. Extend this test to
assert that the marshaled payload from backend.PayloadForCall for a plain text
call also omits parallel_tool_calls entirely (not just false), so it matches the
intended no-tools behavior and catches the current payload.go implementation.
In `@internal/plugins/backends/openaicodex/payload.go`:
- Around line 75-104: The no-tools path in payload construction is still leaking
a tool-related signal through the final parallel-tool-calls fallback. Update the
payload assembly in the OpenAICodex payload builder so the defaulting of
p.ParallelToolCalls to false only happens when call.Tools is non-empty,
alongside the existing tool_choice logic. Keep the no-tools case from
serializing any tool protocol fields, and extend the existing payload test
around TestPayloadForCall_noToolsOmitsToolChoice to assert parallel_tool_calls
is also omitted.
In `@internal/plugins/backends/openaicodex/stream_internal_test.go`:
- Around line 390-423: The test in
TestHandleData_toolArgsBeforeAddedWaitForToolName is too permissive and can miss
duplicate or mismatched ToolCallStarted events. Update it to assert there is
exactly one EventToolCallStarted emitted from handleData/DrainPending and verify
its ToolCallID matches the eventual call_id ("call_late"), not just the tool
name; keep the existing argument buffering check so this scenario also proves
the args stay associated with the correct tool call.
In `@internal/plugins/backends/openaicodex/stream.go`:
- Around line 337-359: The tool-call ID mapping in codexEventMapper is keeping a
stale provisional item ID even after rememberToolCallID learns the real call_id,
causing duplicate canonical IDs and mismatched tool events when args arrive
before output_item.added. Update toolCallID to prefer the learned callID from
toolCallIDs over any provisional[itemID] flag, and ensure handleOutputItemAdded
and handleOutputItemDone use the same canonicalization path so ToolCallStarted,
deltas, and completion all resolve to one ID.
In `@internal/plugins/backends/openaicodex/toolschema.go`:
- Around line 12-19: `isStrictCompatibleSchema` is allowing parameterless object
schemas to pass strict validation without `additionalProperties:false`, which
breaks strict mode. Update the object-handling logic in
`isStrictCompatibleSchema` and the related `addStrictAdditionalProperties` flow
so that empty object schemas are treated like other object schemas and still get
`additionalProperties:false` injected, using the existing helpers (`hasRef`,
`strictCompatibleCompositions`, `strictCompatibleArrayItems`,
`addStrictAdditionalProperties`) to locate the fix.
In `@internal/plugins/backends/openaicodex/ws.go`:
- Around line 554-580: The close-error handling in readFirstNonEmptyWSMessage is
a dead conditional because both the websocket.IsCloseError and fallback paths
return the same wrapped error. Either simplify the branch away and return the
wrapped read error directly, or restore the intended distinction by matching
wsStream.readMessage so a clean normal/going-away close is handled consistently
(for example as io.EOF) while other read failures still become
newWSTransportError. Use readFirstNonEmptyWSMessage and wsStream.readMessage as
the reference points when updating this logic.
- Around line 244-258: newWSDialer currently only copies proxy and TLS settings
when client.Transport is exactly *http.Transport, so wrapped RoundTrippers
silently lose those settings. Update newWSDialer to detect and log when it falls
back to default websocket dialing behavior for non-*http.Transport clients, and
make the fallback explicit by either extracting settings from the underlying
transport path or warning that Proxy/NetDialContext/TLSClientConfig were not
propagated. Keep the fix localized to newWSDialer and its websocket.Dialer setup
so the WebSocket handshake behavior is visible when transport wrapping prevents
config propagation.
In `@internal/plugins/features/codexclientcompat/compat.go`:
- Around line 595-598: The no-tools branch in the compat prompt builder is
contradictory because it still appends tool-usage guidance via
criticalInstruction("OpenCode") even when hasTools is false. Update the no-tools
path in the relevant prompt construction logic to emit only the “no callable
tools” message and return without adding any OpenCode/tool instructions, keeping
the behavior confined to the branch that actually has tools.
In `@internal/plugins/features/codexclientcompat/plugin.go`:
- Line 37: The new pre-call in ApplyCompat is no longer nil-safe because
applyIgnoreUnsupportedGenParams dereferences call before the existing guard can
short-circuit. Add an explicit nil check in ApplyCompat before calling
applyIgnoreUnsupportedGenParams, and keep the extension mutation path guarded so
ApplyCompat(nil) continues to return safely without touching call or its
extensions.
In `@internal/plugins/frontends/gemini/handler.go`:
- Around line 169-171: Remove the redundant second diag.EnsureCallDiag call in
the Gemini handler and keep the ctx initialized consistently with the trimmed
ALegID used earlier. In the execute path around streamdebug.LogExecuteOpened and
streamdebug.Wrap, either reuse the existing ctx or, if the call must remain,
pass strings.TrimSpace(call.Session.ALegID) there as well. Use the existing
handler flow in internal/plugins/frontends/gemini/handler.go and the
EnsureCallDiag symbol to locate the change.
In `@internal/plugins/frontends/openailegacy/decode.go`:
- Around line 227-238: The `parseChatContent` handling in `decode.go` has a dead
assignment in the `errEmptyChatContent` branch because `cp = nil` is never used;
simplify the `openailegacy` assistant content path by removing that branch’s
no-op assignment and keeping the existing default `contentParts` behavior, while
preserving the error wrapping and the `if err == nil { contentParts = cp }`
assignment logic.
In `@internal/plugins/frontends/streamdebug/streamdebug_test.go`:
- Around line 21-34: TestWrapFollowsDebugGate currently depends on
diag.DebugTurnsEnabled(), which is memoized and makes the enabled path
nondeterministic in CI. Update the streamdebug tests around Wrap to
deterministically cover both outcomes by using either a subprocess/build-tag
approach or a test-only reset hook in diag, so you can verify both the wrapped
and passthrough cases without relying on process-wide state. Keep the assertions
centered on Wrap and DebugTurnsEnabled so the branch under test is explicit.
In `@internal/plugins/frontends/streamdebug/streamdebug.go`:
- Around line 301-309: The stableStrings helper is duplicating the same manual
insertion sort already present in diag.StableCounts, so replace the custom loop
with sort.Strings on a copied slice, or extract and reuse a shared sorting
helper from diag. Update stableStrings in streamdebug.go to delegate to the
shared approach and keep the behavior identical while removing the duplicated
sort logic.
- Around line 162-194: `stream.Close` only calls `logTerminal` when
`s.inner.Close()` returns an error, so clean early closes never emit the
terminal debug record and the status would be wrong if logged unconditionally.
Update `Close` to always invoke `logTerminal` after closing `s.inner`, and
adjust `logTerminal` to derive a distinct status for nil errors (for example, a
clean/closed state) instead of defaulting to "error"; use the existing
`logTerminal` and `Close` methods in `stream` to keep the terminal metrics and
status accurate for all shutdown paths.
In `@internal/refbackend/openaicodex/server_test.go`:
- Around line 266-324: The WebSocket tests in
TestServer_webSocketUpgrade_streamsEventFramesAndCaptures (and the other WS test
noted in the comment) do unbounded conn.ReadMessage calls that can hang CI if
the handler stops sending frames. Add a per-read deadline before each read, or
otherwise ensure the gorillawebsocket connection has a timeout around the read
loop, so failures surface as test timeouts instead of indefinite blocking. Keep
the fix localized to the WS test helpers/calls that read event frames.
In `@internal/refbackend/openaicodex/server.go`:
- Around line 353-367: The SSE writer in writeStream is using two parallel
arrays, frames from codexEventFrames and a hardcoded events list, which can
drift and cause mismatched or out-of-range event names. Update writeStream to
derive the emitted SSE event name from each frame’s parsed type field instead of
indexing a separate events slice, and keep the streaming loop aligned with
codexEventFrames output.
---
Outside diff comments:
In `@internal/plugins/backends/openaicodex/managed_oauth_files.go`:
- Around line 21-29: The symlink check in managed OAuth file loading still
leaves a TOCTOU race because the later permission check and read re-resolve the
path. Update the file access flow in managed_oauth_files.go so the JSON file is
opened without following symlinks, or use a pre-opened file descriptor
throughout both checkTokenFilePermissions and the read logic, instead of calling
path-based os.ReadFile after ReadDir.
In `@internal/plugins/backends/openaicodex/plugin_test.go`:
- Around line 1225-1240: The test in
TestModelInventory_builtinWhenNoneConfigured is over-constraining the built-in
Codex model inventory by asserting exact slice order with slices.Equal. Update
the assertion to verify the expected native IDs are present without depending on
ordering, or sort the collected IDs and expected list before comparing, using
the existing snap.Models and NativeID values to keep the test stable across
legitimate inventory reordering.
In `@internal/plugins/frontends/anthropic/handler.go`:
- Around line 148-149: The diagnostics context in anthropic/handler.go is being
reset with an untrimmed session ID, which overrides the earlier trimmed value
and can cause inconsistent trace/session correlation. Update the later
EnsureCallDiag call in the handler flow so it uses the same
strings.TrimSpace(call.Session.ALegID) value as the initial setup, keeping the
trace context consistent across h.execute, streaming/non-streaming encoding, and
error logging.
In `@internal/plugins/frontends/gemini/handler.go`:
- Around line 128-144: The error handling in handler.go is inconsistent between
the decode failure path and the adjacent call.Validate() failure path. Update
the validate-call branch in the Gemini handler to use the same logger fallback
pattern as the decode branch: assign h.Log to a local log variable, fall back to
slog.Default() when nil, and pass that logger to diag.LogError so validation
failures are still logged even when h.Log is absent.
In `@internal/plugins/frontends/openailegacy/decode.go`:
- Around line 155-167: Add documentation for the changed decode behavior in
`DecodeChatRequest` and `parseMessages`, and explain the
`errEmptyAssistantMessage` sentinel in place. The issue is that empty assistant
messages are now skipped instead of causing decode to fail, so update the
relevant doc comments to state this compatibility change and its effect on
callers that depended on strict rejection of empty assistant turns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 389846ce-df83-47cc-8e1e-f297116c1204
📒 Files selected for processing (94)
.kiro/steering/routing-and-orchestration.mdREADME.mdconfig/config.yamlconfig/examples/opencode-codex.yamldocs/openai-codex-backend.mdgo.modinternal/core/diag/debug_summary.gointernal/core/routing/parser_test.gointernal/core/routing/routeprefix.gointernal/core/routing/routeprefix_test.gointernal/core/runtime/executor_open_attempt.gointernal/core/runtime/executor_test.gointernal/infra/runtimebundle/build.gointernal/infra/runtimebundle/build_test.gointernal/infra/runtimebundle/built.gointernal/infra/runtimebundle/opencode_zen_registry_live_test.gointernal/pluginreg/backends_install_test.gointernal/pluginreg/backends_openaicodex.gointernal/pluginreg/backends_openaicodex_test.gointernal/pluginreg/frontends_install.gointernal/plugins/backends/openaicodex/attempt.gointernal/plugins/backends/openaicodex/attempt_internal_test.gointernal/plugins/backends/openaicodex/authjson.gointernal/plugins/backends/openaicodex/authjson_internal_test.gointernal/plugins/backends/openaicodex/authjson_test.gointernal/plugins/backends/openaicodex/config.gointernal/plugins/backends/openaicodex/config_test.gointernal/plugins/backends/openaicodex/continuation.gointernal/plugins/backends/openaicodex/continuation_test.gointernal/plugins/backends/openaicodex/debug.gointernal/plugins/backends/openaicodex/downgrade.gointernal/plugins/backends/openaicodex/downgrade_policy_test.gointernal/plugins/backends/openaicodex/gpt55_downgrade_test.gointernal/plugins/backends/openaicodex/headers.gointernal/plugins/backends/openaicodex/headers_internal_test.gointernal/plugins/backends/openaicodex/main_test.gointernal/plugins/backends/openaicodex/managed_oauth_files.gointernal/plugins/backends/openaicodex/managed_oauth_internal_test.gointernal/plugins/backends/openaicodex/managed_oauth_test.gointernal/plugins/backends/openaicodex/oauth.gointernal/plugins/backends/openaicodex/oauth_refresh_test.gointernal/plugins/backends/openaicodex/payload.gointernal/plugins/backends/openaicodex/payload_input.gointernal/plugins/backends/openaicodex/payload_test.gointernal/plugins/backends/openaicodex/payload_tools.gointernal/plugins/backends/openaicodex/perf_benchmark_test.gointernal/plugins/backends/openaicodex/plugin.gointernal/plugins/backends/openaicodex/plugin_internal_test.gointernal/plugins/backends/openaicodex/plugin_test.gointernal/plugins/backends/openaicodex/prompt_cache_http_test.gointernal/plugins/backends/openaicodex/stream.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/backends/openaicodex/toolschema.gointernal/plugins/backends/openaicodex/transport.gointernal/plugins/backends/openaicodex/transport_fallback_test.gointernal/plugins/backends/openaicodex/transport_internal_test.gointernal/plugins/backends/openaicodex/transport_test.gointernal/plugins/backends/openaicodex/usage_estimator_test.gointernal/plugins/backends/openaicodex/ws.gointernal/plugins/backends/openaicodex/ws_continuation_rotation_internal_test.gointernal/plugins/backends/openaicodex/ws_first_event_internal_test.gointernal/plugins/backends/openaicodex/ws_internal_test.gointernal/plugins/backends/protocols/openairesponsestream/mapper.gointernal/plugins/features/codexclientcompat/compat.gointernal/plugins/features/codexclientcompat/plugin.gointernal/plugins/features/codexclientcompat/plugin_test.gointernal/plugins/frontends/anthropic/handler.gointernal/plugins/frontends/anthropic/integration_test.gointernal/plugins/frontends/gemini/handler.gointernal/plugins/frontends/gemini/integration_test.gointernal/plugins/frontends/openailegacy/codex_body_routing_test.gointernal/plugins/frontends/openailegacy/decode.gointernal/plugins/frontends/openailegacy/decode_test.gointernal/plugins/frontends/openailegacy/handler.gointernal/plugins/frontends/openailegacy/handler_gzip_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/plugins/frontends/openairesponses/codex_body_routing_test.gointernal/plugins/frontends/openairesponses/handler.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/frontends/reqbody/body.gointernal/plugins/frontends/reqbody/body_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/frontends/streamdebug/streamdebug.gointernal/plugins/frontends/streamdebug/streamdebug_test.gointernal/refbackend/openaicodex/server.gointernal/refbackend/openaicodex/server_test.gointernal/stdhttp/default_route_frontends_test.gointernal/stdhttp/mount.gointernal/stdhttp/server.gointernal/stdhttp/standard_wiring_roundtrip_test.gopkg/lipapi/route_params.gopkg/lipapi/route_params_test.gopkg/lipsdk/factory.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (18)
internal/core/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/core/, keep runtime orchestration, routing, continuity, streams, hooks/extensions, config, and diagnostics.
Files:
internal/core/routing/routeprefix_test.gointernal/core/routing/routeprefix.gointernal/core/runtime/executor_open_attempt.gointernal/core/runtime/executor_test.gointernal/core/routing/parser_test.gointernal/core/diag/debug_summary.go
**/*.go
📄 CodeRabbit inference engine (Custom checks)
**/*.go: For server, CLI, worker, and network code, propagatecontext.Contextcorrectly, respect cancellation, and ensure new goroutines cannot leak indefinitely.
For changes underpkg/**or any exported Go identifier, warn if the PR changes exported types, function signatures, error behavior, JSON fields, CLI flags, config keys, or documented behavior without clearly explaining the compatibility impact.
Files:
internal/core/routing/routeprefix_test.gointernal/plugins/backends/openaicodex/config_test.gointernal/stdhttp/standard_wiring_roundtrip_test.gointernal/plugins/backends/openaicodex/main_test.gointernal/plugins/frontends/openailegacy/handler_gzip_test.gointernal/infra/runtimebundle/built.gopkg/lipsdk/factory.gointernal/plugins/backends/openaicodex/oauth.gointernal/plugins/backends/openaicodex/ws_continuation_rotation_internal_test.gointernal/plugins/backends/openaicodex/managed_oauth_files.gointernal/plugins/frontends/openairesponses/codex_body_routing_test.gointernal/core/routing/routeprefix.gointernal/plugins/frontends/openailegacy/codex_body_routing_test.gointernal/stdhttp/server.gointernal/core/runtime/executor_open_attempt.gointernal/plugins/backends/openaicodex/attempt_internal_test.gointernal/infra/runtimebundle/opencode_zen_registry_live_test.gointernal/stdhttp/mount.gointernal/plugins/backends/openaicodex/authjson_internal_test.gointernal/plugins/frontends/gemini/integration_test.gointernal/plugins/backends/openaicodex/usage_estimator_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/stdhttp/default_route_frontends_test.gopkg/lipapi/route_params.gointernal/plugins/backends/openaicodex/prompt_cache_http_test.gointernal/plugins/backends/openaicodex/toolschema.gointernal/plugins/backends/openaicodex/oauth_refresh_test.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/backends/openaicodex/payload_tools.gointernal/plugins/backends/openaicodex/authjson.gointernal/core/runtime/executor_test.gointernal/pluginreg/backends_openaicodex.gointernal/plugins/frontends/reqbody/body.gointernal/pluginreg/frontends_install.gointernal/plugins/backends/openaicodex/perf_benchmark_test.gointernal/plugins/frontends/openailegacy/decode_test.gointernal/plugins/backends/openaicodex/transport_internal_test.gointernal/pluginreg/backends_install_test.gointernal/plugins/backends/openaicodex/debug.gointernal/infra/runtimebundle/build_test.gointernal/plugins/features/codexclientcompat/plugin.gointernal/plugins/backends/openaicodex/managed_oauth_internal_test.gointernal/plugins/backends/openaicodex/plugin_internal_test.gointernal/plugins/backends/openaicodex/transport.gointernal/core/routing/parser_test.gointernal/plugins/backends/openaicodex/downgrade_policy_test.gointernal/plugins/backends/openaicodex/attempt.gointernal/plugins/backends/openaicodex/headers_internal_test.gointernal/plugins/backends/openaicodex/ws_first_event_internal_test.gointernal/plugins/frontends/anthropic/handler.gointernal/plugins/frontends/anthropic/integration_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/frontends/gemini/handler.gointernal/core/diag/debug_summary.gopkg/lipapi/route_params_test.gointernal/plugins/backends/openaicodex/transport_fallback_test.gointernal/plugins/backends/openaicodex/transport_test.gointernal/plugins/frontends/reqbody/body_test.gointernal/plugins/frontends/openailegacy/handler.gointernal/plugins/backends/openaicodex/config.gointernal/infra/runtimebundle/build.gointernal/plugins/backends/openaicodex/gpt55_downgrade_test.gointernal/plugins/frontends/streamdebug/streamdebug_test.gointernal/plugins/backends/openaicodex/authjson_test.gointernal/plugins/backends/protocols/openairesponsestream/mapper.gointernal/pluginreg/backends_openaicodex_test.gointernal/plugins/backends/openaicodex/downgrade.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/frontends/openailegacy/decode.gointernal/plugins/backends/openaicodex/ws_internal_test.gointernal/plugins/frontends/openairesponses/handler.gointernal/plugins/backends/openaicodex/stream.gointernal/plugins/backends/openaicodex/ws.gointernal/plugins/backends/openaicodex/continuation.gointernal/plugins/features/codexclientcompat/compat.gointernal/refbackend/openaicodex/server_test.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/backends/openaicodex/headers.gointernal/plugins/backends/openaicodex/plugin.gointernal/plugins/backends/openaicodex/continuation_test.gointernal/plugins/backends/openaicodex/payload_input.gointernal/plugins/backends/openaicodex/managed_oauth_test.gointernal/plugins/features/codexclientcompat/plugin_test.gointernal/plugins/backends/openaicodex/plugin_test.gointernal/refbackend/openaicodex/server.gointernal/plugins/frontends/streamdebug/streamdebug.gointernal/plugins/backends/openaicodex/payload.gointernal/plugins/backends/openaicodex/payload_test.go
⚙️ CodeRabbit configuration file
**/*.go: Review as production Go code. Prioritize correctness, race conditions, goroutine leaks, context cancellation, timeout handling, error wrapping, nil-pointer risks, resource cleanup, defer placement, API compatibility, interface design, dependency boundaries, and testability. Avoid generic style comments when gofmt/golangci-lint already covers the issue.
Files:
internal/core/routing/routeprefix_test.gointernal/plugins/backends/openaicodex/config_test.gointernal/stdhttp/standard_wiring_roundtrip_test.gointernal/plugins/backends/openaicodex/main_test.gointernal/plugins/frontends/openailegacy/handler_gzip_test.gointernal/infra/runtimebundle/built.gopkg/lipsdk/factory.gointernal/plugins/backends/openaicodex/oauth.gointernal/plugins/backends/openaicodex/ws_continuation_rotation_internal_test.gointernal/plugins/backends/openaicodex/managed_oauth_files.gointernal/plugins/frontends/openairesponses/codex_body_routing_test.gointernal/core/routing/routeprefix.gointernal/plugins/frontends/openailegacy/codex_body_routing_test.gointernal/stdhttp/server.gointernal/core/runtime/executor_open_attempt.gointernal/plugins/backends/openaicodex/attempt_internal_test.gointernal/infra/runtimebundle/opencode_zen_registry_live_test.gointernal/stdhttp/mount.gointernal/plugins/backends/openaicodex/authjson_internal_test.gointernal/plugins/frontends/gemini/integration_test.gointernal/plugins/backends/openaicodex/usage_estimator_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/stdhttp/default_route_frontends_test.gopkg/lipapi/route_params.gointernal/plugins/backends/openaicodex/prompt_cache_http_test.gointernal/plugins/backends/openaicodex/toolschema.gointernal/plugins/backends/openaicodex/oauth_refresh_test.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/backends/openaicodex/payload_tools.gointernal/plugins/backends/openaicodex/authjson.gointernal/core/runtime/executor_test.gointernal/pluginreg/backends_openaicodex.gointernal/plugins/frontends/reqbody/body.gointernal/pluginreg/frontends_install.gointernal/plugins/backends/openaicodex/perf_benchmark_test.gointernal/plugins/frontends/openailegacy/decode_test.gointernal/plugins/backends/openaicodex/transport_internal_test.gointernal/pluginreg/backends_install_test.gointernal/plugins/backends/openaicodex/debug.gointernal/infra/runtimebundle/build_test.gointernal/plugins/features/codexclientcompat/plugin.gointernal/plugins/backends/openaicodex/managed_oauth_internal_test.gointernal/plugins/backends/openaicodex/plugin_internal_test.gointernal/plugins/backends/openaicodex/transport.gointernal/core/routing/parser_test.gointernal/plugins/backends/openaicodex/downgrade_policy_test.gointernal/plugins/backends/openaicodex/attempt.gointernal/plugins/backends/openaicodex/headers_internal_test.gointernal/plugins/backends/openaicodex/ws_first_event_internal_test.gointernal/plugins/frontends/anthropic/handler.gointernal/plugins/frontends/anthropic/integration_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/frontends/gemini/handler.gointernal/core/diag/debug_summary.gopkg/lipapi/route_params_test.gointernal/plugins/backends/openaicodex/transport_fallback_test.gointernal/plugins/backends/openaicodex/transport_test.gointernal/plugins/frontends/reqbody/body_test.gointernal/plugins/frontends/openailegacy/handler.gointernal/plugins/backends/openaicodex/config.gointernal/infra/runtimebundle/build.gointernal/plugins/backends/openaicodex/gpt55_downgrade_test.gointernal/plugins/frontends/streamdebug/streamdebug_test.gointernal/plugins/backends/openaicodex/authjson_test.gointernal/plugins/backends/protocols/openairesponsestream/mapper.gointernal/pluginreg/backends_openaicodex_test.gointernal/plugins/backends/openaicodex/downgrade.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/frontends/openailegacy/decode.gointernal/plugins/backends/openaicodex/ws_internal_test.gointernal/plugins/frontends/openairesponses/handler.gointernal/plugins/backends/openaicodex/stream.gointernal/plugins/backends/openaicodex/ws.gointernal/plugins/backends/openaicodex/continuation.gointernal/plugins/features/codexclientcompat/compat.gointernal/refbackend/openaicodex/server_test.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/backends/openaicodex/headers.gointernal/plugins/backends/openaicodex/plugin.gointernal/plugins/backends/openaicodex/continuation_test.gointernal/plugins/backends/openaicodex/payload_input.gointernal/plugins/backends/openaicodex/managed_oauth_test.gointernal/plugins/features/codexclientcompat/plugin_test.gointernal/plugins/backends/openaicodex/plugin_test.gointernal/refbackend/openaicodex/server.gointernal/plugins/frontends/streamdebug/streamdebug.gointernal/plugins/backends/openaicodex/payload.gointernal/plugins/backends/openaicodex/payload_test.go
internal/**
⚙️ CodeRabbit configuration file
internal/**: Focus on package boundaries, hidden coupling, unexported API design, concurrency safety, deterministic behavior, and whether logic belongs in this internal package.
Files:
internal/core/routing/routeprefix_test.gointernal/plugins/backends/openaicodex/config_test.gointernal/stdhttp/standard_wiring_roundtrip_test.gointernal/plugins/backends/openaicodex/main_test.gointernal/plugins/frontends/openailegacy/handler_gzip_test.gointernal/infra/runtimebundle/built.gointernal/plugins/backends/openaicodex/oauth.gointernal/plugins/backends/openaicodex/ws_continuation_rotation_internal_test.gointernal/plugins/backends/openaicodex/managed_oauth_files.gointernal/plugins/frontends/openairesponses/codex_body_routing_test.gointernal/core/routing/routeprefix.gointernal/plugins/frontends/openailegacy/codex_body_routing_test.gointernal/stdhttp/server.gointernal/core/runtime/executor_open_attempt.gointernal/plugins/backends/openaicodex/attempt_internal_test.gointernal/infra/runtimebundle/opencode_zen_registry_live_test.gointernal/stdhttp/mount.gointernal/plugins/backends/openaicodex/authjson_internal_test.gointernal/plugins/frontends/gemini/integration_test.gointernal/plugins/backends/openaicodex/usage_estimator_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/stdhttp/default_route_frontends_test.gointernal/plugins/backends/openaicodex/prompt_cache_http_test.gointernal/plugins/backends/openaicodex/toolschema.gointernal/plugins/backends/openaicodex/oauth_refresh_test.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/backends/openaicodex/payload_tools.gointernal/plugins/backends/openaicodex/authjson.gointernal/core/runtime/executor_test.gointernal/pluginreg/backends_openaicodex.gointernal/plugins/frontends/reqbody/body.gointernal/pluginreg/frontends_install.gointernal/plugins/backends/openaicodex/perf_benchmark_test.gointernal/plugins/frontends/openailegacy/decode_test.gointernal/plugins/backends/openaicodex/transport_internal_test.gointernal/pluginreg/backends_install_test.gointernal/plugins/backends/openaicodex/debug.gointernal/infra/runtimebundle/build_test.gointernal/plugins/features/codexclientcompat/plugin.gointernal/plugins/backends/openaicodex/managed_oauth_internal_test.gointernal/plugins/backends/openaicodex/plugin_internal_test.gointernal/plugins/backends/openaicodex/transport.gointernal/core/routing/parser_test.gointernal/plugins/backends/openaicodex/downgrade_policy_test.gointernal/plugins/backends/openaicodex/attempt.gointernal/plugins/backends/openaicodex/headers_internal_test.gointernal/plugins/backends/openaicodex/ws_first_event_internal_test.gointernal/plugins/frontends/anthropic/handler.gointernal/plugins/frontends/anthropic/integration_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/frontends/gemini/handler.gointernal/core/diag/debug_summary.gointernal/plugins/backends/openaicodex/transport_fallback_test.gointernal/plugins/backends/openaicodex/transport_test.gointernal/plugins/frontends/reqbody/body_test.gointernal/plugins/frontends/openailegacy/handler.gointernal/plugins/backends/openaicodex/config.gointernal/infra/runtimebundle/build.gointernal/plugins/backends/openaicodex/gpt55_downgrade_test.gointernal/plugins/frontends/streamdebug/streamdebug_test.gointernal/plugins/backends/openaicodex/authjson_test.gointernal/plugins/backends/protocols/openairesponsestream/mapper.gointernal/pluginreg/backends_openaicodex_test.gointernal/plugins/backends/openaicodex/downgrade.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/frontends/openailegacy/decode.gointernal/plugins/backends/openaicodex/ws_internal_test.gointernal/plugins/frontends/openairesponses/handler.gointernal/plugins/backends/openaicodex/stream.gointernal/plugins/backends/openaicodex/ws.gointernal/plugins/backends/openaicodex/continuation.gointernal/plugins/features/codexclientcompat/compat.gointernal/refbackend/openaicodex/server_test.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/backends/openaicodex/headers.gointernal/plugins/backends/openaicodex/plugin.gointernal/plugins/backends/openaicodex/continuation_test.gointernal/plugins/backends/openaicodex/payload_input.gointernal/plugins/backends/openaicodex/managed_oauth_test.gointernal/plugins/features/codexclientcompat/plugin_test.gointernal/plugins/backends/openaicodex/plugin_test.gointernal/refbackend/openaicodex/server.gointernal/plugins/frontends/streamdebug/streamdebug.gointernal/plugins/backends/openaicodex/payload.gointernal/plugins/backends/openaicodex/payload_test.go
**/*_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Review tests for meaningful assertions, table-driven coverage, race-prone tests, t.Parallel misuse, nondeterminism, leaked goroutines, real network or filesystem dependencies, fragile sleeps, and missing edge cases. Prefer testing observable behavior over implementation details.
Files:
internal/core/routing/routeprefix_test.gointernal/plugins/backends/openaicodex/config_test.gointernal/stdhttp/standard_wiring_roundtrip_test.gointernal/plugins/backends/openaicodex/main_test.gointernal/plugins/frontends/openailegacy/handler_gzip_test.gointernal/plugins/backends/openaicodex/ws_continuation_rotation_internal_test.gointernal/plugins/frontends/openairesponses/codex_body_routing_test.gointernal/plugins/frontends/openailegacy/codex_body_routing_test.gointernal/plugins/backends/openaicodex/attempt_internal_test.gointernal/infra/runtimebundle/opencode_zen_registry_live_test.gointernal/plugins/backends/openaicodex/authjson_internal_test.gointernal/plugins/frontends/gemini/integration_test.gointernal/plugins/backends/openaicodex/usage_estimator_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/stdhttp/default_route_frontends_test.gointernal/plugins/backends/openaicodex/prompt_cache_http_test.gointernal/plugins/backends/openaicodex/oauth_refresh_test.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/core/runtime/executor_test.gointernal/plugins/backends/openaicodex/perf_benchmark_test.gointernal/plugins/frontends/openailegacy/decode_test.gointernal/plugins/backends/openaicodex/transport_internal_test.gointernal/pluginreg/backends_install_test.gointernal/infra/runtimebundle/build_test.gointernal/plugins/backends/openaicodex/managed_oauth_internal_test.gointernal/plugins/backends/openaicodex/plugin_internal_test.gointernal/core/routing/parser_test.gointernal/plugins/backends/openaicodex/downgrade_policy_test.gointernal/plugins/backends/openaicodex/headers_internal_test.gointernal/plugins/backends/openaicodex/ws_first_event_internal_test.gointernal/plugins/frontends/anthropic/integration_test.gopkg/lipapi/route_params_test.gointernal/plugins/backends/openaicodex/transport_fallback_test.gointernal/plugins/backends/openaicodex/transport_test.gointernal/plugins/frontends/reqbody/body_test.gointernal/plugins/backends/openaicodex/gpt55_downgrade_test.gointernal/plugins/frontends/streamdebug/streamdebug_test.gointernal/plugins/backends/openaicodex/authjson_test.gointernal/pluginreg/backends_openaicodex_test.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/backends/openaicodex/ws_internal_test.gointernal/refbackend/openaicodex/server_test.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/backends/openaicodex/continuation_test.gointernal/plugins/backends/openaicodex/managed_oauth_test.gointernal/plugins/features/codexclientcompat/plugin_test.gointernal/plugins/backends/openaicodex/plugin_test.gointernal/plugins/backends/openaicodex/payload_test.go
internal/plugins/backends/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/plugins/backends/, implement provider/local/compatible backend adapters, with the exact bundle defined ininternal/pluginreg/standard_table.go.
Files:
internal/plugins/backends/openaicodex/config_test.gointernal/plugins/backends/openaicodex/main_test.gointernal/plugins/backends/openaicodex/oauth.gointernal/plugins/backends/openaicodex/ws_continuation_rotation_internal_test.gointernal/plugins/backends/openaicodex/managed_oauth_files.gointernal/plugins/backends/openaicodex/attempt_internal_test.gointernal/plugins/backends/openaicodex/authjson_internal_test.gointernal/plugins/backends/openaicodex/usage_estimator_test.gointernal/plugins/backends/openaicodex/prompt_cache_http_test.gointernal/plugins/backends/openaicodex/toolschema.gointernal/plugins/backends/openaicodex/oauth_refresh_test.gointernal/plugins/backends/openaicodex/payload_tools.gointernal/plugins/backends/openaicodex/authjson.gointernal/plugins/backends/openaicodex/perf_benchmark_test.gointernal/plugins/backends/openaicodex/transport_internal_test.gointernal/plugins/backends/openaicodex/debug.gointernal/plugins/backends/openaicodex/managed_oauth_internal_test.gointernal/plugins/backends/openaicodex/plugin_internal_test.gointernal/plugins/backends/openaicodex/transport.gointernal/plugins/backends/openaicodex/downgrade_policy_test.gointernal/plugins/backends/openaicodex/attempt.gointernal/plugins/backends/openaicodex/headers_internal_test.gointernal/plugins/backends/openaicodex/ws_first_event_internal_test.gointernal/plugins/backends/openaicodex/transport_fallback_test.gointernal/plugins/backends/openaicodex/transport_test.gointernal/plugins/backends/openaicodex/config.gointernal/plugins/backends/openaicodex/gpt55_downgrade_test.gointernal/plugins/backends/openaicodex/authjson_test.gointernal/plugins/backends/protocols/openairesponsestream/mapper.gointernal/plugins/backends/openaicodex/downgrade.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/backends/openaicodex/ws_internal_test.gointernal/plugins/backends/openaicodex/stream.gointernal/plugins/backends/openaicodex/ws.gointernal/plugins/backends/openaicodex/continuation.gointernal/plugins/backends/openaicodex/headers.gointernal/plugins/backends/openaicodex/plugin.gointernal/plugins/backends/openaicodex/continuation_test.gointernal/plugins/backends/openaicodex/payload_input.gointernal/plugins/backends/openaicodex/managed_oauth_test.gointernal/plugins/backends/openaicodex/plugin_test.gointernal/plugins/backends/openaicodex/payload.gointernal/plugins/backends/openaicodex/payload_test.go
.kiro/steering/**
📄 CodeRabbit inference engine (AGENTS.md)
Treat
.kiro/steering/as durable project memory and source of truth.
Files:
.kiro/steering/routing-and-orchestration.md
.kiro/steering/routing-and-orchestration.md
📄 CodeRabbit inference engine (AGENTS.md)
Use
.kiro/steering/routing-and-orchestration.mdas the source of truth for routing, failover, and B2BUA rules.
Files:
.kiro/steering/routing-and-orchestration.md
internal/stdhttp/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/stdhttp/, compose the standard distribution.
Files:
internal/stdhttp/standard_wiring_roundtrip_test.gointernal/stdhttp/server.gointernal/stdhttp/mount.gointernal/stdhttp/default_route_frontends_test.go
internal/plugins/frontends/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/plugins/frontends/, implement OpenAI Responses, OpenAI legacy, Anthropic, and Gemini frontends.
Files:
internal/plugins/frontends/openailegacy/handler_gzip_test.gointernal/plugins/frontends/openairesponses/codex_body_routing_test.gointernal/plugins/frontends/openailegacy/codex_body_routing_test.gointernal/plugins/frontends/gemini/integration_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/frontends/reqbody/body.gointernal/plugins/frontends/openailegacy/decode_test.gointernal/plugins/frontends/anthropic/handler.gointernal/plugins/frontends/anthropic/integration_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/frontends/gemini/handler.gointernal/plugins/frontends/reqbody/body_test.gointernal/plugins/frontends/openailegacy/handler.gointernal/plugins/frontends/streamdebug/streamdebug_test.gointernal/plugins/frontends/openailegacy/decode.gointernal/plugins/frontends/openairesponses/handler.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/frontends/streamdebug/streamdebug.go
internal/infra/runtimebundle/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/infra/runtimebundle/, compose the standard distribution.
Files:
internal/infra/runtimebundle/built.gointernal/infra/runtimebundle/opencode_zen_registry_live_test.gointernal/infra/runtimebundle/build_test.gointernal/infra/runtimebundle/build.go
pkg/lipsdk/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/lipsdk/**/*.go: Inpkg/lipsdk/, keep plugin SDK, facades, and registration contracts.
Keep publicpkg/lipsdkcontracts minimal, documented, and versionable.
Files:
pkg/lipsdk/factory.go
pkg/**
⚙️ CodeRabbit configuration file
pkg/**: Treat exported identifiers as public API. Flag breaking changes, ambiguous contracts, missing error semantics, poor interface boundaries, and changes that make downstream usage harder.
Files:
pkg/lipsdk/factory.gopkg/lipapi/route_params.gopkg/lipapi/route_params_test.go
pkg/lipapi/**/*.{go,md}
📄 CodeRabbit inference engine (AGENTS.md)
In
pkg/lipapi/, keep canonical request, event, capability, and error contracts.
Files:
pkg/lipapi/route_params.gopkg/lipapi/route_params_test.go
pkg/lipapi/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Keep public
pkg/lipapicontracts minimal, documented, and versionable.
Files:
pkg/lipapi/route_params.gopkg/lipapi/route_params_test.go
internal/pluginreg/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/pluginreg/, compose the standard distribution.
Files:
internal/pluginreg/backends_openaicodex.gointernal/pluginreg/frontends_install.gointernal/pluginreg/backends_install_test.gointernal/pluginreg/backends_openaicodex_test.go
internal/plugins/features/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/plugins/features/, implement official feature and reference plugins.
Files:
internal/plugins/features/codexclientcompat/plugin.gointernal/plugins/features/codexclientcompat/compat.gointernal/plugins/features/codexclientcompat/plugin_test.go
**/go.mod
⚙️ CodeRabbit configuration file
**/go.mod: Review dependency changes for unnecessary additions, major-version upgrades, replace directives, indirect dependency churn, and module path correctness.
Files:
go.mod
internal/refbackend/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/refbackend/, keep test-only emulators.
Files:
internal/refbackend/openaicodex/server_test.gointernal/refbackend/openaicodex/server.go
Comment on lines
+339
to
+363
| func TestOpenAICodexBackendFactory_invalidTransportFromYAML(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| ts := httptest.NewServer(refbackend.New(refbackend.Config{Token: "sk-codex"}).Handler()) | ||
| t.Cleanup(ts.Close) | ||
| reg := NewRegistry() | ||
| if err := InstallStandardBackendsOn(reg, UpstreamAPIKeys{}); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| var cfg yaml.Node | ||
| yamlText := "base_url: " + ts.URL + "/backend-api/codex\naccess_token: sk-codex\ntransport: quic\n" | ||
| if err := yaml.Unmarshal([]byte(yamlText), &cfg); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| be, err := reg.BuildBackend("openai-codex", cfg, ts.Client(), BackendFactoryDeps{}) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| _, err = be.Open(context.Background(), lipapi.Call{ | ||
| Messages: []lipapi.Message{{Role: lipapi.RoleUser, Parts: []lipapi.Part{lipapi.TextPart("hi")}}}, | ||
| }, routing.AttemptCandidate{Primary: routing.Primary{Model: "gpt-5.3-codex-spark"}}) | ||
| if err == nil { | ||
| t.Fatal("expected invalid transport config error") | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating transport at config/build time rather than only at Open.
TestOpenAICodexBackendFactory_invalidTransportFromYAML confirms BuildBackend succeeds for transport: quic and the error only surfaces when a request calls Open. If this is unintentional, fail-fast validation at build time would surface misconfiguration before traffic is served.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/pluginreg/backends_openaicodex_test.go` around lines 339 - 363, The
openai-codex backend currently accepts an invalid transport value from YAML and
only fails later in Open, so move validation into the configuration/build path
instead. Update the backend factory path used by
NewRegistry/InstallStandardBackendsOn and BuildBackend so transport is checked
when constructing the backend, and return an error for unsupported values like
quic before the backend is usable. Keep Open as a fallback only if needed, but
prefer failing fast in the build/config parsing flow for
OpenAICodexBackendFactory.
Comment on lines
+21
to
+34
| func TestWrapFollowsDebugGate(t *testing.T) { | ||
| t.Parallel() | ||
| inner := &testStream{} | ||
| wrapped := Wrap(context.Background(), slog.New(slog.NewTextHandler(io.Discard, nil)), "test", &lipapi.Call{ID: "call"}, inner, time.Now()) | ||
| if diag.DebugTurnsEnabled() { | ||
| if wrapped == inner { | ||
| t.Fatal("Wrap enabled returned original stream, want debug wrapper") | ||
| } | ||
| return | ||
| } | ||
| if wrapped != inner { | ||
| t.Fatalf("Wrap disabled returned %T, want original stream", wrapped) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
Test only reliably exercises the "disabled" branch in typical CI.
TestWrapFollowsDebugGate branches on diag.DebugTurnsEnabled(), but since that value is memoized process-wide via sync.OnceValue in diag, and LIP_CODEX_DEBUG_TURNS is presumably unset in CI, the "enabled" branch (asserting Wrap returns a real wrapper) is unlikely to ever run. Consider a build-tag/subprocess test, or exposing a test-only reset hook in diag, to deterministically cover both branches.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/plugins/frontends/streamdebug/streamdebug_test.go` around lines 21 -
34, TestWrapFollowsDebugGate currently depends on diag.DebugTurnsEnabled(),
which is memoized and makes the enabled path nondeterministic in CI. Update the
streamdebug tests around Wrap to deterministically cover both outcomes by using
either a subprocess/build-tag approach or a test-only reset hook in diag, so you
can verify both the wrapped and passthrough cases without relying on
process-wide state. Keep the assertions centered on Wrap and DebugTurnsEnabled
so the branch under test is explicit.
Resolve CodeRabbit review findings: - tool-call ID canonicalization: remap provisional item-only IDs onto the learned call_id so args-before-added no longer fragments one tool call into two (openairesponsestream.RemapToolCallID + stream.go) - parameterless object tool schemas inject additionalProperties:false and required:[] so strict:true is accepted by the Responses API - no-tools payloads omit parallel_tool_calls (tool-protocol signal leak) - authjson rejects non-regular token files (FIFO/device) - managed oauth files count skipped symlinks for an accurate error - codexclientcompat drops tool-use instructions when no tools; add nil guard - streamdebug Close always emits the terminal debug record with correct status - gemini handler trims ALegID consistently across EnsureCallDiag calls - openailegacy decode drops dead cp=nil assignment - diag/streamdebug use sort.Strings instead of duplicated insertion sorts - ws dialer documents custom-RoundTripper fallback; drop dead close branch - refbackend SSE event name derived from frame type; WS tests add read deadlines Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/plugins/backends/openaicodex/payload_test.go (1)
195-222: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd coverage for explicit no-tools
parallel_tool_calls.This only covers the unset/default path. Add a case where
call.Options.ParallelToolCallsis explicitlyfalse; that currently still serializes the field despite no tools.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/plugins/backends/openaicodex/payload_test.go` around lines 195 - 222, The no-tools payload test only covers the default unset path, so add coverage in TestPayloadForCall_noToolsOmitsToolChoice for an explicit false call.Options.ParallelToolCalls value. Update the existing PayloadForCall-based test to build a call with ParallelToolCalls set to false and verify the serialized payload from backend.PayloadForCall still omits both tool_choice and parallel_tool_calls when no tools are present, using the same JSON inspection approach already in the test.internal/plugins/backends/openaicodex/toolschema.go (1)
29-35: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject malformed
propertiesandrequiredshapes before marking strict.The unchecked assertions collapse present-but-invalid values to empty maps/slices. For example, after Line 83 injects
required: [], atype:"object"schema with non-mappropertiescan pass strict compatibility and be emitted asstrict:true.Proposed fix
- props, _ := schema["properties"].(map[string]any) - reqRaw, _ := schema["required"].([]any) + var props map[string]any + if rawProps, ok := schema["properties"]; ok { + var propsOK bool + props, propsOK = rawProps.(map[string]any) + if !propsOK { + return false + } + } + rawReq, ok := schema["required"] + if !ok { + return false + } + reqRaw, ok := rawReq.([]any) + if !ok { + return false + } required := make(map[string]bool, len(reqRaw)) for _, r := range reqRaw { - if s, ok := r.(string); ok { - required[s] = true + s, ok := r.(string) + if !ok { + return false } + required[s] = true }Also applies to: 83-86
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/plugins/backends/openaicodex/toolschema.go` around lines 29 - 35, The strict-schema handling in schemaToToolSchema is treating malformed `properties` and `required` values as empty collections because of unchecked type assertions, which can incorrectly allow invalid object schemas through. Update schemaToToolSchema to explicitly validate the shape of schema["properties"] and schema["required"] before building the required map or setting strict, and reject or return an error when either field is present but not the expected map/slice type. Also ensure the injected required path in the strict conversion logic preserves validation so a type:"object" schema with bad properties cannot end up emitted as strict:true.internal/plugins/backends/openaicodex/stream.go (1)
221-229: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReuse the remembered
call_idinoutput_item.done.Line 221 saves the canonical
call_id, but Lines 223 and 227 still readev.Item.CallIDdirectly. Ifoutput_item.addedlearned thecall_idandoutput_item.donelater omits it, this path finishes the tool call under the fallback item ID and appends an output item with an emptycall_id.Proposed fix
m.rememberToolCallID(ev.Item.ID, ev.Item.CallID) - m.remapProvisionalToolCall(ev.Item.ID, ev.Item.CallID) - if item, ok := outputFunctionCallInputItem(ev.Item.Type, ev.Item.ID, ev.Item.CallID, ev.Item.Name, ev.Item.Arguments); ok { + callID := strings.TrimSpace(ev.Item.CallID) + if callID == "" { + callID = strings.TrimSpace(m.toolCallIDs[ev.Item.ID]) + } + m.remapProvisionalToolCall(ev.Item.ID, callID) + if item, ok := outputFunctionCallInputItem(ev.Item.Type, ev.Item.ID, callID, ev.Item.Name, ev.Item.Arguments); ok { m.outputItems = append(m.outputItems, item) } return m.mapper.FinishToolCallArguments( - codexCanonicalToolCallID(ev.Item.ID, ev.Item.CallID), + codexCanonicalToolCallID(ev.Item.ID, callID), ev.Item.Name, ev.Item.Arguments, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/plugins/backends/openaicodex/stream.go` around lines 221 - 229, Reuse the remembered call_id when completing output_item.done in stream.go. The path in the handler that calls rememberToolCallID and remapProvisionalToolCall should look up and use the stored call_id instead of reading ev.Item.CallID directly, so outputFunctionCallInputItem and m.mapper.FinishToolCallArguments both receive the canonical remembered value. Update the tool-call completion flow around ev.Item.ID, ev.Item.CallID, and codexCanonicalToolCallID to ensure the output item is finished and appended with the same call_id learned from output_item.added.
♻️ Duplicate comments (1)
internal/plugins/backends/openaicodex/ws.go (1)
244-262: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRuntime fallback still loses client networking behavior.
The new comment documents the limitation, but the WebSocket path still silently falls back when
client.Transportis custom or nil, so proxy/TLS/dial settings can diverge from the HTTPS path. Consider initializing fromwebsocket.DefaultDialerand surfacing the custom-transport fallback explicitly.🩹 Partial hardening for default proxy behavior
func newWSDialer(client *http.Client) *websocket.Dialer { - d := &websocket.Dialer{HandshakeTimeout: wsHandshakeTimeout} + base := *websocket.DefaultDialer + d := &base + d.HandshakeTimeout = wsHandshakeTimeoutgorilla/websocket v1.5.3 Dialer Proxy nil behavior DefaultDialer ProxyFromEnvironment🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/plugins/backends/openaicodex/ws.go` around lines 244 - 262, The WebSocket dialer setup in newWSDialer still drops proxy/TLS/dial behavior when client.Transport is custom or nil, causing it to diverge from the HTTPS path. Update newWSDialer to start from websocket.DefaultDialer so default proxy behavior is preserved, then override settings only when an *http.Transport is available. Make the custom-transport fallback explicit in the code path and ensure any unsupported transport case is clearly handled rather than silently falling back.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/plugins/backends/openaicodex/payload.go`:
- Around line 99-104: The payload builder in the OpenAICodex path is still
copying call.Options.ParallelToolCalls even when there are no tools, which leaks
a tool-protocol field on no-tools turns. Update the ParallelToolCalls handling
in the payload assembly code so the entire block only runs when len(call.Tools)
> 0, preserving the existing fallback behavior only for tool-bearing calls and
preventing explicit parallel_tool_calls from being sent otherwise.
In `@internal/plugins/backends/openaicodex/toolschema.go`:
- Around line 69-75: The normalization in addStrictAdditionalProperties is too
broad because it recurses into every map/array value and can mutate non-schema
payloads like enum, default, or examples. Update addStrictAdditionalProperties
to only descend through actual subschema-bearing keywords in the OpenAICodex
schema traversal, and keep the existing strict-additional-properties behavior
limited to schema objects rather than arbitrary nested object literals.
In `@internal/refbackend/openaicodex/server_test.go`:
- Line 349: The websocket test in `server_test.go` is ignoring the result of the
forced-failure `WriteMessage` call, which can hide a premature server close and
weaken the “closes before first event” assertion. Update the test around the
`conn.WriteMessage` send of `response.create` to check and fail on any write
error, using the existing connection setup in the test so it only proceeds when
the frame is actually sent successfully.
---
Outside diff comments:
In `@internal/plugins/backends/openaicodex/payload_test.go`:
- Around line 195-222: The no-tools payload test only covers the default unset
path, so add coverage in TestPayloadForCall_noToolsOmitsToolChoice for an
explicit false call.Options.ParallelToolCalls value. Update the existing
PayloadForCall-based test to build a call with ParallelToolCalls set to false
and verify the serialized payload from backend.PayloadForCall still omits both
tool_choice and parallel_tool_calls when no tools are present, using the same
JSON inspection approach already in the test.
In `@internal/plugins/backends/openaicodex/stream.go`:
- Around line 221-229: Reuse the remembered call_id when completing
output_item.done in stream.go. The path in the handler that calls
rememberToolCallID and remapProvisionalToolCall should look up and use the
stored call_id instead of reading ev.Item.CallID directly, so
outputFunctionCallInputItem and m.mapper.FinishToolCallArguments both receive
the canonical remembered value. Update the tool-call completion flow around
ev.Item.ID, ev.Item.CallID, and codexCanonicalToolCallID to ensure the output
item is finished and appended with the same call_id learned from
output_item.added.
In `@internal/plugins/backends/openaicodex/toolschema.go`:
- Around line 29-35: The strict-schema handling in schemaToToolSchema is
treating malformed `properties` and `required` values as empty collections
because of unchecked type assertions, which can incorrectly allow invalid object
schemas through. Update schemaToToolSchema to explicitly validate the shape of
schema["properties"] and schema["required"] before building the required map or
setting strict, and reject or return an error when either field is present but
not the expected map/slice type. Also ensure the injected required path in the
strict conversion logic preserves validation so a type:"object" schema with bad
properties cannot end up emitted as strict:true.
---
Duplicate comments:
In `@internal/plugins/backends/openaicodex/ws.go`:
- Around line 244-262: The WebSocket dialer setup in newWSDialer still drops
proxy/TLS/dial behavior when client.Transport is custom or nil, causing it to
diverge from the HTTPS path. Update newWSDialer to start from
websocket.DefaultDialer so default proxy behavior is preserved, then override
settings only when an *http.Transport is available. Make the custom-transport
fallback explicit in the code path and ensure any unsupported transport case is
clearly handled rather than silently falling back.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 91169c95-512e-499c-9ba9-979b9cf342df
📒 Files selected for processing (18)
internal/core/diag/debug_summary.gointernal/plugins/backends/openaicodex/authjson.gointernal/plugins/backends/openaicodex/managed_oauth_files.gointernal/plugins/backends/openaicodex/payload.gointernal/plugins/backends/openaicodex/payload_test.gointernal/plugins/backends/openaicodex/stream.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/backends/openaicodex/toolschema.gointernal/plugins/backends/openaicodex/ws.gointernal/plugins/backends/protocols/openairesponsestream/mapper.gointernal/plugins/backends/protocols/openairesponsestream/mapper_test.gointernal/plugins/features/codexclientcompat/compat.gointernal/plugins/features/codexclientcompat/plugin.gointernal/plugins/frontends/gemini/handler.gointernal/plugins/frontends/openailegacy/decode.gointernal/plugins/frontends/streamdebug/streamdebug.gointernal/refbackend/openaicodex/server.gointernal/refbackend/openaicodex/server_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: qa
🧰 Additional context used
📓 Path-based instructions (8)
internal/plugins/backends/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/plugins/backends/, implement provider/local/compatible backend adapters, with the exact bundle defined ininternal/pluginreg/standard_table.go.
Files:
internal/plugins/backends/openaicodex/managed_oauth_files.gointernal/plugins/backends/protocols/openairesponsestream/mapper_test.gointernal/plugins/backends/openaicodex/authjson.gointernal/plugins/backends/protocols/openairesponsestream/mapper.gointernal/plugins/backends/openaicodex/stream.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/backends/openaicodex/ws.gointernal/plugins/backends/openaicodex/toolschema.gointernal/plugins/backends/openaicodex/payload.gointernal/plugins/backends/openaicodex/payload_test.go
**/*.go
📄 CodeRabbit inference engine (Custom checks)
**/*.go: For server, CLI, worker, and network code, propagatecontext.Contextcorrectly, respect cancellation, and ensure new goroutines cannot leak indefinitely.
For changes underpkg/**or any exported Go identifier, warn if the PR changes exported types, function signatures, error behavior, JSON fields, CLI flags, config keys, or documented behavior without clearly explaining the compatibility impact.
Files:
internal/plugins/backends/openaicodex/managed_oauth_files.gointernal/plugins/features/codexclientcompat/plugin.gointernal/core/diag/debug_summary.gointernal/plugins/backends/protocols/openairesponsestream/mapper_test.gointernal/plugins/backends/openaicodex/authjson.gointernal/refbackend/openaicodex/server_test.gointernal/plugins/frontends/openailegacy/decode.gointernal/plugins/backends/protocols/openairesponsestream/mapper.gointernal/plugins/backends/openaicodex/stream.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/backends/openaicodex/ws.gointernal/plugins/features/codexclientcompat/compat.gointernal/refbackend/openaicodex/server.gointernal/plugins/frontends/gemini/handler.gointernal/plugins/backends/openaicodex/toolschema.gointernal/plugins/frontends/streamdebug/streamdebug.gointernal/plugins/backends/openaicodex/payload.gointernal/plugins/backends/openaicodex/payload_test.go
⚙️ CodeRabbit configuration file
**/*.go: Review as production Go code. Prioritize correctness, race conditions, goroutine leaks, context cancellation, timeout handling, error wrapping, nil-pointer risks, resource cleanup, defer placement, API compatibility, interface design, dependency boundaries, and testability. Avoid generic style comments when gofmt/golangci-lint already covers the issue.
Files:
internal/plugins/backends/openaicodex/managed_oauth_files.gointernal/plugins/features/codexclientcompat/plugin.gointernal/core/diag/debug_summary.gointernal/plugins/backends/protocols/openairesponsestream/mapper_test.gointernal/plugins/backends/openaicodex/authjson.gointernal/refbackend/openaicodex/server_test.gointernal/plugins/frontends/openailegacy/decode.gointernal/plugins/backends/protocols/openairesponsestream/mapper.gointernal/plugins/backends/openaicodex/stream.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/backends/openaicodex/ws.gointernal/plugins/features/codexclientcompat/compat.gointernal/refbackend/openaicodex/server.gointernal/plugins/frontends/gemini/handler.gointernal/plugins/backends/openaicodex/toolschema.gointernal/plugins/frontends/streamdebug/streamdebug.gointernal/plugins/backends/openaicodex/payload.gointernal/plugins/backends/openaicodex/payload_test.go
internal/**
⚙️ CodeRabbit configuration file
internal/**: Focus on package boundaries, hidden coupling, unexported API design, concurrency safety, deterministic behavior, and whether logic belongs in this internal package.
Files:
internal/plugins/backends/openaicodex/managed_oauth_files.gointernal/plugins/features/codexclientcompat/plugin.gointernal/core/diag/debug_summary.gointernal/plugins/backends/protocols/openairesponsestream/mapper_test.gointernal/plugins/backends/openaicodex/authjson.gointernal/refbackend/openaicodex/server_test.gointernal/plugins/frontends/openailegacy/decode.gointernal/plugins/backends/protocols/openairesponsestream/mapper.gointernal/plugins/backends/openaicodex/stream.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/backends/openaicodex/ws.gointernal/plugins/features/codexclientcompat/compat.gointernal/refbackend/openaicodex/server.gointernal/plugins/frontends/gemini/handler.gointernal/plugins/backends/openaicodex/toolschema.gointernal/plugins/frontends/streamdebug/streamdebug.gointernal/plugins/backends/openaicodex/payload.gointernal/plugins/backends/openaicodex/payload_test.go
internal/plugins/features/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/plugins/features/, implement official feature and reference plugins.
Files:
internal/plugins/features/codexclientcompat/plugin.gointernal/plugins/features/codexclientcompat/compat.go
internal/core/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/core/, keep runtime orchestration, routing, continuity, streams, hooks/extensions, config, and diagnostics.
Files:
internal/core/diag/debug_summary.go
**/*_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Review tests for meaningful assertions, table-driven coverage, race-prone tests, t.Parallel misuse, nondeterminism, leaked goroutines, real network or filesystem dependencies, fragile sleeps, and missing edge cases. Prefer testing observable behavior over implementation details.
Files:
internal/plugins/backends/protocols/openairesponsestream/mapper_test.gointernal/refbackend/openaicodex/server_test.gointernal/plugins/backends/openaicodex/stream_internal_test.gointernal/plugins/backends/openaicodex/payload_test.go
internal/refbackend/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/refbackend/, keep test-only emulators.
Files:
internal/refbackend/openaicodex/server_test.gointernal/refbackend/openaicodex/server.go
internal/plugins/frontends/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
In
internal/plugins/frontends/, implement OpenAI Responses, OpenAI legacy, Anthropic, and Gemini frontends.
Files:
internal/plugins/frontends/openailegacy/decode.gointernal/plugins/frontends/gemini/handler.gointernal/plugins/frontends/streamdebug/streamdebug.go
🔇 Additional comments (14)
internal/plugins/backends/openaicodex/managed_oauth_files.go (1)
25-30: LGTM!internal/plugins/frontends/openailegacy/decode.go (1)
48-48: LGTM!Also applies to: 227-236
internal/plugins/features/codexclientcompat/compat.go (1)
580-614: LGTM!internal/plugins/backends/openaicodex/payload_test.go (1)
820-856: LGTM!internal/plugins/features/codexclientcompat/plugin.go (1)
37-40: LGTM!internal/plugins/backends/protocols/openairesponsestream/mapper_test.go (1)
320-390: LGTM!internal/plugins/backends/openaicodex/authjson.go (1)
142-144: LGTM!internal/plugins/backends/protocols/openairesponsestream/mapper.go (1)
268-294: LGTM!internal/plugins/backends/openaicodex/stream_internal_test.go (1)
390-438: LGTM!internal/core/diag/debug_summary.go (1)
6-6: LGTM!Also applies to: 34-45
internal/refbackend/openaicodex/server_test.go (1)
266-325: LGTM!internal/refbackend/openaicodex/server.go (1)
52-56: LGTM!Also applies to: 82-82, 353-376
internal/plugins/frontends/gemini/handler.go (1)
35-35: LGTM!Also applies to: 118-118, 128-133, 153-155, 169-171
internal/plugins/frontends/streamdebug/streamdebug.go (1)
9-9: LGTM!Also applies to: 163-196, 303-307
| } | ||
| defer func() { _ = conn.Close() }() | ||
| _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) | ||
| _ = conn.WriteMessage(gorillawebsocket.TextMessage, []byte(`{"type":"response.create"}`)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fail if the forced-failure request frame cannot be sent.
Ignoring WriteMessage lets this test pass even if the server closes before receiving response.create, so it no longer verifies “closes before first event” behavior.
🩹 Proposed fix
- _ = conn.WriteMessage(gorillawebsocket.TextMessage, []byte(`{"type":"response.create"}`))
+ if err := conn.WriteMessage(gorillawebsocket.TextMessage, []byte(`{"type":"response.create"}`)); err != nil {
+ t.Fatalf("write: %v", err)
+ }As per path instructions, **/*_test.go: “Review tests for meaningful assertions”.
📝 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
| _ = conn.WriteMessage(gorillawebsocket.TextMessage, []byte(`{"type":"response.create"}`)) | |
| if err := conn.WriteMessage(gorillawebsocket.TextMessage, []byte(`{"type":"response.create"}`)); err != nil { | |
| t.Fatalf("write: %v", err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/refbackend/openaicodex/server_test.go` at line 349, The websocket
test in `server_test.go` is ignoring the result of the forced-failure
`WriteMessage` call, which can hide a premature server close and weaken the
“closes before first event” assertion. Update the test around the
`conn.WriteMessage` send of `response.create` to check and fail on any write
error, using the existing connection setup in the test so it only proceeds when
the frame is actually sent successfully.
Source: Path instructions
- payload.go: gate the whole parallel_tool_calls block on tools present so an explicit parallel_tool_calls value is not leaked on no-tools turns - toolschema.go: addStrictAdditionalProperties now recurses only through subschema-bearing keywords (properties/items/oneOf/anyOf/allOf) so non-schema payloads like enum/default/examples are not mutated with strict keywords - add tests for both behaviors (no-tools explicit parallel_tool_calls omitted; default value not mutated) Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
openai-codexWebSocket transport with HTTPS fallback, cooldown, continuation handling, and refbackend WS coverageinternal/core/diagand remove dead payload state from the Codex open pathValidation
go test ./internal/plugins/backends/openaicodex ./internal/plugins/frontends/streamdebug ./internal/core/diag ./internal/infra/runtimebundlegit diff --checkmake qaNotes