From 02402c5605a5c20b9076c2d8cc6a3056ed277924 Mon Sep 17 00:00:00 2001 From: Senna46 <29295263+Senna46@users.noreply.github.com> Date: Sat, 25 Jul 2026 06:03:46 +0900 Subject: [PATCH 1/2] fix: Cap fix retries and classify failures instead of retrying forever A bug whose fix could not be pushed was re-marked FAILED every cycle and retried indefinitely, re-running a full claude -p fix generation every ~2 minutes. The d6e-valuation workflow-scope incident burned six full generations before the token was fixed manually; nothing would have stopped it. - Classify failures: permanent (branch protection, missing token scope, denied permission) gives up at once; transient (usage limit, expired auth, network) retries without spending an attempt; anything else retries up to 3 times. - Back retries off 5/15/30 minutes instead of every cycle. - Post a PR comment when a bug is given up on, so the cause is visible. - Bound the since-filter bypass to a 30-day window; dropping the filter entirely paginated every review comment in the repo on every cycle (6 pages/cycle on the busiest repo) for as long as any bug was failing. Adds processed_bugs.attempts with an in-place migration, and switches the writes to UPSERT so INSERT OR REPLACE cannot reset the counter. --- README.md | 12 +++ src/bugbotMonitor.ts | 34 ++++++-- src/failureClassifier.ts | 63 +++++++++++++++ src/fixGenerator.ts | 2 +- src/main.ts | 165 +++++++++++++++++++++++++++++++++++---- src/state.ts | 159 +++++++++++++++++++++++++++++++------ 6 files changed, 387 insertions(+), 48 deletions(-) create mode 100644 src/failureClassifier.ts diff --git a/README.md b/README.md index d820d48..d1f071e 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,18 @@ Monitored repositories are auto-discovered from the GitHub App installations. | `AUTOFIX_CLAUDE_MODEL` | No | CLI default | Claude model to use | | `AUTOFIX_LOG_LEVEL` | No | `info` | Log level (debug/info/warn/error) | +### Retry policy + +When a fix attempt fails, the failure is classified before Fixooly decides whether to try again: + +| Class | Examples | Behavior | +|---|---|---| +| Permanent | Push rejected by branch protection, PAT missing the `workflow` scope, write permission denied | Given up on immediately — a retry would fail identically | +| Transient | Claude usage limit, expired/invalid auth, network errors | Retried after a backoff, without consuming a retry attempt | +| Other | Fix generation errored or timed out | Retried after a backoff, consuming one of 3 attempts | + +Retries back off (5 → 15 → 30 minutes) instead of running every cycle, so a failing bug cannot burn a full fix generation every couple of minutes. Once a bug is given up on, Fixooly posts a comment on the PR explaining why and never picks it up again. + ### Claude Authentication | Variable | Required | Description | diff --git a/src/bugbotMonitor.ts b/src/bugbotMonitor.ts index de558f8..117eda6 100644 --- a/src/bugbotMonitor.ts +++ b/src/bugbotMonitor.ts @@ -9,10 +9,16 @@ import { isBugbotComment, parseBugbotComment } from "./bugParser.js"; import type { GitHubClient } from "./githubClient.js"; import { logger } from "./logger.js"; +import { BUG_STATUS } from "./state.js"; import type { StateStore } from "./state.js"; import type { Config, PrBugReport } from "./types.js"; const DEFAULT_LOOKBACK_DAYS = 7; +// Widened window used while a repo still has retryable bugs, so a bug whose +// comment has aged out of the default window can still be re-found. Bounded +// on purpose: dropping the filter entirely paginates every review comment in +// the repo on every cycle. +const RETRY_LOOKBACK_DAYS = 30; export class BugbotMonitor { private github: GitHubClient; @@ -110,7 +116,8 @@ export class BugbotMonitor { return null; } - // First pass: filter out already-processed bugs (cheap local check) + // First pass: filter out already-processed and backed-off bugs + // (cheap local checks, no API calls) const candidateComments = []; for (const comment of bugbotComments) { const bug = parseBugbotComment(comment); @@ -119,6 +126,12 @@ export class BugbotMonitor { logger.debug("Bug already processed, skipping.", { bugId: bug.bugId }); continue; } + if (this.state.isInRetryBackoff(bug.bugId)) { + logger.debug("Bug is in retry backoff, skipping this cycle.", { + bugId: bug.bugId, + }); + continue; + } candidateComments.push({ comment, bug }); } @@ -137,7 +150,7 @@ export class BugbotMonitor { repo: `${owner}/${repo}`, prNumber, })), - "SKIPPED_PR_CLOSED" + BUG_STATUS.SKIPPED_PR_CLOSED ); logger.debug( `PR #${prNumber} in ${owner}/${repo} is closed/inaccessible, skipping ${candidateComments.length} bug(s).`, @@ -177,7 +190,7 @@ export class BugbotMonitor { repo: `${owner}/${repo}`, prNumber, })), - "SKIPPED_RESOLVED" + BUG_STATUS.SKIPPED_RESOLVED ); logger.debug( `Marked ${resolvedBugs.length} resolved bug(s) as SKIPPED_RESOLVED.`, @@ -228,12 +241,17 @@ export class BugbotMonitor { // Compute the "since" timestamp for the API query // ============================================================ - private computeSinceForRepo(repo: string): string | undefined { - if (this.state.hasRetryableBugsForRepo(repo)) { - logger.debug("Skipping since filter to retry failed/skipped bugs.", { repo }); - return undefined; + private computeSinceForRepo(repo: string): string { + const hasRetryable = this.state.hasRetryableBugsForRepo(repo); + if (hasRetryable) { + logger.debug("Widening since filter to retry failed bugs.", { + repo, + lookbackDays: RETRY_LOOKBACK_DAYS, + }); } - const lookbackMs = DEFAULT_LOOKBACK_DAYS * 24 * 60 * 60 * 1000; + + const days = hasRetryable ? RETRY_LOOKBACK_DAYS : DEFAULT_LOOKBACK_DAYS; + const lookbackMs = days * 24 * 60 * 60 * 1000; return new Date(Date.now() - lookbackMs).toISOString(); } } diff --git a/src/failureClassifier.ts b/src/failureClassifier.ts new file mode 100644 index 0000000..bf3f8fa --- /dev/null +++ b/src/failureClassifier.ts @@ -0,0 +1,63 @@ +// Classifies fix-generation failures so the daemon can decide whether +// retrying is worth another full fix generation. +// permanent — the same run will fail the same way (push rejected by a +// branch rule, missing token scope). Give up immediately. +// transient — an outage unrelated to the bug (usage limit, expired auth, +// network). Retry later without spending a retry attempt. +// retryable — anything else. Retry, but count it against the cap. + +export type FailureKind = "permanent" | "transient" | "retryable"; + +export interface FailureClassification { + kind: FailureKind; + // Short human-readable cause, used in logs and the PR comment. + reason: string; +} + +const PERMANENT_PATTERNS: Array<{ pattern: RegExp; reason: string }> = [ + { + pattern: /without .?workflow.? scope/i, + reason: "The push token lacks the `workflow` scope required to update .github/workflows files.", + }, + { + pattern: /protected branch|GH006|pre-receive hook declined/i, + reason: "The PR head branch rejects pushes (branch protection or a pre-receive hook).", + }, + { + pattern: /permission to .+ denied|403 Forbidden/i, + reason: "The push credentials are not allowed to write to this repository.", + }, + { + pattern: /remote rejected|refusing to allow/i, + reason: "GitHub rejected the push to the PR head branch.", + }, +]; + +const TRANSIENT_PATTERNS: Array<{ pattern: RegExp; reason: string }> = [ + { + pattern: /hit your limit|usage limit|rate limit|\b429\b/i, + reason: "Claude usage limit reached.", + }, + { + pattern: + /oauth (?:access )?(?:token|session) (?:has )?expired|invalid authentication credentials|\b401\b/i, + reason: "Claude authentication expired or was rejected.", + }, + { + pattern: + /ECONNRESET|ETIMEDOUT|ENOTFOUND|EADDRNOTAVAIL|EAI_AGAIN|socket hang up|\b50[234]\b/i, + reason: "Network or upstream service error.", + }, +]; + +export function classifyFailure(message: string): FailureClassification { + for (const { pattern, reason } of PERMANENT_PATTERNS) { + if (pattern.test(message)) return { kind: "permanent", reason }; + } + + for (const { pattern, reason } of TRANSIENT_PATTERNS) { + if (pattern.test(message)) return { kind: "transient", reason }; + } + + return { kind: "retryable", reason: "Fix generation failed." }; +} diff --git a/src/fixGenerator.ts b/src/fixGenerator.ts index 55a21a6..110a3e6 100644 --- a/src/fixGenerator.ts +++ b/src/fixGenerator.ts @@ -954,7 +954,7 @@ function parseFixDetails(claudeOutput: string): Map { // Utility: strip leaked tokens from git error messages // ============================================================ -function sanitizeGitError(message: string): string { +export function sanitizeGitError(message: string): string { return message .replace(/x-access-token:[^\s@]+/g, "x-access-token:[REDACTED]") .replace(/http\.[^\s]*\.extraheader=Authorization: basic [A-Za-z0-9+/=]+/g, "http.extraheader=[REDACTED]") diff --git a/src/main.ts b/src/main.ts index 1cd8d4c..d03036d 100644 --- a/src/main.ts +++ b/src/main.ts @@ -7,19 +7,23 @@ // Limitations: Single-threaded; processes PRs sequentially // within each polling cycle. Graceful shutdown on SIGINT/SIGTERM. +import { createHash } from "crypto"; import { mkdirSync, openSync, closeSync, unlinkSync, writeFileSync, readFileSync } from "fs"; import { dirname, join } from "path"; import { BugbotMonitor } from "./bugbotMonitor.js"; import { loadConfig } from "./config.js"; -import { FixGenerator } from "./fixGenerator.js"; +import { classifyFailure } from "./failureClassifier.js"; +import { FixGenerator, sanitizeGitError } from "./fixGenerator.js"; import { GitHubClient } from "./githubClient.js"; import { logger, setLogLevel } from "./logger.js"; -import { StateStore } from "./state.js"; -import type { Config, FixResult, PrBugReport } from "./types.js"; +import { BUG_STATUS, MAX_FIX_ATTEMPTS, StateStore } from "./state.js"; +import type { BugbotBug, Config, FixResult, PrBugReport } from "./types.js"; const AUTOFIX_COMMENT_MARKER = ""; const AUTOFIX_NO_CHANGES_MARKER = ""; +const AUTOFIX_FAILED_MARKER = ""; +const MAX_ERROR_DETAIL_CHARS = 1_000; class FixoolyDaemon { private config: Config; @@ -220,7 +224,7 @@ class FixoolyDaemon { repo: repoFullName, prNumber: pr.number, })), - "SKIPPED_NO_CHANGES" + BUG_STATUS.SKIPPED_NO_CHANGES ); logger.info( @@ -229,20 +233,84 @@ class FixoolyDaemon { ); } } catch (error) { - const message = error instanceof Error ? error.message : String(error); + const raw = error instanceof Error ? error.message : String(error); + await this.handleFixFailure(report, sanitizeGitError(raw)); + } + } + + // ============================================================ + // Failure handling: classify, cap retries, report on the PR + // ============================================================ + + private async handleFixFailure( + report: PrBugReport, + message: string + ): Promise { + const { pr, bugs } = report; + const repoFullName = `${pr.owner}/${pr.repo}`; + const records = bugs.map((b) => ({ + bugId: b.bugId, + repo: repoFullName, + prNumber: pr.number, + })); + + const { kind, reason } = classifyFailure(message); + + if (kind === "permanent") { + this.state.recordProcessedBugs(records, BUG_STATUS.FAILED_PERMANENT); logger.error( - `Error processing PR #${pr.number} in ${repoFullName}. Bugs will be retried next cycle.`, - { error: message, prNumber: pr.number, repo: repoFullName } + `Unrecoverable failure on PR #${pr.number} in ${repoFullName}. Bugs will not be retried.`, + { error: message, reason, prNumber: pr.number, repo: repoFullName } ); - this.state.recordProcessedBugs( - bugs.map((b) => ({ - bugId: b.bugId, - repo: repoFullName, + await this.postFailureComment(pr, bugs, reason, message); + return; + } + + // Transient outages must not burn the retry budget of a fixable bug. + const countAttempt = kind === "retryable"; + const results = this.state.recordFailedBugs(records, { countAttempt }); + + const exhausted = results.filter((r) => r.attempts >= MAX_FIX_ATTEMPTS); + const retrying = results.filter((r) => r.attempts < MAX_FIX_ATTEMPTS); + + if (retrying.length > 0) { + logger.error( + `Error processing PR #${pr.number} in ${repoFullName}. ${retrying.length} bug(s) will be retried after backoff.`, + { + error: message, + reason, + kind, prNumber: pr.number, - })), - "FAILED" + repo: repoFullName, + attempts: retrying, + } ); } + + if (exhausted.length === 0) return; + + const exhaustedIds = new Set(exhausted.map((r) => r.bugId)); + this.state.recordProcessedBugs( + records.filter((r) => exhaustedIds.has(r.bugId)), + BUG_STATUS.FAILED_PERMANENT + ); + + const exhaustedBugs = bugs.filter((b) => exhaustedIds.has(b.bugId)); + logger.error( + `Giving up on ${exhaustedBugs.length} bug(s) on PR #${pr.number} in ${repoFullName} after ${MAX_FIX_ATTEMPTS} attempts.`, + { + error: message, + prNumber: pr.number, + repo: repoFullName, + bugIds: exhaustedBugs.map((b) => b.bugId), + } + ); + await this.postFailureComment( + pr, + exhaustedBugs, + `Fix generation failed ${MAX_FIX_ATTEMPTS} times.`, + message + ); } // ============================================================ @@ -298,7 +366,7 @@ class FixoolyDaemon { private async postNoChangesComment( pr: { owner: string; repo: string; number: number }, - bugs: import("./types.js").BugbotBug[] + bugs: BugbotBug[] ): Promise { try { const alreadyPosted = await this.github.hasIssueCommentContaining( @@ -345,6 +413,75 @@ class FixoolyDaemon { } } + // ============================================================ + // Post give-up comment on the PR + // ============================================================ + + private async postFailureComment( + pr: { owner: string; repo: string; number: number }, + bugs: BugbotBug[], + reason: string, + errorDetail: string + ): Promise { + if (bugs.length === 0) return; + + // One marker per bug set, so a repeat of the same failure is not + // re-posted while a different failure still gets its own comment. + const digest = createHash("sha1") + .update( + bugs + .map((b) => b.bugId) + .sort() + .join(",") + ) + .digest("hex") + .slice(0, 12); + const marker = ``; + + try { + const alreadyPosted = await this.github.hasIssueCommentContaining( + pr.owner, + pr.repo, + pr.number, + marker + ); + if (alreadyPosted) { + logger.debug("Failure comment already exists on PR, skipping.", { + prNumber: pr.number, + repo: `${pr.owner}/${pr.repo}`, + }); + return; + } + + const bugList = bugs.map((b) => `- ❌ **${b.title}**`).join("\n"); + const detail = + errorDetail.length > MAX_ERROR_DETAIL_CHARS + ? `${errorDetail.slice(0, MAX_ERROR_DETAIL_CHARS)}\n... (truncated)` + : errorDetail; + + const body = + `${AUTOFIX_COMMENT_MARKER}\n${AUTOFIX_FAILED_MARKER}\n${marker}\n` + + `[Fixooly](https://github.com/Senna46/fixooly) gave up on ` + + `${bugs.length} bug(s) and will not retry them.\n\n` + + `**Reason:** ${reason}\n\n` + + `${bugList}\n\n` + + `
Error detail\n\n\`\`\`\n${detail}\n\`\`\`\n\n
`; + + await this.github.createIssueComment(pr.owner, pr.repo, pr.number, body); + logger.debug("Posted failure comment on PR.", { + prNumber: pr.number, + repo: `${pr.owner}/${pr.repo}`, + }); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger.warn("Failed to post failure comment on PR.", { + error: message, + prNumber: pr.number, + repo: `${pr.owner}/${pr.repo}`, + }); + } + } + // ============================================================ // Shutdown // ============================================================ diff --git a/src/state.ts b/src/state.ts index 00b8d7e..04ced9e 100644 --- a/src/state.ts +++ b/src/state.ts @@ -1,6 +1,7 @@ // SQLite-based state management for Claude Code Bugbot Autofix. // Tracks which Cursor Bugbot bug IDs have been processed -// to prevent duplicate fix attempts. +// to prevent duplicate fix attempts, and how many times a failed +// bug has been retried so retries can be capped and backed off. // Limitations: Single-process only; no concurrent access support. import Database from "better-sqlite3"; @@ -10,6 +11,25 @@ import { mkdirSync } from "fs"; import { logger } from "./logger.js"; import type { ProcessedBugRecord } from "./types.js"; +// Values stored in fix_commit_sha. Anything other than FAILED is terminal: +// the bug is never picked up again. +export const BUG_STATUS = { + FAILED: "FAILED", + FAILED_PERMANENT: "FAILED_PERMANENT", + SKIPPED_NO_CHANGES: "SKIPPED_NO_CHANGES", + SKIPPED_PR_CLOSED: "SKIPPED_PR_CLOSED", + SKIPPED_RESOLVED: "SKIPPED_RESOLVED", +} as const; + +// A bug that keeps failing is given up on after this many counted attempts. +// Without a cap, an unfixable bug re-runs a full fix generation every cycle. +export const MAX_FIX_ATTEMPTS = 3; + +// Minimum wait before retrying a failed bug, indexed by attempts already +// counted. Transient failures don't count an attempt, so they land on the +// first entry and simply stop the every-cycle hammering. +const RETRY_BACKOFF_MS = [5 * 60_000, 15 * 60_000, 30 * 60_000]; + export class StateStore { private db: Database.Database; @@ -34,12 +54,29 @@ export class StateStore { repo TEXT NOT NULL, pr_number INTEGER NOT NULL, processed_at TEXT NOT NULL, - fix_commit_sha TEXT + fix_commit_sha TEXT, + attempts INTEGER NOT NULL DEFAULT 0 ); CREATE INDEX IF NOT EXISTS idx_processed_bugs_repo_pr ON processed_bugs (repo, pr_number); `); + + this.migrateAttemptsColumn(); + } + + // Databases created before retry capping lack the attempts column. + private migrateAttemptsColumn(): void { + const columns = this.db + .prepare("PRAGMA table_info(processed_bugs)") + .all() as Array<{ name: string }>; + + if (columns.some((column) => column.name === "attempts")) return; + + this.db.exec( + "ALTER TABLE processed_bugs ADD COLUMN attempts INTEGER NOT NULL DEFAULT 0" + ); + logger.info("Migrated state DB: added processed_bugs.attempts column."); } // ============================================================ @@ -50,16 +87,39 @@ export class StateStore { const row = this.db .prepare("SELECT fix_commit_sha FROM processed_bugs WHERE bug_id = ?") .get(bugId) as { fix_commit_sha: string | null } | undefined; - // Only FAILED bugs should be retried; SKIPPED_NO_CHANGES is terminal - return row !== undefined && row.fix_commit_sha !== "FAILED"; + // Only FAILED bugs should be retried; every other status is terminal + return row !== undefined && row.fix_commit_sha !== BUG_STATUS.FAILED; + } + + // True while a failed bug is still inside its backoff window. Keeps a + // repeatedly failing bug from re-running a fix generation every cycle. + isInRetryBackoff(bugId: string, now: number = Date.now()): boolean { + const row = this.db + .prepare( + "SELECT processed_at, attempts, fix_commit_sha FROM processed_bugs WHERE bug_id = ?" + ) + .get(bugId) as + | { processed_at: string; attempts: number; fix_commit_sha: string | null } + | undefined; + + if (!row || row.fix_commit_sha !== BUG_STATUS.FAILED) return false; + + const lastAttempt = Date.parse(row.processed_at); + if (Number.isNaN(lastAttempt)) return false; + + const index = Math.min( + Math.max(row.attempts, 0), + RETRY_BACKOFF_MS.length - 1 + ); + return now - lastAttempt < RETRY_BACKOFF_MS[index]; } hasRetryableBugsForRepo(repo: string): boolean { const row = this.db .prepare( - "SELECT 1 FROM processed_bugs WHERE fix_commit_sha = 'FAILED' AND repo = ? LIMIT 1" + "SELECT 1 FROM processed_bugs WHERE fix_commit_sha = ? AND repo = ? LIMIT 1" ) - .get(repo); + .get(BUG_STATUS.FAILED, repo); return row !== undefined; } @@ -69,36 +129,29 @@ export class StateStore { prNumber: number, fixCommitSha: string | null ): void { - this.db - .prepare( - `INSERT OR REPLACE INTO processed_bugs - (bug_id, repo, pr_number, processed_at, fix_commit_sha) - VALUES (?, ?, ?, ?, ?)` - ) - .run(bugId, repo, prNumber, new Date().toISOString(), fixCommitSha); - - logger.debug("Recorded processed bug.", { - bugId, - repo, - prNumber, - fixCommitSha, - }); + this.recordProcessedBugs([{ bugId, repo, prNumber }], fixCommitSha); } recordProcessedBugs( bugs: Array<{ bugId: string; repo: string; prNumber: number }>, fixCommitSha: string | null ): void { - const insert = this.db.prepare( - `INSERT OR REPLACE INTO processed_bugs - (bug_id, repo, pr_number, processed_at, fix_commit_sha) - VALUES (?, ?, ?, ?, ?)` + // Upsert rather than INSERT OR REPLACE so the attempt counter survives. + const upsert = this.db.prepare( + `INSERT INTO processed_bugs + (bug_id, repo, pr_number, processed_at, fix_commit_sha, attempts) + VALUES (?, ?, ?, ?, ?, 0) + ON CONFLICT(bug_id) DO UPDATE SET + repo = excluded.repo, + pr_number = excluded.pr_number, + processed_at = excluded.processed_at, + fix_commit_sha = excluded.fix_commit_sha` ); const now = new Date().toISOString(); const transaction = this.db.transaction(() => { for (const bug of bugs) { - insert.run(bug.bugId, bug.repo, bug.prNumber, now, fixCommitSha); + upsert.run(bug.bugId, bug.repo, bug.prNumber, now, fixCommitSha); } }); @@ -110,6 +163,62 @@ export class StateStore { }); } + // Record a retryable failure. countAttempt is false for transient failures + // (usage limits, expired auth, network errors) so an outage that is not the + // bug's fault cannot exhaust its retry budget. Returns the attempt count + // per bug so the caller can give up once the cap is reached. + recordFailedBugs( + bugs: Array<{ bugId: string; repo: string; prNumber: number }>, + options: { countAttempt: boolean } + ): Array<{ bugId: string; attempts: number }> { + const increment = options.countAttempt ? 1 : 0; + + const upsert = this.db.prepare( + `INSERT INTO processed_bugs + (bug_id, repo, pr_number, processed_at, fix_commit_sha, attempts) + VALUES (?, ?, ?, ?, ?, ?) + ON CONFLICT(bug_id) DO UPDATE SET + repo = excluded.repo, + pr_number = excluded.pr_number, + processed_at = excluded.processed_at, + fix_commit_sha = excluded.fix_commit_sha, + attempts = processed_bugs.attempts + ?` + ); + const readAttempts = this.db.prepare( + "SELECT attempts FROM processed_bugs WHERE bug_id = ?" + ); + + const now = new Date().toISOString(); + const transaction = this.db.transaction(() => { + const results: Array<{ bugId: string; attempts: number }> = []; + for (const bug of bugs) { + upsert.run( + bug.bugId, + bug.repo, + bug.prNumber, + now, + BUG_STATUS.FAILED, + increment, + increment + ); + const row = readAttempts.get(bug.bugId) as + | { attempts: number } + | undefined; + results.push({ bugId: bug.bugId, attempts: row?.attempts ?? increment }); + } + return results; + }); + + const results = transaction(); + + logger.debug(`Recorded ${bugs.length} failed bug attempt(s).`, { + countAttempt: options.countAttempt, + results, + }); + + return results; + } + getProcessedBugsForPr( repo: string, prNumber: number From 17b3cdd71a59b1bbbab21711d2792ac12d73fcda Mon Sep 17 00:00:00 2001 From: Senna46 <29295263+Senna46@users.noreply.github.com> Date: Sat, 25 Jul 2026 06:03:46 +0900 Subject: [PATCH 2/2] fix: Carry CLI failure output into the error and retire stale FAILED rows Addresses both Bugbot findings on this PR, plus the push-token failure it surfaced while running against its own PR. - claude -p rejected with only a generic exit-code message, so the text that identifies a usage limit or expired auth never reached the classifier and every such failure was misread as retryable. The stdout or stderr tail is now part of the error message. - A FAILED row that discovery stopped surfacing (comment deleted, or aged out of the window) held its repo on the widened 30-day scan forever. Rows untouched for 24h are retired to FAILED_PERMANENT each cycle. - Invalid push credentials break every repo at once, so they are treated as transient: abandoning bugs and posting give-up comments across every PR while a token is rotated would be worse than waiting. - On a transient failure the rest of the cycle is skipped. Otherwise one environment outage costs a full fix generation per PR to rediscover the same problem. --- src/failureClassifier.ts | 8 ++++++++ src/fixGenerator.ts | 21 ++++++++++++++++++++- src/main.ts | 30 ++++++++++++++++++++++++------ src/state.ts | 28 ++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 7 deletions(-) diff --git a/src/failureClassifier.ts b/src/failureClassifier.ts index bf3f8fa..d0ea1fc 100644 --- a/src/failureClassifier.ts +++ b/src/failureClassifier.ts @@ -38,6 +38,14 @@ const TRANSIENT_PATTERNS: Array<{ pattern: RegExp; reason: string }> = [ pattern: /hit your limit|usage limit|rate limit|\b429\b/i, reason: "Claude usage limit reached.", }, + { + // Bad push credentials break every repo at once, so this must not be + // treated as a per-bug fault: it would abandon unrelated bugs and post + // give-up comments everywhere while the token is being rotated. + pattern: + /invalid username or token|authentication failed for|could not read Username/i, + reason: "The git push token is invalid or expired (AUTOFIX_PUSH_TOKEN).", + }, { pattern: /oauth (?:access )?(?:token|session) (?:has )?expired|invalid authentication credentials|\b401\b/i, diff --git a/src/fixGenerator.ts b/src/fixGenerator.ts index 110a3e6..9f79977 100644 --- a/src/fixGenerator.ts +++ b/src/fixGenerator.ts @@ -386,8 +386,13 @@ export class FixGenerator { stderr: stderr.substring(0, 1000) || "(empty)", stdoutTail: stdout.substring(Math.max(0, stdout.length - 2000)) || "(empty)", }); + // The actionable cause (usage limit, expired auth) is printed by the + // CLI rather than raised as an exit reason, so carry it in the error + // message — callers classify failures from that message alone. reject( - new Error(`claude -p fix generation exited with code ${code}.`) + new Error( + `claude -p fix generation exited with code ${code}. ${extractFailureDetail(stdout, stderr)}` + ) ); return; } @@ -950,6 +955,20 @@ function parseFixDetails(claudeOutput: string): Map { return details; } +// ============================================================ +// Utility: pull the actionable tail out of a failed claude -p run +// ============================================================ + +const MAX_FAILURE_DETAIL_CHARS = 500; + +function extractFailureDetail(stdout: string, stderr: string): string { + const source = stderr.trim() || stdout.trim(); + if (!source) return "(no output)"; + return source.length > MAX_FAILURE_DETAIL_CHARS + ? source.slice(-MAX_FAILURE_DETAIL_CHARS) + : source; +} + // ============================================================ // Utility: strip leaked tokens from git error messages // ============================================================ diff --git a/src/main.ts b/src/main.ts index d03036d..15eb0d4 100644 --- a/src/main.ts +++ b/src/main.ts @@ -151,6 +151,8 @@ class FixoolyDaemon { private async pollCycle(): Promise { logger.info("Starting polling cycle..."); + this.state.expireStaleFailures(); + const reports = await this.monitor.discoverUnprocessedBugs(); if (reports.length === 0) { @@ -165,7 +167,14 @@ class FixoolyDaemon { for (const report of reports) { if (this.isShuttingDown) break; - await this.processReport(report); + const outcome = await this.processReport(report); + if (outcome === "abort-cycle") { + logger.warn( + "Environment-level failure detected. Skipping the rest of this cycle.", + { remainingPrs: reports.length - reports.indexOf(report) - 1 } + ); + break; + } } } @@ -173,7 +182,9 @@ class FixoolyDaemon { // Process a single PR bug report // ============================================================ - private async processReport(report: PrBugReport): Promise { + private async processReport( + report: PrBugReport + ): Promise<"ok" | "abort-cycle"> { const { pr, bugs } = report; const repoFullName = `${pr.owner}/${pr.repo}`; @@ -234,8 +245,10 @@ class FixoolyDaemon { } } catch (error) { const raw = error instanceof Error ? error.message : String(error); - await this.handleFixFailure(report, sanitizeGitError(raw)); + return this.handleFixFailure(report, sanitizeGitError(raw)); } + + return "ok"; } // ============================================================ @@ -245,7 +258,7 @@ class FixoolyDaemon { private async handleFixFailure( report: PrBugReport, message: string - ): Promise { + ): Promise<"ok" | "abort-cycle"> { const { pr, bugs } = report; const repoFullName = `${pr.owner}/${pr.repo}`; const records = bugs.map((b) => ({ @@ -263,7 +276,7 @@ class FixoolyDaemon { { error: message, reason, prNumber: pr.number, repo: repoFullName } ); await this.postFailureComment(pr, bugs, reason, message); - return; + return "ok"; } // Transient outages must not burn the retry budget of a fixable bug. @@ -287,7 +300,10 @@ class FixoolyDaemon { ); } - if (exhausted.length === 0) return; + // A transient failure is an environment problem (usage limit, bad push + // token) that will hit every remaining PR the same way. Stop the cycle + // instead of spending a full fix generation per PR to rediscover it. + if (exhausted.length === 0) return kind === "transient" ? "abort-cycle" : "ok"; const exhaustedIds = new Set(exhausted.map((r) => r.bugId)); this.state.recordProcessedBugs( @@ -311,6 +327,8 @@ class FixoolyDaemon { `Fix generation failed ${MAX_FIX_ATTEMPTS} times.`, message ); + + return "ok"; } // ============================================================ diff --git a/src/state.ts b/src/state.ts index 04ced9e..356e5e4 100644 --- a/src/state.ts +++ b/src/state.ts @@ -30,6 +30,12 @@ export const MAX_FIX_ATTEMPTS = 3; // first entry and simply stop the every-cycle hammering. const RETRY_BACKOFF_MS = [5 * 60_000, 15 * 60_000, 30 * 60_000]; +// A live bug resolves within an hour (3 attempts, 30 min of backoff at most). +// A FAILED row older than this is one whose comment discovery never returned +// again — deleted, or aged out of the lookback window. Left alone it would +// keep the repo on the widened scan window forever, so it is retired instead. +const STALE_FAILURE_MS = 24 * 60 * 60 * 1000; + export class StateStore { private db: Database.Database; @@ -114,6 +120,28 @@ export class StateStore { return now - lastAttempt < RETRY_BACKOFF_MS[index]; } + // Retire FAILED rows that discovery has stopped surfacing, so they cannot + // hold a repo on the widened scan window indefinitely. Returns the number + // of rows retired. + expireStaleFailures(now: number = Date.now()): number { + const cutoff = new Date(now - STALE_FAILURE_MS).toISOString(); + const result = this.db + .prepare( + `UPDATE processed_bugs SET fix_commit_sha = ? + WHERE fix_commit_sha = ? AND processed_at < ?` + ) + .run(BUG_STATUS.FAILED_PERMANENT, BUG_STATUS.FAILED, cutoff); + + if (result.changes > 0) { + logger.info( + `Retired ${result.changes} stale failed bug(s) that were never re-discovered.`, + { cutoff } + ); + } + + return result.changes; + } + hasRetryableBugsForRepo(repo: string): boolean { const row = this.db .prepare(