feat(FN-2662): enforce model override fallback hierarchy
- Fix triage planning model resolution to fall back through project/global planning settings and default overrides - Fix reviewer model selection to honor validator-specific settings before default provider/model overrides - Update merger model resolution to apply default override fallback and align shared task setting types/executor flow - Add regression coverage for triage, reviewer, and merger fallback behavior and update settings hierarchy documentation
This commit is contained in:
@@ -6331,7 +6331,7 @@ describe("aiMergeTask — in-merge verification fix", () => {
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
mockedCreateFnAgent.mockImplementation(async (opts: any) => {
|
||||
mockedCreateFnAgent.mockImplementation(async (_opts: any) => {
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
@@ -6362,6 +6362,61 @@ describe("aiMergeTask — in-merge verification fix", () => {
|
||||
expect(fixAgentCall[0].defaultModelId).toBe("claude-sonnet-4-5");
|
||||
});
|
||||
|
||||
it("uses project default override for merge agent model", async () => {
|
||||
mockedExecSync.mockImplementation((cmd: any) => {
|
||||
const cmdStr = String(cmd);
|
||||
if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git log")) return "- feat: something" as any;
|
||||
if (cmdStr.includes("merge-base")) return Buffer.from("abc123");
|
||||
if (cmdStr.includes("git diff") && cmdStr.includes("--stat")) return "1 file changed" as any;
|
||||
if (cmdStr.includes("merge --squash")) return Buffer.from("");
|
||||
if (cmdStr.includes("vitest run")) {
|
||||
const err = new Error("Test failed") as any;
|
||||
err.status = 1;
|
||||
err.stdout = "";
|
||||
err.stderr = "";
|
||||
throw err;
|
||||
}
|
||||
if (cmdStr.includes("diff --cached --quiet")) return "1" as any;
|
||||
if (cmdStr.includes("diff --cached")) return "" as any;
|
||||
if (cmdStr.includes("branch -d") || cmdStr.includes("branch -D")) return Buffer.from("");
|
||||
if (cmdStr.includes("worktree remove")) return Buffer.from("");
|
||||
if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123";
|
||||
return Buffer.from("");
|
||||
});
|
||||
|
||||
mockedCreateFnAgent.mockImplementation(async (_opts: any) => {
|
||||
return {
|
||||
session: {
|
||||
prompt: vi.fn().mockResolvedValue(undefined),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
|
||||
const store = createMockStore(
|
||||
{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050" },
|
||||
[{ id: "FN-050", worktree: "/tmp/root/.worktrees/KB-050", column: "in-review" } as Task],
|
||||
);
|
||||
(store.getSettings as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
...DEFAULT_SETTINGS,
|
||||
testCommand: "vitest run",
|
||||
verificationFixRetries: 0,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModelId: "claude-sonnet-4-5",
|
||||
defaultProviderOverride: "openai",
|
||||
defaultModelIdOverride: "gpt-4o",
|
||||
});
|
||||
|
||||
await expect(aiMergeTask(store, "/tmp/root", "FN-050")).rejects.toMatchObject({
|
||||
name: "VerificationError",
|
||||
});
|
||||
|
||||
const mergeAgentCall = mockedCreateFnAgent.mock.calls[0];
|
||||
expect(mergeAgentCall[0].defaultProvider).toBe("openai");
|
||||
expect(mergeAgentCall[0].defaultModelId).toBe("gpt-4o");
|
||||
});
|
||||
|
||||
it("fix agent session is disposed", async () => {
|
||||
const disposeMock = vi.fn();
|
||||
|
||||
|
||||
@@ -425,6 +425,51 @@ describe("reviewStep — validator model overrides", () => {
|
||||
expect(opts.defaultModelId).toBe("gemini-2.5");
|
||||
});
|
||||
|
||||
it("uses project default override when validator lanes are absent", async () => {
|
||||
mockedCreateFnAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nLooks good."),
|
||||
);
|
||||
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "FN-100", 1, "Test Step", "plan", "# prompt",
|
||||
undefined,
|
||||
{
|
||||
defaultProvider: "anthropic",
|
||||
defaultModelId: "claude-sonnet-4-5",
|
||||
projectDefaultOverrideProvider: "openai",
|
||||
projectDefaultOverrideModelId: "gpt-4o",
|
||||
// No validator lanes set
|
||||
},
|
||||
);
|
||||
|
||||
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(1);
|
||||
const opts = mockedCreateFnAgent.mock.calls[0][0];
|
||||
expect(opts.defaultProvider).toBe("openai");
|
||||
expect(opts.defaultModelId).toBe("gpt-4o");
|
||||
});
|
||||
|
||||
it("falls through to execution default when project default override is incomplete", async () => {
|
||||
mockedCreateFnAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nLooks good."),
|
||||
);
|
||||
|
||||
await reviewStep(
|
||||
"/tmp/worktree", "FN-100", 1, "Test Step", "plan", "# prompt",
|
||||
undefined,
|
||||
{
|
||||
defaultProvider: "anthropic",
|
||||
defaultModelId: "claude-sonnet-4-5",
|
||||
projectDefaultOverrideProvider: "openai",
|
||||
// projectDefaultOverrideModelId intentionally omitted
|
||||
},
|
||||
);
|
||||
|
||||
expect(mockedCreateFnAgent).toHaveBeenCalledTimes(1);
|
||||
const opts = mockedCreateFnAgent.mock.calls[0][0];
|
||||
expect(opts.defaultProvider).toBe("anthropic");
|
||||
expect(opts.defaultModelId).toBe("claude-sonnet-4-5");
|
||||
});
|
||||
|
||||
it("falls back to execution default when no validator lanes are set", async () => {
|
||||
mockedCreateFnAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nLooks good."),
|
||||
|
||||
@@ -2195,9 +2195,139 @@ describe("taskCreate tool model inheritance", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to global defaults when neither task nor settings have planning model", async () => {
|
||||
it("uses project default override when planning lanes are absent", async () => {
|
||||
const task = {
|
||||
id: "FN-402",
|
||||
description: "Test fallback to project default override",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as unknown as Task;
|
||||
|
||||
const mockDispose = vi.fn();
|
||||
const mockPrompt = vi.fn().mockResolvedValue(undefined);
|
||||
const mockGetLeafId = vi.fn().mockReturnValue(null);
|
||||
const mockNavigateTree = vi.fn();
|
||||
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue({ ...task, attachments: [] }),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModelId: "claude-sonnet-4-5",
|
||||
defaultProviderOverride: "openai",
|
||||
defaultModelIdOverride: "gpt-4o",
|
||||
// No planningProvider/planningModelId set
|
||||
} as Settings),
|
||||
});
|
||||
|
||||
mockCreateFnAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: mockPrompt,
|
||||
dispose: mockDispose,
|
||||
sessionManager: {
|
||||
getLeafId: mockGetLeafId,
|
||||
navigateTree: mockNavigateTree,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { promptWithFallback } = await import("../pi.js");
|
||||
(promptWithFallback as ReturnType<typeof vi.fn>).mockRejectedValueOnce(
|
||||
new Error("test stop after model check"),
|
||||
);
|
||||
|
||||
const processor = new TriageProcessor(store, "/test/root", {
|
||||
pollIntervalMs: 100_000,
|
||||
});
|
||||
|
||||
await processor.specifyTask(task);
|
||||
|
||||
// Should use project default override when planning lanes are absent
|
||||
expect(mockCreateFnAgent).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
defaultProvider: "openai",
|
||||
defaultModelId: "gpt-4o",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("falls through to global default when project default override is incomplete", async () => {
|
||||
const task = {
|
||||
id: "FN-403",
|
||||
description: "Test fallback when project default override is incomplete",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as unknown as Task;
|
||||
|
||||
const mockDispose = vi.fn();
|
||||
const mockPrompt = vi.fn().mockResolvedValue(undefined);
|
||||
const mockGetLeafId = vi.fn().mockReturnValue(null);
|
||||
const mockNavigateTree = vi.fn();
|
||||
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue({ ...task, attachments: [] }),
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
defaultProvider: "anthropic",
|
||||
defaultModelId: "claude-sonnet-4-5",
|
||||
defaultProviderOverride: "openai",
|
||||
// defaultModelIdOverride intentionally omitted
|
||||
// No planningProvider/planningModelId set
|
||||
} as Settings),
|
||||
});
|
||||
|
||||
mockCreateFnAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: mockPrompt,
|
||||
dispose: mockDispose,
|
||||
sessionManager: {
|
||||
getLeafId: mockGetLeafId,
|
||||
navigateTree: mockNavigateTree,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const { promptWithFallback } = await import("../pi.js");
|
||||
(promptWithFallback as ReturnType<typeof vi.fn>).mockRejectedValueOnce(
|
||||
new Error("test stop after model check"),
|
||||
);
|
||||
|
||||
const processor = new TriageProcessor(store, "/test/root", {
|
||||
pollIntervalMs: 100_000,
|
||||
});
|
||||
|
||||
await processor.specifyTask(task);
|
||||
|
||||
// Incomplete override should fall through to global defaults
|
||||
expect(mockCreateFnAgent).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
defaultProvider: "anthropic",
|
||||
defaultModelId: "claude-sonnet-4-5",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to global defaults when neither task nor settings have planning model", async () => {
|
||||
const task = {
|
||||
id: "FN-404",
|
||||
description: "Test fallback to global defaults",
|
||||
column: "triage",
|
||||
dependencies: [],
|
||||
|
||||
@@ -2908,6 +2908,9 @@ export class TaskExecutor {
|
||||
// Global validator lane
|
||||
globalValidatorProvider: settings.validatorGlobalProvider,
|
||||
globalValidatorModelId: settings.validatorGlobalModelId,
|
||||
// Project-level default override (fallback before execution defaults)
|
||||
projectDefaultOverrideProvider: settings.defaultProviderOverride,
|
||||
projectDefaultOverrideModelId: settings.defaultModelIdOverride,
|
||||
store,
|
||||
taskId,
|
||||
task: detail,
|
||||
|
||||
@@ -922,8 +922,12 @@ A merge has been applied and the verification command failed. Your job is to fix
|
||||
onThinking: logger.onThinking,
|
||||
onToolStart: logger.onToolStart,
|
||||
onToolEnd: logger.onToolEnd,
|
||||
defaultProvider: settings.defaultProvider,
|
||||
defaultModelId: settings.defaultModelId,
|
||||
defaultProvider: settings.defaultProviderOverride && settings.defaultModelIdOverride
|
||||
? settings.defaultProviderOverride
|
||||
: settings.defaultProvider,
|
||||
defaultModelId: settings.defaultProviderOverride && settings.defaultModelIdOverride
|
||||
? settings.defaultModelIdOverride
|
||||
: settings.defaultModelId,
|
||||
defaultThinkingLevel: settings.defaultThinkingLevel,
|
||||
// Skill selection: use assigned agent skills if available, otherwise role fallback
|
||||
...(skillContext?.skillSelectionContext ? { skillSelection: skillContext.skillSelectionContext } : {}),
|
||||
@@ -1808,8 +1812,12 @@ You are assisting with a paused \`git pull --rebase\`.
|
||||
onThinking: agentLogger.onThinking,
|
||||
onToolStart: agentLogger.onToolStart,
|
||||
onToolEnd: agentLogger.onToolEnd,
|
||||
defaultProvider: settings.defaultProvider,
|
||||
defaultModelId: settings.defaultModelId,
|
||||
defaultProvider: settings.defaultProviderOverride && settings.defaultModelIdOverride
|
||||
? settings.defaultProviderOverride
|
||||
: settings.defaultProvider,
|
||||
defaultModelId: settings.defaultProviderOverride && settings.defaultModelIdOverride
|
||||
? settings.defaultModelIdOverride
|
||||
: settings.defaultModelId,
|
||||
defaultThinkingLevel: settings.defaultThinkingLevel,
|
||||
});
|
||||
|
||||
@@ -3490,8 +3498,12 @@ async function runAiAgentForCommit(params: AiAgentParams): Promise<{ success: bo
|
||||
onThinking: agentLogger.onThinking,
|
||||
onToolStart: agentLogger.onToolStart,
|
||||
onToolEnd: agentLogger.onToolEnd,
|
||||
defaultProvider: settings.defaultProvider,
|
||||
defaultModelId: settings.defaultModelId,
|
||||
defaultProvider: settings.defaultProviderOverride && settings.defaultModelIdOverride
|
||||
? settings.defaultProviderOverride
|
||||
: settings.defaultProvider,
|
||||
defaultModelId: settings.defaultProviderOverride && settings.defaultModelIdOverride
|
||||
? settings.defaultModelIdOverride
|
||||
: settings.defaultModelId,
|
||||
defaultThinkingLevel: settings.defaultThinkingLevel,
|
||||
// Skill selection: use assigned agent skills if available, otherwise role fallback
|
||||
...(skillContext?.skillSelectionContext ? { skillSelection: skillContext.skillSelectionContext } : {}),
|
||||
|
||||
@@ -201,10 +201,14 @@ export interface ReviewOptions {
|
||||
projectValidatorProvider?: string;
|
||||
/** Project-level validator model ID override. Takes precedence over global validator lane. */
|
||||
projectValidatorModelId?: string;
|
||||
/** Global validator lane provider. Takes precedence over execution defaults. */
|
||||
/** Global validator lane provider. Takes precedence over project default override + execution defaults. */
|
||||
globalValidatorProvider?: string;
|
||||
/** Global validator lane model ID. Takes precedence over execution defaults. */
|
||||
/** Global validator lane model ID. Takes precedence over project default override + execution defaults. */
|
||||
globalValidatorModelId?: string;
|
||||
/** Project-level default provider override, used when validator lanes are absent. */
|
||||
projectDefaultOverrideProvider?: string;
|
||||
/** Project-level default model override, used when validator lanes are absent. */
|
||||
projectDefaultOverrideModelId?: string;
|
||||
/** Fallback model provider used when the primary reviewer model hits a retryable provider-side error. */
|
||||
fallbackProvider?: string;
|
||||
/** Fallback model ID used with `fallbackProvider`. */
|
||||
@@ -269,21 +273,26 @@ export async function reviewStep(
|
||||
// 1. Task-level validator override pair (taskValidatorProvider + taskValidatorModelId)
|
||||
// 2. Project-level validator override pair (projectValidatorProvider + projectValidatorModelId)
|
||||
// 3. Global validator lane pair (globalValidatorProvider + globalValidatorModelId)
|
||||
// 4. Execution default pair (defaultProvider + defaultModelId)
|
||||
// 4. Project default override pair (projectDefaultOverrideProvider + projectDefaultOverrideModelId)
|
||||
// 5. Execution default pair (defaultProvider + defaultModelId)
|
||||
const validatorProvider = options.taskValidatorProvider && options.taskValidatorModelId
|
||||
? options.taskValidatorProvider
|
||||
: (options.projectValidatorProvider && options.projectValidatorModelId
|
||||
? options.projectValidatorProvider
|
||||
: (options.globalValidatorProvider && options.globalValidatorModelId
|
||||
? options.globalValidatorProvider
|
||||
: options.defaultProvider));
|
||||
: (options.projectDefaultOverrideProvider && options.projectDefaultOverrideModelId
|
||||
? options.projectDefaultOverrideProvider
|
||||
: options.defaultProvider)));
|
||||
const validatorModelId = options.taskValidatorProvider && options.taskValidatorModelId
|
||||
? options.taskValidatorModelId
|
||||
: (options.projectValidatorProvider && options.projectValidatorModelId
|
||||
? options.projectValidatorModelId
|
||||
: (options.globalValidatorProvider && options.globalValidatorModelId
|
||||
? options.globalValidatorModelId
|
||||
: options.defaultModelId));
|
||||
: (options.projectDefaultOverrideProvider && options.projectDefaultOverrideModelId
|
||||
? options.projectDefaultOverrideModelId
|
||||
: options.defaultModelId)));
|
||||
|
||||
// Resolve validator fallback using lane hierarchy:
|
||||
// 1. Project-level validator fallback (projectValidatorFallbackProvider + projectValidatorFallbackModelId)
|
||||
|
||||
@@ -909,23 +909,28 @@ export class TriageProcessor {
|
||||
onToolEnd: agentLogger.onToolEnd,
|
||||
// Resolve planning model using canonical lane hierarchy:
|
||||
// 1. Task planning override pair (planningModelProvider + planningModelId)
|
||||
// 2. Project planning override pair (planningProvider + planningModelId)
|
||||
// 2. Project planning lane pair (planningProvider + planningModelId)
|
||||
// 3. Global planning lane pair (planningGlobalProvider + planningGlobalModelId)
|
||||
// 4. Default pair (defaultProvider + defaultModelId)
|
||||
// 4. Project default override pair (defaultProviderOverride + defaultModelIdOverride)
|
||||
// 5. Global default pair (defaultProvider + defaultModelId)
|
||||
defaultProvider: task.planningModelProvider && task.planningModelId
|
||||
? task.planningModelProvider
|
||||
: (settings.planningProvider && settings.planningModelId
|
||||
? settings.planningProvider
|
||||
: (settings.planningGlobalProvider && settings.planningGlobalModelId
|
||||
? settings.planningGlobalProvider
|
||||
: settings.defaultProvider)),
|
||||
: (settings.defaultProviderOverride && settings.defaultModelIdOverride
|
||||
? settings.defaultProviderOverride
|
||||
: settings.defaultProvider))),
|
||||
defaultModelId: task.planningModelProvider && task.planningModelId
|
||||
? task.planningModelId
|
||||
: (settings.planningProvider && settings.planningModelId
|
||||
? settings.planningModelId
|
||||
: (settings.planningGlobalProvider && settings.planningGlobalModelId
|
||||
? settings.planningGlobalModelId
|
||||
: settings.defaultModelId)),
|
||||
: (settings.defaultProviderOverride && settings.defaultModelIdOverride
|
||||
? settings.defaultModelIdOverride
|
||||
: settings.defaultModelId))),
|
||||
fallbackProvider: settings.planningFallbackProvider && settings.planningFallbackModelId
|
||||
? settings.planningFallbackProvider
|
||||
: settings.fallbackProvider,
|
||||
@@ -1711,6 +1716,9 @@ export class TriageProcessor {
|
||||
// Global validator lane
|
||||
globalValidatorProvider: currentSettings.validatorGlobalProvider,
|
||||
globalValidatorModelId: currentSettings.validatorGlobalModelId,
|
||||
// Project-level default override (fallback before execution defaults)
|
||||
projectDefaultOverrideProvider: currentSettings.defaultProviderOverride,
|
||||
projectDefaultOverrideModelId: currentSettings.defaultModelIdOverride,
|
||||
defaultThinkingLevel: currentSettings.defaultThinkingLevel,
|
||||
store,
|
||||
taskId,
|
||||
|
||||
Reference in New Issue
Block a user