diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 22a72debb..ae1e200eb 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,12 @@ ## [Unreleased] +### Fixed + +- Fixed an advisor refusal skipping the model fallback chain. `AdvisorRuntime` treated a classifier refusal as terminal once its one stripped-reasoning resend failed, returning before the `onTurnError` hook that owns fallback (`#recoverAdvisorTurn`), so a `Refusal (cyber)` on one model disabled the advisor even with a configured chain. The refusal path now takes the same fallback pass the primary turn-recovery path already allows, and only reports the advisor unavailable when the host declines to switch. +- Bounded advisor refusal recovery to one attempt per model. The cascade walks the fallback chain to exhaustion, but a model switch re-arms `#includeThinking` via `#syncModelIdentity`, so a chain whose keys point back at each other (A→B, B→A) would strip-and-resend against the same pair forever. Each cascade now visits a model at most once; a successful turn or a reset starts a fresh walk. +- Fixed `/advisor status` throwing when the roster is empty but an advisor is live. `formatAdvisorStatus` dereferenced `stats.advisors[0]` after a guard that only covered the inactive case, so a status call landing in the window where `#advisorStatuses` is cleared for a rebuild hit `undefined.contextWindow`. Reporting status now never throws. + ## [17.2.8] - 2026-08-04 ### Changed diff --git a/packages/coding-agent/src/advisor/runtime.ts b/packages/coding-agent/src/advisor/runtime.ts index 071a6c014..37ce6aabe 100644 --- a/packages/coding-agent/src/advisor/runtime.ts +++ b/packages/coding-agent/src/advisor/runtime.ts @@ -296,6 +296,14 @@ export class AdvisorRuntime { #failureNotified = false; /** Consecutive quarantined turns since the last success/reset (issue #6661). */ #consecutiveQuarantines = 0; + /** + * Model identities this refusal cascade has already tried. The cascade walks + * the fallback chain to exhaustion — that is what the chain is for — but + * visits each model at most once, so a chain whose keys point back at each + * other (A→B, B→A) cannot ping-pong forever. Cleared by a successful turn or + * a reset, so a later refusal starts a fresh walk. + */ + readonly #refusalModelsTried = new Set(); /** Whether primary reasoning is included in advisor deltas for the current model. */ #includeThinking = true; #modelIdentity: string | undefined; @@ -541,6 +549,7 @@ export class AdvisorRuntime { this.#failing = false; this.#droppedBacklogs = 0; this.#consecutiveQuarantines = 0; + this.#refusalModelsTried.clear(); this.#failureNotified = false; this.#resetAdvisorContext(true, true); } @@ -936,6 +945,7 @@ export class AdvisorRuntime { this.#failureNotified = false; this.#droppedBacklogs = 0; this.#consecutiveQuarantines = 0; + this.#refusalModelsTried.clear(); if (this.host.onTurnSuccess) { try { await raceWithSignal(Promise.resolve(this.host.onTurnSuccess()), iterationAbort.signal); @@ -995,6 +1005,47 @@ export class AdvisorRuntime { continue; } } + // A refusal that outlives the strip is this model's policy call, not + // a malformed request, so hand it to the host's model fallback before + // declaring the advisor dead. The cascade walks the chain to + // exhaustion (the host returns false once candidates run out); the + // tried-set only stops a cyclic chain from revisiting a model. The + // primary turn-recovery path already allows fallback on refusals. + const refusalModel = this.host.getModelIdentity?.() ?? this.#modelIdentity ?? ""; + let refusalRecovered = false; + try { + if (!this.#refusalModelsTried.has(refusalModel)) { + this.#refusalModelsTried.add(refusalModel); + refusalRecovered = + (await raceWithSignal( + Promise.resolve(this.host.onTurnError?.(err, failedMessages, iterationAbort.signal)), + iterationAbort.signal, + )) === true; + } else { + logger.debug("advisor refusal chain exhausted", { model: refusalModel }); + } + } catch (hookErr) { + logger.debug("advisor onTurnError hook failed after refusal", { err: String(hookErr) }); + } + if (this.#epoch !== epoch) continue; + if (this.#sessionTransitionPaused) { + this.#pending.unshift(...popped); + continue; + } + if (refusalRecovered) { + this.#consecutiveFailures = 0; + this.#failureNotified = false; + this.#pending.unshift({ + text: batch, + rawMessages, + renderRevision: this.#renderRevision, + turns: finalTurns, + wip, + overflowRecovery: recoveringOverflow || undefined, + }); + logger.debug("advisor refusal recovered by model fallback"); + continue; + } this.#notifyFailureOnce(err); this.#clearSeenContext(); this.#backlog = Math.max(0, this.#backlog - finalTurns); diff --git a/packages/coding-agent/src/session/session-advisors.ts b/packages/coding-agent/src/session/session-advisors.ts index f72615bdc..9c880514b 100644 --- a/packages/coding-agent/src/session/session-advisors.ts +++ b/packages/coding-agent/src/session/session-advisors.ts @@ -1790,7 +1790,14 @@ export class SessionAdvisors { } if (stats.advisors.length <= 1) { const s = stats.advisors[0]; - if (s && s.status === "no_model") { + if (!s) { + // A rebuild clears #advisorStatuses before repopulating it, so a status + // call landing in that window sees live advisors with an empty roster. + // Reporting a status must never throw — this used to fall through to + // `s.contextWindow`. + return stats.active ? "Advisor is starting up." : "Advisor is disabled."; + } + if (s.status === "no_model") { return stats.configured ? "Advisor setting is enabled, but no model is assigned to the 'advisor' role." : "Advisor is disabled."; diff --git a/packages/coding-agent/test/advisor/advisor.test.ts b/packages/coding-agent/test/advisor/advisor.test.ts index 2f70d1d7f..34e2a7db6 100644 --- a/packages/coding-agent/test/advisor/advisor.test.ts +++ b/packages/coding-agent/test/advisor/advisor.test.ts @@ -3617,6 +3617,261 @@ describe("advisor", () => { expect(failures).toHaveLength(1); }); + it("asks the host to switch models when a refusal outlives the stripped resend", async () => { + const promptInputs: string[] = []; + const failures: unknown[] = []; + const state: { messages: AgentMessage[]; error?: string } = { messages: [] }; + // The first model refuses every time; the host's fallback hook swaps in a + // model that answers, exactly as `#recoverAdvisorTurn` does for a session. + let modelRefuses = true; + const agent: AdvisorAgent = { + prompt: async input => { + promptInputs.push(input); + if (modelRefuses) { + state.error = "Refusal (cyber): blocked under Anthropic's Usage Policy"; + state.messages.push({ + role: "assistant", + content: [], + stopReason: "error", + stopDetails: { type: "refusal", category: "cyber" }, + errorMessage: state.error, + timestamp: promptInputs.length + 1, + } as unknown as AgentMessage); + return; + } + state.error = undefined; + state.messages.push({ + role: "assistant", + content: [], + stopReason: "stop", + timestamp: promptInputs.length + 1, + } as unknown as AgentMessage); + }, + abort: () => {}, + reset: () => {}, + rollbackTo: count => { + state.messages.length = count; + state.error = undefined; + }, + state, + }; + const messages = [ + { + role: "assistant", + content: [ + { type: "thinking", thinking: "private reasoning" }, + { type: "text", text: "answer" }, + ], + timestamp: 1, + } as AgentMessage, + ]; + let fallbackCalls = 0; + const runtime = new AdvisorRuntime( + agent, + { + snapshotMessages: () => messages, + enqueueAdvice: () => {}, + notifyFailure: error => failures.push(error), + onTurnError: async () => { + fallbackCalls++; + modelRefuses = false; + return true; + }, + }, + 0, + ); + + runtime.onTurnEnd(messages); + await settleUntil(() => runtime.backlog === 0); + + // Refuse, strip-and-resend, refuse again, then the swapped model answers. + expect(fallbackCalls).toBe(1); + expect(promptInputs).toHaveLength(3); + expect(failures).toEqual([]); + }); + + it("surfaces the refusal when the host declines to switch models", async () => { + const promptInputs: string[] = []; + const failures: unknown[] = []; + const state: { messages: AgentMessage[]; error?: string } = { messages: [] }; + const agent: AdvisorAgent = { + prompt: async input => { + promptInputs.push(input); + state.error = "Refusal (cyber): blocked under Anthropic's Usage Policy"; + state.messages.push({ + role: "assistant", + content: [], + stopReason: "error", + stopDetails: { type: "refusal", category: "cyber" }, + errorMessage: state.error, + timestamp: promptInputs.length + 1, + } as unknown as AgentMessage); + }, + abort: () => {}, + reset: () => {}, + rollbackTo: count => { + state.messages.length = count; + state.error = undefined; + }, + state, + }; + const messages = [ + { + role: "assistant", + content: [ + { type: "thinking", thinking: "private reasoning" }, + { type: "text", text: "answer" }, + ], + timestamp: 1, + } as AgentMessage, + ]; + const runtime = new AdvisorRuntime( + agent, + { + snapshotMessages: () => messages, + enqueueAdvice: () => {}, + notifyFailure: error => failures.push(error), + onTurnError: async () => false, + }, + 0, + ); + + runtime.onTurnEnd(messages); + await settleUntil(() => failures.length === 1 && runtime.backlog === 0); + + expect(promptInputs).toHaveLength(2); + expect(failures).toHaveLength(1); + }); + + it("walks the whole fallback chain before reporting a refusal", async () => { + // Every model refuses. The cascade must reach the last chain entry, then + // stop once the host runs out of candidates. + const promptInputs: string[] = []; + const failures: unknown[] = []; + const state: { messages: AgentMessage[]; error?: string } = { messages: [] }; + let identity = "anthropic/claude-fable-5"; + const agent: AdvisorAgent = { + prompt: async input => { + promptInputs.push(input); + state.error = "Refusal (cyber): blocked under Anthropic's Usage Policy"; + state.messages.push({ + role: "assistant", + content: [], + stopReason: "error", + stopDetails: { type: "refusal", category: "cyber" }, + errorMessage: state.error, + timestamp: promptInputs.length + 1, + } as unknown as AgentMessage); + }, + abort: () => {}, + reset: () => {}, + rollbackTo: count => { + state.messages.length = count; + state.error = undefined; + }, + state, + }; + const messages = [ + { + role: "assistant", + content: [ + { type: "thinking", thinking: "private reasoning" }, + { type: "text", text: "answer" }, + ], + timestamp: 1, + } as AgentMessage, + ]; + const chain = ["openai-codex/gpt-5.6-sol", "synthetic/hf:moonshotai/Kimi-K3", "fireworks/kimi-k3"]; + const switched: string[] = []; + const runtime = new AdvisorRuntime( + agent, + { + snapshotMessages: () => messages, + enqueueAdvice: () => {}, + notifyFailure: error => failures.push(error), + getModelIdentity: () => identity, + onTurnError: async () => { + const next = chain[switched.length]; + if (!next) return false; + switched.push(next); + identity = next; + return true; + }, + }, + 0, + ); + + runtime.onTurnEnd(messages); + await settleUntil(() => failures.length === 1 && runtime.backlog === 0); + + expect(switched).toEqual(chain); + expect(failures).toHaveLength(1); + }); + + it("stops a refusal walk that a cyclic chain would loop forever", async () => { + const promptInputs: string[] = []; + const failures: unknown[] = []; + const state: { messages: AgentMessage[]; error?: string } = { messages: [] }; + // A chain configured A -> B -> A: the host never runs out of candidates, + // so only the per-cascade tried-set can end the walk. + const cycle = ["model/b", "model/a"]; + let identity = "model/a"; + const agent: AdvisorAgent = { + prompt: async input => { + promptInputs.push(input); + state.error = "Refusal (cyber): blocked under Anthropic's Usage Policy"; + state.messages.push({ + role: "assistant", + content: [], + stopReason: "error", + stopDetails: { type: "refusal", category: "cyber" }, + errorMessage: state.error, + timestamp: promptInputs.length + 1, + } as unknown as AgentMessage); + }, + abort: () => {}, + reset: () => {}, + rollbackTo: count => { + state.messages.length = count; + state.error = undefined; + }, + state, + }; + const messages = [ + { + role: "assistant", + content: [ + { type: "thinking", thinking: "private reasoning" }, + { type: "text", text: "answer" }, + ], + timestamp: 1, + } as AgentMessage, + ]; + let hops = 0; + const runtime = new AdvisorRuntime( + agent, + { + snapshotMessages: () => messages, + enqueueAdvice: () => {}, + notifyFailure: error => failures.push(error), + getModelIdentity: () => identity, + onTurnError: async () => { + identity = cycle[hops % cycle.length] ?? "model/a"; + hops++; + return true; + }, + }, + 0, + ); + + runtime.onTurnEnd(messages); + await settleUntil(() => failures.length === 1 && runtime.backlog === 0); + + // a (refuse, strip) -> b (refuse, strip) -> back to a, already tried: stop. + expect(hops).toBe(2); + expect(failures).toHaveLength(1); + }); + it("degrades on a category-less refusal", async () => { const promptInputs: string[] = []; const failures: unknown[] = [];