Skip to content

_WIRE_ROLES permits a 'tool' message that _for_the_wire has already stripped the tool_call_id from #223

Description

@thedancingdeveloper

Spotted while reviewing my own adapter work, confirmed against main after the hardening in #217, and not previously written down.

The contradiction

_for_the_wire reduces every message to exactly two fields:

{"role": role if role in _WIRE_ROLES else "user", "content": content}

and _WIRE_ROLES is:

frozenset({"system", "user", "assistant", "tool"})

A tool message is only valid when it carries the tool_call_id it answers. _for_the_wire has just discarded that. So the allow-list permits through the one role the function guarantees it has broken.

Why nothing is failing

Nothing currently emits a tool role. format_observation_messages deliberately returns observations as user turns, and the docstring says why: a tool message without its id is a malformed request, and that is what upstream_rejected was.

So this is latent, not live. The code is correct in what it does and wrong in what it permits.

Why it is worth closing anyway

The reasoning is already recorded in format_observation_messagesdo not send a tool role, because we cannot send its id. _WIRE_ROLES is the enforcement point for that rule and currently contradicts it. The next person who adds a second tool, or reinstates the loop's own observation formatter (which does pair tool_call_id correctly), will find the allow-list agreeing with them and the stripper silently disagreeing.

The failure mode when it does happen is the expensive one: a refusal from the gateway with no indication which message was malformed. That has already cost this repository a live run once.

The fix, and the alternative

Either:

  • drop tool from _WIRE_ROLES, so the contradiction cannot be reached and an accidental tool message degrades to a user turn — consistent with how every other unknown role is already handled; or
  • carry tool_call_id through _for_the_wire for tool messages specifically, and use the loop's own paired formatter. That is the better shape if more than one tool is ever offered, and it is a larger change.

The first is right until there is a reason for the second.

Blind spots

  • Whether mini-swe-agent upgrades might start emitting tool roles through a path this adapter does not control has not been checked; if they can, the second option becomes necessary rather than preferable.
  • No other allow-list in the adapter has been audited for the same shape of contradiction.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:model-clientThe ModelClient: routing, retry classification, per-endpoint cooldowntype:taskUnit of implementation work

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions