fix(circular): break SF safety/autonomous-rollback chain (7-edge ring)
Some checks are pending
sf self-deploy / build, test, and publish server image (push) Waiting to run
sf self-deploy / deploy test and probe (push) Blocked by required conditions
sf self-deploy / promote prod (push) Blocked by required conditions

The cycle was a clean 7-edge ring:
  preferences → preferences-models → uok/auto-runaway-guard →
  detectors/periodic-runner → detectors/crash-loop-classifier →
  last-green → experimental → preferences

Three targeted cuts, each chosen for being a real architectural smell:

1. experimental → commands-prefs-wizard: the wizard was just
   re-routing the same `serializePreferencesToFrontmatter` import
   from preferences-serializer. experimental.js now imports from
   preferences-serializer directly. Edge removed.

2. crash-loop-classifier → safety/autonomous-rollback: detection
   should not directly trigger action — that couples concerns and
   creates the runtime cycle. Switched to a lazy `await import()`
   inside `crashLoopGate.execute()` (which is already async). The
   call site is unchanged from the caller's perspective; the
   runtime module-graph edge is gone. Walker skips dynamic
   imports.

3. preferences-models → uok/auto-runaway-guard: preferences-models
   only needed 6 runaway-threshold CONSTANTS, but pulling them from
   auto-runaway-guard dragged the whole detector/preferences/
   experimental subsystem into the preferences-models graph.
   Extracted those 6 constants to a new leaf module
   uok/runaway-defaults.js. Both preferences-models and the guard
   import from there. auto-runaway-guard re-exports the constants
   so existing call sites keep working without churn.

Net: 2 cycles → 1 cycle. 29/29 tests pass across the 5 touched
modules (autonomous-rollback, experimental-flags, crash-loop-
classifier detector, auto-runaway-guard, preferences-models).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mikael Hugo 2026-05-18 00:36:40 +02:00
parent e2c7484598
commit 5ac550d62a
5 changed files with 81 additions and 13 deletions

View file

@ -10,7 +10,21 @@ import { mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs";
import { join } from "node:path";
import { compareToLastGreen, getLastGreenEntry } from "../last-green.js";
import { maybeRollbackCrashLoop } from "../safety/autonomous-rollback.js";
// `maybeRollbackCrashLoop` is imported lazily via dynamic `import()` to
// break the safety/autonomous-rollback → ... → crash-loop-classifier →
// safety/autonomous-rollback runtime cycle. The classifier's `execute()`
// is already async, so the lazy import has zero observable effect on
// callers. Dynamic imports are skipped by scripts/check-circular-deps.mjs
// since they don't run at module-graph evaluation time.
let _maybeRollbackCrashLoop = null;
async function loadMaybeRollbackCrashLoop() {
if (_maybeRollbackCrashLoop === null) {
const mod = await import("../safety/autonomous-rollback.js");
_maybeRollbackCrashLoop = mod.maybeRollbackCrashLoop;
}
return _maybeRollbackCrashLoop;
}
export const CRASH_LOOP_WINDOW_MS = 90_000;
export const CRASH_LOOP_THRESHOLD = 3;
@ -106,10 +120,15 @@ export const crashLoopGate = {
ctx.options,
);
if (result.stuck) {
const rollback =
ctx.autoRollback === true || ctx.options?.autoRollback === true
? maybeRollbackCrashLoop(ctx.basePath ?? ctx.cwd, result, ctx.options)
: null;
let rollback = null;
if (ctx.autoRollback === true || ctx.options?.autoRollback === true) {
const maybeRollbackCrashLoop = await loadMaybeRollbackCrashLoop();
rollback = maybeRollbackCrashLoop(
ctx.basePath ?? ctx.cwd,
result,
ctx.options,
);
}
return {
outcome: "fail",
failureClass: "verification",

View file

@ -10,7 +10,11 @@
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
import { dirname } from "node:path";
import { serializePreferencesToFrontmatter } from "./commands-prefs-wizard.js";
// Import directly from the serializer module instead of going through
// commands-prefs-wizard. The wizard just re-routes this same import, and
// the detour created a runtime cycle:
// experimental → commands-prefs-wizard → preferences → ... → experimental
import { serializePreferencesToFrontmatter } from "./preferences-serializer.js";
import {
getProjectSFPreferencesPath,
loadEffectiveSFPreferences,

View file

@ -20,13 +20,18 @@ import {
resolveProfileDefaults,
} from "./preferences-profile.js";
import { sfHome } from "./sf-home.js";
// Import runaway defaults from the leaf constants module rather than
// auto-runaway-guard, which would pull the entire detector/preferences/
// experimental chain into the preferences-models module graph and form
// a runtime cycle (preferences-models → auto-runaway-guard → ... →
// preferences → preferences-models).
import {
DEFAULT_RUNAWAY_CHANGED_FILES_WARNING,
DEFAULT_RUNAWAY_DIAGNOSTIC_TURNS,
DEFAULT_RUNAWAY_ELAPSED_MINUTES,
DEFAULT_RUNAWAY_TOKEN_WARNING,
DEFAULT_RUNAWAY_TOOL_CALL_WARNING,
} from "./uok/auto-runaway-guard.js";
} from "./uok/runaway-defaults.js";
import { getKeyManagerAuthStorage } from "./key-manager.js";
import { lookupDiscoveredModelCost } from "./model-catalog-cache.js";

View file

@ -20,12 +20,27 @@ import {
detectorSweepSignal,
zeroProgressSignal,
} from "../supervision/loop-signals.js";
export const DEFAULT_RUNAWAY_TOOL_CALL_WARNING = 60;
export const DEFAULT_RUNAWAY_TOKEN_WARNING = 1_000_000;
export const DEFAULT_RUNAWAY_ELAPSED_MINUTES = 20;
export const DEFAULT_RUNAWAY_CHANGED_FILES_WARNING = 75;
export const DEFAULT_RUNAWAY_DIAGNOSTIC_TURNS = 2;
export const DEFAULT_RUNAWAY_MIN_INTERVAL_MS = 120_000;
// Re-exported from the leaf module so existing call sites that do
// `import { DEFAULT_RUNAWAY_* } from "./uok/auto-runaway-guard.js"`
// keep working. The new canonical home is runaway-defaults.js — adding
// new defaults goes there, not inline here. The split exists to break the
// preferences-models → auto-runaway-guard → ... → preferences-models cycle.
export {
DEFAULT_RUNAWAY_CHANGED_FILES_WARNING,
DEFAULT_RUNAWAY_DIAGNOSTIC_TURNS,
DEFAULT_RUNAWAY_ELAPSED_MINUTES,
DEFAULT_RUNAWAY_MIN_INTERVAL_MS,
DEFAULT_RUNAWAY_TOKEN_WARNING,
DEFAULT_RUNAWAY_TOOL_CALL_WARNING,
} from "./runaway-defaults.js";
import {
DEFAULT_RUNAWAY_CHANGED_FILES_WARNING,
DEFAULT_RUNAWAY_DIAGNOSTIC_TURNS,
DEFAULT_RUNAWAY_ELAPSED_MINUTES,
DEFAULT_RUNAWAY_MIN_INTERVAL_MS,
DEFAULT_RUNAWAY_TOKEN_WARNING,
DEFAULT_RUNAWAY_TOOL_CALL_WARNING,
} from "./runaway-defaults.js";
const EXECUTE_NO_PROGRESS_TOOL_WARNING = 25;
const EXECUTE_NO_PROGRESS_TOKEN_WARNING = 500_000;
const DURABLE_SF_ARTIFACT_PATHS = [".sf/milestones", ".sf/approvals"];

View file

@ -0,0 +1,25 @@
/**
* runaway-defaults.js default thresholds for the autonomous-loop runaway guard.
*
* Purpose: hold the runaway-guard defaults in a leaf module so both the guard
* implementation (uok/auto-runaway-guard.js) and the preferences resolver
* (preferences-models.js) can import from a single point WITHOUT pulling the
* full guard logic into the preferences module graph.
*
* Why this file exists: preferences-models needs only the numeric defaults to
* compute resolved-config fallbacks, but importing from auto-runaway-guard
* brought the entire detector/preferences/experimental subsystem into the
* preferences cycle. Splitting just the constants here lets both modules read
* the same source of truth without a runtime cycle.
*
* Consumer: uok/auto-runaway-guard.js (re-exports for internal call sites) and
* preferences-models.js (default-value lookups). Add NEW runaway-related
* constants here, not inline in the guard.
*/
export const DEFAULT_RUNAWAY_TOOL_CALL_WARNING = 60;
export const DEFAULT_RUNAWAY_TOKEN_WARNING = 1_000_000;
export const DEFAULT_RUNAWAY_ELAPSED_MINUTES = 20;
export const DEFAULT_RUNAWAY_CHANGED_FILES_WARNING = 75;
export const DEFAULT_RUNAWAY_DIAGNOSTIC_TURNS = 2;
export const DEFAULT_RUNAWAY_MIN_INTERVAL_MS = 120_000;