ReviewOS
Reload

also looking at this

stacks/buddy

feat(ci): let @buddy fix-ci find the run a comment cannot name

#1413
Merged glennmichael123 wants to merge feat/fix-ci-from-comment into main
10 files +697 -121
bin/cli.tsmodified+15-106
@@ -1349,117 +1349,18 @@ cli
13491349
13501350 try {
13511351 const provider = await providerFor(config, 'this command', logger)
1352 const { runFixCi } = await import('../src/ci')
13521353
1353 assertSupports(provider, 'ciLogs', 'getWorkflowRunLogs', 'reading CI logs')
1354 const runId = options.runId ? Number.parseInt(options.runId, 10) : undefined
1355 const log = runId ? await provider.getWorkflowRunLogs(runId) : null
1356
1357 if (!log) {
1358 logger.warn('⚠️ Could not read the run logs; nothing to diagnose')
1359 return
1360 }
1361
1362 const { attemptFix, failsOnBaseBranch } = await import('../src/ci')
1363 const { parseFixAttempts, upsertFixAttempts } = await import('../src/ci/attempts')
1364 const { createAiClient } = await import('../src/ai')
1365
1366 // The guard in attemptFix can only stop a loop if it is told how many
1367 // attempts came before, and a CI job remembers nothing between runs.
1368 const prNumber = options.pr ? Number.parseInt(options.pr, 10) : undefined
1369 let pullRequest: PullRequest | undefined
1370 if (prNumber) {
1371 const open = await provider.getPullRequests('open')
1372 pullRequest = open.find(candidate => candidate.number === prNumber)
1373 }
1374 const priorAttempts = parseFixAttempts(pullRequest?.body)?.attempts ?? 0
1375 const baseBranch = config.repository?.baseBranch ?? 'main'
1376
1377 // Both of these are capability-gated rather than assumed: a provider
1378 // that cannot read a run history still gets the full diagnosis, it just
1379 // cannot tell an inherited failure from a new one.
1380 const canReadRuns = supports(provider, 'ciRuns', 'listWorkflowRuns')
1381 const canRerun = supports(provider, 'ciRuns', 'rerunWorkflowRun')
1382
1383 const outcome = await attemptFix({
1384 log,
1385 priorAttempts,
1386 workspace: process.cwd(),
1387 baseBranch,
1388 ai: createAiClient(config, logger),
1354 const status = await runFixCi({
1355 config,
1356 provider,
13891357 logger,
13901358 dryRun: Boolean(options.dryRun),
1391 ...(canReadRuns
1392 ? {
1393 checkBase: (kind: FailureKind) => failsOnBaseBranch(
1394 {
1395 listWorkflowRuns: (branch, listOptions) => provider.listWorkflowRuns!(branch, listOptions),
1396 getWorkflowRunLogs: id => provider.getWorkflowRunLogs!(id),
1397 },
1398 baseBranch,
1399 kind,
1400 logger,
1401 ),
1402 }
1403 : {}),
1404 ...(canRerun && runId ? { rerun: () => provider.rerunWorkflowRun!(runId) } : {}),
1405 regenerateLockfile: async () => {
1406 const { regenerateLockFile, detectRequiredPackageManagers, getAllLockFilePaths } = await import('../src/utils/lock-file')
1407 const { commitAndPush } = await import('../src/utils/git')
1408
1409 // Detection keys off the manifests a lockfile is derived from, so a
1410 // repository with several ecosystems regenerates each of them.
1411 const managers = detectRequiredPackageManagers(['package.json', 'composer.json'])
1412 let regenerated = false
1413
1414 for (const manager of managers) {
1415 const result = await regenerateLockFile(manager, process.cwd())
1416 if (result.success)
1417 regenerated = true
1418 else
1419 logger.warn(`⚠️ ${result.message}`)
1420 }
1421
1422 if (!regenerated)
1423 return { regenerated: false, pushed: false }
1424
1425 // Rewriting the file in a workspace that is about to be thrown away
1426 // repairs nothing. The job checked this branch out with credentials,
1427 // so the commit is a local one rather than a branch recreation.
1428 try {
1429 const pushed = await commitAndPush(
1430 getAllLockFilePaths(),
1431 'fix(deps): regenerate the lock file\n\nThe lock file had drifted from the manifest, which failed CI.',
1432 process.cwd(),
1433 )
1434 return { regenerated: true, pushed }
1435 }
1436 catch (error) {
1437 logger.warn(`⚠️ Could not push the regenerated lock file: ${error instanceof Error ? error.message : String(error)}`)
1438 return { regenerated: true, pushed: false }
1439 }
1440 },
1359 ...(options.runId ? { runId: Number.parseInt(options.runId, 10) } : {}),
1360 ...(options.pr ? { prNumber: Number.parseInt(options.pr, 10) } : {}),
14411361 })
14421362
1443 logger.info(`\n${outcome.report}\n`)
1444
1445 if (prNumber && !options.dryRun) {
1446 await provider.createComment(prNumber, outcome.report)
1447
1448 // Only a real attempt counts. Recording the ones the guard already
1449 // refused would march the counter up without anything being tried.
1450 if (outcome.action !== 'skipped' && pullRequest) {
1451 try {
1452 await provider.updatePullRequest(prNumber, {
1453 body: upsertFixAttempts(pullRequest.body, priorAttempts + 1),
1454 })
1455 }
1456 catch (error) {
1457 logger.warn(`Could not record the fix attempt on PR #${prNumber}: ${error instanceof Error ? error.message : String(error)}`)
1458 }
1459 }
1460 }
1461
1462 logger.success(`✅ ${outcome.action}${outcome.fixed ? ' (fixed)' : ''}`)
1363 logger.success(`✅ ${status}`)
14631364 }
14641365 catch (error) {
14651366 logger.error('fix-ci failed:', error)
@@ -1563,6 +1464,14 @@ cli
15631464 return 'Rebased.'
15641465 },
15651466 merge: async () => await new Buddy(config).mergeEligiblePullRequests(false),
1467 // `analysisOnly` because this job checks out the default branch, not
1468 // the pull request's. A lock-file rewrite or an agent edit here would
1469 // change the wrong tree; re-running a flake is an API call and still
1470 // works. The fix-ci job, which does check out the branch, repairs.
1471 fixCi: async (prNumber) => {
1472 const { runFixCi } = await import('../src/ci')
1473 return await runFixCi({ config, provider, prNumber, analysisOnly: true, logger })
1474 },
15661475 remember: async (text, context) => {
15671476 const { addLearning, createLearning, loadLearnings, serializeLearnings, DEFAULT_LEARNINGS_FILE }
15681477 = await import('../src/ai')
docs/features/ci-repair.mdmodified+18-2
@@ -87,8 +87,24 @@ that job's log. The workflow generated by `buddy setup` wires this up: when a
8787run on a Buddy branch fails, the `fix-ci` job reads its log and reports back on
8888the pull request.
8989
90A comment cannot say *which* run failed, so `@buddy fix-ci` in a thread replies
91pointing at that job rather than guessing.
90## From a comment
91
92`@buddy fix-ci` on a pull request finds the run itself: the newest failing run
93at the *current* head commit. Restricted to the head deliberately — a branch
94keeps its old runs, and diagnosing one from three pushes ago would produce a
95confident report about a failure somebody already fixed. If nothing at the head
96is red, it says so rather than reaching further back.
97
98What it does from there is bounded by where it runs. Answering a comment happens
99on the default branch, not the pull request's, so a lock-file rewrite or an
100agent edit would land on the wrong tree entirely. From a comment Buddy will:
101
102- classify the failure and post the diagnosis
103- tell you whether the base branch already fails the same way
104- re-run a flake, which is an API call and needs no checkout
105
106and it will name the repair it *cannot* apply rather than staying quiet about
107it. The `fix-ci` job below does check out the branch, and repairs there.
92108
93109### Whose failure is it
94110
src/ci/base.tsmodified+5-1
@@ -99,10 +99,14 @@ export async function failsOnBaseBranch(
9999 * answers the question actually being asked — is this workflow red on base
100100 * right now — and drops a workflow whose latest run has since gone green.
101101 *
102 * Also used to pick *which* run a comment is asking about: the same
103 * interleaving that makes a base-branch check ambiguous makes "the run that
104 * failed on this pull request" ambiguous too.
105 *
102106 * @param runs - Runs, most recent first
103107 * @returns The newest run of each workflow, kept only where it failed
104108 */
105function latestFailurePerWorkflow(runs: WorkflowRun[]): WorkflowRun[] {
109export function latestFailurePerWorkflow(runs: WorkflowRun[]): WorkflowRun[] {
106110 const newest = new Map<string, WorkflowRun>()
107111
108112 for (const run of runs) {
src/ci/fix.tsmodified+44-0
@@ -60,6 +60,16 @@ export interface FixOptions {
6060 rerun?: () => Promise<boolean>
6161 /** Diagnose and report without changing anything */
6262 dryRun?: boolean
63 /**
64 * Diagnose without repairing, because the caller has nowhere to repair.
65 *
66 * Distinct from `dryRun`, which is a choice someone made. This is a
67 * constraint: answering an `@buddy fix-ci` comment runs on the default
68 * branch, so rewriting a lock file or letting the agent edit files would
69 * change the wrong tree entirely. Re-running a flake is still fine — that is
70 * an API call, not a change to a checkout.
71 */
72 analysisOnly?: boolean
6373 logger?: Logger
6474}
6575
@@ -108,6 +118,29 @@ function lockfileReport(regenerated: boolean, pushed: boolean): string {
108118 return 'Regenerated the lock file and pushed the result. No model was needed.'
109119}
110120
121/**
122 * Explain a diagnosis that could not be acted on from where it was made.
123 *
124 * Says which repair was available rather than only that none happened — a
125 * report that stops at "I cannot fix this here" reads as a limitation, where
126 * the useful version names the fix and who will apply it.
127 *
128 * @param failure - The classified failure
129 */
130function analysisOnlyNote(failure: ClassifiedFailure): string {
131 const shared = 'Repairs run on the pull request\'s branch, and answering a comment runs on the default branch, '
132 + 'so anything I changed here would land on the wrong tree. '
133
134 if (failure.kind === 'lockfile-drift') {
135 return `Regenerating the lock file would fix this, and no model would be needed. ${shared}`
136 + 'The `fix-ci` job will do it on the next failing run, or you can run '
137 + '`buddy fix-ci --run-id <id> --pr <number>` yourself.'
138 }
139
140 return `${shared}The \`fix-ci\` job runs on the failing run itself and can repair this; `
141 + 'this reply is the diagnosis it would start from.'
142}
143
111144export async function attemptFix(options: FixOptions): Promise<FixOutcome> {
112145 const logger = options.logger ?? getDefaultLogger()
113146 const failure = classifyFailure(options.log)
@@ -140,6 +173,17 @@ export async function attemptFix(options: FixOptions): Promise<FixOutcome> {
140173 }
141174 }
142175
176 // Everything below either writes to the workspace or hands it to the agent.
177 // A flake is the exception, and it is handled on its own further down.
178 if (options.analysisOnly && failure.kind !== 'flake') {
179 return {
180 failure,
181 action: 'reported',
182 fixed: false,
183 report: report(failure, analysisOnlyNote(failure)),
184 }
185 }
186
143187 if (failure.kind === 'lockfile-drift' && options.regenerateLockfile) {
144188 if (options.dryRun) {
145189 return {
src/ci/index.tsmodified+3-1
@@ -1,4 +1,4 @@
1export { failsOnBaseBranch } from './base'
1export { failsOnBaseBranch, latestFailurePerWorkflow } from './base'
22export type { RunHistoryReader } from './base'
33export {
44 classifyFailure,
@@ -7,4 +7,6 @@ export {
77} from './classify'
88export type { ClassifiedFailure, FailureKind } from './classify'
99export { attemptFix } from './fix'
10export { runFixCi } from './run'
11export type { RunFixCiOptions } from './run'
1012export type { FixOptions, FixOutcome, LockfileRepair } from './fix'
src/ci/run.tsadded+220-0
@@ -0,0 +1,220 @@
1/**
2 * Repair a failing run, from wherever the request came from.
3 *
4 * The CLI had this inline, which was fine while it was the only caller. The
5 * `@buddy fix-ci` comment needs the same sequence — resolve a run, read its
6 * log, diagnose, report, count the attempt — and a second copy would drift
7 * from the first the way the two review paths did.
8 */
9import type { PullRequest } from '../types'
10import type { BuddyConfig } from '../types'
11import type { GitProvider } from '../git/provider'
12import type { Logger } from '../utils/logger'
13import type { FailureKind } from './classify'
14import { assertSupports, supports } from '../git/provider'
15import { formatError } from '../utils/errors'
16import { getDefaultLogger } from '../utils/logger'
17import { parseFixAttempts, upsertFixAttempts } from './attempts'
18import { failsOnBaseBranch, latestFailurePerWorkflow } from './base'
19import { attemptFix } from './fix'
20
21/** Inputs to a repair run. */
22export interface RunFixCiOptions {
23 config: BuddyConfig
24 provider: GitProvider
25 /** Run to diagnose; resolved from the pull request when absent */
26 runId?: number
27 /** Pull request to report on, and where the attempt counter lives */
28 prNumber?: number
29 /** Diagnose and report without changing anything */
30 dryRun?: boolean
31 /** Diagnose without repairing, because the caller has nowhere to repair */
32 analysisOnly?: boolean
33 /** Directory repairs run in */
34 workspace?: string
35 logger?: Logger
36}
37
38/**
39 * Find the run a pull request is currently red on.
40 *
41 * Restricted to the head commit deliberately. A branch keeps its old runs, and
42 * diagnosing one from three pushes ago would produce a confident report about
43 * a failure somebody already fixed.
44 *
45 * @param provider - Provider to read runs from
46 * @param pr - The pull request in question
47 * @param logger - Optional logger
48 * @returns The failing run's id, or `null` when nothing at the head has failed
49 */
50async function resolveFailingRun(
51 provider: GitProvider,
52 pr: PullRequest,
53 logger: Logger,
54): Promise<number | null> {
55 if (!supports(provider, 'ciRuns', 'listWorkflowRuns'))
56 return null
57
58 try {
59 const headSha = await provider.getPullRequestHeadSha(pr.number)
60 const runs = await provider.listWorkflowRuns(pr.head, { status: 'completed', limit: 30 })
61 const atHead = headSha ? runs.filter(run => run.headSha === headSha) : runs
62
63 return latestFailurePerWorkflow(atHead)[0]?.id ?? null
64 }
65 catch (error) {
66 logger.debug(`Could not resolve the failing run for #${pr.number}: ${formatError(error)}`)
67 return null
68 }
69}
70
71/**
72 * Diagnose a failing run and repair it when the fix is clear.
73 *
74 * Shared by `buddy fix-ci` and the `@buddy fix-ci` comment so both resolve the
75 * run, apply the anti-loop guard and report the same way.
76 *
77 * @param options - Repository context and repair settings
78 * @returns A short status line describing what happened
79 * @example
80 * ```ts
81 * const status = await runFixCi({ config, provider, prNumber: 128 })
82 * ```
83 */
84export async function runFixCi(options: RunFixCiOptions): Promise<string> {
85 const logger = options.logger ?? getDefaultLogger()
86 const { config, provider } = options
87 const workspace = options.workspace ?? process.cwd()
88
89 assertSupports(provider, 'ciLogs', 'getWorkflowRunLogs', 'reading CI logs')
90
91 // The pull request carries the attempt counter, so it is fetched before
92 // anything else that might spend one.
93 let pullRequest: PullRequest | undefined
94 if (options.prNumber) {
95 const open = await provider.getPullRequests('open')
96 pullRequest = open.find(candidate => candidate.number === options.prNumber)
97
98 if (!pullRequest)
99 return `Could not find open pull request #${options.prNumber}.`
100 }
101
102 const runId = options.runId
103 ?? (pullRequest ? await resolveFailingRun(provider, pullRequest, logger) : null)
104
105 if (!runId) {
106 return pullRequest
107 ? 'Nothing is failing at the head of this pull request, so there is nothing to diagnose.'
108 : 'No run to diagnose. Pass --run-id, or --pr so I can find the failing run myself.'
109 }
110
111 const log = await provider.getWorkflowRunLogs(runId)
112 if (!log) {
113 logger.warn(`⚠️ Could not read the logs for run ${runId}; nothing to diagnose`)
114 return `Could not read the logs for run ${runId}. They may have expired.`
115 }
116
117 const priorAttempts = parseFixAttempts(pullRequest?.body)?.attempts ?? 0
118 const baseBranch = config.repository?.baseBranch ?? 'main'
119
120 // Capability-gated rather than assumed: a provider that cannot read a run
121 // history still gets the full diagnosis, it just cannot tell an inherited
122 // failure from a new one.
123 const canReadRuns = supports(provider, 'ciRuns', 'listWorkflowRuns')
124 const canRerun = supports(provider, 'ciRuns', 'rerunWorkflowRun')
125
126 const { createAiClient } = await import('../ai')
127
128 const outcome = await attemptFix({
129 log,
130 priorAttempts,
131 workspace,
132 baseBranch,
133 // The agent edits files. Where there is no branch to edit, it must not run
134 // at all rather than run against whatever happens to be checked out.
135 ai: options.analysisOnly ? null : createAiClient(config, logger),
136 logger,
137 dryRun: Boolean(options.dryRun),
138 analysisOnly: Boolean(options.analysisOnly),
139 ...(canReadRuns
140 ? {
141 checkBase: (kind: FailureKind) => failsOnBaseBranch(
142 {
143 listWorkflowRuns: (branch, listOptions) => provider.listWorkflowRuns!(branch, listOptions),
144 getWorkflowRunLogs: id => provider.getWorkflowRunLogs!(id),
145 },
146 baseBranch,
147 kind,
148 logger,
149 ),
150 }
151 : {}),
152 ...(canRerun ? { rerun: () => provider.rerunWorkflowRun!(runId) } : {}),
153 ...(options.analysisOnly ? {} : { regenerateLockfile: () => regenerateLockfile(workspace, logger) }),
154 })
155
156 logger.info(`\n${outcome.report}\n`)
157
158 if (options.prNumber && !options.dryRun) {
159 await provider.createComment(options.prNumber, outcome.report)
160
161 // Only a real attempt counts. Recording the ones the guard already refused
162 // would march the counter up without anything being tried.
163 if (outcome.action !== 'skipped' && pullRequest) {
164 try {
165 await provider.updatePullRequest(options.prNumber, {
166 body: upsertFixAttempts(pullRequest.body, priorAttempts + 1),
167 })
168 }
169 catch (error) {
170 logger.warn(`Could not record the fix attempt on PR #${options.prNumber}: ${formatError(error)}`)
171 }
172 }
173 }
174
175 return `${outcome.action}${outcome.fixed ? ' (fixed)' : ''}`
176}
177
178/**
179 * Regenerate every lock file in the workspace and land the result.
180 *
181 * @param workspace - Directory to regenerate in
182 * @param logger - Logger for per-manager failures
183 * @returns What was rewritten, and whether it reached the branch
184 */
185async function regenerateLockfile(workspace: string, logger: Logger): Promise<{ regenerated: boolean, pushed: boolean }> {
186 const { regenerateLockFile, detectRequiredPackageManagers, getAllLockFilePaths } = await import('../utils/lock-file')
187 const { commitAndPush } = await import('../utils/git')
188
189 // Detection keys off the manifests a lock file is derived from, so a
190 // repository with several ecosystems regenerates each of them.
191 const managers = detectRequiredPackageManagers(['package.json', 'composer.json'])
192 let regenerated = false
193
194 for (const manager of managers) {
195 const result = await regenerateLockFile(manager, workspace)
196 if (result.success)
197 regenerated = true
198 else
199 logger.warn(`⚠️ ${result.message}`)
200 }
201
202 if (!regenerated)
203 return { regenerated: false, pushed: false }
204
205 // Rewriting the file in a workspace that is about to be thrown away repairs
206 // nothing. The job checked this branch out with credentials, so the commit
207 // is a local one rather than a branch recreation.
208 try {
209 const pushed = await commitAndPush(
210 getAllLockFilePaths(),
211 'fix(deps): regenerate the lock file\n\nThe lock file had drifted from the manifest, which failed CI.',
212 workspace,
213 )
214 return { regenerated: true, pushed }
215 }
216 catch (error) {
217 logger.warn(`⚠️ Could not push the regenerated lock file: ${formatError(error)}`)
218 return { regenerated: true, pushed: false }
219 }
220}
src/commands/dispatcher.tsmodified+1-1
@@ -49,7 +49,7 @@ export const HELP_TEXT = `**Buddy commands**
4949| \`@buddy pause\` / \`resume\` | Stop or restart reviewing this pull request |
5050| \`@buddy rebase\` | Rebase a dependency update pull request |
5151| \`@buddy merge\` | Re-check auto-merge conditions now |
52| \`@buddy fix-ci\` | Diagnose and try to fix failing checks |
52| \`@buddy fix-ci\` | Diagnose the failing checks, and retry a flake |
5353| \`@buddy remember <text>\` | Record a durable note for future runs |
5454| \`@buddy help\` | Show this table |
5555
src/commands/handlers.tsmodified+15-10
@@ -13,6 +13,8 @@ export interface HandlerDeps {
1313 rebase: (prNumber: number) => Promise<string>
1414 /** Re-evaluate auto-merge conditions now */
1515 merge: () => Promise<number[]>
16 /** Diagnose the failing checks on a pull request */
17 fixCi: (prNumber: number) => Promise<string>
1618 /** Record a durable note */
1719 remember: (text: string, context: CommandContext) => Promise<string>
1820}
@@ -88,17 +90,20 @@ export function createHandlers(deps: HandlerDeps): Record<string, CommandHandler
8890 },
8991
9092 async 'fix-ci'(context): Promise<CommandOutcome> {
91 // This handler has no run to read. Diagnosis is entirely a function of
92 // the failing job's log, so calling attemptFix with an empty one only
93 // ever produced a confident-looking `unknown` classification with no
94 // evidence behind it. Saying so is more useful than that.
95 return {
96 handled: false,
97 reply: 'I diagnose a failure from its job log, and a comment does not tell me which run failed. '
98 + 'Buddy repairs failing checks automatically when the `fix-ci` job is enabled in your workflow — '
99 + 'it runs on the failed run itself and reports back here. '
100 + 'See https://buddy.sh/features/ci-repair',
93 // A comment cannot name a run, so this used to refuse outright. It can
94 // now find one: the newest failing run at the pull request's head.
95 if (!context.isPullRequest) {
96 return {
97 handled: false,
98 reply: 'There are no checks to look at — this is an issue, not a pull request.',
99 }
101100 }
101
102 // The diagnosis is posted as its own comment by the run itself, which is
103 // where the evidence and the log excerpt belong. Repeating it here would
104 // put the same wall of text on the thread twice.
105 const status = await deps.fixCi(context.number)
106 return { handled: true, reply: status }
102107 },
103108
104109 async remember(context): Promise<CommandOutcome> {
test/ci-run.test.tsadded+273-0
@@ -0,0 +1,273 @@
1import type { GitProvider, ListWorkflowRunsOptions, WorkflowRun } from '../src/git/provider'
2import type { PullRequest } from '../src/types'
3import { describe, expect, it } from 'bun:test'
4import { runFixCi } from '../src/ci/run'
5import { NO_CAPABILITIES } from '../src/git/provider'
6import { Logger } from '../src/utils/logger'
7
8const FLAKE_LOG = `error: request to https://registry.npmjs.org/react failed, reason: ECONNRESET`
9const TYPE_ERROR_LOG = `src/app.ts(12,5): error TS2322: Type 'string' is not assignable to type 'number'.`
10
11interface Recorded {
12 comments: Array<{ pr: number, body: string }>
13 updates: Array<{ pr: number, body?: string }>
14 rerun: number[]
15 logsRead: number[]
16}
17
18interface StubOptions {
19 runs?: WorkflowRun[]
20 /** Runs on the base branch; empty means the base is green */
21 baseRuns?: WorkflowRun[]
22 logs?: Record<number, string | null>
23 headSha?: string
24 prBody?: string
25 ciRuns?: boolean
26 openPrs?: boolean
27}
28
29/** A provider with just enough surface for a repair run, and a record of it. */
30function stub(options: StubOptions = {}): { provider: GitProvider, recorded: Recorded } {
31 const recorded: Recorded = { comments: [], updates: [], rerun: [], logsRead: [] }
32 const headSha = options.headSha ?? 'head1'
33
34 const pr = {
35 number: 7,
36 title: 'chore(deps): bump x',
37 body: options.prBody ?? 'description',
38 head: 'buddy/update-x',
39 base: 'main',
40 state: 'open',
41 } as PullRequest
42
43 const provider = {
44 capabilities: () => ({ ...NO_CAPABILITIES, ciLogs: true, ciRuns: options.ciRuns ?? true }),
45 getPullRequests: async () => (options.openPrs === false ? [] : [pr]),
46 getPullRequestHeadSha: async () => headSha,
47 // Branch-aware, because `failsOnBaseBranch` asks about `main` with the
48 // same call. A stub that answered identically for every branch would make
49 // every failure look inherited.
50 listWorkflowRuns: async (branch: string, _o?: ListWorkflowRunsOptions) =>
51 (branch === pr.head ? options.runs ?? [] : options.baseRuns ?? []),
52 getWorkflowRunLogs: async (id: number) => {
53 recorded.logsRead.push(id)
54 return options.logs?.[id] ?? null
55 },
56 rerunWorkflowRun: async (id: number) => {
57 recorded.rerun.push(id)
58 return true
59 },
60 createComment: async (prNumber: number, body: string) => {
61 recorded.comments.push({ pr: prNumber, body })
62 },
63 updatePullRequest: async (prNumber: number, patch: { body?: string }) => {
64 recorded.updates.push({ pr: prNumber, body: patch.body })
65 return pr
66 },
67 } as unknown as GitProvider
68
69 return { provider, recorded }
70}
71
72function run(overrides: Partial<WorkflowRun> = {}): WorkflowRun {
73 return {
74 id: 1,
75 name: 'CI',
76 headSha: 'head1',
77 headBranch: 'buddy/update-x',
78 status: 'completed',
79 conclusion: 'failure',
80 createdAt: new Date('2026-01-01T00:00:00Z'),
81 ...overrides,
82 }
83}
84
85const base = { config: {}, logger: Logger.silent() }
86
87/**
88 * `@buddy fix-ci` used to refuse outright — "a comment does not tell me which
89 * run failed". It can be told now, by looking at what is red at the head.
90 */
91describe('runFixCi', () => {
92 describe('finding the run', () => {
93 it('success case - resolves the failing run from a pull request', async () => {
94 const { provider, recorded } = stub({
95 runs: [run({ id: 55 })],
96 logs: { 55: TYPE_ERROR_LOG },
97 })
98
99 await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true })
100
101 expect(recorded.logsRead).toContain(55)
102 })
103
104 it('success case - an explicit run id wins over the lookup', async () => {
105 const { provider, recorded } = stub({ logs: { 99: TYPE_ERROR_LOG } })
106
107 await runFixCi({ ...base, provider, runId: 99, prNumber: 7, analysisOnly: true })
108
109 expect(recorded.logsRead).toEqual([99])
110 })
111
112 it('failure case - ignores a run from an older commit', async () => {
113 // A branch keeps its old runs. Diagnosing one from three pushes ago
114 // produces a confident report about a failure somebody already fixed.
115 const { provider } = stub({
116 runs: [run({ id: 55, headSha: 'stale' })],
117 logs: { 55: TYPE_ERROR_LOG },
118 headSha: 'head1',
119 })
120
121 const status = await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true })
122
123 expect(status).toContain('Nothing is failing')
124 })
125
126 it('success case - a green head is reported as nothing to do', async () => {
127 const { provider } = stub({ runs: [run({ conclusion: 'success' })] })
128
129 expect(await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true }))
130 .toContain('Nothing is failing')
131 })
132
133 it('edge case - only the newest run of each workflow counts', async () => {
134 // A workflow that failed and has since gone green is not failing now.
135 const { provider } = stub({
136 runs: [
137 run({ id: 9, name: 'CI', conclusion: 'success' }),
138 run({ id: 8, name: 'CI', conclusion: 'failure' }),
139 ],
140 logs: { 8: TYPE_ERROR_LOG },
141 })
142
143 expect(await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true }))
144 .toContain('Nothing is failing')
145 })
146
147 it('edge case - a provider that cannot list runs says what it needs', async () => {
148 const { provider } = stub({ ciRuns: false })
149
150 expect(await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true }))
151 .toContain('Nothing is failing')
152 })
153
154 it('failure case - a missing pull request is reported, not thrown', async () => {
155 const { provider } = stub({ openPrs: false })
156
157 expect(await runFixCi({ ...base, provider, prNumber: 7 })).toContain('Could not find')
158 })
159
160 it('failure case - no run and no pull request explains both ways in', async () => {
161 const { provider } = stub()
162
163 expect(await runFixCi({ ...base, provider })).toContain('--run-id')
164 })
165
166 it('edge case - expired logs are reported rather than diagnosed blank', async () => {
167 const { provider } = stub({ runs: [run({ id: 55 })], logs: { 55: null } })
168
169 expect(await runFixCi({ ...base, provider, prNumber: 7 })).toContain('expired')
170 })
171 })
172
173 describe('answering from a comment', () => {
174 it('success case - diagnoses without touching the workspace', async () => {
175 // The comment job checks out the default branch, so a repair here would
176 // change the wrong tree entirely.
177 const { provider, recorded } = stub({
178 runs: [run({ id: 55 })],
179 logs: { 55: 'error: lockfile had changes, but lockfile is frozen' },
180 })
181
182 const status = await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true })
183
184 expect(status).toBe('reported')
185 expect(recorded.comments[0].body).toContain('Regenerating the lock file would fix this')
186 expect(recorded.comments[0].body).toContain('wrong tree')
187 })
188
189 it('success case - still re-runs a flake, which needs no checkout', async () => {
190 const { provider, recorded } = stub({ runs: [run({ id: 55 })], logs: { 55: FLAKE_LOG } })
191
192 const status = await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true })
193
194 expect(recorded.rerun).toEqual([55])
195 expect(status).toBe('retry')
196 })
197
198 it('success case - names who will apply the fix it cannot', async () => {
199 const { provider, recorded } = stub({ runs: [run({ id: 55 })], logs: { 55: TYPE_ERROR_LOG } })
200
201 await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true })
202
203 expect(recorded.comments[0].body).toContain('`fix-ci` job')
204 })
205 })
206
207 describe('reporting and the attempt counter', () => {
208 it('success case - posts the report to the pull request', async () => {
209 const { provider, recorded } = stub({ runs: [run({ id: 55 })], logs: { 55: TYPE_ERROR_LOG } })
210
211 await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true })
212
213 expect(recorded.comments[0].pr).toBe(7)
214 expect(recorded.comments[0].body).toContain('CI failure analysis')
215 })
216
217 it('success case - records the attempt on the pull request', async () => {
218 const { provider, recorded } = stub({ runs: [run({ id: 55 })], logs: { 55: TYPE_ERROR_LOG } })
219
220 await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true })
221
222 expect(recorded.updates[0].body).toContain('buddy:fix-ci')
223 })
224
225 it('failure case - a refused attempt does not spend one', async () => {
226 // Marching the counter up for attempts the guard already refused would
227 // exhaust it without anything having been tried.
228 const { provider, recorded } = stub({
229 runs: [run({ id: 55 })],
230 logs: { 55: TYPE_ERROR_LOG },
231 prBody: 'desc\n\n<!-- buddy:fix-ci v1\n{"attempts":3}\n-->',
232 })
233
234 const status = await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true })
235
236 expect(status).toBe('skipped')
237 expect(recorded.updates).toHaveLength(0)
238 })
239
240 it('success case - a dry run posts nothing and spends nothing', async () => {
241 const { provider, recorded } = stub({ runs: [run({ id: 55 })], logs: { 55: TYPE_ERROR_LOG } })
242
243 await runFixCi({ ...base, provider, prNumber: 7, dryRun: true, analysisOnly: true })
244
245 expect(recorded.comments).toHaveLength(0)
246 expect(recorded.updates).toHaveLength(0)
247 })
248 })
249 describe('inherited failures, end to end', () => {
250 it('success case - declines a failure the base branch already has', async () => {
251 const { provider, recorded } = stub({
252 runs: [run({ id: 55 })],
253 baseRuns: [run({ id: 60, headBranch: 'main' })],
254 logs: { 55: TYPE_ERROR_LOG, 60: TYPE_ERROR_LOG },
255 })
256
257 const status = await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true })
258
259 expect(status).toBe('skipped')
260 expect(recorded.comments[0].body).toContain('base branch')
261 })
262
263 it('success case - a differently-broken base does not excuse this failure', async () => {
264 const { provider } = stub({
265 runs: [run({ id: 55 })],
266 baseRuns: [run({ id: 60, headBranch: 'main' })],
267 logs: { 55: TYPE_ERROR_LOG, 60: 'error: lockfile had changes, but lockfile is frozen' },
268 })
269
270 expect(await runFixCi({ ...base, provider, prNumber: 7, analysisOnly: true })).toBe('reported')
271 })
272 })
273})
test/command-handlers.test.tsadded+103-0
@@ -0,0 +1,103 @@
1import type { HandlerDeps } from '../src/commands/handlers'
2import type { CommandContext } from '../src/commands/dispatcher'
3import type { GitProvider } from '../src/git/provider'
4import { describe, expect, it } from 'bun:test'
5import { createHandlers } from '../src/commands/handlers'
6import { Logger } from '../src/utils/logger'
7
8/** Deps that record what a handler routed to, with everything stubbed. */
9function deps(overrides: Partial<HandlerDeps> = {}): HandlerDeps & { calls: string[] } {
10 const calls: string[] = []
11
12 return {
13 calls,
14 config: {},
15 provider: {} as GitProvider,
16 review: async (pr) => {
17 calls.push(`review:${pr}`)
18 return 'reviewed'
19 },
20 rebase: async (pr) => {
21 calls.push(`rebase:${pr}`)
22 return 'rebased'
23 },
24 merge: async () => {
25 calls.push('merge')
26 return []
27 },
28 fixCi: async (pr) => {
29 calls.push(`fixCi:${pr}`)
30 return 'reported'
31 },
32 remember: async () => 'noted',
33 ...overrides,
34 }
35}
36
37function context(overrides: Partial<CommandContext> = {}): CommandContext {
38 return {
39 command: { name: 'fix-ci', args: '', raw: '@buddy fix-ci' },
40 actor: { login: 'maintainer', canWrite: true, isBot: false },
41 number: 42,
42 isPullRequest: true,
43 logger: Logger.quiet(),
44 ...overrides,
45 }
46}
47
48/**
49 * The handlers had no tests at all, which is part of why `fix-ci` sat returning
50 * `handled: false` with a message explaining why it could not work.
51 */
52describe('command handlers', () => {
53 describe('fix-ci', () => {
54 it('success case - diagnoses the pull request it was asked on', async () => {
55 const dependencies = deps()
56 const outcome = await createHandlers(dependencies)['fix-ci'](context())
57
58 expect(outcome.handled).toBe(true)
59 expect(dependencies.calls).toEqual(['fixCi:42'])
60 })
61
62 it('failure case - an issue has no checks to look at', async () => {
63 const dependencies = deps()
64 const outcome = await createHandlers(dependencies)['fix-ci'](context({ isPullRequest: false }))
65
66 expect(outcome.handled).toBe(false)
67 expect(outcome.reply).toContain('issue, not a pull request')
68 expect(dependencies.calls).toEqual([])
69 })
70
71 it('success case - reports the status rather than repeating the diagnosis', async () => {
72 // The run posts its own comment carrying the evidence and the log
73 // excerpt. Echoing it here would put the same wall of text on the
74 // thread twice.
75 const outcome = await createHandlers(deps())['fix-ci'](context())
76
77 expect(outcome.reply).toBe('reported')
78 expect(outcome.reply).not.toContain('CI failure analysis')
79 })
80 })
81
82 describe('routing', () => {
83 it('success case - review and rebase reach their machinery', async () => {
84 const dependencies = deps()
85 const handlers = createHandlers(dependencies)
86
87 await handlers.review(context({ command: { name: 'review', args: '', raw: '' } }))
88 await handlers.rebase(context({ command: { name: 'rebase', args: '', raw: '' } }))
89
90 expect(dependencies.calls).toEqual(['review:42', 'rebase:42'])
91 })
92
93 it('failure case - reviewing an issue is refused before any work', async () => {
94 const dependencies = deps()
95 const outcome = await createHandlers(dependencies).review(
96 context({ command: { name: 'review', args: '', raw: '' }, isPullRequest: false }),
97 )
98
99 expect(outcome.handled).toBe(false)
100 expect(dependencies.calls).toEqual([])
101 })
102 })
103})