fix(FN-1989): stabilize triage fallback
This commit is contained in:
@@ -507,6 +507,54 @@ describe("createKbAgent", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("falls back during prompt when the primary model has an auth failure", async () => {
|
||||
const primaryPrompt = vi.fn().mockRejectedValue(new Error("401 unauthorized: invalid api key"));
|
||||
const fallbackPrompt = vi.fn().mockResolvedValue(undefined);
|
||||
const primaryDispose = vi.fn();
|
||||
|
||||
createAgentSessionMock
|
||||
.mockResolvedValueOnce({
|
||||
session: {
|
||||
prompt: primaryPrompt,
|
||||
subscribe: vi.fn(),
|
||||
dispose: primaryDispose,
|
||||
setThinkingLevel: vi.fn(),
|
||||
},
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
session: {
|
||||
prompt: fallbackPrompt,
|
||||
subscribe: vi.fn(),
|
||||
dispose: vi.fn(),
|
||||
setThinkingLevel: vi.fn(),
|
||||
},
|
||||
});
|
||||
|
||||
const { createKbAgent } = await import("./pi.js");
|
||||
|
||||
const { session } = await createKbAgent({
|
||||
cwd: "/tmp",
|
||||
systemPrompt: "test",
|
||||
tools: "readonly",
|
||||
defaultProvider: "zai",
|
||||
defaultModelId: "glm-5.1",
|
||||
fallbackProvider: "openai-codex",
|
||||
fallbackModelId: "gpt-5.3-codex",
|
||||
});
|
||||
|
||||
await (session as any).promptWithFallback("make a spec");
|
||||
|
||||
expect(primaryPrompt).toHaveBeenCalledWith("make a spec");
|
||||
expect(primaryDispose).toHaveBeenCalled();
|
||||
expect(fallbackPrompt).toHaveBeenCalledWith("make a spec");
|
||||
expect(createAgentSessionMock).toHaveBeenNthCalledWith(1, expect.objectContaining({
|
||||
model: { provider: "zai", id: "glm-5.1" },
|
||||
}));
|
||||
expect(createAgentSessionMock).toHaveBeenNthCalledWith(2, expect.objectContaining({
|
||||
model: { provider: "openai-codex", id: "gpt-5.3-codex" },
|
||||
}));
|
||||
});
|
||||
|
||||
it("enables auto-compaction to prevent context-window overflow", async () => {
|
||||
const { createKbAgent } = await import("./pi.js");
|
||||
|
||||
|
||||
@@ -226,6 +226,14 @@ function isRetryableModelSelectionError(message: string): boolean {
|
||||
return normalized.includes("rate limit")
|
||||
|| normalized.includes("too many requests")
|
||||
|| normalized.includes("429")
|
||||
|| normalized.includes("401")
|
||||
|| normalized.includes("403")
|
||||
|| normalized.includes("unauthorized")
|
||||
|| normalized.includes("forbidden")
|
||||
|| normalized.includes("authentication")
|
||||
|| normalized.includes("invalid api key")
|
||||
|| normalized.includes("invalid key")
|
||||
|| normalized.includes("api key")
|
||||
|| normalized.includes("overloaded")
|
||||
|| normalized.includes("quota")
|
||||
|| normalized.includes("capacity")
|
||||
|
||||
@@ -1385,6 +1385,55 @@ describe("taskCreate tool model inheritance", () => {
|
||||
});
|
||||
|
||||
describe("bounded recovery retries for triage", () => {
|
||||
it("marks triage failed when the agent exits without calling review_spec", async () => {
|
||||
const task = {
|
||||
id: "FN-202",
|
||||
description: "Test triage task",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as unknown as Task;
|
||||
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue({ ...task, attachments: [], comments: [] }),
|
||||
});
|
||||
|
||||
mockCreateKbAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
sessionManager: {
|
||||
getLeafId: vi.fn().mockReturnValue(null),
|
||||
navigateTree: vi.fn(),
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { promptWithFallback } = await import("./pi.js");
|
||||
(promptWithFallback as ReturnType<typeof vi.fn>).mockResolvedValueOnce(undefined);
|
||||
|
||||
const processor = new TriageProcessor(store, "/test/root", {
|
||||
pollIntervalMs: 100_000,
|
||||
});
|
||||
|
||||
await processor.specifyTask(task);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-202", expect.objectContaining({
|
||||
status: "failed",
|
||||
error: expect.stringContaining("review_spec was never called"),
|
||||
recoveryRetryCount: null,
|
||||
nextRecoveryAt: null,
|
||||
}));
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-202",
|
||||
expect.stringContaining("Specification failed: spec review not approved"),
|
||||
);
|
||||
});
|
||||
|
||||
it("sets recoveryRetryCount and nextRecoveryAt on first transient error via specifyTask", async () => {
|
||||
const task = {
|
||||
id: "FN-200",
|
||||
@@ -1456,6 +1505,38 @@ describe("taskCreate tool model inheritance", () => {
|
||||
});
|
||||
|
||||
describe("recovery due-time gating (nextRecoveryAt)", () => {
|
||||
it("skips failed triage tasks until they are explicitly retried", async () => {
|
||||
const task = {
|
||||
id: "FN-102",
|
||||
description: "Failed triage task",
|
||||
column: "triage",
|
||||
status: "failed",
|
||||
error: "Specification failed",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as unknown as Task;
|
||||
|
||||
const store = createMockStore({
|
||||
listTasks: vi.fn().mockResolvedValue([task]),
|
||||
});
|
||||
|
||||
const processor = new TriageProcessor(store, "/test/root", {
|
||||
pollIntervalMs: 100_000,
|
||||
});
|
||||
const specifySpy = vi.spyOn(processor, "specifyTask");
|
||||
|
||||
processor.start();
|
||||
await new Promise((r) => setTimeout(r, 50));
|
||||
processor.stop();
|
||||
|
||||
expect(specifySpy).not.toHaveBeenCalled();
|
||||
specifySpy.mockRestore();
|
||||
});
|
||||
|
||||
it("skips triage tasks whose nextRecoveryAt is in the future", async () => {
|
||||
const future = new Date(Date.now() + 60_000).toISOString();
|
||||
const task = {
|
||||
@@ -2043,6 +2124,8 @@ describe("pause-abort status clearing (bug fix)", () => {
|
||||
navigateTree: vi.fn(),
|
||||
},
|
||||
});
|
||||
const { promptWithFallback } = await import("./pi.js");
|
||||
(promptWithFallback as ReturnType<typeof vi.fn>).mockReturnValueOnce(disposePromise);
|
||||
|
||||
const task: Task = { id: "FN-001", description: "test", column: "triage", dependencies: [], steps: [], currentStep: 0, log: [], createdAt: "", updatedAt: "" };
|
||||
const processor = new TriageProcessor(store, "/tmp/root");
|
||||
@@ -2093,6 +2176,8 @@ describe("stuck task detector integration", () => {
|
||||
navigateTree: vi.fn(),
|
||||
},
|
||||
});
|
||||
const { promptWithFallback } = await import("./pi.js");
|
||||
(promptWithFallback as ReturnType<typeof vi.fn>).mockReturnValueOnce(disposePromise);
|
||||
|
||||
const task: Task = { id: "FN-001", description: "test", column: "triage", dependencies: [], steps: [], currentStep: 0, log: [], createdAt: "", updatedAt: "" };
|
||||
const processor = new TriageProcessor(store, "/tmp/root");
|
||||
|
||||
@@ -541,6 +541,9 @@ export class TriageProcessor {
|
||||
(t) => t.column === "triage" && !this.processing.has(t.id) && !t.paused
|
||||
// Skip tasks awaiting manual plan approval — they should not be auto-discovered
|
||||
&& t.status !== "awaiting-approval"
|
||||
// Skip failed specifications until the user explicitly retries them.
|
||||
&& t.status !== "failed"
|
||||
&& t.status !== "stuck-killed"
|
||||
// Skip tasks with a recovery backoff that hasn't elapsed yet
|
||||
&& !(t.nextRecoveryAt && new Date(t.nextRecoveryAt).getTime() > now),
|
||||
);
|
||||
@@ -687,7 +690,7 @@ export class TriageProcessor {
|
||||
projectRootDir: this.rootDir,
|
||||
});
|
||||
|
||||
const { session } = await createKbAgent({
|
||||
let { session } = await createKbAgent({
|
||||
cwd: this.rootDir,
|
||||
systemPrompt: triageSystemPrompt,
|
||||
tools: "coding",
|
||||
@@ -790,6 +793,22 @@ export class TriageProcessor {
|
||||
// Re-raise errors that pi-coding-agent swallowed after exhausting retries.
|
||||
checkSessionError(session);
|
||||
|
||||
if (this.pauseAborted.has(task.id)) {
|
||||
this.pauseAborted.delete(task.id);
|
||||
triageLog.log(`${task.id} aborted by pause — clearing status`);
|
||||
const restoreStatus = task.status === "needs-respecify" ? "needs-respecify" : null;
|
||||
await this.store.updateTask(task.id, { status: restoreStatus }).catch(() => {});
|
||||
return;
|
||||
}
|
||||
|
||||
if (this.stuckAborted.has(task.id)) {
|
||||
this.stuckAborted.delete(task.id);
|
||||
triageLog.log(`${task.id} killed by stuck detector — clearing status for retry`);
|
||||
const restoreStatus = task.status === "needs-respecify" ? "needs-respecify" : null;
|
||||
await this.store.updateTask(task.id, { status: restoreStatus }).catch(() => {});
|
||||
return;
|
||||
}
|
||||
|
||||
if (createdSubtasksRef.current.length > 0) {
|
||||
const childTaskIds = createdSubtasksRef.current.join(", ");
|
||||
await this.store.logEntry(
|
||||
@@ -801,6 +820,85 @@ export class TriageProcessor {
|
||||
return;
|
||||
}
|
||||
|
||||
const planningFallbackProvider = settings.planningFallbackProvider;
|
||||
const planningFallbackModelId = settings.planningFallbackModelId;
|
||||
const canRetryWithPlanningFallback =
|
||||
specReviewVerdictRef.current !== "APPROVE" &&
|
||||
planningFallbackProvider &&
|
||||
planningFallbackModelId &&
|
||||
modelDesc !== `${planningFallbackProvider}/${planningFallbackModelId}`;
|
||||
|
||||
if (canRetryWithPlanningFallback) {
|
||||
const verdictDesc =
|
||||
specReviewVerdictRef.current === null
|
||||
? "review_spec was never called"
|
||||
: `verdict was ${specReviewVerdictRef.current}`;
|
||||
const fallbackDesc = `${planningFallbackProvider}/${planningFallbackModelId}`;
|
||||
triageLog.warn(
|
||||
`${task.id} primary planning model produced no approved spec (${verdictDesc}) — retrying with fallback ${fallbackDesc}`,
|
||||
);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Primary planning model produced no approved spec (${verdictDesc}) — retrying with fallback ${fallbackDesc}`,
|
||||
);
|
||||
|
||||
session.dispose();
|
||||
this.activeSessions.delete(task.id);
|
||||
stuckDetector?.untrackTask(task.id);
|
||||
specReviewVerdictRef.current = null;
|
||||
approvedCommentFingerprintRef.current = "";
|
||||
|
||||
const fallbackResult = await createKbAgent({
|
||||
cwd: this.rootDir,
|
||||
systemPrompt: triageSystemPrompt,
|
||||
tools: "coding",
|
||||
customTools,
|
||||
onText: agentLogger.onText,
|
||||
onThinking: agentLogger.onThinking,
|
||||
onToolStart: agentLogger.onToolStart,
|
||||
onToolEnd: agentLogger.onToolEnd,
|
||||
defaultProvider: planningFallbackProvider,
|
||||
defaultModelId: planningFallbackModelId,
|
||||
defaultThinkingLevel: settings.defaultThinkingLevel,
|
||||
...(skillContext.skillSelectionContext ? { skillSelection: skillContext.skillSelectionContext } : {}),
|
||||
});
|
||||
|
||||
session = fallbackResult.session;
|
||||
const fallbackModelDesc = describeModel(session);
|
||||
triageLog.log(`${task.id}: using fallback model ${fallbackModelDesc}`);
|
||||
await this.store.logEntry(task.id, `Triage using fallback model: ${fallbackModelDesc}`);
|
||||
await this.store.appendAgentLog(
|
||||
task.id,
|
||||
`Triage using fallback model: ${fallbackModelDesc}`,
|
||||
"text",
|
||||
undefined,
|
||||
"triage",
|
||||
);
|
||||
|
||||
sessionRef.current = session;
|
||||
this.activeSessions.set(task.id, session);
|
||||
stuckDetector?.trackTask(task.id, session);
|
||||
stuckDetector?.recordActivity(task.id);
|
||||
|
||||
await promptWithFallback(
|
||||
session,
|
||||
agentPrompt,
|
||||
imageContents.length > 0 ? { images: imageContents } : undefined,
|
||||
);
|
||||
checkSessionError(session);
|
||||
|
||||
if (createdSubtasksRef.current.length > 0) {
|
||||
const childTaskIds = createdSubtasksRef.current.join(", ");
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Converted into subtasks: ${childTaskIds}`,
|
||||
);
|
||||
await this.store.deleteTask(task.id);
|
||||
triageLog.log(`✓ ${task.id} split into subtasks (${childTaskIds}) and closed`);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Post-session APPROVE gate: only advance to todo when the spec
|
||||
// reviewer explicitly approved. Any other verdict (REVISE,
|
||||
// RETHINK, UNAVAILABLE) or a missing review (null) keeps the task
|
||||
@@ -810,17 +908,21 @@ export class TriageProcessor {
|
||||
specReviewVerdictRef.current === null
|
||||
? "review_spec was never called"
|
||||
: `verdict was ${specReviewVerdictRef.current}`;
|
||||
const failureMessage =
|
||||
`Specification failed: spec review not approved (${verdictDesc}). ` +
|
||||
"Retry after adjusting the task prompt or model.";
|
||||
triageLog.log(
|
||||
`${task.id} spec review not approved (${verdictDesc}) — not moving to todo`,
|
||||
`${task.id} spec review not approved (${verdictDesc}) — marking specification failed`,
|
||||
);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Spec review not approved (${verdictDesc}) — specification not approved`,
|
||||
failureMessage,
|
||||
);
|
||||
// For re-specification, keep the needs-respecify status so it can be retried
|
||||
// For new specs, clear the status
|
||||
await this.store.updateTask(task.id, {
|
||||
status: isRespecify ? "needs-respecify" : null,
|
||||
status: "failed",
|
||||
error: failureMessage,
|
||||
recoveryRetryCount: null,
|
||||
nextRecoveryAt: null,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user