fix(autopilot): derive the full-cycle timeout floor from the handler anchors (#2781) (#3656)

Adversarial review: survived a hostile reviewer plus two independent refuters, each told to assume the PR was broken and to default to refuting when uncertain. 19 of 62 PRs cleared that bar.

Full-cycle maintenance jobs were stamped with the outer 600s timeout instead of the 30-minute handler anchor — a regression from #3338 that killed long cycles mid-run. Fixed with a named `fullCycleTimeoutMs` derived from the handler anchors, which now fail loudly rather than silently defaulting; reverting fails 3 of 8 tests.

Verified before merge: the PR's own tests fail when the production change is reverted; typecheck clean; MERGEABLE/CLEAN on the current base after batches 1 and 2 landed, not a stale one.

Known gap, recorded rather than hidden: the '38 dead cycles in 24h' figure from the description was not reproduced; the stamp arithmetic was verified by code inspection.

src/commands/autopilot-timeout.ts

@@ -1,9 +1,42 @@
+import { defaultTimeoutMsFor } from '../core/minions/handler-timeouts.ts';
+
+// #2781: the full-cycle floor used to be a literal `1_800_000` that merely
+// HAPPENED to match the 'autopilot-cycle' / 'autopilot-global-maintenance'
+// handler anchors (`HANDLER_DEFAULT_TIMEOUT_MS`, #1737) instead of being
+// derived from them. A duplicated literal can silently drift from the
+// handler default it's supposed to track — which is exactly the bug class
+// #2781 reported (an explicit `timeout_ms` stamp permanently overrides the
+// handler default per `queue.ts`'s `opts?.timeout_ms ?? defaultTimeoutMsFor`,
+// so a stale/lower literal here would starve a phase the handler default
+// was sized for). Deriving the floor from `defaultTimeoutMsFor` for both
+// full-cycle job names keeps the stamp coupled to its anchor by construction.
+// Fail fast (not `?? 0`) if either handler ever loses its entry in
+// HANDLER_DEFAULT_TIMEOUT_MS — silently falling back to "no floor" would
+// reintroduce #2781 rather than surface the drift.
+function requireHandlerAnchorMs(jobName: string): number {
+  const ms = defaultTimeoutMsFor(jobName);
+  if (ms === null) {
+    throw new Error(
+      `resolveAutopilotDispatchTimeoutMs: '${jobName}' has no entry in HANDLER_DEFAULT_TIMEOUT_MS ` +
+      '(handler-timeouts.ts) — the full-cycle timeout floor can no longer be derived from it. ' +
+      'See #2781: a missing/removed anchor here silently reintroduces the interval-derived stamp ' +
+      'permanently overriding the handler default.',
+    );
+  }
+  return ms;
+}
+
+const FULL_CYCLE_TIMEOUT_FLOOR_MS = Math.max(
+  requireHandlerAnchorMs('autopilot-cycle'),
+  requireHandlerAnchorMs('autopilot-global-maintenance'),
+);
+
 export function resolveAutopilotDispatchTimeoutMs(
   baseIntervalSeconds: number,
   fullCycle: boolean,
 ): number {
   const intervalDerivedTimeoutMs = Math.max(baseIntervalSeconds * 2 * 1000, 300_000);
   return fullCycle
-    ? Math.max(intervalDerivedTimeoutMs, 1_800_000)
+    ? Math.max(intervalDerivedTimeoutMs, FULL_CYCLE_TIMEOUT_FLOOR_MS)
     : intervalDerivedTimeoutMs;
 }

src/commands/autopilot.ts

@@ -981,12 +981,21 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
           // can't shrink throughput (codex #9/D5). autopilot-cycle jobs run on
           // the 'default' queue, so that's the concurrency we compare against.
           const fanoutMax = await resolveEffectiveFanoutMax(engine, 'default');
+          // #2781: both 'autopilot-cycle' (per-source) and 'autopilot-global-
+          // maintenance' carry a 30-min handler anchor (handler-timeouts.ts)
+          // because a full cycle can outlive short daemon intervals — unlike
+          // the lighter interval-derived `timeoutMs` above (sync/freshness,
+          // extract-atoms-drain, targeted small-plan steps), which have no
+          // such anchor and are meant to stay interval-derived. Naming this
+          // separately (rather than reusing the outer `timeoutMs`) avoids
+          // the #2781 bug class: dispatchGlobalMaintenance previously reused
+          // the outer non-full-cycle `timeoutMs` by shorthand, silently
+          // dropping its own handler anchor.
+          const fullCycleTimeoutMs = resolveAutopilotDispatchTimeoutMs(baseInterval, true);
           const result = await dispatchPerSource(engine, queue, {
             repoPath,
             slot,
-            // Full cycles can outlive short daemon intervals. Keep lighter dispatches
-            // interval-derived while giving per-source consolidation enough time.
-            timeoutMs: resolveAutopilotDispatchTimeoutMs(baseInterval, true),
+            timeoutMs: fullCycleTimeoutMs,
             fanoutMax,
             jsonMode,
           });
@@ -997,7 +1006,7 @@ export async function runAutopilot(engine: BrainEngine, args: string[]) {
           // the per-source path (legacy single-source still runs everything).
           if (!result.legacy_fallback) {
             try {
-              await dispatchGlobalMaintenance(engine, queue, { repoPath, slot, timeoutMs, jsonMode });
+              await dispatchGlobalMaintenance(engine, queue, { repoPath, slot, timeoutMs: fullCycleTimeoutMs, jsonMode });
             } catch (e) {
               if (jsonMode) process.stderr.write(JSON.stringify({ event: 'global_maintenance_dispatch_failed', error: e instanceof Error ? e.message : String(e) }) + '\n');
             }

test/autopilot-fanout-wiring.test.ts

@@ -16,12 +16,18 @@ import { describe, expect, test } from 'bun:test';
 import { readFileSync } from 'fs';
 import { join } from 'path';
 import { resolveAutopilotDispatchTimeoutMs } from '../src/commands/autopilot-timeout.ts';
+import { defaultTimeoutMsFor } from '../src/core/minions/handler-timeouts.ts';
 
 const AUTOPILOT_SRC = readFileSync(
   join(import.meta.dir, '..', 'src', 'commands', 'autopilot.ts'),
   'utf8',
 );
 
+const AUTOPILOT_TIMEOUT_SRC = readFileSync(
+  join(import.meta.dir, '..', 'src', 'commands', 'autopilot-timeout.ts'),
+  'utf8',
+);
+
 describe('autopilot.ts ↔ dispatchPerSource wiring', () => {
   test('imports dispatchPerSource from the fan-out helper', () => {
     expect(AUTOPILOT_SRC).toMatch(
@@ -59,9 +65,33 @@ describe('autopilot.ts ↔ dispatchPerSource wiring', () => {
     expect(AUTOPILOT_SRC).toContain(
       'const timeoutMs = resolveAutopilotDispatchTimeoutMs(baseInterval, false);',
     );
-    expect(AUTOPILOT_SRC).toMatch(
-      /dispatchPerSource\(engine, queue, \{[\s\S]{0,300}timeoutMs: resolveAutopilotDispatchTimeoutMs\(baseInterval, true\)/,
+    expect(AUTOPILOT_SRC).toContain(
+      'const fullCycleTimeoutMs = resolveAutopilotDispatchTimeoutMs(baseInterval, true);',
     );
+    expect(AUTOPILOT_SRC).toMatch(
+      /dispatchPerSource\(engine, queue, \{[\s\S]{0,300}timeoutMs: fullCycleTimeoutMs/,
+    );
+  });
+
+  test('#2781: dispatchGlobalMaintenance gets the full-cycle floor, not the outer (non-full-cycle) timeoutMs', () => {
+    // Live #2781 regression, found in review: dispatchGlobalMaintenance's
+    // call used the object-shorthand `timeoutMs`, which resolved to the
+    // OUTER `const timeoutMs = resolveAutopilotDispatchTimeoutMs(baseInterval, false)`
+    // declared earlier in the same function for the sync/freshness dispatch
+    // — not to the full-cycle value computed for dispatchPerSource a few
+    // lines above it. 'autopilot-global-maintenance' carries the same
+    // 30-min handler anchor as 'autopilot-cycle' (handler-timeouts.ts), so
+    // this silently starved brain-wide maintenance (embed/orphans/purge/…)
+    // at exactly the #2781 symptom (600s budget at the default 300s
+    // interval) even after the per-source path was fixed. Pin the correct
+    // wiring by source-shape: the call must pass the *full-cycle* variable.
+    const dispatchGlobalIdx = AUTOPILOT_SRC.indexOf('dispatchGlobalMaintenance(engine, queue');
+    expect(dispatchGlobalIdx).toBeGreaterThan(-1);
+    const dispatchGlobalCall = AUTOPILOT_SRC.slice(dispatchGlobalIdx, dispatchGlobalIdx + 200);
+    expect(dispatchGlobalCall).toContain('timeoutMs: fullCycleTimeoutMs');
+    // Guard against the exact regression: the shorthand `timeoutMs` (bare,
+    // no colon) resolving to the non-full-cycle outer const.
+    expect(dispatchGlobalCall).not.toMatch(/\{\s*repoPath,\s*slot,\s*timeoutMs,/);
   });
 
   test('updates lastFullCycleAt on dispatch (so the 60-min floor is honored)', () => {
@@ -70,6 +100,59 @@ describe('autopilot.ts ↔ dispatchPerSource wiring', () => {
     expect(AUTOPILOT_SRC).toMatch(/lastFullCycleAt\s*=\s*Date\.now\(\)/);
   });
 
+  test('#2781: full-cycle floor is derived from BOTH handler anchors, not a duplicated literal', () => {
+    // #2781's root cause: autopilot stamped an explicit `timeout_ms` that was
+    // only a `Math.max(interval-derived, 300_000)`-shaped literal, so it
+    // silently overrode the 'autopilot-cycle' handler's own #1737 anchor
+    // (`queue.ts`: an explicit stamp always wins over `defaultTimeoutMsFor`).
+    // A prior fix (#2852) hardcoded a matching `1_800_000` floor for
+    // full-cycle dispatch, but a literal that merely happens to equal the
+    // handler anchor can drift from it again if the anchor is ever retuned
+    // in handler-timeouts.ts without a matching edit here — reintroducing
+    // the exact #2781 bug class.
+    //
+    // Prove the floor is *derived* (not just numerically coincidental) two
+    // ways: (a) it equals Math.max of BOTH job names' anchors — a bare
+    // duplicated literal could accidentally match a SINGLE anchor (as the
+    // prior #2852 fix did) but wiring `Math.max(cycle, global)` is what
+    // actually protects a future divergence between the two anchors; (b) a
+    // source-shape check that both job-name string literals reach
+    // `defaultTimeoutMsFor` (directly or via a thin wrapper), and that no
+    // bare numeric literal sits in the full-cycle branch.
+    const cycleAnchorMs = defaultTimeoutMsFor('autopilot-cycle');
+    const globalAnchorMs = defaultTimeoutMsFor('autopilot-global-maintenance');
+    if (cycleAnchorMs === null) throw new Error("expected a handler anchor for 'autopilot-cycle'");
+    if (globalAnchorMs === null) throw new Error("expected a handler anchor for 'autopilot-global-maintenance'");
+    const expectedFloorMs = Math.max(cycleAnchorMs, globalAnchorMs);
+
+    // A short interval collapses the interval-derived component to its
+    // 300_000ms minimum, so the full-cycle result must equal the derived
+    // floor exactly.
+    expect(resolveAutopilotDispatchTimeoutMs(1, true)).toBe(expectedFloorMs);
+    // A regular (non-full-cycle) dispatch — e.g. the 'sync' freshness job,
+    // which has no long-job handler anchor — must NOT pick up the floor.
+    expect(resolveAutopilotDispatchTimeoutMs(1, false)).toBe(300_000);
+
+    // Guard against reintroducing a hardcoded literal floor directly in the
+    // full-cycle branch instead of the derived FULL_CYCLE_TIMEOUT_FLOOR_MS.
+    expect(AUTOPILOT_TIMEOUT_SRC).not.toMatch(/fullCycle\s*\?\s*Math\.max\([^)]*,\s*1_?800_?000\)/);
+    // Pin the derivation END TO END in source shape (codex round-2): both
+    // job-name anchor lookups must participate in the floor's Math.max, and
+    // the full-cycle branch must consume that derived const — otherwise the
+    // floor could be swapped back to a bare literal while the wrapper,
+    // import, and job-name strings survive as dead code and the assertions
+    // above still pass.
+    expect(AUTOPILOT_TIMEOUT_SRC).toMatch(
+      /FULL_CYCLE_TIMEOUT_FLOOR_MS\s*=\s*Math\.max\(\s*requireHandlerAnchorMs\('autopilot-cycle'\),\s*requireHandlerAnchorMs\('autopilot-global-maintenance'\),?\s*\)/,
+    );
+    expect(AUTOPILOT_TIMEOUT_SRC).toMatch(
+      /fullCycle\s*\?\s*Math\.max\(intervalDerivedTimeoutMs,\s*FULL_CYCLE_TIMEOUT_FLOOR_MS\)/,
+    );
+    // The wrapper itself must consult defaultTimeoutMsFor (fail-loud on a
+    // missing anchor, never a numeric fallback).
+    expect(AUTOPILOT_TIMEOUT_SRC).toMatch(/requireHandlerAnchorMs[\s\S]{0,200}defaultTimeoutMsFor\(jobName\)/);
+  });
+
   test('does NOT regress to the single-job dispatch on the full-cycle path', () => {
     // Pre-PR: the shouldFullCycle branch did:
     //   const job = await queue.add('autopilot-cycle', { repoPath }, {