fix(agent): relax forced toolChoice to auto on follow-up turns (DEV-785)

[devin-ai-integration]

Summary

makeFollowupRequest spread this.resolvedRequest unchanged into every follow-up dispatch, so a caller-supplied forced toolChoice ('required', a specific-tool object, or allowed_tools with mode: 'required') was reapplied on every turn. A model that honors required can then never answer in text after a successful tool round — it loops calling tools until stopWhen exhausts the budget and the forced final-response (toolChoice: 'none') turn kicks in (DEV-785, surfaced during openrouter:subagent testing in DEV-763).

Fix: a forced tool choice is "spent" once a tool round has executed. makeFollowupRequest now relaxes it when accumulating the next turn's request:

relaxForcedToolChoice(toolChoice):
  'required'                          -> 'auto'
  { type: 'function' | 'shell' | … }  -> 'auto'
  allowed_tools mode 'required'       -> same tools, mode 'auto'
  'auto' | 'none' | allowed_tools mode 'auto' | undefined -> unchanged

The initial turn still sends the forced choice intact, tools stay in follow-up requests, and the budget-exhaustion path (makeFinalResponseRequest, toolChoice: 'none') is unchanged — covered by the new unit tests, including a case where the relaxed 'auto' turn returns another tool call and the run still ends via the forced final turn.

Both streaming and non-streaming results share this single follow-up dispatch path, so one fix covers both.

Link to Devin session: https://openrouter.devinenterprise.com/sessions/a301382e980d4b5295bb6d3c30dfe984

API example

const result = callModel(client, {
  model: 'openai/gpt-4o',\n  input: 'Plan, research, then submit.',\n  tools: [planTool, searchTool, submitTool] as const,\n  toolChoice: ({ numberOfTurns }) =>\n    numberOfTurns === 0\n      ? { type: 'function', name: 'plan' }\n      : numberOfTurns === 3\n        ? { type: 'function', name: 'submit' }\n        : 'auto',\n});\n```

[devin-ai-integration]

Original prompt from dennis.jeong

can you read https://linear.app/openrouter/issue/DEV-785/server-tools-sdk-loop-reapplies-tool-choice-required-on-every-turn, verify it with an end to end test with gpt-5.6-luna and a server tool, and verify that this is a problem, and then create a fix?

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR that start with 'DevinAI' or '@devin'.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[cortex-github-agent]

• Keep up to date — merge the base branch into this PR as it moves
• Merge when ready — GitHub auto-merges once its required checks and approvals pass

cortex review — 38a8b29

✅ Security · ✅ Experience (DX · UX · A11y) · ✅ Performance

_{Automatic first-pass review · updated in place on every push}

[cortex-github-agent]

⚠️ APPROVE withheld by policy — PR author @devin-ai-integration[bot] is not a member of OpenRouterTeam (association: CONTRIBUTOR). Review posted as COMMENT; a maintainer must approve out-of-band.

cortex panel verdict: comment — details in the consolidated review comment.

[LukasParke]

Adversarial review of the forced-toolChoice relaxation (DEV-785). The core fix is sound: the initial turn sends the forced choice intact, consumption happens on emission before any pause can persist, the forcedToolChoiceSatisfied bit is additive-compatible with older state, and the toolChoice: 'none' finalization path is untouched. Unit suite (66 files / 718 tests), typecheck, and biome all pass locally. Four findings below, ordered by severity.

[w0nche0l]

Adversarial follow-up in f55c109: tool-choice policy changes are now transactional. Unforced clears and forced-choice consumption commit only after the caller choice actually reaches the wire and a response is materialized; pauses before dispatch preserve state, and advisor overrides cannot consume an undispatched caller choice. Added deferred-pause and advisor-override regression tests. All five CI checks pass.

[perry-the-pr-reviewer]

Perry's Review

Verdict: 💬 Comments / questions

Note: I would have APPROVEd, but the maintainer app lacks pull_requests:write on this org (GitHub 403), so I'm posting as a comment review instead of an approval — the findings below are non-blocking and the change is merge-ready from my read.

Reviewed f55c109c (auto re-review; previous review was on 20e0b11a). CI is green across the board (lint, typecheck, unit-tests, e2e-tests, structural-gate).

This is a well-structured fix for DEV-785. The core insight — a forced toolChoice is "spent" once the turn it forced actually produces a tool call — is implemented as a clean prepare → dispatch → commit state machine (applyResolvedForcedToolChoicePolicy → beginToolChoiceDispatch → commitToolChoiceDispatch). The commit is correctly gated on (a) the caller's choice actually reaching the wire (engine-owned none/advisor overrides produce a different semantic key and don't commit caller state — covered by the doom-loop-escalation test) and (b) the materialized response containing a tool call. Pauses before dispatch leave persisted state untouched, which the async-tool-deferred test pins.

The relaxation handlers (relaxForcedToolChoice / isForcedToolChoice) exhaustively cover every member of the SDK OpenAIResponsesToolChoiceUnion (strings, function, shell, apply_patch, web_search_preview, allowed_tools with both modes). The persisted state (consumedForcedToolChoiceKey) is a single optional string, backward-compatible with existing conversation state, and is correctly cleared on every terminal exit (markStateComplete, interruption, doom-stop, terminal approval continuation) and re-armed on unforced/different-value turns. The new tool-choice-relaxation.test.ts covers the static, dynamic, approval-resume, client-tool-resume, and budget-exhaustion paths. The e2e stopWhen adjustment is the right call to keep that test exercising the forced-final path.

Two non-blocking items below; neither changes my LGTM.

Findings

Question — static toolChoice: 'required' is now one-shot (behavior change)

This change alters the observable contract of a static forced choice ('required', a specific-tool object, or allowed_tools mode 'required'): it now forces only the first turn that resolves that semantic value, then relaxes to 'auto'. A caller who today relies on toolChoice: 'required' forcing a tool call on every turn until stopWhen will, after this lands, see the model answer in text as soon as it can. That is the intended fix for the loop bug, and the README now documents the one-shot semantics — but I couldn't find a migration note for the (unusual but valid) "force a tool every turn" pattern, and the new re-arm rules don't offer a way to re-force the same 'required' value on consecutive turns (it only re-arms after an unforced turn or a different forced value). Confirm that dropping per-turn forcing is intended and that downstream callers have been considered; if any need it, the README's dynamic-callback escape hatch should call that out explicitly.

Suggestion — allowed_tools identity is order-sensitive on the tools array

forcedToolChoiceKey runs canonicalizeKeyMaterial on the whole toolChoice object. JCS sorts object keys but preserves array order, so allowed_tools with [{name:'a'},{name:'b'}] and [{name:'b'},{name:'a'}] produce different keys and are treated as different semantic values. A dynamic allowed_tools callback that returns the same tool set in a different order across turns will therefore not be recognized as "the same forced choice" and won't be relaxed — it re-forces (old behavior). The consequence is benign (re-force, not a crash or wrong data), but it undercuts the "same semantic value" framing. Consider sorting the tools array (or its fingerprints) before keying so identity is set-order-independent, or note the limitation in the docstring.

Nit — changeset level

.changeset/warm-lions-drum.md is patch. This fixes a bug, but it also changes the observable behavior of a public option (toolChoice: 'required'/specific-tool). For 0.x semver that's defensible, but if you consider the per-turn-forcing removal a contract change rather than a pure bug fix, minor may communicate the impact more accurately. Minor either way; flagging for your judgment.

Risk assessment

Risk: 🟡 Medium

Dimension	Severity	Risk	Reasoning
Implementation risk	🟩	Low	State machine is sound; prepare/dispatch/commit gating is correct; exhaustive union coverage; comprehensive unit + e2e tests; CI green.
Premise risk	🟨🟨	Medium	Diagnosis (DEV-785 loop) is sound and verified against the code, but the fix removes a public "force every turn" pattern with no migration path; the review can't confirm no downstream caller depends on it.
Estimated impact	🟨🟨	Medium	Worst case a caller relying on per-turn 'required' gets earlier text answers; reversible by revert, observable, no data/financial/security impact.

Risk Factor	Severity	Risk	Reasoning
Reversibility	🟩	Low	A revert fully restores prior per-turn forcing.
Detectability	🟩	Low	The behavior change is directly observable in model output.
Blast radius	🟨🟨	Medium	All callers using a static forced toolChoice see different follow-up behavior.
Data integrity		None	No persisted data is mutated beyond a single optional state key.
Financial exposure		None	No billing/payment path is touched.
Security and privacy exposure		None	No auth, credential, or tenant-isolation surface.
Propagation	🟩	Low	Effect is confined to the agent loop's toolChoice handling.
Availability		None	The change cannot take a serving path down.
Recovery cost	🟩	Low	Revert or a dynamic-callback workaround corrects it.
Time to correct	🟩	Low	A one-commit revert or per-callback adjustment suffices.

Estimated impact: Worst case a caller relying on per-turn toolChoice: 'required' forcing receives text answers one turn earlier than before; reversible by revert, observable in output, and contained to toolChoice callers with no data, financial, or security impact.

▶ Prompt for agents: Address the two inline items if you agree — (1) confirm the per-turn-forcing removal is intended and downstream callers are covered (or add a README migration note), and (2) decide whether allowed_tools identity should be order-independent. Neither blocks merge.