feat(FN-1062): fix step status tracking and expose step session settings in CLI
- Fix step status not being tracked correctly in executor callbacks (done/skipped were not updated) - Expose runStepsInNewSessions and maxParallelSteps settings in CLI settings command - Add AGENTS.md documentation for runStepsInNewSessions and maxParallelSteps settings - Add changeset for the published CLI package - Add tests for CLI settings validation and executor step status tracking
This commit is contained in:
@@ -139,4 +139,70 @@ describe("settings commands", () => {
|
||||
expect(errorSpy).toHaveBeenCalledWith('Error: Setting "maxConcurrent" is project-only. Use --project or run from a project directory.');
|
||||
expect(resolveProject).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("runSettingsSet with project updates runStepsInNewSessions", async () => {
|
||||
const updateSettings = vi.fn().mockResolvedValue(makeSettings({ runStepsInNewSessions: true }));
|
||||
const getSettings = vi.fn().mockResolvedValue(makeSettings({ runStepsInNewSessions: true }));
|
||||
vi.mocked(resolveProject).mockResolvedValue({
|
||||
projectId: "proj-1",
|
||||
projectName: "demo-project",
|
||||
projectPath: "/projects/demo",
|
||||
isRegistered: true,
|
||||
store: { updateSettings, getSettings } as any,
|
||||
});
|
||||
|
||||
await runSettingsSet("runStepsInNewSessions", "true", "demo-project");
|
||||
|
||||
expect(updateSettings).toHaveBeenCalledWith({ runStepsInNewSessions: true });
|
||||
});
|
||||
|
||||
it("runSettingsSet with project updates maxParallelSteps", async () => {
|
||||
const updateSettings = vi.fn().mockResolvedValue(makeSettings({ maxParallelSteps: 3 }));
|
||||
const getSettings = vi.fn().mockResolvedValue(makeSettings({ maxParallelSteps: 3 }));
|
||||
vi.mocked(resolveProject).mockResolvedValue({
|
||||
projectId: "proj-1",
|
||||
projectName: "demo-project",
|
||||
projectPath: "/projects/demo",
|
||||
isRegistered: true,
|
||||
store: { updateSettings, getSettings } as any,
|
||||
});
|
||||
|
||||
await runSettingsSet("maxParallelSteps", "3", "demo-project");
|
||||
|
||||
expect(updateSettings).toHaveBeenCalledWith({ maxParallelSteps: 3 });
|
||||
});
|
||||
|
||||
it("rejects maxParallelSteps values outside range", async () => {
|
||||
vi.mocked(resolveProject).mockResolvedValue({
|
||||
projectId: "proj-1",
|
||||
projectName: "demo-project",
|
||||
projectPath: "/projects/demo",
|
||||
isRegistered: true,
|
||||
store: { updateSettings: vi.fn(), getSettings: vi.fn() } as any,
|
||||
});
|
||||
|
||||
await expect(runSettingsSet("maxParallelSteps", "5", "demo-project")).rejects.toThrow("process.exit:1");
|
||||
expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining("Value out of range for maxParallelSteps"));
|
||||
});
|
||||
|
||||
it("runSettingsShow displays Execution section with step-session settings", async () => {
|
||||
const getSettings = vi.fn().mockResolvedValue(makeSettings({
|
||||
runStepsInNewSessions: true,
|
||||
maxParallelSteps: 3,
|
||||
}));
|
||||
vi.mocked(resolveProject).mockResolvedValue({
|
||||
projectId: "proj-1",
|
||||
projectName: "demo-project",
|
||||
projectPath: "/projects/demo",
|
||||
isRegistered: true,
|
||||
store: { getSettings } as any,
|
||||
});
|
||||
|
||||
await runSettingsShow("demo-project");
|
||||
|
||||
const output = logSpy.mock.calls.map((args) => args.join(" ")).join("\n");
|
||||
expect(output).toContain("Execution");
|
||||
expect(output).toContain("Run Steps In New Sessions");
|
||||
expect(output).toContain("Max Parallel Steps");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -13,6 +13,8 @@ export const VALID_SETTINGS = [
|
||||
"requirePlanApproval",
|
||||
"ntfyEnabled",
|
||||
"defaultModel",
|
||||
"runStepsInNewSessions",
|
||||
"maxParallelSteps",
|
||||
] as const;
|
||||
|
||||
const GLOBAL_ONLY_SETTINGS = ["ntfyEnabled", "ntfyTopic", "defaultModel"] as const;
|
||||
@@ -24,6 +26,8 @@ const PROJECT_ONLY_SETTINGS = [
|
||||
"autoResolveConflicts",
|
||||
"smartConflictResolution",
|
||||
"requirePlanApproval",
|
||||
"runStepsInNewSessions",
|
||||
"maxParallelSteps",
|
||||
] as const;
|
||||
|
||||
type ValidSettingKey = (typeof VALID_SETTINGS)[number];
|
||||
@@ -34,9 +38,10 @@ const BOOLEAN_SETTINGS: readonly string[] = [
|
||||
"smartConflictResolution",
|
||||
"requirePlanApproval",
|
||||
"ntfyEnabled",
|
||||
"runStepsInNewSessions",
|
||||
];
|
||||
|
||||
const NUMBER_SETTINGS: readonly string[] = ["maxConcurrent", "maxWorktrees"];
|
||||
const NUMBER_SETTINGS: readonly string[] = ["maxConcurrent", "maxWorktrees", "maxParallelSteps"];
|
||||
|
||||
const ENUM_SETTINGS: Record<string, readonly string[]> = {
|
||||
worktreeNaming: ["random", "task-id", "task-title"],
|
||||
@@ -48,6 +53,7 @@ const STRING_SETTINGS: readonly string[] = ["taskPrefix", "ntfyTopic", "defaultM
|
||||
const NUMBER_RANGES: Record<string, { min: number; max: number }> = {
|
||||
maxConcurrent: { min: 1, max: 10 },
|
||||
maxWorktrees: { min: 1, max: 20 },
|
||||
maxParallelSteps: { min: 1, max: 4 },
|
||||
};
|
||||
|
||||
async function getGlobalSettingsStore(): Promise<GlobalSettingsStore> {
|
||||
@@ -185,6 +191,10 @@ export async function runSettingsShow(projectName?: string): Promise<void> {
|
||||
title: "Engine",
|
||||
keys: ["maxConcurrent", "maxWorktrees", "autoResolveConflicts", "smartConflictResolution"],
|
||||
},
|
||||
{
|
||||
title: "Execution",
|
||||
keys: ["runStepsInNewSessions", "maxParallelSteps"],
|
||||
},
|
||||
{
|
||||
title: "Worktrees",
|
||||
keys: ["worktreeNaming", "recycleWorktrees"],
|
||||
|
||||
@@ -8435,4 +8435,65 @@ describe("StepSessionExecutor integration", () => {
|
||||
// which should mark the task as failed with "Workflow step failed"
|
||||
expect(onError).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("onStepStart callback updates step status to in-progress", async () => {
|
||||
const store = createStepSessionStore();
|
||||
store.updateStep.mockResolvedValue({} as any);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
await executor.execute(createTaskWithSteps());
|
||||
|
||||
// Capture the StepSessionExecutor constructor options
|
||||
expect(mockedStepSessionExecutor).toHaveBeenCalled();
|
||||
const ctorOptions = mockedStepSessionExecutor.mock.calls[mockedStepSessionExecutor.mock.calls.length - 1][0];
|
||||
|
||||
// Invoke the onStepStart callback
|
||||
ctorOptions.onStepStart!(0);
|
||||
|
||||
// Should update step status in store
|
||||
expect(store.updateStep).toHaveBeenCalledWith("FN-200", 0, "in-progress");
|
||||
});
|
||||
|
||||
it("onStepComplete callback updates step status to done on success", async () => {
|
||||
const store = createStepSessionStore();
|
||||
store.updateStep.mockResolvedValue({} as any);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
await executor.execute(createTaskWithSteps());
|
||||
|
||||
const ctorOptions = mockedStepSessionExecutor.mock.calls[mockedStepSessionExecutor.mock.calls.length - 1][0];
|
||||
|
||||
ctorOptions.onStepComplete!(0, { stepIndex: 0, success: true, retries: 0 });
|
||||
|
||||
expect(store.updateStep).toHaveBeenCalledWith("FN-200", 0, "done");
|
||||
});
|
||||
|
||||
it("onStepComplete callback updates step status to skipped on failure", async () => {
|
||||
const store = createStepSessionStore();
|
||||
store.updateStep.mockResolvedValue({} as any);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
await executor.execute(createTaskWithSteps());
|
||||
|
||||
const ctorOptions = mockedStepSessionExecutor.mock.calls[mockedStepSessionExecutor.mock.calls.length - 1][0];
|
||||
|
||||
ctorOptions.onStepComplete!(1, { stepIndex: 1, success: false, retries: 3 });
|
||||
|
||||
expect(store.updateStep).toHaveBeenCalledWith("FN-200", 1, "skipped");
|
||||
});
|
||||
|
||||
it("step status update errors do not block execution", async () => {
|
||||
const store = createStepSessionStore();
|
||||
store.updateStep.mockRejectedValue(new Error("DB error"));
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
// Should not throw even when updateStep rejects
|
||||
await executor.execute(createTaskWithSteps());
|
||||
|
||||
const ctorOptions = mockedStepSessionExecutor.mock.calls[mockedStepSessionExecutor.mock.calls.length - 1][0];
|
||||
|
||||
// Invoking callbacks should not throw
|
||||
expect(() => ctorOptions.onStepStart!(0)).not.toThrow();
|
||||
expect(() => ctorOptions.onStepComplete!(0, { stepIndex: 0, success: true, retries: 0 })).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -755,9 +755,23 @@ export class TaskExecutor {
|
||||
stuckTaskDetector: this.options.stuckTaskDetector,
|
||||
onStepStart: (stepIndex) => {
|
||||
this.options.stuckTaskDetector?.recordProgress(task.id);
|
||||
try {
|
||||
this.store.updateStep(task.id, stepIndex, "in-progress").catch((err) => {
|
||||
executorLog.warn(`${task.id}: failed to update step ${stepIndex} status to in-progress: ${err}`);
|
||||
});
|
||||
} catch (err) {
|
||||
executorLog.warn(`${task.id}: failed to update step ${stepIndex} status to in-progress: ${err}`);
|
||||
}
|
||||
},
|
||||
onStepComplete: (stepIndex, result) => {
|
||||
executorLog.log(`${task.id}: step ${stepIndex} ${result.success ? "succeeded" : "failed"} (${result.retries} retries)`);
|
||||
try {
|
||||
this.store.updateStep(task.id, stepIndex, result.success ? "done" : "skipped").catch((err) => {
|
||||
executorLog.warn(`${task.id}: failed to update step ${stepIndex} status: ${err}`);
|
||||
});
|
||||
} catch (err) {
|
||||
executorLog.warn(`${task.id}: failed to update step ${stepIndex} status: ${err}`);
|
||||
}
|
||||
},
|
||||
});
|
||||
this.activeStepExecutors.set(task.id, stepExecutor);
|
||||
|
||||
Reference in New Issue
Block a user