fix(ui): stream tool results in order and gate approval on real state

[IBJunior]

Two regressions that shipped in #29 (render_chart / artifact delivery) and were surfaced — not caused — by the Skills work. load_skill is the first tool that reliably fires before the real work, so turns now contain two tool rounds instead of one, which is exactly the shape that exposes both.

Symptoms: tool results appeared in a block after the final AI message, the APPROVE button sometimes never showed, and read-only tools (load_skill, run_sql) offered an approval gate they don't have.

1. The tool pump blocked on the whole run

let artifact = artifacts.get(call.callId);
if (artifact === undefined) {
  await collectArtifacts(await run.output);   // waits for the ENTIRE run

run.output resolves only when the graph finishes. The fallback fired whenever an artifact wasn't already in the map — and for every tool except render_chart that is always, since only it uses responseFormat: "content_and_artifact". An absent artifact and a not-yet-arrived one are both undefined, so the code could not tell "this tool has none" from "keep waiting".

So pumpToolCalls parked on the first result while pumpMessages ran on unblocked — serializing every turn as all AI text → all tool results. render_chart itself skipped the fallback, so the one tool the feature was built for behaved differently from every other: the bug hid from its own feature's testing.

Fixed with a positive signal: src/lib/agent/artifactTools.ts, a zero-import leaf (same pattern and same reason as mutatingTools.ts) listing the tools that actually produce an artifact.

2. The approval gate keyed off list position

showApprovalButtons={index === messages.length - 1}

index indexes uniqueMessages (deduplicated) but is compared against messages.length (the original). Whenever dedup removed anything the condition was never true for any message — the missing APPROVE button.

And position is the wrong predicate regardless: bug 1 meant tool results arrived after the final AI message, so the message carrying the pending call was no longer last. The inverse gave read-only tools a gate, because ToolCallDisplay opened the amber panel purely from showApprovalButtons && id && approvalCallbacks, never consulting isMutatingTool.

The server knows the truth and now says it: after the run settles, streamMessages re-emits the AI message with pendingToolCallIds — the calls the model requested that never executed, which is exactly what the gate paused. ToolCallDisplay additionally requires isMutatingTool(name); two independent conditions, because that panel claims "this writes to your data."

3. Dedup and React keys on type:id

Ids are unique only within a type — an AI message's synthetic id and a tool message's call id are different namespaces — so keying on id alone could drop a distinct message. This is what made bug 2 intermittent rather than constant.

Testability

streamMessages moves to src/services/messageStream.ts. Not cosmetic: agentService.ts reaches Postgres at import time, so a test importing it hangs (verified). The ordering contract had no free-testable surface, which is why two regressions shipped clean. The new module has no I/O of its own.

Verification

• 193/193 unit tests pass, including 8 new ones in messageStream.test.ts.
• The new tests were observed failing first: removing the one-line guard makes both ordering tests hang and time out, which is the bug's real signature.
• Live-tested against the dev stack only (mydb_dev:5544, port 3101); production on 3100 untouched.

Live SSE ordering after the fix — results interleave with their calls, and read-only tools get no gate:

AI:tool_calls  ['describe_finance_schema']
TOOL:result    describe_finance_schema      ← was dumped at the end
AI:tool_calls  ['run_sql']
TOOL:result    run_sql

Full gate cycle: log_expense paused → AI:PENDING ['toolu_01FQ…'] → DB still 10 rows → approved → result streams, zero pending chunks → 11 rows. Chart artifacts still deliver (spec, columns, rows).

Note

render_chart still lands slightly after its following text — it's the one tool that legitimately waits on run.output. That's now the intended narrow case rather than every tool's behavior. Tightening it means having the values stream deliver artifacts before the tool result resolves; deliberately out of scope here.

🤖 Generated with Claude Code