-
Notifications
You must be signed in to change notification settings - Fork 421
fix(proxy): recover compact bridge continuations #1727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,6 +66,8 @@ | |
| _PENDING_TOOL_CALL_ITEM_TYPES = frozenset(_PENDING_TOOL_CALL_OUTPUT_ITEM_TYPE_BY_CALL_TYPE) | ||
| _PENDING_TOOL_CALL_OUTPUT_ITEM_TYPES = frozenset(_PENDING_TOOL_CALL_OUTPUT_ITEM_TYPE_BY_CALL_TYPE.values()) | ||
| _TTFT_OUTPUT_ITEM_TYPES = _PENDING_TOOL_CALL_ITEM_TYPES - {"function_call"} | ||
| _AGENT_CONTROL_TOOL_NAMESPACES = frozenset({"collaboration", "multi_agent_v1"}) | ||
| _AGENT_CONTROL_TOOL_NAMES = frozenset({"close_agent", "resume_agent", "send_input", "spawn_agent", "wait_agent"}) | ||
| _WEBSOCKET_FULL_REPLAY_WAIT_MIN_ITEMS = 20 | ||
| _WEBSOCKET_FULL_REPLAY_WAIT_POLL_SECONDS = 0.05 | ||
| _HARD_HTTP_BRIDGE_AFFINITY_KINDS = frozenset( | ||
|
|
@@ -78,6 +80,34 @@ | |
| } | ||
| ) | ||
| _ACCOUNT_SELECTION_RECOVERY_MIN_SLEEP_SECONDS = 1.0 | ||
|
|
||
|
|
||
| def _protected_agent_control_tool_call_ids(input_value: JsonValue) -> set[str]: | ||
| if not isinstance(input_value, list): | ||
| return set() | ||
| call_ids: set[str] = set() | ||
| for item in input_value: | ||
| if not isinstance(item, Mapping): | ||
| continue | ||
| item_type = item.get("type") | ||
| if not isinstance(item_type, str) or item_type not in _PENDING_TOOL_CALL_ITEM_TYPES: | ||
| continue | ||
| namespace = item.get("namespace") | ||
| name = item.get("name") | ||
| is_agent_control = ( | ||
| isinstance(namespace, str) | ||
| and namespace in _AGENT_CONTROL_TOOL_NAMESPACES | ||
| or isinstance(name, str) | ||
| and name in _AGENT_CONTROL_TOOL_NAMES | ||
|
Comment on lines
+97
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a non-agent namespaced tool happens to use a common name such as Useful? React with 👍 / 👎. |
||
| ) | ||
| if not is_agent_control: | ||
| continue | ||
| call_id = item.get("call_id") | ||
| if isinstance(call_id, str) and call_id: | ||
| call_ids.add(call_id) | ||
| return call_ids | ||
|
|
||
|
|
||
| _HARD_AFFINITY_RECOVERY_SLEEP_SECONDS = 2.0 | ||
| _ACCOUNT_SELECTION_RECOVERY_DEFAULT_SLEEP_SECONDS = 30.0 | ||
| _ACCOUNT_SELECTION_RECOVERY_MAX_SLEEP_SECONDS = 300.0 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1609,6 +1609,21 @@ def _matching_websocket_request_states_for_previous_response_error( | |
| ] | ||
| if matching_requests: | ||
| return matching_requests | ||
| visible_unresolved_followups = [ | ||
| request_state | ||
| for request_state in followup_requests | ||
| if request_state.response_id is None and _http_bridge_request_counts_against_queue(request_state) | ||
| ] | ||
| if len(visible_unresolved_followups) == 1: | ||
| return visible_unresolved_followups | ||
|
Comment on lines
+1617
to
+1618
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The only new coverage for this branch calls AGENTS.md reference: AGENTS.md:L125-L128 Useful? React with 👍 / 👎. |
||
| if len(visible_unresolved_followups) > 1: | ||
| unique_previous_response_ids = { | ||
| request_state.previous_response_id | ||
| for request_state in visible_unresolved_followups | ||
| if request_state.previous_response_id | ||
| } | ||
| if len(unique_previous_response_ids) == 1: | ||
| return visible_unresolved_followups | ||
| unresolved_followups = [request_state for request_state in followup_requests if request_state.response_id is None] | ||
| if len(unresolved_followups) == 1: | ||
| return unresolved_followups | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,15 @@ | ||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: Replayed tool-call namespace metadata is local-only on upstream input | ||
| ### 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. | ||
|
Comment on lines
+3
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 AGENTS.md reference: AGENTS.md:L24-L28 Useful? React with 👍 / 👎. |
||
|
|
||
| #### Scenario: Standard Responses replay omits tool-call namespaces upstream | ||
| 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. | ||
|
|
||
| #### Scenario: Standard Responses replay preserves tool-call namespaces upstream | ||
|
|
||
| - **WHEN** a standard Responses request replays `function_call` and `custom_tool_call` input items with `namespace` | ||
| - **THEN** the upstream payload omits only those items' `namespace` | ||
| - **THEN** the upstream payload preserves those items' `namespace` | ||
| - **AND** preserves their remaining call fields | ||
| - **AND** the local request input retains the namespace metadata | ||
|
|
||
|
|
@@ -17,16 +19,16 @@ For standard and compact Responses requests, the proxy MUST omit `namespace` fro | |
| - **THEN** its upstream payload omits the input item's `namespace` | ||
| - **AND** preserves the remaining tool-call fields | ||
|
|
||
| #### Scenario: WebSocket response.create omits tool-call namespaces upstream | ||
| #### Scenario: WebSocket response.create preserves tool-call namespaces upstream | ||
|
|
||
| - **WHEN** a Responses WebSocket request replays namespaced `function_call` and `custom_tool_call` input items | ||
| - **THEN** the upstream `response.create` frame omits only those items' `namespace` | ||
| - **THEN** the upstream `response.create` frame preserves those items' `namespace` | ||
| - **AND** preserves their remaining call fields | ||
|
|
||
| #### Scenario: Configured Responses model source omits tool-call namespaces upstream | ||
| #### Scenario: Configured Responses model source preserves tool-call namespaces upstream | ||
|
|
||
| - **WHEN** `/v1/responses` routes a replayed namespaced tool call to a configured OpenAI-compatible Responses model source | ||
| - **THEN** the source payload omits only the call item's `namespace` | ||
| - **THEN** the source payload preserves the call item's `namespace` | ||
| - **AND** preserves source-compatible request fields that the Codex upstream path does not support | ||
|
|
||
| #### Scenario: Account-neutral replay classification retains namespace identity | ||
|
|
@@ -45,3 +47,10 @@ For standard and compact Responses requests, the proxy MUST omit `namespace` fro | |
|
|
||
| - **WHEN** the client includes a top-level tool entry whose `type` is `namespace` | ||
| - **THEN** standard Responses serialization forwards that tool entry byte-identically | ||
|
|
||
| #### Scenario: Historical agent wait output remains visible after slimming | ||
|
|
||
| - **WHEN** a response.create payload contains a historical `multi_agent_v1.wait_agent` call with a large matching output | ||
| - **AND** also contains an unrelated large historical shell output | ||
| - **THEN** the agent wait output remains byte-preserved in the upstream input | ||
| - **AND** the unrelated shell output MAY be replaced with the historical tool-output omission notice | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3263,6 +3263,8 @@ For a namespaced side-effect function or custom-tool call, the service MUST use | |
|
|
||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This new requirement is not applied to the separate direct-upstream WebSocket path: AGENTS.md reference: AGENTS.md:L125-L128 Useful? React with 👍 / 👎. |
||
|
|
||
| #### Scenario: Distinct namespaced spawns use identical arguments | ||
|
|
||
| - **WHEN** two `collaboration.spawn_agent` calls have identical arguments and different call IDs | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fresh pre-serialization capture here addresses the earlier slimming-set comment, but
upstream_payloadis still built immediately afterward withResponsesRequest.to_payload(), which invokes_strip_unsupported_fields()with replayed-namespace stripping enabled. Consequently, standard HTTP-bridge and WebSocket replays containingcollaborationormulti_agent_v1calls 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 👍 / 👎.