fix(proxy): suppress duplicate side-effect tool calls

[Komzpa]

Summary

• suppress same-response replayed side-effect tool-call events even when upstream changes call_id
• rewrite duplicate nested operations inside multi_tool_use.parallel before forwarding the batch to the client
• keep read-only calls and repeated operations under later response ids intact

Validation

• uv run ruff check app/modules/proxy/service.py tests/unit/test_proxy_utils.py
• uv run ty check app/modules/proxy/service.py tests/unit/test_proxy_utils.py
• uv run pytest tests/unit/test_proxy_utils.py -q -k 'tool_call or stream_responses_keeps_distinct_http_tool_calls_across_response_ids or stream_responses_suppresses_same_response_http_tool_call_replay or parallel_tool'\n\nSplit from fix(proxy): trim Codex websocket full-replay continuations #555.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a337e6aa9

ℹ️ 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".

[Soju06]

Thanks @Komzpa -- the side-effect tool-call dedupe is exactly the right shape (per-response id cache, side-effect allowlist, multi_tool_use.parallel nested rewriting, distinct read-only / later-response-id calls intact), and the OpenSpec change folder is in place. The 566 lines of unit coverage are also a great safety net. Codex review picked up two issues that should land before merging.

P1 -- wait_agent dedupe must not raise on malformed targets

app/modules/proxy/service.py:~8653 calls sorted(targets) on tool-call payload that is model-generated JSON. If the model emits functions.wait_agent with mixed-type targets (or any otherwise non-comparable list), sorted raises TypeError and aborts the whole stream-processing path. That converts what's supposed to be a best-effort dedupe into a hard stream failure on input the proxy has no control over.

Suggested fix: feature-detect comparability, or just sort safely with a key that's always comparable, then fall back to the original list on failure:

def _normalize_wait_agent_targets(targets):
    if not isinstance(targets, list):
        return targets
    try:
        return sorted(targets, key=lambda t: (t.__class__.__name__, str(t)))
    except (TypeError, ValueError):
        return list(targets)  # fall through; dedupe is best-effort

And a regression to pin behavior:

def test_wait_agent_dedupe_tolerates_mixed_type_targets():
    parallel = {
        "call_id": "call_x",
        "name": "multi_tool_use.parallel",
        "arguments": json.dumps({
            "tool_uses": [
                {"recipient_name": "functions.wait_agent", "parameters": {"targets": [1, "a", {}]}}
            ]
        }),
    }
    # should not raise, should forward the event unchanged (or at worst with a no-op dedupe)
    result = _maybe_dedupe_parallel_tool_use(parallel, dedupe_cache={})
    assert result is not None

P2 -- normalize top-level exec_command / write_stdin keys before dedupe

The top-level side-effect path keys on the raw arguments string:

# app/modules/proxy/service.py:~8619
dedupe_key = (call_name, arguments_str)

while the nested multi_tool_use.parallel path already strips volatile fields (yield_time_ms, max_output_tokens, etc.) before keying. That asymmetry defeats the dedupe in the exact replay scenario this PR is targeting:

• Same response_id, same exec_command body, but a replayed event with yield_time_ms: 5000 vs yield_time_ms: 6000 -> two distinct keys -> both events forwarded -> command executed twice.

Suggested fix: share the volatile-field-stripping helper between the top-level and nested paths, and key on the normalized arguments dict (or its canonical-JSON dump):

_SIDE_EFFECT_VOLATILE_ARG_KEYS = frozenset({"yield_time_ms", "max_output_tokens", "timeout_ms"})

def _side_effect_dedupe_key(call_name: str, arguments: dict) -> str:
    operative = {k: v for k, v in arguments.items() if k not in _SIDE_EFFECT_VOLATILE_ARG_KEYS}
    return f"{call_name}::{json.dumps(operative, sort_keys=True, separators=(',', ':'))}"

Then both top-level (exec_command/write_stdin) and the parallel inner-dedupe use the same function.

Test to pin:

def test_top_level_exec_command_dedupes_across_yield_time_ms_differences():
    # same response_id, two events with identical exec_command `command` but different `yield_time_ms`
    state = _make_dedupe_state(response_id="resp_a")
    first = _exec_event(call_id="call_a", command="echo hi", yield_time_ms=5000)
    second = _exec_event(call_id="call_b", command="echo hi", yield_time_ms=6000)
    assert _consume_side_effect_event(first, state) is not None      # forwarded
    assert _consume_side_effect_event(second, state) is None         # suppressed

Not blocking otherwise

• Scope (split from fix(proxy): trim Codex websocket full-replay continuations #555) is clean.
• Cache limit (_WEBSOCKET_TOOL_CALL_DEDUPE_CACHE_LIMIT = 1024) is reasonable for per-response keying and won't grow unbounded.
• Allowlist for side-effect tool names is conservative -- read-only calls and later-response ids are still passed through.

After the P1 fix and (ideally) the P2 normalization land, I'll re-run @codex review and queue for merge.

[chatgpt-codex-connector]

To use Codex here, create an environment for this repo.