docs(FN-3538): clarify pause-safe stuck timeout behavior
- Update enginePaused setting docs to specify stuck-task timers are suspended while pauses are active - Document that paused wall-clock time does not count toward taskStuckTimeoutMs, including shared globalPause windows - Clarify that unpausing restores scheduling and grants active sessions a fresh stuck-task grace window before detection resumes Fusion-Task-Id: FN-3538
This commit is contained in:
@@ -1561,6 +1561,142 @@ describe("ProjectEngine paused in-review auto-merge behavior", () => {
|
||||
|
||||
await engine.stop();
|
||||
});
|
||||
|
||||
it("calls stuck detector pause/resume hooks for enginePaused transitions", async () => {
|
||||
const mockStore = createMockStore(baseSettings);
|
||||
mocks.currentStore = mockStore.store;
|
||||
const engine = createEngine();
|
||||
await engine.start();
|
||||
|
||||
const pause = vi.fn();
|
||||
const resume = vi.fn();
|
||||
const runtime = engine.getRuntime() as unknown as object;
|
||||
Object.defineProperty(runtime, "stuckTaskDetector", {
|
||||
get: () => ({ pause, resume }),
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, enginePaused: true },
|
||||
{ ...baseSettings, enginePaused: false },
|
||||
);
|
||||
expect(pause).toHaveBeenCalledTimes(1);
|
||||
expect(resume).not.toHaveBeenCalled();
|
||||
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, enginePaused: false },
|
||||
{ ...baseSettings, enginePaused: true },
|
||||
);
|
||||
expect(resume).toHaveBeenCalledTimes(1);
|
||||
|
||||
await engine.stop();
|
||||
});
|
||||
|
||||
it("calls stuck detector pause/resume hooks for globalPause transitions", async () => {
|
||||
const mockStore = createMockStore(baseSettings);
|
||||
mocks.currentStore = mockStore.store;
|
||||
const engine = createEngine();
|
||||
await engine.start();
|
||||
|
||||
const pause = vi.fn();
|
||||
const resume = vi.fn();
|
||||
const runtime = engine.getRuntime() as unknown as object;
|
||||
Object.defineProperty(runtime, "stuckTaskDetector", {
|
||||
get: () => ({ pause, resume }),
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, globalPause: true },
|
||||
{ ...baseSettings, globalPause: false },
|
||||
);
|
||||
expect(pause).toHaveBeenCalledTimes(1);
|
||||
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, globalPause: false },
|
||||
{ ...baseSettings, globalPause: true },
|
||||
);
|
||||
expect(resume).toHaveBeenCalledTimes(1);
|
||||
|
||||
await engine.stop();
|
||||
});
|
||||
|
||||
it("does not resume stuck detector until both global and engine pause are cleared", async () => {
|
||||
const mockStore = createMockStore(baseSettings);
|
||||
mocks.currentStore = mockStore.store;
|
||||
const engine = createEngine();
|
||||
await engine.start();
|
||||
|
||||
const pause = vi.fn();
|
||||
const resume = vi.fn();
|
||||
const runtime = engine.getRuntime() as unknown as object;
|
||||
Object.defineProperty(runtime, "stuckTaskDetector", {
|
||||
get: () => ({ pause, resume }),
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, globalPause: true, enginePaused: true },
|
||||
{ ...baseSettings, globalPause: false, enginePaused: false },
|
||||
);
|
||||
expect(pause).toHaveBeenCalledTimes(1);
|
||||
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, globalPause: false, enginePaused: true },
|
||||
{ ...baseSettings, globalPause: true, enginePaused: true },
|
||||
);
|
||||
expect(resume).not.toHaveBeenCalled();
|
||||
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, globalPause: false, enginePaused: false },
|
||||
{ ...baseSettings, globalPause: false, enginePaused: true },
|
||||
);
|
||||
expect(resume).toHaveBeenCalledTimes(1);
|
||||
|
||||
await engine.stop();
|
||||
});
|
||||
|
||||
it("reserves stuck-detector checkNow for timeout-setting changes", async () => {
|
||||
const mockStore = createMockStore(baseSettings);
|
||||
mocks.currentStore = mockStore.store;
|
||||
const engine = createEngine();
|
||||
await engine.start();
|
||||
|
||||
const checkNow = vi.fn(async () => undefined);
|
||||
const runtime = engine.getRuntime() as unknown as object;
|
||||
Object.defineProperty(runtime, "stuckTaskDetector", {
|
||||
get: () => ({ pause: vi.fn(), resume: vi.fn(), checkNow }),
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, enginePaused: true },
|
||||
{ ...baseSettings, enginePaused: false },
|
||||
);
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, enginePaused: false },
|
||||
{ ...baseSettings, enginePaused: true },
|
||||
);
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, globalPause: true },
|
||||
{ ...baseSettings, globalPause: false },
|
||||
);
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, globalPause: false },
|
||||
{ ...baseSettings, globalPause: true },
|
||||
);
|
||||
|
||||
expect(checkNow).not.toHaveBeenCalled();
|
||||
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, taskStuckTimeoutMs: 600_000 },
|
||||
{ ...baseSettings, taskStuckTimeoutMs: 300_000 },
|
||||
);
|
||||
|
||||
expect(checkNow).toHaveBeenCalledTimes(1);
|
||||
|
||||
await engine.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("ProjectEngine swallowed error hardening", () => {
|
||||
@@ -1789,4 +1925,45 @@ describe("ProjectEngine swallowed error hardening", () => {
|
||||
|
||||
await engine.stop();
|
||||
});
|
||||
|
||||
it("warns when stuck-detector pause/resume hooks throw", async () => {
|
||||
const mockStore = createMockStore(baseSettings);
|
||||
mocks.currentStore = mockStore.store;
|
||||
const engine = createEngine();
|
||||
await engine.start();
|
||||
warnSpy.mockClear();
|
||||
|
||||
const runtime = engine.getRuntime() as unknown as object;
|
||||
Object.defineProperty(runtime, "stuckTaskDetector", {
|
||||
get() {
|
||||
return {
|
||||
pause: () => {
|
||||
throw new Error("pause hook failed");
|
||||
},
|
||||
resume: () => {
|
||||
throw new Error("resume hook failed");
|
||||
},
|
||||
};
|
||||
},
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, enginePaused: true },
|
||||
{ ...baseSettings, enginePaused: false },
|
||||
);
|
||||
await mockStore.emitSettingsUpdated(
|
||||
{ ...baseSettings, enginePaused: false },
|
||||
{ ...baseSettings, enginePaused: true },
|
||||
);
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Engine pause: stuck detector pause hook failed"),
|
||||
);
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining("Engine unpause: stuck detector resume hook failed"),
|
||||
);
|
||||
|
||||
await engine.stop();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -604,6 +604,75 @@ describe("StuckTaskDetector", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("pause lifecycle", () => {
|
||||
it("skips stuck evaluation while paused", async () => {
|
||||
const getSettings = vi.fn().mockResolvedValue({ taskStuckTimeoutMs: 60000 });
|
||||
store = createMockStore({ getSettings });
|
||||
const onStuck = vi.fn();
|
||||
const customDetector = new StuckTaskDetector(store, { onStuck });
|
||||
const session = createMockSession();
|
||||
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
customDetector.trackTask("FN-001", session);
|
||||
customDetector.pause();
|
||||
vi.advanceTimersByTime(61_000);
|
||||
|
||||
await customDetector.checkNow();
|
||||
|
||||
expect(getSettings).not.toHaveBeenCalled();
|
||||
expect(onStuck).not.toHaveBeenCalled();
|
||||
expect(session.dispose).not.toHaveBeenCalled();
|
||||
expect(customDetector.trackedCount).toBe(1);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("resets tracked timing on resume so paused interval is not immediately stuck", async () => {
|
||||
store = createMockStore({
|
||||
getSettings: vi.fn().mockResolvedValue({ taskStuckTimeoutMs: 60000 }),
|
||||
});
|
||||
const onStuck = vi.fn();
|
||||
const customDetector = new StuckTaskDetector(store, { onStuck });
|
||||
const session = createMockSession();
|
||||
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
customDetector.trackTask("FN-001", session);
|
||||
customDetector.pause();
|
||||
vi.advanceTimersByTime(120_000);
|
||||
customDetector.resume();
|
||||
|
||||
await customDetector.checkNow();
|
||||
expect(onStuck).not.toHaveBeenCalled();
|
||||
|
||||
vi.advanceTimersByTime(61_000);
|
||||
await customDetector.checkNow();
|
||||
expect(onStuck).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ taskId: "FN-001", reason: "inactivity" }),
|
||||
);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("does not refresh tracked timing when resume is called while already unpaused", async () => {
|
||||
store = createMockStore({
|
||||
getSettings: vi.fn().mockResolvedValue({ taskStuckTimeoutMs: 60000 }),
|
||||
});
|
||||
const onStuck = vi.fn();
|
||||
const customDetector = new StuckTaskDetector(store, { onStuck });
|
||||
const session = createMockSession();
|
||||
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
customDetector.trackTask("FN-001", session);
|
||||
vi.advanceTimersByTime(61_000);
|
||||
|
||||
customDetector.resume();
|
||||
await customDetector.checkNow();
|
||||
|
||||
expect(onStuck).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ taskId: "FN-001", reason: "inactivity" }),
|
||||
);
|
||||
vi.useRealTimers();
|
||||
});
|
||||
});
|
||||
|
||||
describe("checkNow", () => {
|
||||
it("checks stuck tasks immediately and disposes session", async () => {
|
||||
store = createMockStore({
|
||||
|
||||
@@ -1901,7 +1901,47 @@ export class ProjectEngine {
|
||||
}
|
||||
|
||||
private wireSettingsListeners(store: TaskStore): void {
|
||||
// 1. Global pause — terminate active merge session AND abort any running
|
||||
const applyDetectorPauseLifecycle = (paused: boolean, source: string): void => {
|
||||
try {
|
||||
const detector = (this.runtime as any).stuckTaskDetector;
|
||||
if (paused) {
|
||||
detector?.pause?.();
|
||||
} else {
|
||||
detector?.resume?.();
|
||||
}
|
||||
} catch (err: unknown) {
|
||||
runtimeLog.warn(
|
||||
`${source}: stuck detector ${paused ? "pause" : "resume"} hook failed: ${err instanceof Error ? err.message : String(err)}`,
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
// 1. Unified pause lifecycle — detector only resumes once BOTH pause sources
|
||||
// are clear, and pauses when either source engages.
|
||||
const onPauseLifecycleTransition = ({
|
||||
settings: s,
|
||||
previous: prev,
|
||||
}: {
|
||||
settings: Settings;
|
||||
previous: Settings;
|
||||
}) => {
|
||||
const wasPaused = prev.globalPause || prev.enginePaused;
|
||||
const isPaused = s.globalPause || s.enginePaused;
|
||||
|
||||
if (!wasPaused && isPaused) {
|
||||
const source = s.globalPause && !prev.globalPause ? "Global pause" : "Engine pause";
|
||||
applyDetectorPauseLifecycle(true, source);
|
||||
}
|
||||
|
||||
if (wasPaused && !isPaused) {
|
||||
const source = prev.globalPause && !s.globalPause ? "Global unpause" : "Engine unpause";
|
||||
applyDetectorPauseLifecycle(false, source);
|
||||
}
|
||||
};
|
||||
store.on("settings:updated", onPauseLifecycleTransition);
|
||||
this.settingsHandlers.push(onPauseLifecycleTransition);
|
||||
|
||||
// 2. Global pause — terminate active merge session AND abort any running
|
||||
// deterministic verification (pnpm test/build). The abort controller gates
|
||||
// both the AI merge agent and the spawned child processes; without it,
|
||||
// verification commands keep churning until they finish naturally.
|
||||
@@ -1922,7 +1962,7 @@ export class ProjectEngine {
|
||||
store.on("settings:updated", onGlobalPause);
|
||||
this.settingsHandlers.push(onGlobalPause);
|
||||
|
||||
// 2. Global unpause — resume orphaned tasks + sweep in-review
|
||||
// 3. Global unpause — resume orphaned tasks + sweep in-review
|
||||
const onGlobalUnpause = async ({
|
||||
settings: s,
|
||||
previous: prev,
|
||||
@@ -1938,7 +1978,7 @@ export class ProjectEngine {
|
||||
store.on("settings:updated", onGlobalUnpause);
|
||||
this.settingsHandlers.push(onGlobalUnpause);
|
||||
|
||||
// 3. Engine unpause — same as global unpause
|
||||
// 4. Engine unpause — same as global unpause
|
||||
const onEngineUnpause = async ({
|
||||
settings: s,
|
||||
previous: prev,
|
||||
@@ -1954,7 +1994,7 @@ export class ProjectEngine {
|
||||
store.on("settings:updated", onEngineUnpause);
|
||||
this.settingsHandlers.push(onEngineUnpause);
|
||||
|
||||
// 4. Stuck task timeout change — trigger immediate check
|
||||
// 5. Stuck task timeout change — trigger immediate check
|
||||
const onStuckTimeoutChange = async ({
|
||||
settings: s,
|
||||
previous: prev,
|
||||
|
||||
@@ -94,6 +94,7 @@ export class StuckTaskDetector {
|
||||
private onStuck?: (event: StuckTaskEvent) => void;
|
||||
private beforeRequeue?: (taskId: string) => Promise<boolean>;
|
||||
private onLoopDetected?: (event: StuckTaskEvent) => Promise<boolean>;
|
||||
private paused = false;
|
||||
|
||||
constructor(
|
||||
private store: TaskStore,
|
||||
@@ -406,6 +407,31 @@ export class StuckTaskDetector {
|
||||
this.tracked.delete(taskId);
|
||||
}
|
||||
|
||||
/**
|
||||
* Pause stuck detection checks while the engine is in a paused lifecycle.
|
||||
* Active tracked sessions are preserved and refreshed on resume.
|
||||
*/
|
||||
pause(): void {
|
||||
if (this.paused) return;
|
||||
this.paused = true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resume stuck detection checks and refresh tracked timestamps so the paused
|
||||
* interval does not count as inactivity/no-progress time.
|
||||
*/
|
||||
resume(): void {
|
||||
if (!this.paused) return;
|
||||
this.paused = false;
|
||||
if (this.tracked.size === 0) return;
|
||||
const now = Date.now();
|
||||
for (const entry of this.tracked.values()) {
|
||||
entry.lastActivity = now;
|
||||
entry.lastProgressAt = now;
|
||||
entry.activitySinceProgress = 0;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check for stuck tasks immediately, outside the normal polling cycle.
|
||||
* Safe to call at any time — will no-op if no tasks are tracked or timeout is disabled.
|
||||
@@ -429,6 +455,10 @@ export class StuckTaskDetector {
|
||||
private async checkStuckTasks(): Promise<void> {
|
||||
if (this.tracked.size === 0) return;
|
||||
|
||||
// Fast pause gate: if lifecycle hooks paused the detector, skip the cycle
|
||||
// without reading settings (avoids noisy settings-read errors while paused).
|
||||
if (this.paused) return;
|
||||
|
||||
let settings: Settings;
|
||||
try {
|
||||
settings = await this.store.getSettings();
|
||||
@@ -437,10 +467,7 @@ export class StuckTaskDetector {
|
||||
return; // Can't read settings — skip this cycle
|
||||
}
|
||||
|
||||
// Pause gate: when globalPause or enginePaused is on, sessions are
|
||||
// intentionally idle (engine listeners dispose them on transition) and
|
||||
// long pauses would otherwise look like inactivity → trigger spurious
|
||||
// stuck-kill / requeue cycles. Skip detection while paused.
|
||||
// Defensive fallback for pause windows where lifecycle hooks haven't run yet.
|
||||
if (settings.globalPause || settings.enginePaused) return;
|
||||
|
||||
const timeoutMs = settings.taskStuckTimeoutMs;
|
||||
|
||||
Reference in New Issue
Block a user