smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(advisor): fail a refusal over to the model fallback chain

Browse Source 
A classifier refusal returned before the `onTurnError` hook that owns
model fallback, so `AdvisorRuntime` treated one provider's policy verdict
as terminal: `Refusal (cyber)` on the advisor model disabled the advisor
outright even with a fallback chain configured. Its only recovery was
stripping echoed primary reasoning and resending once, which does nothing
for a refusal about the content itself.

Route a refusal that outlives the strip through the same hook the generic
failure path uses, mirroring its epoch guard, session-transition requeue,
and requeue-on-recovery. The cascade walks the chain to exhaustion and
only reports the advisor unavailable once the host runs out of candidates,
matching what turn-recovery already allows for the primary.

Each cascade visits a model at most once. A switch re-arms
`#includeThinking` through `#syncModelIdentity`, so chain keys that point
back at each other (A to B, B to A) would otherwise strip-and-resend
against the same pair forever. A successful turn or a reset starts a
fresh walk.

Also stop `/advisor status` throwing when a live advisor has no roster
entry: `formatAdvisorStatus` guarded only the inactive case before
dereferencing `stats.advisors[0]`, and `#ensureAdvisors` clears
`#advisorStatuses` before repopulating it, so a status call landing in
that window hit `undefined.contextWindow`.
This commit is contained in:
Mantas Vidutis
2026-08-04 10:54:27 -07:00
parent 5af71dc9cf
commit 6e32705e71
4 changed files with 320 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/CHANGELOG.md
+6
View File
@@ -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
packages/coding-agent/src/advisor/runtime.ts
+51
View File
@@ -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<string>();
/** 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);
packages/coding-agent/src/session/session-advisors.ts
+8 -1
View File
@@ -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.";
packages/coding-agent/test/advisor/advisor.test.ts
+255
View File
@@ -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[] = [];