From f8f81360217a29cfe39f9c7cb68fe6f09a500089 Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 19 Jun 2026 22:24:12 +0200 Subject: [PATCH] refactor(coding-agent): privatized the legacy `nextToolChoice` method to - Privatized the legacy `nextToolChoice` method to `#nextHardToolChoice` to ensure all tool-choice directives flow through the unified `nextToolChoiceDirective` entry point. - Eliminated redundant dual entry points for fetching tool choices, which previously bypassed the soft pending-preview lifecycle. - Updated test suites to consume `nextToolChoiceDirective` where appropriate to maintain consistency with internal agent-loop logic. --- docs/resolve-tool-runtime.md | 2 +- packages/coding-agent/CHANGELOG.md | 4 ++++ packages/coding-agent/src/session/agent-session.ts | 9 +++++---- .../test/agent-session-eager-compaction.test.ts | 2 +- .../coding-agent/test/agent-session-eager-task.test.ts | 2 +- .../coding-agent/test/agent-session-eager-todo.test.ts | 2 +- .../test/agent-session-force-tool-choice.test.ts | 10 +++++----- .../agent-session-plan-reference-compaction.test.ts | 2 +- .../test/agent-session-resolve-reminder.test.ts | 2 +- 9 files changed, 20 insertions(+), 15 deletions(-) diff --git a/docs/resolve-tool-runtime.md b/docs/resolve-tool-runtime.md index d7a692863..6aadf6168 100644 --- a/docs/resolve-tool-runtime.md +++ b/docs/resolve-tool-runtime.md @@ -44,7 +44,7 @@ Runtime behavior: - the pending invoker owns the `apply`/`reject` callbacks, - `resolve` dispatches via `peekQueueInvoker() ?? peekPendingInvoker() ?? peekStandingResolveHandler()`, -- a genuine hard forced tool choice (queued via `nextToolChoice`) preempts the soft requirement, +- a genuine hard forced tool choice (dequeued first by `nextToolChoiceDirective`) preempts the soft requirement, - if an apply callback throws, the helper re-registers the same pending invoker (same id) so the preview can still be discarded or retried. `resolve` also checks a standing resolve handler after the invokers; this is used by long-lived approval flows that are not ordinary preview tool calls. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index a7eb99d13..e420489d2 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Changed + +- Removed the legacy `AgentSession.nextToolChoice()` method. The per-turn tool-choice directive now flows solely through `nextToolChoiceDirective()` (which folds in the hard-choice dequeue plus active-tool filtering as a private helper), eliminating the dual entry point that let callers consume the queue while bypassing the soft pending-preview lifecycle. The underlying `ToolChoiceQueue.nextToolChoice()` is unchanged. + ## [16.1.4] - 2026-06-19 ### Fixed diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index b00782c0b..0de528d96 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -2141,8 +2141,9 @@ export class AgentSession { return this.#agentId; } - /** Advance the tool-choice queue and return the next directive for the upcoming LLM call. */ - nextToolChoice(): ToolChoice | undefined { + /** Dequeue the next HARD forced tool choice for the upcoming LLM call, dropping + * (and rejecting) one whose named tool is no longer active. */ + #nextHardToolChoice(): ToolChoice | undefined { const choice = this.#toolChoiceQueue.nextToolChoice(); if (isToolChoiceActive(choice, this.agent.state.tools)) { return choice; @@ -2154,7 +2155,7 @@ export class AgentSession { /** * The per-turn tool-choice directive for the agent loop's `getToolChoice`. Priority: * 1. a HARD forced choice from the queue (genuine forces: user-force, eager-todo, …) — - * consuming, unchanged from `nextToolChoice`; + * consuming (advances the queue generator); * 2. else, when a non-forcing preview is pending, a {@link SoftToolRequirement} — a * PEEK (advances/pops nothing), so the agent-loop injects the reminder once per head * and escalates to a forced `resolve` only if the model declines. A compliant turn @@ -2162,7 +2163,7 @@ export class AgentSession { * 3. else undefined. */ nextToolChoiceDirective(): ToolChoiceDirective | undefined { - const hard = this.nextToolChoice(); + const hard = this.#nextHardToolChoice(); if (hard !== undefined) return hard; const head = this.#toolChoiceQueue.peekPendingHead(); if (head !== undefined) { diff --git a/packages/coding-agent/test/agent-session-eager-compaction.test.ts b/packages/coding-agent/test/agent-session-eager-compaction.test.ts index 5f095aa31..3dc44b768 100644 --- a/packages/coding-agent/test/agent-session-eager-compaction.test.ts +++ b/packages/coding-agent/test/agent-session-eager-compaction.test.ts @@ -192,7 +192,7 @@ describe("AgentSession eager prelude re-injection after compaction", () => { getApiKey: () => "test-key", initialState: { model, systemPrompt: ["Test"], tools, messages: [] }, convertToLlm, - getToolChoice: () => session?.nextToolChoice(), + getToolChoice: () => session?.nextToolChoiceDirective(), streamFn: (_model, context, options) => { const call: ObservedPromptCall = { toolChoice: getToolChoiceName(options?.toolChoice), diff --git a/packages/coding-agent/test/agent-session-eager-task.test.ts b/packages/coding-agent/test/agent-session-eager-task.test.ts index 15cc6d99d..566e75306 100644 --- a/packages/coding-agent/test/agent-session-eager-task.test.ts +++ b/packages/coding-agent/test/agent-session-eager-task.test.ts @@ -139,7 +139,7 @@ describe("AgentSession eager task prelude", () => { messages: [], }, convertToLlm, - getToolChoice: () => session?.nextToolChoice(), + getToolChoice: () => session?.nextToolChoiceDirective(), streamFn: (_model, context, options) => { const lastMessage = context.messages.at(-1); if (!lastMessage) { diff --git a/packages/coding-agent/test/agent-session-eager-todo.test.ts b/packages/coding-agent/test/agent-session-eager-todo.test.ts index 2eb2d3b0a..d5feea1d2 100644 --- a/packages/coding-agent/test/agent-session-eager-todo.test.ts +++ b/packages/coding-agent/test/agent-session-eager-todo.test.ts @@ -132,7 +132,7 @@ describe("AgentSession eager todo enforcement", () => { messages: [], }, convertToLlm, - getToolChoice: () => session?.nextToolChoice(), + getToolChoice: () => session?.nextToolChoiceDirective(), streamFn: (_model, context, options) => { streamCallCount++; const lastMessage = context.messages.at(-1); diff --git a/packages/coding-agent/test/agent-session-force-tool-choice.test.ts b/packages/coding-agent/test/agent-session-force-tool-choice.test.ts index 6c0d82d9c..00b3e6db2 100644 --- a/packages/coding-agent/test/agent-session-force-tool-choice.test.ts +++ b/packages/coding-agent/test/agent-session-force-tool-choice.test.ts @@ -78,9 +78,9 @@ afterEach(async () => { it("forces specific tool, then transitions to none, then clears", () => { session.setForcedToolChoice("write"); - const first = session.nextToolChoice(); - const second = session.nextToolChoice(); - const third = session.nextToolChoice(); + const first = session.nextToolChoiceDirective(); + const second = session.nextToolChoiceDirective(); + const third = session.nextToolChoiceDirective(); expect(first).toEqual({ type: "tool", name: "write" }); // After the forced call, "none" prevents the loop from making more tool calls @@ -93,11 +93,11 @@ it("requeues a forced choice whose tool is filtered out before dequeue", async ( session.setForcedToolChoice("write"); await session.setActiveToolsByName(["bash"]); - expect(session.nextToolChoice()).toBeUndefined(); + expect(session.nextToolChoiceDirective()).toBeUndefined(); expect(session.toolChoiceQueue.hasInFlight).toBe(false); await session.setActiveToolsByName(["bash", "write"]); - expect(session.nextToolChoice()).toEqual({ type: "tool", name: "write" }); + expect(session.nextToolChoiceDirective()).toEqual({ type: "tool", name: "write" }); session.toolChoiceQueue.clear(); }); diff --git a/packages/coding-agent/test/agent-session-plan-reference-compaction.test.ts b/packages/coding-agent/test/agent-session-plan-reference-compaction.test.ts index 2dd557727..7ee165c5b 100644 --- a/packages/coding-agent/test/agent-session-plan-reference-compaction.test.ts +++ b/packages/coding-agent/test/agent-session-plan-reference-compaction.test.ts @@ -155,7 +155,7 @@ describe("AgentSession approved-plan reference re-injection after compaction (is getApiKey: () => "test-key", initialState: { model, systemPrompt: ["Test"], tools: [], messages: [] }, convertToLlm, - getToolChoice: () => session?.nextToolChoice(), + getToolChoice: () => session?.nextToolChoiceDirective(), streamFn: (_model, context) => { observedCalls.push({ messageTexts: context.messages.map(message => getMessageText(message)) }); const call = observedCalls[observedCalls.length - 1]; diff --git a/packages/coding-agent/test/agent-session-resolve-reminder.test.ts b/packages/coding-agent/test/agent-session-resolve-reminder.test.ts index ff32e58d5..bc27100c1 100644 --- a/packages/coding-agent/test/agent-session-resolve-reminder.test.ts +++ b/packages/coding-agent/test/agent-session-resolve-reminder.test.ts @@ -77,7 +77,7 @@ describe("AgentSession resolve reminder", () => { }); // Forcing was removed — staging a preview never queues a hard tool_choice. - expect(session.nextToolChoice()).toBeUndefined(); + expect(session.toolChoiceQueue.nextToolChoice()).toBeUndefined(); // The reminder now rides an agent-level soft requirement (delivered once by // the agent loop) instead of a host-side steer that churned the prefix.