update: rename duplicate tool-call ids on ingest so results land on the right card

CHANGELOG.md

@@ -4,6 +4,9 @@ All notable changes to agentty. Versions follow [SemVer](https://semver.org/).
 
 ## [Unreleased]
 
+### Fixed
+- **Tool results no longer vanish onto a dead card when a provider reuses tool-call ids.** Some OpenAI-compatible gateways mint a deterministic id per (tool, index) — literally `"bash:0"` for every bash call on every turn — instead of a unique `ToolCallId`. One agent turn holds several assistant messages in the live tail, so the next sub-turn's `"bash:0"` collided with the previous one's (already Done): the result was stamped onto the dead card, the real call stayed Pending, and its card hung until the step timeout. A duplicate import is now renamed at ingest (`bash:0#2`, …) with the wire id preserved for routing of the in-flight delta/end/result events, and tagged-back calls prefer the *first non-terminal* carrier, so every tool card completes with its own output. (`src/runtime/app/update/stream.cpp`; new `dup_tool_call_id_test`, 5 scenarios green.)
+
 ## [0.3.0] - 2026-08-14
 
 ### Added

CMakeLists.txt

@@ -2159,6 +2159,14 @@ if(AGENTTY_BUILD_TESTS)
     agentty_add_full_test(settings_default_test)
     set_tests_properties(settings_default_test PROPERTIES TIMEOUT 30)
 
+    # dup_tool_call_id_test — duplicate tool-call id ingest (stream.cpp
+    # `uniquify` + ToolUse::wire_id). OpenAI-compatible gateways that mint a
+    # per-(tool,index) id like "bash:0" every turn must not smash a live
+    # call's result onto the prior sub-turn's already-Done card (the stuck
+    # "card hangs until step timeout" bug): the newcomer is renamed to id#N
+    # and wire events keep routing via wire_id.
+    agentty_add_full_test(dup_tool_call_id_test)
+
     # salvage_dedup_test — re-leaked salvaged-tool-call dedup. Unit-tests
     # dedup_releaked_salvage_calls (cmd_factory.cpp): a weak local model that
     # re-leaks a tool call it already ran this turn must not run it twice.
@@ -2405,6 +2413,7 @@ if(AGENTTY_BUILD_TESTS)
         decomposition_memory_test
         smart_cascade_gate_test
         settings_default_test
+        dup_tool_call_id_test
         salvage_dedup_test
         doom_loop_test
         acp_integration_test

include/agentty/domain/conversation.hpp

@@ -83,6 +83,15 @@ struct ToolUse {
     using Status = std::variant<Pending, Approved, Running, Done, Failed, Rejected>;
 
     ToolCallId     id;
+    // The provider's ORIGINAL id, set only when `id` had to be rewritten at
+    // ingest because the wire id already belonged to another call in the
+    // live tail (update/stream.cpp's `uniquify`). The wire keeps addressing
+    // its own id on the following input_json_delta / tool_use_end events, so
+    // find_streaming_tool falls back to matching this. Empty whenever no
+    // rewrite happened, which is the overwhelmingly common case.
+    // Streaming-time scratch only — not persisted; a reloaded thread has no
+    // live stream to route.
+    ToolCallId     wire_id;
     ToolName       name;
     nlohmann::json args;
     std::string    args_streaming;

include/agentty/runtime/app/update/internal.hpp

@@ -228,6 +228,19 @@ void mark_tool_rejected(Model& m, const ToolCallId& id,
 // false — caller treats it as a no-op, matching the existing
 // "idempotent on terminal" behaviour of apply_tool_output.
 //
+// Duplicate ids: a ToolCallId is supposed to be unique, but providers
+// break that (see the `uniquify` guard in update/stream.cpp) and the
+// live tail routinely holds SEVERAL assistant messages within one turn
+// — kick_pending_tools appends a fresh placeholder per sub-turn. When
+// two calls share an id, matching the first one found routes the second
+// tool's result onto the first tool's already-terminal card, where
+// apply_tool_output drops it as a late duplicate: the tool really ran,
+// its output vanished, and the card stayed Running until the 330 s
+// wedge net failed it. So prefer the first NON-terminal match and only
+// fall back to a terminal one when no live call carries the id. Every
+// caller wants this: the four exec/permission sites bail on terminal
+// anyway, and ToggleToolExpanded flipping the running card is the
+// better guess of the two.
 // The callback is invoked as `f(ToolUse&)`. `ToolMutator` pins that shape so a
 // wrong-signature lambda is a clean concept error at the call site, not a
 // template-depth error inside the loop.
@@ -236,15 +249,22 @@ concept ToolMutator = std::invocable<F&, ToolUse&>;
 
 template <ToolMutator F>
 bool with_live_tool(Model& m, const ToolCallId& id, F&& f) {
+    ToolUse* settled = nullptr;   // first terminal match — fallback only
     for (std::size_t i = m.ui.frozen_through;
          i < m.d.current.messages.size(); ++i) {
         for (auto& tc : m.d.current.messages[i].tool_calls) {
-            if (tc.id == id) {
+            if (tc.id != id) continue;
+            if (!tc.is_terminal()) {
                 std::forward<F>(f)(tc);
                 return true;
             }
+            if (!settled) settled = &tc;
         }
     }
+    if (settled) {
+        std::forward<F>(f)(*settled);
+        return true;
+    }
     return false;
 }
 

src/runtime/app/update/stream.cpp

@@ -896,7 +896,47 @@ Step stream_update(Model m, msg::StreamMsg sm) {
             return nullptr;
         auto& calls = m.d.current.messages.back().tool_calls;
         auto it = std::ranges::find(calls, id, &ToolUse::id);
-        return it == calls.end() ? nullptr : &*it;
+        if (it != calls.end()) return &*it;
+        // Renamed at ingest (see `uniquify` below) — the wire goes on
+        // addressing the original id in its delta / end events. Match the
+        // NEWEST call carrying it: with the sequential collisions gateways
+        // actually produce, that is always the one still streaming.
+        for (auto rit = calls.rbegin(); rit != calls.rend(); ++rit)
+            if (!rit->wire_id.empty() && rit->wire_id == id) return &*rit;
+        return nullptr;
+    };
+
+    // ── Duplicate tool-call id guard ─────────────────────────────────────
+    // The ToolCallId is the ONLY key the runtime has for routing a tool's
+    // result, progress snapshots, timeout and permission decision back to
+    // its card (`with_live_tool`), and it is what the wire uses to pair a
+    // role:"tool" message with its assistant tool_call. Providers are
+    // supposed to make it unique. Several OpenAI-compatible gateways
+    // instead mint a deterministic id per (tool, index) — literally
+    // "bash:0" for every bash call on every turn.
+    //
+    // That is fatal here because one turn holds SEVERAL assistant messages
+    // in the live tail (kick_pending_tools appends a fresh placeholder per
+    // sub-turn), so turn 2's "bash:0" lands beside turn 1's, which is
+    // already Done. Rename the newcomer so every live call is uniquely
+    // addressable again; `wire_id` keeps the original for the stream events
+    // still to come. The rewritten id is what goes back out on the wire in
+    // both the assistant tool_call and its paired tool result, so the
+    // request stays self-consistent — and unlike the original, unambiguous.
+    auto uniquify = [&](const ToolCallId& id) -> ToolCallId {
+        auto taken = [&](const ToolCallId& cand) {
+            for (std::size_t i = m.ui.frozen_through;
+                 i < m.d.current.messages.size(); ++i)
+                for (const auto& tc : m.d.current.messages[i].tool_calls)
+                    if (tc.id == cand) return true;
+            return false;
+        };
+        if (id.empty() || !taken(id)) return id;
+        for (int n = 2; n < 1000; ++n) {
+            ToolCallId cand{id.value + "#" + std::to_string(n)};
+            if (!taken(cand)) return cand;
+        }
+        return id;   // 1000 collisions on one id: give up, not worth more
     };
 
     return std::visit(overload{
@@ -1047,7 +1087,14 @@ Step stream_update(Model m, msg::StreamMsg sm) {
             if (!m.d.current.messages.empty()
                 && m.d.current.messages.back().role == Role::Assistant) {
                 ToolUse tc;
-                tc.id   = e.id;
+                // Never let a second live call share an id with a first —
+                // see `uniquify` above for why that silently eats results.
+                if (auto uid = uniquify(e.id); uid != e.id) {
+                    tc.wire_id = e.id;
+                    tc.id      = std::move(uid);
+                } else {
+                    tc.id = e.id;
+                }
                 tc.name = e.name;
                 tc.args = json::object();
                 // Stamp start now so the card shows a live timer during the