fix(proxy): recover compact bridge continuations - #1727
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8d512c479
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
7c20e6c to
ff94b58
Compare
Komzpa
changed the title
Komzpa
added
🤖 codex: needs work
c6fb862 to
efdffb9
Compare
Komzpa
changed the title
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: efdffb9be8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| protected_tool_output_call_ids = _protected_agent_control_tool_call_ids(payload.input) | ||
| upstream_payload = dict(payload.to_payload()) |
There was a problem hiding this comment.
The fresh pre-serialization capture here addresses the earlier slimming-set comment, but upstream_payload is still built immediately afterward with ResponsesRequest.to_payload(), which invokes _strip_unsupported_fields() with replayed-namespace stripping enabled. Consequently, standard HTTP-bridge and WebSocket replays containing collaboration or multi_agent_v1 calls still reach upstream without the routing namespace promised by the newly edited live-egress contract; only the model-source path was changed to avoid this serializer. Use the namespace-preserving serializer on every standard live path and assert the actual HTTP and WebSocket payloads.
AGENTS.md reference: AGENTS.md:L129-L132
Useful? React with 👍 / 👎.
| is_agent_control = ( | ||
| isinstance(namespace, str) | ||
| and namespace in _AGENT_CONTROL_TOOL_NAMESPACES | ||
| or isinstance(name, str) | ||
| and name in _AGENT_CONTROL_TOOL_NAMES |
There was a problem hiding this comment.
When a non-agent namespaced tool happens to use a common name such as database.send_input or workflow.wait_agent, this or classifies it as agent control solely by name. Its oversized historical output is then exempted from slimming, so a request whose excess size comes from that output remains above the wire limit and is rejected instead of receiving the normal omission notice. Restrict the name-only fallback to calls with no usable namespace so an explicit unrelated namespace takes precedence.
Useful? React with 👍 / 👎.
| ### Requirement: Replayed tool-call namespace metadata survives live Responses egress | ||
|
|
||
| For standard and compact Responses requests, the proxy MUST omit `namespace` from every replayed `input` item whose `type` is `function_call`, `custom_tool_call`, or `apply_patch_call` before forwarding the request upstream. The proxy MUST preserve all other fields on that item, MUST retain the original namespace metadata for local call-identity and replay-deduplication processing, and MUST NOT alter client-provided top-level tool entries as part of this normalization. | ||
| For standard live Responses requests, including WebSocket `response.create` and configured Responses model-source egress, the proxy MUST preserve `namespace` on replayed `input` items whose `type` is `function_call`, `custom_tool_call`, or `apply_patch_call`. Tool namespaces are part of the live tool-routing identity. Compact Responses egress MAY omit those namespaces only as part of the compact-specific upstream compatibility serializer. The proxy MUST preserve all other fields on each tool-call item and MUST NOT alter client-provided top-level tool entries as part of this normalization. |
There was a problem hiding this comment.
This archived delta now requires standard live Responses egress to preserve replayed namespaces, but the normative main spec still contains the opposite requirement and scenarios at openspec/specs/responses-api-compat/spec.md:4357-4383, saying that standard, WebSocket, and model-source egress must omit them. Because the main capability spec is the repository's source of truth, leaving only the archived artifact updated documents the wrong compatibility contract and invites the stripping behavior to be restored; replace the obsolete main requirement and scenarios as part of this change.
AGENTS.md reference: AGENTS.md:L24-L28
Useful? React with 👍 / 👎.
|
|
||
| Flat legacy side-effect calls MAY continue to use argument-based replay identity so reconnects that change only a call ID do not repeat shell, patch, or terminal side effects. | ||
|
|
||
| Historical response.create slimming MUST preserve outputs for namespaced agent-control calls, including `collaboration` and `multi_agent_v1` calls, even when those outputs are large. Such outputs carry the completed spawn/wait state needed by the next model turn and MUST NOT be replaced with a generic historical tool-output omission notice. Unrelated historical tool outputs MAY still be slimmed under the normal payload-budget policy. |
There was a problem hiding this comment.
This new requirement is not applied to the separate direct-upstream WebSocket path: app/core/clients/proxy.py:1958 calls its own _prepare_websocket_response_create_payload(), whose private slimmer at lines 2242-2265 still replaces every oversized historical tool output without identifying agent-control calls. Therefore, when the regular Responses route selects transport == "websocket" rather than the HTTP bridge, a large completed spawn/wait output is still replaced by the omission notice and the next model turn loses the agent state. Route that product path through the same protection logic and add a regression test through the regular Responses endpoint.
AGENTS.md reference: AGENTS.md:L125-L128
Useful? React with 👍 / 👎.
efdffb9 to
680a73f
Compare
github-actions
Bot
added
the
db migration
936be38 to
5878e74
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5878e74529
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1,22 @@ | |||
| ## ADDED Requirements | |||
There was a problem hiding this comment.
This proxy/session behavior change adds only tasks.md and a delta spec under fix-ownerless-bridge-claim; repository-wide inspection of that change folder finds no proposal.md. This leaves the change without the required proposal artifact and fails the repository's hard OpenSpec merge gate, so add the proposal before treating the checked-off tasks as ready.
AGENTS.md reference: AGENTS.md:L92-L98
Useful? React with 👍 / 👎.
| if len(visible_unresolved_followups) == 1: | ||
| return visible_unresolved_followups |
There was a problem hiding this comment.
The only new coverage for this branch calls _matching_websocket_request_states_for_previous_response_error() directly; inspection of the changed HTTP bridge integration tests finds no route-level case with one live follow-up and one unresolved draining request. Because this selection controls which request is popped, settled, and receives an anonymous previous_response_not_found, a wiring regression could still terminate the wrong continuation; add a regression through the actual Responses HTTP bridge path.
AGENTS.md reference: AGENTS.md:L125-L128
Useful? React with 👍 / 👎.
Komzpa
added
the
needs rebase
5878e74 to
074ac69
Compare
|
Folded into #1730: post-compaction continuation and namespace preservation join the quarantine replay carrier as one post-compaction continuation PR; series replayed onto a clean base with authorship preserved. |
1 participant
Summary
namespacemetadata on standard/live Responses egress, including WebSocketresponse.createand configured Responses model-source forwardingValidation
uv run pytest tests/unit/test_openai_requests.py tests/unit/test_proxy_utils.py tests/integration/test_api_keys_api.py tests/integration/test_proxy_websocket_responses.py -qgit diff --checkorigin/main...HEAD; changed files are limited to proxy/request code, Responses compatibility specs, and focused tests.