fix(worker): stop running pre-PR repair cycles by default (#320)

* fix(worker): stop running pre-PR repair cycles by default

* docs: plan the auto-fix loop on failing PR checks

Author: outof-place

apps/worker/src/pre-pr-checks/runner.ts

@@ -22,7 +22,13 @@ import type {
   RunBudgetState,
 } from "../workflows/run-budget.js";
 
-export const MAX_PRE_PR_FIX_CYCLES = 3;
+/** Repair cycles the Pre-PR gate runs when a graph does not author its own:
+ *  none. Every cycle re-ran a tenant's entire check batch, so three cycles of an
+ *  810s batch burned 54 minutes of a 100 minute run budget, and the loop could
+ *  not tell a broken environment from broken code. Remediation belongs after the
+ *  pull request is open, where provider CI has already reported what failed. A
+ *  graph can still opt in per node. */
+export const MAX_PRE_PR_FIX_CYCLES = 0;
 
 /** Longest launch cause carried into the Pre-PR repair failure detail. Same
  *  bound the workspace gate puts on a carried inspection reason (AIW-223): long

apps/worker/src/workflow-definition/block-registry.ts

@@ -774,7 +774,7 @@ const definitions: Record<WorkflowBlockType, ContractDefinition> = {
       "Runs the product's configured pre-publication validation and fix cycle.",
       "✓",
     ),
-    defaults: { maxFixCycles: 3 },
+    defaults: { maxFixCycles: 0 },
     inputs: {},
     output: statusOutput({
       ok: booleanType(),

apps/worker/src/workflows/agent.ts

@@ -152,6 +152,7 @@ import {
 } from "./blocks/call-llm.js";
 import { pollPhaseUntilDone } from "./blocks/poll-phase.js";
 import {
+  MAX_PRE_PR_FIX_CYCLES,
   loadPrePrCheckConfigStep,
   runPrePrChecksWithFixes,
 } from "./blocks/pre-pr-checks.js";
@@ -5267,7 +5268,7 @@ async function agentWorkflowBody(
                 : undefined);
             if (
               ctx.schemaVersion === 2 &&
-              (maxFixCycles ?? 3) > 0 &&
+              (maxFixCycles ?? MAX_PRE_PR_FIX_CYCLES) > 0 &&
               !repairRuntime
             ) {
               return executionError(

apps/worker/src/workflows/blocks/pre-pr-checks.test.ts

@@ -122,7 +122,11 @@ function collected(overrides: {
 
 describe("runPrePrChecksWithFixes", () => {
   beforeEach(() => {
-    vi.clearAllMocks();
+    // resetAllMocks, not clearAllMocks: clear leaves queued mockImplementationOnce
+    // entries behind, so a test that consumes fewer of them than it queued leaks
+    // the rest into whatever runs next. That made five unrelated tests fail the
+    // moment the repair loop stopped running by default.
+    vi.resetAllMocks();
     mocks.pollPhaseUntilDone.mockImplementation(pollEnds("finished", 30_000));
     mocks.startRepoCheckBatchStep.mockImplementation(async (
       _sandboxId: string,
@@ -446,7 +450,7 @@ describe("runPrePrChecksWithFixes", () => {
     });
     mocks.collectPrePrRepairStep.mockResolvedValue({ usage: null });
 
-    const result = await runPrePrChecksWithFixes(options());
+    const result = await runPrePrChecksWithFixes(options({ maxFixCycles: 3 }));
 
     expect(result.passed).toBe(true);
     expect(result.fixCycles).toBe(1);
@@ -619,7 +623,7 @@ describe("runPrePrChecksWithFixes", () => {
     });
     mocks.collectPrePrRepairStep.mockResolvedValue({ usage: null });
 
-    const result = await runPrePrChecksWithFixes(options({ config }));
+    const result = await runPrePrChecksWithFixes(options({ maxFixCycles: 3, config }));
 
     expect(result.setupFailed).toBe(false);
     expect(mocks.startPrePrRepairStep).toHaveBeenCalledTimes(1);
@@ -735,6 +739,32 @@ describe("runPrePrChecksWithFixes", () => {
     expect(mocks.startPrePrRepairStep).not.toHaveBeenCalled();
   });
 
+  it("runs no fix cycles when the graph does not author maxFixCycles", async () => {
+    // Pins the shipped default. Repair before the pull request re-ran the whole
+    // batch on every cycle and could not tell a broken environment from broken
+    // code, so remediation moved behind the pull request. A graph that wants the
+    // old behaviour has to ask for it by number.
+    mocks.collectRepoCheckBatchStep.mockResolvedValue(
+      collected({
+        results: [{ provider: "github", repoPath: "acme/web", command: "pnpm typecheck", exitCode: 1 }],
+        failures: [{
+          provider: "github",
+          repoPath: "acme/web",
+          command: "pnpm typecheck",
+          exitCode: 1,
+          stdout: "",
+          stderr: "still failing",
+        }],
+      }),
+    );
+
+    const result = await runPrePrChecksWithFixes(options());
+
+    expect(result.fixCycles).toBe(0);
+    expect(result.passed).toBe(false);
+    expect(mocks.startPrePrRepairStep).not.toHaveBeenCalled();
+  });
+
   it("returns a repair agent failure without starting another cycle", async () => {
     mocks.collectRepoCheckBatchStep.mockResolvedValue(
       collected({
@@ -754,7 +784,7 @@ describe("runPrePrChecksWithFixes", () => {
       failure: { ok: false, category: "provider", diagnostic: { failureKind: "setup_failed" } },
     });
 
-    const result = await runPrePrChecksWithFixes(options());
+    const result = await runPrePrChecksWithFixes(options({ maxFixCycles: 3 }));
 
     expect(result.fixCycles).toBe(1);
     expect(result.agentFailure).toMatchObject({
@@ -843,6 +873,7 @@ describe("runPrePrChecksWithFixes", () => {
 
     const result = await runPrePrChecksWithFixes(
       options({
+        maxFixCycles: 3,
         budget: {
           state: createRunBudgetState(),
           limits: { maxDurationMs: 60_000, maxTokens: 12 },

apps/worker/src/workflows/blocks/pre-pr-checks.ts

@@ -1,4 +1,5 @@
 import type { PrePrCheckConfig } from "../../pre-pr-checks/config.js";
+export { MAX_PRE_PR_FIX_CYCLES } from "../../pre-pr-checks/runner.js";
 import {
   MAX_PRE_PR_FIX_CYCLES,
   PRE_PR_CHECK_BATCH_MAX_MINUTES,