test(autofix-pr): re-run a timed-out job and lose its answer in the chain test (#16138)

The fake gh only re-ran FAILURE jobs and applied loseNextAnswer to comment
POSTs only, so the helpers' handling of a timed-out job's re-run and of a
re-run whose answer is lost could not be reached from the chain test.

- `run rerun` re-runs FAILURE and TIMED_OUT jobs, refuses a run with no
  failed job, and with loseNextAnswer queues the jobs then fails the way gh
  words a non-403 error.
- A job conclusion the fake does not list is refused as unsupported, on
  every path that reads a run.
- FAKE_GH_WORLD, and the names derived from it, are read after the call is
  logged and inside the guard, so a missing or malformed world shows as
  unsupported instead of a bare crash.
- New case: a timed-out job whose re-run's answer is lost is left for a
  person; when the re-run times out again, rerun refuses a second one, and
  status lists both. The re-run's attempt key is pinned, here and in the
  re-run case.
- Design §12 (both languages) lists the conflict case and the new case.
This commit is contained in:
Shaojin Wen authored and GitHub committed 2026-11-03 09:17:45 +08:00
1 parent 212fedbdc7
commit a6f1885833
4 files changed
+150 -38

No files matched your search

+7 -4
View File
@@ -1839,10 +1839,13 @@ Validation:
`schedule` printed it; `status` and `cleanup` — against real git with a
bare remote, the test making the fix agent's commits. Its cases: a watch
from start to merge, including a reply whose answer was lost; a push by
someone else during a round; a re-run spent, then refused; a check left
for a person after three pushes, one head's result asked of GitHub by
commit; and whether the PR's author can push, asked of GitHub. Each helper
is also tested on its own, with `gh` stubbed in-process.
someone else during a round; a re-run spent, then refused; a job that
timed out re-run once, a re-run whose answer was lost counted; a check
left for a person after three pushes, one head's result asked of GitHub
by commit; a conflict left for a person once per head, a head the watch
pushed before GitHub computed it included; and whether the PR's author
can push, asked of GitHub. Each helper is also tested on its own, with
`gh` stubbed in-process.
- Manual end-to-end on a sandbox repository: a failing test is fixed and
pushed; a collaborator comment is fixed and answered; a stranger's comment is
ignored; merging the PR ends the loop and removes the worktree.
@@ -555,7 +555,7 @@ resync 时如果新 head 不包含 worktree 的 `HEAD`(作者 rebase 或 force
- 用录制的 `gh` JSON fixture 做单元测试:事件对比、旧 SHA 上的过时检查、各信任类别、回复标记和 id 的自我排除、动作优先级、预算;闸门游标规则(每个 head 一次、重跑后的失败、冲突标记、当前用户自己的评论、合并与重新打开)和 check 汇总翻页;评审者探针的事件、过滤项和游标清理。
- 用本地 bare 仓库测试推送保护:拒绝非 fast-forward、拒绝默认分支、别人推送后 lease 被拒。
- 在 `PATH` 上放一个假的 `gh` 做集成测试(`autofix-pr/chain.integration.test.ts`):按监视运行它们的顺序依次运行各辅助命令——`prepare` 和 `schedule`;一轮里的 `sync`、`snapshot`、`ci-log`、`rerun`、`push` 和 `reply`;按 `schedule` 打印的原样执行的 gate;`status` 和 `cleanup`——用真实 git 和裸远端仓库,修复 agent 的提交由测试代为完成。用例:从启动到合入的一次监视,其中有一次回复的应答丢失;一轮进行中别人推送;重跑用掉一次后被拒;三次推送后仍失败的检查交给人,其中一个 head 的结果按提交向 GitHub 查询;向 GitHub 查询 PR 作者能否推送。每个辅助命令另有各自的测试,`gh` 在进程内打桩。
- 在 `PATH` 上放一个假的 `gh` 做集成测试(`autofix-pr/chain.integration.test.ts`):按监视运行它们的顺序依次运行各辅助命令——`prepare` 和 `schedule`;一轮里的 `sync`、`snapshot`、`ci-log`、`rerun`、`push` 和 `reply`;按 `schedule` 打印的原样执行的 gate;`status` 和 `cleanup`——用真实 git 和裸远端仓库,修复 agent 的提交由测试代为完成。用例:从启动到合入的一次监视,其中有一次回复的应答丢失;一轮进行中别人推送;重跑用掉一次后被拒;超时的 job 只重跑一次,应答丢失的那次重跑也计数;三次推送后仍失败的检查交给人,其中一个 head 的结果按提交向 GitHub 查询;冲突每个 head 只交给人一次,监视自己推送、GitHub 还没算出结果的 head 也算在内;向 GitHub 查询 PR 作者能否推送。每个辅助命令另有各自的测试,`gh` 在进程内打桩。
- 在沙箱仓库上手工端到端验证:失败的测试被修复并推送;协作者的评论被修复并回复;陌生人的评论被忽略;合并 PR 后循环结束,worktree 被删除。
验收标准:
@@ -15,9 +15,9 @@
// missing as it would be. Anything this file does not know — a command, a
// flag, a path, a jq expression, a `--json` field, a GraphQL field or
// alias, a host other than the world's — fails the call with exit 2 and is
// logged as unsupported, and so does a crash while answering. The test
// checks the log: some helpers take a failed call for "no answer" and go
// on, so the exit code alone would not show it.
// logged as unsupported, and so do a world it cannot read and a crash
// while answering. The test checks the log: some helpers take a failed
// call for "no answer" and go on, so the exit code alone would not show it.
import { execFileSync } from 'node:child_process';
import {
@@ -30,8 +30,6 @@ import process from 'node:process';
const ACTIONS_APP = 15368;
const worldPath = process.env['FAKE_GH_WORLD'];
const world = JSON.parse(readFileSync(worldPath, 'utf8'));
const args = process.argv.slice(2);
const stdin = args.includes('--input') ? readFileSync(0, 'utf8') : '';
const log = (entry) =>
@@ -50,6 +48,20 @@ function unsupported(what) {
fail(`fake gh: unsupported ${what}: gh ${args.join(' ')}`, 2);
}
// The world, and the names it gives the repository and the PR: read once
// the call is logged, so a world that cannot be read is logged too.
const worldPath = process.env['FAKE_GH_WORLD'];
let world, slug, repoUrl, pr, prUrl;
try {
world = JSON.parse(readFileSync(worldPath, 'utf8'));
slug = `${world.owner}/${world.repo}`;
repoUrl = `https://${world.host}/${slug}`;
pr = world.pr;
prUrl = `${repoUrl}/pull/${pr.number}`;
} catch (err) {
unsupported(`world ${worldPath}: ${err.message}`);
}
function print(value) {
process.stdout.write(
typeof value === 'string' ? `${value}\n` : `${JSON.stringify(value)}\n`,
@@ -60,6 +72,15 @@ function save() {
writeFileSync(worldPath, `${JSON.stringify(world, null, 2)}\n`);
}
// A write is saved, then answered: with `loseNextAnswer`, it reaches GitHub
// and its answer is lost on the way back.
function saveWrite(lostMessage) {
const lost = world.loseNextAnswer;
world.loseNextAnswer = false;
save();
if (lost) fail(lostMessage);
}
function git(...gitArgs) {
return execFileSync('git', ['--git-dir', world.bare, ...gitArgs], {
encoding: 'utf8',
@@ -75,11 +96,6 @@ function isAncestor(a, b) {
}
}
const slug = `${world.owner}/${world.repo}`;
const repoUrl = `https://${world.host}/${slug}`;
const pr = world.pr;
const prUrl = `${repoUrl}/pull/${pr.number}`;
function headSha() {
return git('rev-parse', `refs/heads/${pr.headRefName}`);
}
@@ -91,6 +107,12 @@ function nextId() {
// --- checks -----------------------------------------------------------------
// The conclusions of a job that failed: a re-run with `--failed` re-runs
// them. (It re-runs a cancelled job too, a conclusion this file does not
// model.)
const FAILED_JOB = new Set(['FAILURE', 'TIMED_OUT']);
const JOB_CONCLUSIONS = new Set(['SUCCESS', ...FAILED_JOB]);
// The latest attempt of each job of a run: a re-run appends new jobs to it.
function latestJobs(run) {
const latest = new Map();
@@ -109,21 +131,30 @@ function runStatus(run) {
function runConclusion(run) {
if (runStatus(run) !== 'COMPLETED') return null;
return latestJobs(run).some((job) => job.conclusion === 'FAILURE')
return latestJobs(run).some((job) => FAILED_JOB.has(job.conclusion))
? 'FAILURE'
: 'SUCCESS';
}
// A run with no job is one GitHub could not start, which this file does
// not model.
function runsOn(sha) {
const runs = world.runs.filter((run) => run.sha === sha);
// A run with no job is one GitHub could not start, and a job conclusion
// it does not list is one the helpers read their own way: this file models
// neither.
function modelled(runs) {
if (runs.some((run) => run.jobs.length === 0)) {
unsupported('a run with no job');
}
for (const job of runs.flatMap((run) => run.jobs)) {
if (job.status === 'COMPLETED' && !JOB_CONCLUSIONS.has(job.conclusion)) {
unsupported(`job conclusion ${job.conclusion}`);
}
}
return runs;
}
function runsOn(sha) {
return modelled(world.runs.filter((run) => run.sha === sha));
}
function contextNodes(sha) {
return runsOn(sha).flatMap((run) =>
run.jobs.map((job) => ({
@@ -172,9 +203,9 @@ function suiteNodes(sha) {
}
function findRun(id) {
return (
world.runs.find((run) => run.id === Number(id)) ?? unsupported('run id')
);
const run =
world.runs.find((r) => r.id === Number(id)) ?? unsupported('run id');
return modelled([run])[0];
}
// --- comments ---------------------------------------------------------------
@@ -509,13 +540,8 @@ function api() {
let m;
if (method === 'POST') {
const body = JSON.parse(stdin).body;
// `loseNextAnswer`: the POST reaches GitHub and its answer is lost on
// the way back.
const answered = (response) => {
const lost = world.loseNextAnswer;
world.loseNextAnswer = false;
save();
if (lost) fail('gh: HTTP 502: Bad Gateway');
saveWrite('gh: HTTP 502: Bad Gateway');
print(response);
};
if ((m = /^issues\/(\d+)\/comments$/.exec(route))) {
@@ -587,7 +613,8 @@ function api() {
}
// `gh run rerun <id> --failed -R <slug>`: a new attempt of each failed job,
// queued.
// queued. gh words an answer other than a 403 as "failed to rerun: <the
// HTTP error>", and an error page that is not JSON by its status line.
function runRerun() {
const [, , id, failed, r, repo, ...rest] = args;
if (failed !== '--failed' || r !== '-R' || rest.length > 0) {
@@ -598,11 +625,14 @@ function runRerun() {
if (runStatus(run) !== 'COMPLETED') {
fail(`run ${run.id} cannot be rerun; This workflow is already running`);
}
for (const job of latestJobs(run)) {
if (job.conclusion !== 'FAILURE') continue;
const jobs = latestJobs(run).filter((job) => FAILED_JOB.has(job.conclusion));
if (jobs.length === 0) unsupported('a re-run of a run with no failed job');
for (const job of jobs) {
run.jobs.push({ id: nextId(), name: job.name, status: 'QUEUED' });
}
save();
saveWrite(
`failed to rerun: HTTP 502: 502 Bad Gateway (https://${world.host}/api/v3/repos/${slug}/actions/runs/${run.id}/rerun-failed-jobs)`,
);
}
// Every call goes to the host the watch was prepared for: one that drops it
@@ -65,7 +65,7 @@ const FAKE_GH = fileURLToPath(
new URL('./__fixtures__/fake-gh.mjs', import.meta.url),
);
type Conclusion = 'SUCCESS' | 'FAILURE';
type Conclusion = 'SUCCESS' | 'FAILURE' | 'TIMED_OUT';
interface Job {
id: number;
@@ -120,7 +120,8 @@ interface World {
comments: Comment[];
}>;
nextId: number;
/** The next comment POST reaches GitHub, and its answer is lost. */
/** The next write (a comment, a re-run) reaches GitHub, and its answer
* is lost. */
loseNextAnswer?: boolean;
}
@@ -270,7 +271,7 @@ describe.skipIf(process.platform === 'win32')(
}
/** A workflow run on `sha`: a job per name, finished unless it is
* `IN_PROGRESS`. */
* `IN_PROGRESS`, and a failed one with `log`. */
function ci(
sha: string,
runId: number,
@@ -289,7 +290,7 @@ describe.skipIf(process.platform === 'win32')(
...(result === 'IN_PROGRESS'
? { status: 'IN_PROGRESS' as const }
: { status: 'COMPLETED' as const, conclusion: result }),
...(result === 'FAILURE' ? { log } : {}),
...(result === 'FAILURE' || result === 'TIMED_OUT' ? { log } : {}),
})),
});
});
@@ -742,7 +743,7 @@ describe.skipIf(process.platform === 'win32')(
const { events } = await snapshot('--task', id);
expect(events).toMatchObject([{ type: 'ci_failed', runId: 11 }]);
const key = events[0]['key'] as string;
expect(key).not.toBe(`test@${head}@#110`);
expect(key).toBe(`test@${head}@#100001`);
await cli('ci-log', '7', '11', '--task', id);
const [refused] = await cli('rerun', '7', '11', '--task', id);
expect(refused).toMatchObject({ rerun: false, checks: ['test'] });
@@ -771,6 +772,84 @@ describe.skipIf(process.platform === 'win32')(
expect(checkoutState()).toBe(checkoutBefore);
});
it('re-runs a job that timed out once, counting the re-run whose answer was lost', async () => {
const head = prHead();
const timedOut =
'The job has exceeded the maximum execution time of 10m0s\n';
ci(head, 12, { e2e: 'TIMED_OUT', lint: 'SUCCESS' }, timedOut);
await prepare();
const { gate: gateCommand, task: id } = await schedule();
// The start's round: the job hung, and the fix agent answered
// `rerun`. GitHub re-runs it, and its answer is lost on the way back.
await cli('sync', '7', '--task', id);
const first = `e2e@${head}@#120`;
expect((await snapshot('--task', id)).events).toMatchObject([
{ type: 'ci_failed', check: 'e2e', key: first },
]);
expect(await cli('ci-log', '7', '12', '--task', id)).toMatchObject([
{ jobs: ['e2e'] },
]);
updateWorld((world) => {
world.loseNextAnswer = true;
});
await expect(cli('rerun', '7', '12', '--task', id)).rejects.toThrow(
/\(or its answer was lost; the re-run is counted\): .*failed to rerun: HTTP 502/,
);
// Not re-run for sure: the skill leaves it for a person.
const lost = writePlan({
summary: { body: 'e2e timed out; its re-run got no answer.' },
handledChecks: [first],
needsHuman: [`${first}: no answer to its re-run`],
});
expect(await cli('reply', '7', lost, '--task', id)).toMatchObject([
{ posted: [{ kind: 'summary' }], handledChecks: 1 },
]);
expect(await gate(gateCommand)).toEqual([]);
// The re-run went ahead all the same, and timed out again.
finish(12, 'e2e', 'TIMED_OUT', timedOut);
// The gate wakes a round, and the re-run whose answer was lost was
// the head's one.
expect(await gate(gateCommand)).toMatchObject([
{ action: 'fix', events: [{ type: 'ci_failed', runId: 12 }] },
]);
await cli('sync', '7', '--task', id);
const { events } = await snapshot('--task', id);
expect(events).toMatchObject([{ type: 'ci_failed', check: 'e2e' }]);
// The re-run's own attempt, the job it queued.
const key = events[0]['key'] as string;
expect(key).toBe(`e2e@${head}@#100001`);
await cli('ci-log', '7', '12', '--task', id);
const [refused] = await cli('rerun', '7', '12', '--task', id);
expect(refused).toMatchObject({ rerun: false, checks: ['e2e'] });
expect(refused['reason']).toMatch(/re-run on this head already/);
const plan = writePlan({
summary: { body: 'e2e times out again after a re-run.' },
handledChecks: [key],
needsHuman: [`${key}: times out again after a re-run`],
});
expect(await cli('reply', '7', plan, '--task', id)).toMatchObject([
{ posted: [{ kind: 'summary' }], handledChecks: 1 },
]);
expect(await gate(gateCommand)).toEqual([]);
expect(await cli('status', '7')).toMatchObject([
{
watches: [{ pushes: 0, needsHuman: [{ sha: head }, { sha: head }] }],
},
]);
expect(await cli('cleanup', '7')).toMatchObject([{ removed: true }]);
expect(writes()).toEqual([
'run rerun 12 --failed -R o/r',
'api --method POST repos/o/r/issues/7/comments --input -',
'api --method POST repos/o/r/issues/7/comments --input -',
]);
expectPushes([]);
expect(checkoutState()).toBe(checkoutBefore);
});
it('leaves a check for a person once three pushes of the watch failed it, asking GitHub about a head no round read', async () => {
const head = prHead();
ci(head, 11, { test: 'FAILURE' }, 'expected 3, got 4\n');