From 81de9f60c5ecbd54fa6f6528f6ab11d8f45c64c0 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 25 Mar 2026 00:34:45 -0400 Subject: [PATCH] fix: supervision timeouts now respect task est: annotations (#2243) (#2434) Added parseEstimateMinutes() to parse estimate strings like "30m", "2h", "1h30m" into minutes. startUnitSupervision now looks up the task estimate from the DB and scales soft/hard timeouts accordingly. A 30m task gets 3x the default timeout, a 2h task gets 12x. Idle timeout is not scaled because idle is idle regardless of task size. Also added taskEstimate field to SupervisionContext interface for explicit estimate passing from callers. Fixes #2243 Co-authored-by: Claude Opus 4.6 (1M context) --- src/resources/extensions/gsd/auto-timers.ts | 67 +++++++++- .../gsd/tests/est-annotation-timeout.test.ts | 120 ++++++++++++++++++ 2 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 src/resources/extensions/gsd/tests/est-annotation-timeout.test.ts diff --git a/src/resources/extensions/gsd/auto-timers.ts b/src/resources/extensions/gsd/auto-timers.ts index f69eb4d01..ae3ded014 100644 --- a/src/resources/extensions/gsd/auto-timers.ts +++ b/src/resources/extensions/gsd/auto-timers.ts @@ -8,6 +8,7 @@ import type { ExtensionAPI, ExtensionContext } from "@gsd/pi-coding-agent"; import { readUnitRuntimeRecord, writeUnitRuntimeRecord } from "./unit-runtime.js"; +import { isDbAvailable, getMilestoneSlices, getSliceTasks } from "./gsd-db.js"; import { resolveAutoSupervisorConfig } from "./preferences.js"; import type { GSDPreferences } from "./preferences.js"; import { computeBudgets, resolveExecutorContextWindow } from "./context-budget.js"; @@ -32,6 +33,8 @@ export interface SupervisionContext { buildSnapshotOpts: () => CloseoutOptions & Record; buildRecoveryContext: () => RecoveryContext; pauseAuto: (ctx?: ExtensionContext, pi?: ExtensionAPI) => Promise; + /** Optional task estimate string (e.g. "30m", "2h") for timeout scaling (#2243). */ + taskEstimate?: string; } /** @@ -41,13 +44,71 @@ export interface SupervisionContext { * 3. Hard timeout (pause + recovery) * 4. Context-pressure monitor (continue-here) */ + +/** + * Parse a task estimate string (e.g. "30m", "2h", "1h30m") into minutes. + * Returns null if the string cannot be parsed. + */ +export function parseEstimateMinutes(estimate: string): number | null { + if (!estimate || typeof estimate !== "string") return null; + const trimmed = estimate.trim(); + if (!trimmed) return null; + + let totalMinutes = 0; + let matched = false; + + // Match hours component + const hoursMatch = trimmed.match(/(\d+)\s*h/i); + if (hoursMatch) { + totalMinutes += Number(hoursMatch[1]) * 60; + matched = true; + } + + // Match minutes component + const minutesMatch = trimmed.match(/(\d+)\s*m/i); + if (minutesMatch) { + totalMinutes += Number(minutesMatch[1]); + matched = true; + } + + return matched ? totalMinutes : null; +} + export function startUnitSupervision(sctx: SupervisionContext): void { const { s, ctx, pi, unitType, unitId, prefs, buildSnapshotOpts, buildRecoveryContext, pauseAuto } = sctx; const supervisor = resolveAutoSupervisorConfig(); - const softTimeoutMs = (supervisor.soft_timeout_minutes ?? 0) * 60 * 1000; - const idleTimeoutMs = (supervisor.idle_timeout_minutes ?? 0) * 60 * 1000; - const hardTimeoutMs = (supervisor.hard_timeout_minutes ?? 0) * 60 * 1000; + + // Scale timeouts based on task estimate annotations (#2243). + // If the task has an est: annotation, use it to extend the hard and soft timeouts + // so longer tasks don't get prematurely timed out. + let taskEstimate = sctx.taskEstimate; + if (!taskEstimate && unitType === "task" && isDbAvailable()) { + // Look up the task estimate from the DB (#2243). + try { + if (s.currentMilestoneId) { + const slices = getMilestoneSlices(s.currentMilestoneId); + for (const slice of slices) { + const tasks = getSliceTasks(s.currentMilestoneId, slice.id); + const task = tasks.find(t => t.id === unitId); + if (task?.estimate) { + taskEstimate = task.estimate; + break; + } + } + } + } catch { + // Non-fatal — fall through with no estimate + } + } + const estimateMinutes = taskEstimate ? parseEstimateMinutes(taskEstimate) : null; + const timeoutScale = estimateMinutes && estimateMinutes > 0 + ? Math.max(1, estimateMinutes / 10) // 10min task = 1x, 30min = 3x, 2h = 12x + : 1; + + const softTimeoutMs = (supervisor.soft_timeout_minutes ?? 0) * 60 * 1000 * timeoutScale; + const idleTimeoutMs = (supervisor.idle_timeout_minutes ?? 0) * 60 * 1000; // idle not scaled — idle is idle + const hardTimeoutMs = (supervisor.hard_timeout_minutes ?? 0) * 60 * 1000 * timeoutScale; // ── 1. Soft timeout warning ── s.wrapupWarningHandle = setTimeout(() => { diff --git a/src/resources/extensions/gsd/tests/est-annotation-timeout.test.ts b/src/resources/extensions/gsd/tests/est-annotation-timeout.test.ts new file mode 100644 index 000000000..973243cc6 --- /dev/null +++ b/src/resources/extensions/gsd/tests/est-annotation-timeout.test.ts @@ -0,0 +1,120 @@ +/** + * est-annotation-timeout.test.ts — Regression tests for #2243. + * + * Tasks with `est: 30m` or `est: 2h` annotations should get extended + * supervision timeouts. The parseEstimateMinutes helper should parse + * estimate strings, and startUnitSupervision should use them. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const timersSrcPath = join(import.meta.dirname, "..", "auto-timers.ts"); +const timersSrc = readFileSync(timersSrcPath, "utf-8"); + +// ─── Source analysis: parseEstimateMinutes exists and is exported ──────────── + +test("#2243: auto-timers.ts should export parseEstimateMinutes", () => { + assert.ok( + timersSrc.includes("export function parseEstimateMinutes"), + "parseEstimateMinutes should be exported from auto-timers.ts", + ); +}); + +// ─── Inline unit test of parseEstimateMinutes logic ───────────────────────── +// Since importing the module pulls in heavy deps, test the parsing logic inline. + +function parseEstimateMinutes(estimate: string): number | null { + if (!estimate || typeof estimate !== "string") return null; + const trimmed = estimate.trim(); + if (!trimmed) return null; + + let totalMinutes = 0; + let matched = false; + + const hoursMatch = trimmed.match(/(\d+)\s*h/i); + if (hoursMatch) { + totalMinutes += Number(hoursMatch[1]) * 60; + matched = true; + } + + const minutesMatch = trimmed.match(/(\d+)\s*m/i); + if (minutesMatch) { + totalMinutes += Number(minutesMatch[1]); + matched = true; + } + + return matched ? totalMinutes : null; +} + +test("#2243: parseEstimateMinutes parses '30m' correctly", () => { + assert.equal(parseEstimateMinutes("30m"), 30); +}); + +test("#2243: parseEstimateMinutes parses '2h' correctly", () => { + assert.equal(parseEstimateMinutes("2h"), 120); +}); + +test("#2243: parseEstimateMinutes parses '1h30m' correctly", () => { + assert.equal(parseEstimateMinutes("1h30m"), 90); +}); + +test("#2243: parseEstimateMinutes parses '15m' correctly", () => { + assert.equal(parseEstimateMinutes("15m"), 15); +}); + +test("#2243: parseEstimateMinutes returns null for empty string", () => { + assert.equal(parseEstimateMinutes(""), null); +}); + +test("#2243: parseEstimateMinutes returns null for invalid string", () => { + assert.equal(parseEstimateMinutes("not a time"), null); +}); + +// ─── Source analysis: startUnitSupervision uses task estimates ─────────────── + +test("#2243: startUnitSupervision should reference task estimates for timeout scaling", () => { + const usesEstimate = + timersSrc.includes("parseEstimateMinutes") && + timersSrc.includes("estimateMinutes") && + timersSrc.includes("taskEstimate"); + + assert.ok( + usesEstimate, + "startUnitSupervision should use task estimate annotations for timeout scaling", + ); +}); + +test("#2243: SupervisionContext should accept an optional taskEstimate field", () => { + const ctxIdx = timersSrc.indexOf("SupervisionContext"); + assert.ok(ctxIdx !== -1, "SupervisionContext interface exists"); + + const ctxEnd = timersSrc.indexOf("}", ctxIdx); + const ctxBlock = timersSrc.slice(ctxIdx, ctxEnd); + + assert.ok( + ctxBlock.includes("taskEstimate"), + "SupervisionContext should include a taskEstimate field", + ); +}); + +test("#2243: timeouts should be scaled by estimate (timeoutScale in source)", () => { + assert.ok( + timersSrc.includes("timeoutScale"), + "auto-timers.ts should use a timeoutScale factor derived from est: annotations", + ); +}); + +test("#2243: idle timeout should NOT be scaled (idle is idle regardless of estimate)", () => { + // Find the idleTimeoutMs line + const idleIdx = timersSrc.indexOf("const idleTimeoutMs"); + assert.ok(idleIdx !== -1, "idleTimeoutMs variable exists"); + + const idleLine = timersSrc.slice(idleIdx, timersSrc.indexOf("\n", idleIdx)); + assert.ok( + !idleLine.includes("timeoutScale"), + "idleTimeoutMs should NOT be scaled — idle is idle", + ); +});