Let consumers pass their own tools to the OpenAI-compatible facade - #176
Conversation
Two levels of tool calling existed, both sourced from the in-cluster CRD catalog: the orchestrator's own Skill-scoped planner loop (ADR 0008/0021) and a sub-agent's internal loop (ADR 0028). The third level the OpenAI wire format assumes -- a consumer passing its own `tools` and getting `tool_calls` back to execute itself -- was silently ignored, so a client that offered tools got prose with no signal its tools were never seen. The caller executes; the turn suspends. `runTool` gains a fourth dispatch kind (`callerTool`, alongside jobTemplate/localExec/agentRunTemplate) and it is the only one that executes nothing: it hands the call back as `tool_calls` with `finish_reason: "tool_calls"` and ends the turn. Resumption is read off the wire rather than from a session, since there is no server-side conversation store: `buildAgentRequest` now lifts `assistant.tool_calls` + `role: "tool"` pairs into structured history. That closes two silent losses -- a `role: "tool"` message used to be dropped outright, and an assistant message carrying only tool_calls has `content: null`, which the history fold skips -- and seeding `actionHistory` is also what keeps MAX_TOOL_STEPS bounding a resumed loop. Keeping this from degrading the two existing levels is most of the work: - Its own Qdrant collection, keyed by a content hash, so a caller's ephemeral definitions never enter another caller's candidate set, selectFallbackTool's catalog query, or a sub-agent's toolRefs resolution. The hash keying makes the collection an embedding cache: since a client resends the same array every turn, steady-state embedding cost is zero. - Pruned to top-K before the planner sees anything, and the index is skipped ENTIRELY below that threshold -- with nothing to prune, a round trip would be pure added latency, so the common case costs exactly what it did before. - Refusable per-skill via `Skill.spec.allowCallerTools` (*bool, nil => allowed, so Go's zero value cannot silently mean "refuse"). No RBAC applies, and that is argued rather than assumed: authorization is vacuous for a function the caller both supplies and runs itself, under its own credentials, in its own process. Isolation instead comes from `has_id` -- every search is restricted to hashes taken from the request being served, so it cannot surface a definition the caller did not just supply. Caller-provided text is rendered to the planner in a distinctly-labelled untrusted block, one level below a Tool CR description and two below Skill markdown. Open WebUI's housekeeping completions still short-circuit before caller tools are parsed, so a title-generation request carrying a tool array can never make the client execute a real function as a side effect of rendering a chat title. See docs/adr/0035 for the alternatives weighed (notably orchestrator-side execution, rejected: no off-the-shelf client would populate it, and it would put caller-controlled egress in the pod holding the k8s identity). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjyBwazMbdQMzPbupZiiXc
`waitFor` promised "poll until timeoutMs" but awaited `probe()` unbounded, which made its own timeout UNREACHABLE whenever a probe failed to settle: the loop stopped, permanently, without ever re-checking the deadline. That is reachable, not theoretical. Several probes `fetch` through a `kubectl port-forward`, Node's fetch has no default timeout, and a forward dropped by a busy apiserver leaves a socket nobody will ever answer. `fakeGithubRequests` -- polled by every comment-wait in the suite -- was one of them. Watched live rather than inferred: a resilience run sat at 0% CPU for eight minutes with no kubectl children, no open sockets and no output, wedged inside a test whose assertion had already been satisfied. Only vitest's per-test timeout would eventually kill it, reporting "the test timed out" -- which says nothing about which hop stalled, and is exactly the diagnosis waitFor exists to provide. It then burned a retry doing it again. So: - Every probe attempt is bounded, by whichever is sooner -- the per-probe ceiling or what remains of the overall budget. Hung attempts are counted and reported SEPARATELY from failures, because "146 attempts, 0 hung" and "146 attempts, all hung" call for completely different fixes. (That distinction immediately diagnosed the next failure underneath this one.) - `fetchThrough` replaces bare `fetch` against cluster services: bounded, and it reports whether the port-forward died mid-request, so a dropped forward reads as a dropped forward rather than as a service that went quiet. - `openPortForward` records its child's exit and fails loudly if it never becomes ready, instead of leaving a doomed forward behind. The practical effect is that a dropped forward now fails one poll and the next poll opens a fresh forward, instead of wedging the whole run. `specs/waitfor-guard.e2e.ts` pins the guarantee -- including "a probe that never settles must still time out" -- and needs no cluster, so a regression here fails in milliseconds rather than after an hour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjyBwazMbdQMzPbupZiiXc
…acade apps/agent-orchestrator's unit tests cover the decisions. They cannot cover either half of what actually breaks this feature in a cluster. **Qdrant has to accept the queries.** Those tests mock the Qdrant client outright, so they prove which method the code MEANT to call and nothing about whether the filter DSL is valid. ADR 0035 added three hand-written filter shapes -- `has_id` for the id-restricted search, a payload-only `setPayload` for cache touches, a delete-by-filter `range` for the TTL sweep -- and a mock accepts all three whether or not Qdrant would. `has_id` matters beyond correctness: it IS the isolation boundary for a collection that carries no RBAC filter, so a mis-shaped filter is both a 500 and a cross-caller leak. The `range` sweep is the only thing bounding a collection Qdrant gives no native TTL. So the first block drives the real store against the real Qdrant, in its own throwaway collection, with a deterministic stand-in embedder -- the subject is the filter DSL, and real embeddings would add cost and nondeterminism to assertions that never look at similarity. **The catalog has to stay clean.** "A caller's definitions go somewhere else and never perturb tool/skill/agent retrieval" is a claim about which collection holds which points, unobservable anywhere but a real Qdrant. Plus the wire contract over the real facade and real planner: `tool_calls` with schema-conforming arguments, resumption from a client-executed result, the per-skill gate taking its refusal branch, `tool_choice: "none"`, and the housekeeping short-circuit. Two things worth knowing before editing it. The planner decision is made deterministic on purpose. Whether the planner CALLS a caller tool is a real OpenAI decision, and this suite's rule is to assert on deterministic dispatch rather than model judgement. A skill's markdown is trusted system-prompt content -- the strongest lever over that decision short of faking the planner -- so the two fixture Skill CRs differ ONLY in `allowCallerTools`, one instructed to call the caller's tool and the other given an explicit no-tool branch. Asserting that branch, rather than merely the absence of a tool call, is what separates the gate working from the model declining anyway. And the tool definitions carry a per-run token, which cost a red build to learn. The index is content-hash keyed and persists, so a fixed set of definitions is indexed by the first run that ever sends them and is a pure cache hit forever after -- an "indexing happened" assertion against fixed definitions passes exactly once in the lifetime of a Qdrant volume, then fails, reporting the cache working correctly as a product failure. Varying the description (part of the hash) keeps the name stable for the skill markdown while making each run genuinely new, and let the assertion tighten from "grew" to "grew by exactly the number sent" -- plus a new one: an EDITED tool that keeps its name must add exactly one point, which a name-keyed cache would fail. `support/invoke.ts` applies the suite's per-entry-point rule to the programmatic `/invoke`: both facades may OFFER tools, only chat can RESUME, and `/invoke` renders a pending call differently. A documented asymmetry with nothing asserting it is just a claim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjyBwazMbdQMzPbupZiiXc
"an orchestrator rollout mid-turn does not fabricate a timeout" waited 5 minutes
for the AgentRun to reach `Succeeded` after rolling the orchestrator. It could
never pass.
Since docs/adr/0033 an agent HOLDS its concluding message until someone acks it
(the protocol's `reply_ack`), re-offering every 10s and giving up only after
`REPLY_ACK_TIMEOUT_MS` -- 10 minutes (packages/agent-runtime/src/runtime.ts).
This test rolls the orchestrator and never re-triggers, so nothing acks: the new
pod has no reason to re-attach. The run therefore stays Running for ten minutes
by design, twice this test's budget and past vitest's per-test timeout. The
assertion predates that change, as its own comment admitted ("this held even
BEFORE the fix"), and ADR 0033 quietly made it unreachable.
It now asserts what is true and still meaningful: the run SURVIVED the roll --
same run, not Failed. That is precisely what made the old fabricated error a lie
rather than a report, since the run it claimed had timed out was alive and holding
an answer. The dropped half is not lost: the very next spec re-triggers the same
issue and asserts `Succeeded` BECAUSE the re-attach acked it, which is the
assertion this one deliberately withholds.
Also gives each trigger a unique issue number. `Date.now() % 100000` repeats every
100 seconds' worth of values; the issue number decides the session id, sessions
outlive a spec, and a turn landing on a session that still carries an
awaiting-reply anchor re-attaches to the old run instead of launching -- surfacing
as "timed out waiting for an AgentRun to be created", a real product behaviour
triggered by a fixture and indistinguishable from a regression. Not the cause of
anything observed here (exact congruence is unlikely); it removes the class of
doubt for free. Test 5's deliberate same-issue re-trigger is unaffected.
Found only after bounding waitFor's probes: "146 attempts, 0 hung" is what said
the harness was healthy and the run genuinely was not terminating.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjyBwazMbdQMzPbupZiiXc
Correcting the verification claim for
|
…trigger This is the bug behind resilience.e2e.ts failing intermittently on a different test each run, and it was never a product fault. Kubernetes stamps `metadata.creationTimestamp` in RFC3339 with SECOND precision and truncates. `since` is a host-side `new Date()` in milliseconds. Compared directly, a run created at 14:07:05.800 is stamped `14:07:05Z`, parses back as 14:07:05.000, and a `since` of 14:07:05.200 excludes it -- permanently. The trigger looks like it launched nothing, however long you poll. The evidence, once the harness stopped hanging and started reporting: the test failed with "timed out after 420000ms waiting for an AgentRun to be created (204 attempts)" -- 204 clean polls, zero hung, no probe errors -- while fake-github had recorded the gateway posting a full `stub-agent-reply` for that very issue, mid-timeout. The run was created, ran, replied, and its answer reached the issue while the test insisted nothing had launched. Which explains every symptom that misled the investigation for hours: - Intermittent, and a different test each run: it is a race on where in the wall-clock second the trigger happens to land. - Passes in isolation: different timing. - `204 attempts, 0 hung`: the harness was working perfectly and kubectl was telling the truth. The FILTER was wrong. - Unreproducible by hand: the probes written to chase it compared run COUNTS before and after, so they never touched the timestamp comparison at all. The cutoff is now widened by the granularity plus a clock-skew allowance -- minikube's VM clock drifts against the host and jumps after the host sleeps, and skew in the node-behind direction hides runs exactly the same way. Two seconds, which is deliberately far below the >=20s spacing between triggers in any spec: a wider window would risk the WORSE failure of matching a previous turn's run and passing an assertion against the wrong one. Three tests in specs/waitfor-guard.e2e.ts pin all of it -- the truncated-below case, a lagging node clock, and the boundary that must NOT reach back to a prior turn -- by stubbing kubectl on PATH, so they need no cluster. resilience.e2e.ts now passes in full (4 passed, 1 skipped by design), in 451s rather than 2317s, since the phantom 7-minute timeouts are gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjyBwazMbdQMzPbupZiiXc
…dget "recovers the reply on a follow-up turn after a rollout" passed whenever this file ran alone and failed inside the full suite -- the signature of a budget with no slack rather than of a broken recovery. The recovery has to complete inside integration-gateway's own poll budget (90s in values-e2e.yaml). A re-attaching turn only sees the held reply on the agent's NEXT re-offer, and that cadence is 10s in production. Under full-suite load the re-offer landed after the gateway had given up, so the gateway posted a failure and the answer never arrived at all -- which is why making the test wait longer cannot fix it, and why the gateway's budget should stay short (it is deliberately tight so an abandoned turn releases the relay quickly). So the fixture turns the re-offer down to 2s, via `AGENT_REPLY_ACK_RETRY_MS` -- a real packages/agent-runtime knob, not a test-only affordance -- and only on this one spec, so every other spec keeps the production cadence and this stays a local, deliberate choice. The whole e2e suite now passes: 6 files, 55 passed, 1 skipped by design, in 745s (down from 3013s, since the phantom timeouts are gone). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjyBwazMbdQMzPbupZiiXc
Resolved — the whole e2e suite is greenSuperseding my earlier correction. The root cause is found and fixed, and the PR The intermittent The evidence is unambiguous: the spec reported That explains every symptom that made this look like a regression: intermittent Fixing the Two things I'd want a reviewer to be sceptical of, so I'll name them myself:
Four separate e2e bugs, all test-side, each needed to see the next. The feature |
|
🤖 Starting work on this now. Watch live or take over the session here: https://claude.ai/code/session_01XATGT61M6b9EKe6NfCbmPy |
![k5s-bot[bot]](https://avatars.githubusercontent.com/in/1052865?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review: caller-supplied tools over the OpenAI facade
I checked out feat/caller-supplied-tools, read the ADR and the full diff, and verified the build/test claims locally. This is a strong, unusually well-documented PR. The design holds up: the two existing tool levels really are untouched (separate caller_tools collection, no writes to the catalog collections, no path into selectFallbackTool's query or sub-agent toolRefs), caller:-namespaced ids can't shadow catalog ids, and the isolation argument for skipping RBAC (has_id restricted to hashes taken from the request being served, resolved back to the request's own descriptor in qdrant-caller-tool-store.ts:131) is sound — a shared content-hash namespace leaks nothing because retrieval never ranges beyond what the caller just supplied.
Verified locally
- Whole-workspace typecheck clean (
tsc --noEmit, after building@controller-agent/messaging). - 568/568 orchestrator unit tests pass (46 files, 189s) — matches the PR's claim. I re-ran the changed areas in isolation too (caller-tools, chat-completions, action-planner, graph, server, skills — 282 tests green).
- The Go
zz_generated.deepcopy.goaddition is the correctcontroller-genshape for a*bool(nil-guard →new(bool)→ deref copy); the CRD YAML matchesskill_types.go. I did not finishgo build/make testin this environment (module download exceeded the sandbox time budget), so the Go side is assessed by inspection only, not executed. - e2e specs were not run (no minikube cluster available here); the four test-harness fixes are assessed by reading. The
agentRunsSincewidening (granularity 1s + 2s skew, argued to stay well under the ≥20s trigger spacing so it can't match a previous run) and thewaitFor/fetchThroughbounding are convincing, and thewithPortForwardsignature change is backward-compatible (existing callers ignore the added 2nd callback arg).
One correctness gap (minor, but real and untested)
The duplicate-call guard in planAction was not updated to carry the seeded result, unlike the other two finish branches in the same node — see the inline comment on apps/agent-orchestrator/src/agent/graph.ts:1759. On a resumed caller-tool turn the result lives only in seeded actionHistory (no runTool ran this invocation to set state.result), so if the planner re-issues a byte-identical call the guard finishes with result undefined and the blocking facade renders renderResult(undefined) → a literal ```json\nundefined``` block instead of the last tool's result. This is exactly the path tool_choice: "required" nudges toward on a resend, since that directive is re-applied on the resumed turn (server.ts sets required whenever tools are present) and pushes the planner to call the one already-run tool again. Two-word fix; details inline.
Smaller notes (non-blocking)
QdrantCallerToolStore.prunereturns 1/0 ("did it run"), not a delete count — Qdrant's delete-by-filter gives no count, so this is inherent; the doc comment says so and theindex.tscaller ignores the value. Fine.parse.tsacceptsparameterswhentypeof === "object", which also admits a JSON array. A non-object schema is the caller's own problem (it only ends up in their planner prompt and their own client), so not worth rejecting, but worth being aware of.- Reviewer notes already flag the two honest residues:
caller_toolsgrows a few points per run until the TTL sweep (no active teardown, unlikecleanupAgentRunsSince), and two e2e assertions are genuinely model-dependent. Both are called out in the PR; no objection.
Net: I'd fix the finish-without-result inconsistency before merge (or add a resume-with-duplicate-call test that would have caught it); everything else is solid.
Reviewed at commit 63a2e8f. Unit tests + typecheck run locally; Go build and e2e not executed in this environment (noted above).
|
Review posted successfully with both the summary and the inline comment. SummaryI reviewed PR #176 ("Let consumers pass their own tools to the OpenAI-compatible facade") by checking out the branch, reading ADR 0035 and the full 48-file diff, and verifying the build/test claims locally. What I verified in-environment:
Assessment: Strong, exceptionally well-documented PR. The core design holds up — separate One concrete finding (minor, non-blocking) posted as an inline comment: the verbatim-repeat ✅ Opened pull request: imaustink/agent-controller#176 |
|
🤖 Starting work on this now. Watch live or take over the session here: https://claude.ai/code/session_01MZkSbDnWnREdmCtDFXPPkU |
On a resumed caller-tool turn (docs/adr/0035) no runTool runs this
invocation, so state.result is unset and the last tool's answer lives
only in the seeded actionHistory. The duplicate-call guard in planAction
finished with a bare `{ plannedAction: "finish" }`, unlike its two
sibling finish branches which carry `...lastHistoryResult(state)`. When
tool_choice: "required" (re-applied on the resend) pushes the planner to
re-issue the byte-identical already-run call, this guard fired and the
blocking facade rendered `renderResult(undefined)` -- a literal
```json\nundefined``` block instead of the last tool's result.
Make it match its siblings and add a resume-with-duplicate-call
regression test that pins final.result to the seeded result.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MZkSbDnWnREdmCtDFXPPkU
Picked back up from ai-triage — review feedback addressedWhat I changed and whyFixed the one correctness gap from @k5s-bot's review (inline at
Commit Synced with Updated the PR description: Verification (re-run locally on the merged branch)
Deliberately not done
|
|
Everything is done and verified. Here's the summary. SummaryI picked up PR #176 ("Let consumers pass their own tools to the OpenAI-compatible facade") from the Feedback found: One code review (from What I changed:
✅ Opened pull request: imaustink/agent-controller#176 |
1 participant
What
Adds a third level of tool calling: a consumer can pass its own
toolstoPOST /v1/chat/completions(or/invoke) and get standardtool_callsback toexecute in its own client. Recorded as ADR 0035.
Before this, the field was silently ignored — a client that offered tools got
prose back with no way to tell "the agent chose not to call them" from "the agent
never saw them". Open WebUI's native tool calling, LibreChat and anything on the
OpenAI SDK all sit on this contract.
How
The caller executes; the turn suspends.
runToolgains a fourth dispatch kind(
callerTool, alongsidejobTemplate/localExec/agentRunTemplate) and it isthe only one that executes nothing — it hands the call back with
finish_reason: "tool_calls"and ends the turn. Resumption is read off the wire,not from a session, because there is no server-side conversation store:
buildAgentRequestnow liftsassistant.tool_calls+role: "tool"pairs intostructured history.
That closes two silent losses. A
role: "tool"message used to be droppedoutright (the history fold keeps only user/assistant), and an assistant message
carrying only
tool_callshascontent: null, which the fold skips — so withoutthis the planner would re-issue the same call forever. Seeding
actionHistoryisalso what keeps
MAX_TOOL_STEPSbounding a resumed loop rather than resetting iteach round trip.
Not degrading the two existing levels is most of the work:
selectFallbackTool's catalog query, or a sub-agent'stoolRefsresolution.Skill.spec.allowCallerTools(*bool, nil ⇒ allowed) lets a sensitive authored skill refuse them.No RBAC applies, and that's argued rather than assumed. Authorization is
vacuous for a function the caller both supplies and runs itself, in its own
process, under its own credentials — the orchestrator gains no capability. Isolation
comes from
has_idinstead: every search is restricted to hashes taken from therequest being served, so it cannot surface anything the caller didn't just supply.
This is the design's least conventional decision and §2 of the ADR defends it.
Open WebUI's housekeeping completions still short-circuit before caller tools are
parsed, so a title-generation request carrying a tool array can never make the
client execute a real function as a side effect of rendering a chat title.
Four e2e bugs found and fixed along the way
All four are pre-existing and test-side — the feature code is unchanged since
this PR opened. They're here because the suite could not report the truth without
them, and each had to be fixed to see the next.
1. The harness could hang forever.
waitForpromised "poll untiltimeoutMs"but awaited
probe()unbounded, making its own timeout unreachable if a probenever settled. With un-timed
fetchover akubectl port-forward, one droppedforward wedged an entire run: observed at 0% CPU for eight minutes, no kubectl
children, no sockets, no output, inside a test whose assertion was already
satisfied — then it burned a retry doing it again. Probes are now bounded, hung
attempts counted separately from failures, and
fetchThroughreports whether theforward died mid-request.
2. A test asserted behaviour ADR 0033 removed. The rollout spec waited 5
minutes for the run to reach
Succeeded. But an agent now holds its reply untilacked, giving up only after
REPLY_ACK_TIMEOUT_MS= 10 minutes — and that specrolls the orchestrator and never re-triggers, so nothing acks. It could only ever
fail; its own comment admitted it predated the change. It now asserts the run
survived the roll, which is what made the old fabricated error a lie rather than
a report. The next spec already owns "it completes once acked".
3.
agentRunsSincehid runs whose timestamp truncated below the trigger. Thiswas the real cause of the intermittent, different-test-each-run failures.
Kubernetes stamps
creationTimestampto the second and truncates;sinceis ahost-side
new Date()in milliseconds. A run created at…05.800stamps to…05Z→ parses as…05.000→ asinceof…05.200excludes it permanently.The proof: the spec reported
timed out after 420000ms waiting for an AgentRun to be created (204 attempts)— 204 clean polls, zero hung, no probe errors — whilefake-github had recorded the gateway posting a full
stub-agent-replyfor thatvery issue, mid-timeout. The run was created, ran, replied, and its answer
reached the issue while the test insisted nothing had launched. Note fixing (1) is
what made this findable:
204 attempts, 0 hungis what said the harness is fine,the filter is lying.
The cutoff is now widened by the granularity plus a 2s clock-skew allowance
(minikube's VM clock drifts against the host). 2s is deliberately far below the
≥20s spacing between triggers, so it cannot do the worse thing — match a previous
turn's run and pass an assertion against the wrong one. A regression test pins that
boundary.
4. The rollout-recovery spec had no headroom. It passed alone and failed under
full-suite load. Recovery must land inside the gateway's 90s poll budget, and a
re-attaching turn only sees the held reply on the agent's next re-offer — 10s in
production. If that lands late the gateway posts a failure and the answer never
arrives, so waiting longer in the test cannot help. The fixture turns the re-offer
down to 2s via
AGENT_REPLY_ACK_RETRY_MS(a real agent-runtime knob), on that onespec only.
Verification
Everything below was run against a real deployed minikube stack.
make testgreen; CRD manifests + deepcopy regenerated withcontroller-gen, not hand-edited.745s (down from 3013s once the phantom timeouts were gone).
caller-tools.e2e.ts18/18 — the three Qdrant filter shapes the unit testscan only mock, the
tool_callsround trip through the real planner, and proofthe catalogs are untouched.
chat-harness.e2e.ts9/9, no cluster — one test feeds the orchestrator'sown
toolCallDeltaChunkoutput through the harness so wire-shape drift failsfast.
waitfor-guard.e2e.ts7/7, no cluster.resilience.e2e.ts4 passed, 1 skipped.Reviewer notes
caller_toolsgrows ~6 points per e2e run, reclaimed by the TTL sweep.Bounded, but real residue — unlike
cleanupAgentRunsSincethere's no activeteardown.
that the refused skill emits its no-tool branch). Made as deterministic as
possible via trusted skill markdown and narrow skill descriptions, but
model-dependent in a way the rest of the suite isn't.
tool_choice: "required"is a planner directive not a guarantee;
/invokecan offer tools but not resume(no message array to carry
role: "tool"); caller tools are unreachable from asub-agent's loop (no channel back to the HTTP caller's client). Per-turn planner
loops are capped as always, but a client can always resend, so per-conversation
depth is unbounded — unchanged from before.
Review follow-up
graph.ts, verbatim-repeatfinishguard). It returned a bare{ plannedAction: "finish" }while its twosibling
finishbranches carry...lastHistoryResult(state). On a resumedcaller-tool turn no
runToolruns, sostate.resultis unset and the answerlives only in seeded
actionHistory; whentool_choice: "required"(re-appliedon the resend) pushed the planner to re-issue the byte-identical already-run
call, the guard finished with
result === undefinedand the blocking facaderendered
```json\nundefined```instead of the last tool's result. Theguard now matches its siblings, with a resume-with-duplicate-call regression test
that fails on the old code (
expected undefined to be '58F and raining') andpasses with the fix. This is the one product-code change since the PR opened; the
four e2e fixes remain test-side.
main(merged, matching this repo's history).🤖 Generated with Claude Code
https://claude.ai/code/session_01SjyBwazMbdQMzPbupZiiXc