From 2ab817cdf0f13ae8cc1b4e20d668bc562d769e50 Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 24 Oct 2026 17:42:59 -0500 Subject: [PATCH 1/4] fix(server): storage cleanup keeps worktrees held for a usage-limit reset A thread whose run failed on a usage limit and is snoozed or set to auto-resume until the reset now counts as busy for storage cleanup, so a fresh worktree is not removed as unchanged before its first turn runs. Fixes #24389 Co-Authored-By: Claude Opus 5.5 --- apps/server/src/storageCleanup.test.ts | 26 ++++++++++++++++++++++++++ apps/server/src/storageCleanup.ts | 16 +++++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index b8332de492884..8a29aae8fc3c9 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -164,6 +164,32 @@ describe("V2 storage cleanup eligibility", () => { ).toBe(false); }); + it.each([ + { autoResume: false, snooze: true, resetIn: DAY_MS, idle: false }, + { autoResume: true, snooze: false, resetIn: DAY_MS, idle: false }, + { autoResume: false, snooze: false, resetIn: DAY_MS, idle: true }, + { autoResume: true, snooze: true, resetIn: -1_000, idle: true }, + ])( + "holds a usage-limited worktree only until its reset (autoResume $autoResume, snooze $snooze, reset in $resetIn ms)", + ({ resetIn, idle, ...recovery }) => { + const runId = RunId.make("run-1"); + expect( + storageCleanupThreadIdle( + { + ...candidateWithStatus("failed"), + latestRunId: runId, + limitRecovery: { + runId, + resetAt: DateTime.formatIso(at(resetIn)), + ...recovery, + }, + }, + NOW_MS, + ), + ).toBe(idle); + }, + ); + it("uses V2 run activity instead of metadata refreshes for retention", () => { const thread = candidate(); const runTime = at(-3 * DAY_MS); diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index 471e78df2a3b1..eb4bcd10ee80f 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -128,7 +128,21 @@ export function storageCleanupThreadIdle(thread: OrchestrationV2ThreadShell, now thread.status === "rolled_back") && (thread.pendingBackgroundTasks?.length ?? 0) === 0 && thread.pendingRuntimeRequest === null && - !threadHasQueuedTurnStart(thread, now) + !threadHasQueuedTurnStart(thread, now) && + !awaitingLimitReset(thread, now) + ); +} + +// A run held for its usage-limit reset has its work still ahead of it, and a +// fresh worktree would otherwise look unchanged and be removed before it runs. +function awaitingLimitReset(thread: OrchestrationV2ThreadShell, now: number): boolean { + const recovery = thread.limitRecovery; + return ( + thread.status === "failed" && + recovery != null && + recovery.runId === thread.latestRunId && + (recovery.autoResume || recovery.snooze === true) && + Date.parse(recovery.resetAt) > now ); } From f2e9fa9f6c7f415e9aafd48007ec71db9e71fd5e Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 24 Oct 2026 20:19:14 -0500 Subject: [PATCH 2/4] fix(server): cleanup holds a usage-limited worktree until recovery acts Read the failure itself before the recovery worker arms it, and keep an armed auto-resume held until its continuation starts a new run. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/storageCleanup.test.ts | 61 +++++++++++++++++++------- apps/server/src/storageCleanup.ts | 44 ++++++++++++++----- 2 files changed, 77 insertions(+), 28 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index 8a29aae8fc3c9..f5986cb72e18d 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -107,14 +107,16 @@ describe("V2 storage cleanup eligibility", () => { const candidate = () => shell({ branch: "feature", worktreePath: "/worktrees/feature" }); it("allows an idle worktree and rejects the project checkout", () => { - expect(storageCleanupThreadIdle(candidate(), NOW_MS)).toBe(true); - expect(storageCleanupThreadIdle(shell(), NOW_MS)).toBe(false); + expect(storageCleanupThreadIdle(candidate(), NOW_MS, DEFAULT_SERVER_SETTINGS)).toBe(true); + expect(storageCleanupThreadIdle(shell(), NOW_MS, DEFAULT_SERVER_SETTINGS)).toBe(false); }); it.each(["running", "starting", "preparing", "waiting", "queued"] as const)( "retains a worktree while its thread is %s", (status) => { - expect(storageCleanupThreadIdle(candidateWithStatus(status), NOW_MS)).toBe(false); + expect( + storageCleanupThreadIdle(candidateWithStatus(status), NOW_MS, DEFAULT_SERVER_SETTINGS), + ).toBe(false); }, ); @@ -128,6 +130,7 @@ describe("V2 storage cleanup eligibility", () => { pendingBackgroundTasks: [{ taskId: "task-1", kind: "command" }], }, NOW_MS, + DEFAULT_SERVER_SETTINGS, ), ).toBe(false); }, @@ -147,6 +150,7 @@ describe("V2 storage cleanup eligibility", () => { }, }, NOW_MS, + DEFAULT_SERVER_SETTINGS, ), ).toBe(false); }, @@ -154,37 +158,62 @@ describe("V2 storage cleanup eligibility", () => { it("retains an active run even if the shell status is idle", () => { expect( - storageCleanupThreadIdle({ ...candidate(), activeRunId: RunId.make("run") }, NOW_MS), + storageCleanupThreadIdle( + { ...candidate(), activeRunId: RunId.make("run") }, + NOW_MS, + DEFAULT_SERVER_SETTINGS, + ), ).toBe(false); }); it("retains a queued prompt before the new run has been projected", () => { expect( - storageCleanupThreadIdle({ ...candidate(), latestUserMessageAt: at(-1_000) }, NOW_MS), + storageCleanupThreadIdle( + { ...candidate(), latestUserMessageAt: at(-1_000) }, + NOW_MS, + DEFAULT_SERVER_SETTINGS, + ), ).toBe(false); }); it.each([ - { autoResume: false, snooze: true, resetIn: DAY_MS, idle: false }, - { autoResume: true, snooze: false, resetIn: DAY_MS, idle: false }, - { autoResume: false, snooze: false, resetIn: DAY_MS, idle: true }, - { autoResume: true, snooze: true, resetIn: -1_000, idle: true }, + { name: "snoozed until a future reset", snooze: true, resetIn: DAY_MS, idle: false }, + { name: "set to auto-resume", autoResume: true, resetIn: DAY_MS, idle: false }, + { name: "with recovery turned off", resetIn: DAY_MS, idle: true }, + { name: "auto-resuming after the reset", autoResume: true, resetIn: -1_000, idle: false }, + { name: "snoozed past its reset", snooze: true, resetIn: -1_000, idle: true }, + { + name: "settled with an auto-resume pending", + autoResume: true, + settled: true, + resetIn: -1_000, + idle: true, + }, + { name: "not yet armed by the recovery worker", armed: false, resetIn: DAY_MS, idle: false }, + { + name: "not armed with recovery settings off", + armed: false, + settingsOff: true, + resetIn: DAY_MS, + idle: true, + }, ])( - "holds a usage-limited worktree only until its reset (autoResume $autoResume, snooze $snooze, reset in $resetIn ms)", - ({ resetIn, idle, ...recovery }) => { + "treats a usage-limited thread $name as idle: $idle", + ({ autoResume = false, snooze = false, armed = true, settled, settingsOff, resetIn, idle }) => { const runId = RunId.make("run-1"); + const resetAt = DateTime.formatIso(at(resetIn)); expect( storageCleanupThreadIdle( { ...candidateWithStatus("failed"), latestRunId: runId, - limitRecovery: { - runId, - resetAt: DateTime.formatIso(at(resetIn)), - ...recovery, - }, + lastErrorClass: "usage_limit", + usageLimitResetAt: resetAt, + settledOverride: settled ? "settled" : null, + limitRecovery: armed ? { runId, resetAt, autoResume, snooze } : null, }, NOW_MS, + { autoResumeLimitedThreads: false, snoozeLimitedThreads: !settingsOff }, ), ).toBe(idle); }, diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index eb4bcd10ee80f..e10fadc8b13cb 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -116,7 +116,16 @@ function sameProjectWorktreePolicies(left: ServerSettings, right: ServerSettings ); } -export function storageCleanupThreadIdle(thread: OrchestrationV2ThreadShell, now: number): boolean { +type LimitRecoverySettings = Pick< + ServerSettings, + "autoResumeLimitedThreads" | "snoozeLimitedThreads" +>; + +export function storageCleanupThreadIdle( + thread: OrchestrationV2ThreadShell, + now: number, + settings: LimitRecoverySettings, +): boolean { return ( thread.worktreePath !== null && thread.activeRunId === null && @@ -129,21 +138,32 @@ export function storageCleanupThreadIdle(thread: OrchestrationV2ThreadShell, now (thread.pendingBackgroundTasks?.length ?? 0) === 0 && thread.pendingRuntimeRequest === null && !threadHasQueuedTurnStart(thread, now) && - !awaitingLimitReset(thread, now) + !heldForLimitRecovery(thread, now, settings) ); } // A run held for its usage-limit reset has its work still ahead of it, and a // fresh worktree would otherwise look unchanged and be removed before it runs. -function awaitingLimitReset(thread: OrchestrationV2ThreadShell, now: number): boolean { +function heldForLimitRecovery( + thread: OrchestrationV2ThreadShell, + now: number, + settings: LimitRecoverySettings, +): boolean { + if ( + thread.status !== "failed" || + thread.lastErrorClass !== "usage_limit" || + !thread.usageLimitResetAt || + thread.archivedAt !== null || + thread.settledOverride === "settled" + ) + return false; + const resetMs = Date.parse(thread.usageLimitResetAt); const recovery = thread.limitRecovery; - return ( - thread.status === "failed" && - recovery != null && - recovery.runId === thread.latestRunId && - (recovery.autoResume || recovery.snooze === true) && - Date.parse(recovery.resetAt) > now - ); + if (recovery?.runId === thread.latestRunId && recovery.resetAt === thread.usageLimitResetAt) + // An auto-resume holds until its continuation replaces the failed run. + return recovery.autoResume || (recovery.snooze === true && resetMs > now); + // The recovery worker arms this failure on a later tick; hold it until then. + return (settings.autoResumeLimitedThreads || settings.snoozeLimitedThreads) && resetMs > now; } /** PR metadata refreshes must not reset the inactivity clock. */ @@ -817,7 +837,7 @@ const make = Effect.gen(function* () { if (project === undefined) return keep("project is unavailable"); if ( thread.branch === null || - !group.every((member) => storageCleanupThreadIdle(member, now)) + !group.every((member) => storageCleanupThreadIdle(member, now, serverSettings)) ) return keep("thread is running or has pending work"); if (group.some((member) => member.branch !== thread.branch)) @@ -988,7 +1008,7 @@ const make = Effect.gen(function* () { shell.branch !== thread.branch || !Equal.equals(shell.settledAt, previous.settledAt) || (shell.settledAt !== null && !Equal.equals(shell.updatedAt, previous.updatedAt)) || - !storageCleanupThreadIdle(shell, now) || + !storageCleanupThreadIdle(shell, now, serverSettings) || path.resolve(shell.worktreePath!) !== worktreePath || storageCleanupActivityAt(shell) !== storageCleanupActivityAt(previous) ); From ecd4e452424d6ea1202d1398fb7e343090b46730 Mon Sep 17 00:00:00 2001 From: Victor Vogel Date: Sat, 24 Oct 2026 21:16:32 -0500 Subject: [PATCH 3/4] fix(server): cleanup holds an unarmed failure the worker will auto-resume Use the recovery worker's own rule for a failure it has not armed yet: auto-resume on, or snooze on with the reset ahead, skipping a reset that had already passed when the run ended. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/storageCleanup.test.ts | 20 ++++++++++++++++++-- apps/server/src/storageCleanup.ts | 8 +++++++- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index f5986cb72e18d..fdd1bc17b8dda 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -190,6 +190,13 @@ describe("V2 storage cleanup eligibility", () => { idle: true, }, { name: "not yet armed by the recovery worker", armed: false, resetIn: DAY_MS, idle: false }, + { + name: "not yet armed, auto-resume after reset", + armed: false, + autoResumeSetting: true, + resetIn: -1_000, + idle: false, + }, { name: "not armed with recovery settings off", armed: false, @@ -199,7 +206,16 @@ describe("V2 storage cleanup eligibility", () => { }, ])( "treats a usage-limited thread $name as idle: $idle", - ({ autoResume = false, snooze = false, armed = true, settled, settingsOff, resetIn, idle }) => { + ({ + autoResume = false, + snooze = false, + armed = true, + autoResumeSetting = false, + settled, + settingsOff, + resetIn, + idle, + }) => { const runId = RunId.make("run-1"); const resetAt = DateTime.formatIso(at(resetIn)); expect( @@ -213,7 +229,7 @@ describe("V2 storage cleanup eligibility", () => { limitRecovery: armed ? { runId, resetAt, autoResume, snooze } : null, }, NOW_MS, - { autoResumeLimitedThreads: false, snoozeLimitedThreads: !settingsOff }, + { autoResumeLimitedThreads: autoResumeSetting, snoozeLimitedThreads: !settingsOff }, ), ).toBe(idle); }, diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index e10fadc8b13cb..db8e8cd1cbf65 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -163,7 +163,13 @@ function heldForLimitRecovery( // An auto-resume holds until its continuation replaces the failed run. return recovery.autoResume || (recovery.snooze === true && resetMs > now); // The recovery worker arms this failure on a later tick; hold it until then. - return (settings.autoResumeLimitedThreads || settings.snoozeLimitedThreads) && resetMs > now; + // Like the worker, skip a reset that had already passed when the run ended. + if ( + !Number.isFinite(resetMs) || + resetMs <= DateTime.toEpochMillis(thread.latestRunCompletedAt ?? thread.updatedAt) + ) + return false; + return settings.autoResumeLimitedThreads || (settings.snoozeLimitedThreads && resetMs > now); } /** PR metadata refreshes must not reset the inactivity clock. */ From 4c6ab1794060348da2118ae6e9d2c2ece347f537 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Fri, 30 Oct 2026 18:16:45 +0000 Subject: [PATCH 4/4] fix(server): recheck recovery settings before worktree removal --- apps/server/src/storageCleanup.test.ts | 27 ++++++++++++++++++++++++++ apps/server/src/storageCleanup.ts | 8 ++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/apps/server/src/storageCleanup.test.ts b/apps/server/src/storageCleanup.test.ts index 9fa150d367a3b..6d87a03e55a76 100644 --- a/apps/server/src/storageCleanup.test.ts +++ b/apps/server/src/storageCleanup.test.ts @@ -2282,6 +2282,33 @@ describe("storage cleanup reports and local file policies", () => { }); describe("settled worktree rechecks", () => { + it.live.each(["autoResumeLimitedThreads", "snoozeLimitedThreads"] as const)( + "keeps a usage-limited checkout when %s is enabled during measurement", + (preference) => + runCleanupTest( + Effect.gen(function* () { + const fixture = yield* pausedMeasurementFixture(); + fixture.setSettings({ autoResumeLimitedThreads: false, snoozeLimitedThreads: false }); + fixture.setThreads([ + shell({ + branch: "feature", + worktreePath: fixture.worktree, + status: "failed", + latestRunId: RunId.make("limited-run"), + lastErrorClass: "usage_limit", + usageLimitResetAt: DateTime.formatIso(at(DAY_MS)), + }), + ]); + const run = yield* fixture.service.runNow.pipe(Effect.forkChild); + yield* Deferred.await(fixture.measuring); + fixture.setSettings({ [preference]: true }); + yield* Deferred.succeed(fixture.resume, undefined); + const report = yield* Fiber.join(run); + expect(report.entries[0]?.outcome).toBe("kept"); + expect(yield* fixture.fs.exists(fixture.worktree)).toBe(true); + }), + ), + ); it.live.each([{ settledAt: null }, { settledAt: at(DAY_MS) }])( "keeps a checkout changed during measurement (%j)", (change) => diff --git a/apps/server/src/storageCleanup.ts b/apps/server/src/storageCleanup.ts index 09b9b75729322..deffe41cec571 100644 --- a/apps/server/src/storageCleanup.ts +++ b/apps/server/src/storageCleanup.ts @@ -1048,11 +1048,11 @@ const make = Effect.gen(function* () { return keep("commit changed since check"); const finalChanges = yield* localChangesReason(worktreePath, settings); if (finalChanges !== null) return keep(finalChanges); - const current = resolveWorktreeCleanup( - yield* settingsService.getSettings, - thread.projectId, - ); + const currentSettings = yield* settingsService.getSettings; + const current = resolveWorktreeCleanup(currentSettings, thread.projectId); if ( + currentSettings.autoResumeLimitedThreads !== serverSettings.autoResumeLimitedThreads || + currentSettings.snoozeLimitedThreads !== serverSettings.snoozeLimitedThreads || Object.keys(settings).some( (key) => current[key as keyof typeof settings] !== settings[key as keyof typeof settings],