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) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher 2026-03-25 00:34:45 -04:00 committed by GitHub
parent 2ddb790141
commit 81de9f60c5
2 changed files with 184 additions and 3 deletions

View file

@ -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<string, unknown>;
buildRecoveryContext: () => RecoveryContext;
pauseAuto: (ctx?: ExtensionContext, pi?: ExtensionAPI) => Promise<void>;
/** 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(() => {

View file

@ -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",
);
});