[local-fix-worker-throttle-ci-watcher-credentia] fix(worker): throttle CI-watcher credential failures and make them actionable

docs/code/worker.md

@@ -542,6 +542,8 @@ Pending, failing, and not-yet-reported CI is checked at the configured workspace
 
 Relay-backed workspaces perform these API calls with the local `GITHUB_TOKEN`. A fine-grained token—or the customer-owned App used by a no-relay worker—needs **Actions: Read** and **Commit statuses: Read** in addition to the normal PR and contents permissions. Existing App installations must be re-approved after adding permissions. No extra webhook event subscription is required because CI is polled. GitHub does not currently expose its separate Checks permission for fine-grained PATs, so check-run-only CI providers are not watched unless they also publish a commit status; GitHub Actions is fully supported through the Actions API.
 
+A rejected or missing credential (HTTP 401, or no `GITHUB_TOKEN` at all) and a credential-scoped permission denial (HTTP 403 with a GitHub credential message such as `Resource not accessible`) share one throttle per affected repository and operation: the watcher logs at most one actionable warning per operation per hour instead of repeating the same failure on every poll. The warning names the PR and explains whether the credential must be renewed or the missing permission granted — for CI reads, Actions: Read and Commit statuses: Read; for the exhaustion comment, Issues: Write or Pull requests: Write. CI reads and comment posts are throttled separately so a CI warning cannot hide a write-permission failure. A credential failure while posting the exhaustion comment is retried on the next poll instead of silently dropping the escalation, and a non-credential failure (for example a locked issue or persistent 5xx) is retried a bounded number of times before the watcher stops trying for that head SHA. CI auto-fix resumes automatically once the credential can read CI again; the watcher logs a single recovery line per repository and operation, and only after the same endpoint that failed succeeds (a metadata-only PR fetch does not clear a CI or review-read throttle). In fleet mode each repository has its own token, so one repository's credential failure never hides another's in either the CI watcher or review polling. Other 403 responses — rate limits, archived read-only repositories, locked issues, and blocked repositories — are not credential failures and keep their normal per-tick error reporting and retry path.
+
 ## Mention the bot on any PR
 
 The worker also reacts to mentions on pull requests it did not create. When a teammate writes a comment like `@devintern address the review feedback` on any PR in the repository, the worker picks it up on the next poll and handles it through the same pipeline. Detection is a repository-wide sweep of new comments (two requests per interval, regardless of how many PRs are open), so mentions work without any webhook setup.

packages/code/src/lib/acquirers/ci-failure-watcher.ts

@@ -39,6 +39,7 @@ import { RowParseError } from "../state/row-parse";
 import type { Acquirer } from "../../worker";
 import type { CiFailureFeedback } from "../review/formatter";
 import { serializePrRun } from "./review-polling";
+import { describeGitHubAuthFailure, isGitHubAuthError } from "../code-host/github/reviews";
 
 import type {
   CiProvider,
@@ -277,6 +278,36 @@ interface CachedCiSnapshot {
 
 type PollOutcome = "active" | "unchanged-green" | "removed";
 
+/** API surface a CI credential failure came from; throttled independently. */
+type CiAuthOperation = "poll" | "comment";
+
+/** Result of one escalation-comment attempt. */
+type EscalationOutcome = "posted" | "retry" | "gave-up";
+
+interface CiAuthFailureEntry {
+  repo: string;
+  operation: CiAuthOperation;
+  lastWarnAt: number | null;
+  active: boolean;
+}
+
+/** What one PR poll actually exercised, so recovery is only claimed where tested. */
+interface CiPollProgress {
+  /**
+   * True once every configured CI stream was fetched for this PR. PR-state
+   * reads alone (fork PRs, a missing head SHA) are metadata-only and cannot
+   * prove the CI endpoints recovered.
+   */
+  ciQueried: boolean;
+}
+
+/**
+ * Non-credential escalation-comment failures tolerated before freezing the
+ * escalation for this head. A permanent 403/422/5xx on the comment endpoint
+ * must not be retried (and warned about) on every poll forever.
+ */
+const MAX_ESCALATION_POST_FAILURES = 3;
+
 interface GreenPollSchedule {
   /** Index of the delay to use after the next unchanged-green observation. */
   nextBackoffIndex: number;
@@ -304,6 +335,26 @@ export class CiFailureWatcherAcquirer implements Acquirer {
   private timer: ReturnType<typeof setInterval> | null = null;
   private busy = false;
   private greenPollSchedules = new Map<string, GreenPollSchedule>();
+  /**
+   * Per-repo/per-operation throttle state for permanent credential failures.
+   * Fleet mode resolves a distinct token per repo, so a 403 in one repo must
+   * not hide an unrelated 401 in another; a comment-post failure also must not
+   * be silenced by a CI-read warning (they need different permissions).
+   */
+  private authFailures = new Map<string, CiAuthFailureEntry>();
+  /**
+   * Repo operations whose credential failed during the current tick. Credential
+   * errors can surface outside the per-PR fetch try/catch (for example posting
+   * the escalation comment), and such an operation must not be reported as
+   * recovered just because its PR state fetched successfully.
+   */
+  private tickAuthFailedOps = new Set<string>();
+  /**
+   * Consecutive non-credential escalation-comment failures per PR, scoped to
+   * the head SHA that was being escalated, so a permanently failing post is
+   * retried a bounded number of times instead of on every poll forever.
+   */
+  private escalationPostFailures = new Map<string, { headSha: string; failures: number }>();
 
   constructor(options: CiFailureWatcherAcquirerOptions) {
     this.options = {
@@ -358,22 +409,33 @@ export class CiFailureWatcherAcquirer implements Acquirer {
     this.busy = true;
 
     try {
+      this.tickAuthFailedOps.clear();
       const watchedPrs =
         this.options.watchedChanges?.() ?? this.options.workerState.listOpenAgentPrs();
       const watchedKeys = new Set(watchedPrs.map((pr) => this.prKey(pr.repo, pr.prNumber)));
+      const watchedRepos = new Set(watchedPrs.map((pr) => pr.repo));
       for (const key of this.greenPollSchedules.keys()) {
         if (!watchedKeys.has(key)) this.greenPollSchedules.delete(key);
       }
+      for (const [key, entry] of this.authFailures) {
+        if (!watchedRepos.has(entry.repo)) this.authFailures.delete(key);
+      }
+      for (const key of this.escalationPostFailures.keys()) {
+        if (!watchedKeys.has(key)) this.escalationPostFailures.delete(key);
+      }
 
+      const ciSucceededRepos = new Set<string>();
       for (const pr of watchedPrs) {
         const key = this.prKey(pr.repo, pr.prNumber);
         const schedule = this.greenPollSchedules.get(key);
         if (schedule && schedule.nextPollAt > this.now()) {
           continue;
         }
 
+        const progress: CiPollProgress = { ciQueried: false };
         try {
-          const outcome = await this.pollPr(pr.repo, pr.prNumber);
+          const outcome = await this.pollPr(pr.repo, pr.prNumber, progress);
+          if (progress.ciQueried) ciSucceededRepos.add(pr.repo);
           if (outcome === "unchanged-green") {
             this.scheduleGreenPoll(key, schedule?.nextBackoffIndex ?? 0);
           } else {
@@ -383,16 +445,84 @@ export class CiFailureWatcherAcquirer implements Acquirer {
           }
         } catch (error) {
           this.greenPollSchedules.delete(key);
+          if (isGitHubAuthError(error)) {
+            // A rejected or under-scoped credential fails identically on every
+            // request, so re-reporting it per PR per tick floods the log.
+            this.warnAuthFailure(pr.repo, pr.prNumber, error as Error, "poll");
+            continue;
+          }
           console.warn(
             `⚠️  [${this.name}] polling ${pr.repo}#${pr.prNumber} failed: ${(error as Error).message}`,
           );
         }
       }
+      // Only a tick where every CI stream was fetched proves polling works
+      // again; a metadata-only success (fork PR, missing head SHA) must not
+      // clear a CI throttle it never re-tested. Credential failures raised
+      // outside the per-PR fetch (escalation comments) are tracked per
+      // operation and do not block CI polling's own recovery.
+      for (const repo of ciSucceededRepos) {
+        if (!this.tickAuthFailedOps.has(CiFailureWatcherAcquirer.authKey(repo, "poll"))) {
+          this.noteAuthRecovery(repo, "poll");
+        }
+      }
     } finally {
       this.busy = false;
     }
   }
 
+  /** One credential warning per repo per operation per rolling hour. */
+  private static AUTH_WARN_INTERVAL_MS = 60 * 60 * 1000;
+
+  private static authKey(repo: string, operation: CiAuthOperation): string {
+    return `${repo.toLowerCase()}\u0000${operation}`;
+  }
+
+  private warnAuthFailure(
+    repo: string,
+    prNumber: number,
+    error: Error,
+    operation: CiAuthOperation,
+  ): void {
+    const key = CiFailureWatcherAcquirer.authKey(repo, operation);
+    this.tickAuthFailedOps.add(key);
+    const entry = this.authFailures.get(key) ?? {
+      repo,
+      operation,
+      lastWarnAt: null,
+      active: false,
+    };
+    entry.active = true;
+    const now = this.now();
+    if (
+      entry.lastWarnAt !== null &&
+      now - entry.lastWarnAt < CiFailureWatcherAcquirer.AUTH_WARN_INTERVAL_MS
+    ) {
+      this.authFailures.set(key, entry);
+      return;
+    }
+    entry.lastWarnAt = now;
+    this.authFailures.set(key, entry);
+    const activity = operation === "comment" ? "posting an escalation comment on" : "polling";
+    console.warn(
+      `⚠️  [${this.name}] GitHub rejected the credential while ${activity} ${repo}#${prNumber}: ` +
+        `${error.message} ${describeGitHubAuthFailure(error, operation === "comment" ? "comment" : "ci")} ` +
+        "See docs/code/worker.md#ci-failures-on-the-agents-prs.",
+    );
+  }
+
+  /** Announce one operation's recovery and rearm its throttle. */
+  private noteAuthRecovery(repo: string, operation: CiAuthOperation): void {
+    const key = CiFailureWatcherAcquirer.authKey(repo, operation);
+    const entry = this.authFailures.get(key);
+    if (!entry?.active) return;
+    this.authFailures.delete(key);
+    const scope = operation === "comment" ? "escalation comment posting" : "CI polling";
+    console.log(
+      `✅ [${this.name}] GitHub credential accepted again for ${repo}; ${scope} recovered`,
+    );
+  }
+
   /** Promptly reconcile one watched change after an authenticated provider hint. */
   async reconcile(repo: string, prNumber: number): Promise<void> {
     if (this.busy || !(this.options.enabled?.() ?? true)) return;
@@ -401,7 +531,7 @@ export class CiFailureWatcherAcquirer implements Acquirer {
     this.busy = true;
     const key = this.prKey(repo, prNumber);
     try {
-      const outcome = await this.pollPr(repo, prNumber);
+      const outcome = await this.pollPr(repo, prNumber, { ciQueried: false });
       if (outcome === "unchanged-green") {
         this.scheduleGreenPoll(key, 0);
       } else {
@@ -484,14 +614,20 @@ export class CiFailureWatcherAcquirer implements Acquirer {
       );
       state.consecutiveFailures = 0;
       state.escalatedSha = undefined;
+      this.escalationPostFailures.delete(this.prKey(repo, prNumber));
       workerState.setCiFixState(repo, prNumber, state);
     }
 
     if (state.consecutiveFailures >= this.maxAttempts) {
       if (!state.escalatedSha && fresh.length > 0) {
-        state.escalatedSha = headSha;
-        workerState.setCiFixState(repo, prNumber, state);
-        await this.escalateToHuman(repo, prNumber, pending);
+        // Persist the escalation only once the comment landed (or retrying it
+        // is abandoned), so a transient credential/API failure is retried
+        // instead of permanently silencing the escalation for this head SHA.
+        const escalation = await this.escalateToHuman(repo, prNumber, pending, headSha);
+        if (escalation !== "retry") {
+          state.escalatedSha = headSha;
+          workerState.setCiFixState(repo, prNumber, state);
+        }
       } else if (verbose) {
         console.log(
           `   [${this.name}] ${this.describe(repo, prNumber)}: retry budget exhausted; waiting for human`,
@@ -547,8 +683,10 @@ export class CiFailureWatcherAcquirer implements Acquirer {
         }
       }
       if (!ok && state.consecutiveFailures >= this.maxAttempts && !state.escalatedSha) {
-        state.escalatedSha = headSha;
-        await this.escalateToHuman(repo, prNumber, pending);
+        const escalation = await this.escalateToHuman(repo, prNumber, pending, headSha);
+        if (escalation !== "retry") {
+          state.escalatedSha = headSha;
+        }
       }
       workerState.setCiFixState(repo, prNumber, state);
       console.log(
@@ -564,7 +702,11 @@ export class CiFailureWatcherAcquirer implements Acquirer {
   }
 
   /** Poll a single PR; triggers at most one fix attempt per poll. */
-  private async pollPr(repo: string, prNumber: number): Promise<PollOutcome> {
+  private async pollPr(
+    repo: string,
+    prNumber: number,
+    progress: CiPollProgress,
+  ): Promise<PollOutcome> {
     const { workerState, provider, verbose } = this.options;
 
     // 1. PR state (ETag-cached): unwatch closed/merged PRs, track head SHA.
@@ -612,6 +754,9 @@ export class CiFailureWatcherAcquirer implements Acquirer {
       headSha,
       prResult,
     );
+    // Every configured stream was fetched: CI polling genuinely exercised the
+    // endpoints a credential failure would have hit.
+    progress.ciQueried = true;
 
     // 4. Split failures into fresh vs already-handled. Mark only after a
     // successful invocation so crashes/no-op runs remain retryable.
@@ -699,30 +844,63 @@ export class CiFailureWatcherAcquirer implements Acquirer {
     }
   }
 
-  /** Post the give-up comment and freeze further attempts until head moves. */
+  /**
+   * Post the give-up comment. Callers only freeze further attempts
+   * (`escalatedSha`) once the comment landed or retrying it is abandoned, so a
+   * transient failure is retried instead of permanently silencing the
+   * escalation. Credential failures use the per-repo/per-operation throttle and
+   * keep retrying (they recover once the token is fixed); non-credential
+   * failures are capped so a permanently rejected post cannot be retried and
+   * warned about on every poll forever.
+   */
   private async escalateToHuman(
     repo: string,
     prNumber: number,
     failures: PendingFailure[],
-  ): Promise<void> {
+    headSha: string,
+  ): Promise<EscalationOutcome> {
     const names = failures.map((f) => `- ${f.name}`).join("\n");
     const body =
       "⚠️ I could not fix the following CI failure(s) automatically after " +
       `${this.maxAttempts} attempt(s):\n\n${names}\n\n` +
       "I have stopped retrying to avoid churn. " +
       (this.options.escalationRecoveryText ??
         "Push a new commit (or mention me) and I will take another look.");
+    const key = this.prKey(repo, prNumber);
     try {
       await this.options.provider.postComment(repo, prNumber, body);
+      this.escalationPostFailures.delete(key);
+      this.noteAuthRecovery(repo, "comment");
       console.log(
         `📣 [${this.name}] ${this.describe(repo, prNumber)}: posted escalation comment after ` +
           `${this.maxAttempts} failed attempt(s)`,
       );
+      return "posted";
     } catch (error) {
+      if (isGitHubAuthError(error)) {
+        // A credential failure is a configuration problem: keep retrying (the
+        // warning is throttled per repo per hour) so the escalation lands as
+        // soon as the token is fixed.
+        this.warnAuthFailure(repo, prNumber, error as Error, "comment");
+        return "retry";
+      }
+      const previous = this.escalationPostFailures.get(key);
+      const failedAttempts = previous?.headSha === headSha ? previous.failures + 1 : 1;
+      this.escalationPostFailures.set(key, { headSha, failures: failedAttempts });
+      if (failedAttempts >= MAX_ESCALATION_POST_FAILURES) {
+        this.escalationPostFailures.delete(key);
+        console.warn(
+          `⚠️  [${this.name}] could not post escalation comment on ${this.describe(repo, prNumber)} ` +
+            `after ${failedAttempts} attempt(s): ${(error as Error).message}; ` +
+            "not retrying until the head changes",
+        );
+        return "gave-up";
+      }
       console.warn(
         `⚠️  [${this.name}] could not post escalation comment on ${this.describe(repo, prNumber)}: ` +
-          `${(error as Error).message}`,
+          `${(error as Error).message}; retrying on the next poll`,
       );
+      return "retry";
     }
   }