Skip to content

Commit 67b8d57

Browse files
committed
Keep update context for retries and diff against the recorded main commit
1 parent c45a1be commit 67b8d57

5 files changed

Lines changed: 156 additions & 21 deletions

File tree

‎.github/prompts/update-docs.md‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,13 @@ If you changed anything, run `mint broken-links` from the docs checkout and fix
8282
broken links your changes introduce, and run `jq empty docs.json` if you edited it.
8383
Do not run `mint dev`.
8484

85-
Read the complete diff against origin/main before finishing. It includes changes from
86-
earlier releases still on this branch. Remove filler, unrelated changes, and anything
87-
that does not follow AGENTS.md.
85+
Read the complete diff against `$MAIN_SHA` before finishing. That is the main commit this
86+
branch was rebuilt on; `origin/main` may have moved since, so do not compare against it.
87+
The diff includes changes from earlier releases still on this branch. Remove filler,
88+
unrelated changes, and anything that does not follow AGENTS.md.
8889

8990
Write `$RUNNER_TEMP/docs-update-report.md`. It replaces the PR description, so it
90-
must describe every change in the diff against origin/main, not only this run's.
91+
must describe every change in the diff against `$MAIN_SHA`, not only this run's.
9192
Carry forward entries from `$RUNNER_TEMP/previous-report.md` that still match the
9293
diff. Use concise sections:
9394

‎.github/prompts/update-mcp-coverage.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Do not downgrade or change that version.
99

1010
1. Read `$RUNNER_TEMP/sdk-diff.patch`, `../sdk/api.md`, and the parameter types in
1111
`../sdk/src/resources`. The diff covers every release since the SDK version on
12-
origin/main, so earlier runs may already have covered part of it on this branch.
12+
`$MAIN_SHA`, so earlier runs may already have covered part of it on this branch.
1313
Enumerate SDK methods and their parameters, including
1414
nested options, new enum values, pagination, streaming, and changed signatures.
1515
2. Read `src/lib/mcp/register.ts`, `src/lib/mcp/tools/`, and shared MCP schemas.
@@ -33,7 +33,7 @@ Do not downgrade or change that version.
3333

3434
The Kernel API spec as of this SDK release is at
3535
`$RUNNER_TEMP/kernel-api/packages/api/openapi.yaml`, and `$RUNNER_TEMP/openapi-diff.patch`
36-
shows spec changes between origin/main's SDK release and this one. The SDK is generated
36+
shows spec changes between `$MAIN_SHA`'s SDK release and this one. The SDK is generated
3737
from this spec, so its endpoint, parameter, and schema descriptions are the source of
3838
truth for what the API does.
3939

@@ -72,11 +72,13 @@ Redis is available on localhost:6379, matching MCP CI. Do not disable tests or w
7272
checks to make this update pass. Fix failures caused by your changes. Do not edit
7373
unrelated code, workflows, generated App bundles, or authentication configuration.
7474

75-
Read the complete diff against origin/main before finishing. Remove unnecessary
75+
Read the complete diff against `$MAIN_SHA` before finishing. That is the main commit this
76+
branch was rebuilt on; `origin/main` may have moved since, so do not compare against it.
77+
Remove unnecessary
7678
comments, type casts, defensive wrappers, and unrelated changes.
7779

7880
Write `$RUNNER_TEMP/mcp-coverage-report.md`. It replaces the PR description, so it
79-
must describe every change in the diff against origin/main, including changes earlier
81+
must describe every change in the diff against `$MAIN_SHA`, including changes earlier
8082
runs left on this branch, not only this run's. Use concise sections describing:
8183

8284
- Added or updated MCP tools, actions, and fields, with their SDK counterparts.

‎.github/tests/update-coverage-security.test.mjs‎

Lines changed: 119 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,14 @@ const updaters = [
2828
file: 'update-mcp-coverage.yml',
2929
agentStep: 'Update SDK dependency and MCP coverage',
3030
context: 'mcp-coverage-context',
31+
prompt: 'update-mcp-coverage.md',
32+
},
33+
{
34+
file: 'update-docs.yml',
35+
agentStep: 'Update docs',
36+
context: 'sdk-docs-context',
37+
prompt: 'update-docs.md',
3138
},
32-
{ file: 'update-docs.yml', agentStep: 'Update docs', context: 'sdk-docs-context' },
3339
];
3440

3541
const mcp = load('update-mcp-coverage.yml');
@@ -52,7 +58,22 @@ const runScript = (entry) => {
5258
.join('\n');
5359
};
5460

55-
for (const { file, agentStep, context } of updaters) {
61+
// Evaluates the job `if:` expressions these workflows use: always(), needs results
62+
// and outputs, string literals, ==, != and &&.
63+
const evaluate = (expression, needs) => {
64+
const js = expression
65+
.replace(/\balways\(\)/g, 'true')
66+
.replace(/needs\.([\w-]+)\.result/g, (_, job) => JSON.stringify(needs[job].result))
67+
.replace(/needs\.([\w-]+)\.outputs\.([\w-]+)/g, (_, job, name) =>
68+
JSON.stringify(needs[job].outputs?.[name] ?? ''),
69+
)
70+
.replace(/'([^']*)'/g, (_, value) => JSON.stringify(value))
71+
.replace(/([!=])=/g, '$1==');
72+
assert.match(js, /^(?:\s|"[^"]*"|true|===|!==|&&)+$/, `unsupported expression: ${expression}`);
73+
return Function(`return (${js});`)();
74+
};
75+
76+
for (const { file, agentStep, context, prompt } of updaters) {
5677
const { jobText, jobs, steps, step } = load(file);
5778

5879
test(`${file}: actions are pinned and checkouts do not persist credentials`, () => {
@@ -135,14 +156,11 @@ for (const { file, agentStep, context } of updaters) {
135156
jobText.context,
136157
/context_artifact_id: \$\{\{ steps\.upload-context\.outputs\.artifact-id \}\}/,
137158
);
138-
assert.match(
139-
jobText.cleanup,
140-
/needs: \[context, generate, publish\]\n\s+if: always\(\) && needs\.context\.outputs\.context_artifact_id != ''/,
141-
);
159+
assert.match(jobText.cleanup, /needs: \[context, generate, publish\]\n/);
142160
assert.match(jobText.cleanup, /permissions:\n\s+actions: write\n\s+steps:/);
143161
assert.match(
144162
step('cleanup', 'Delete update context'),
145-
/ARTIFACT_ID: \$\{\{ needs\.context\.outputs\.context_artifact_id \}\}\n\s+run: gh api --method DELETE "repos\/\$GITHUB_REPOSITORY\/actions\/artifacts\/\$ARTIFACT_ID"/,
163+
/ARTIFACT_ID: \$\{\{ needs\.context\.outputs\.context_artifact_id \}\}\n[\s\S]*gh api --method DELETE "repos\/\$GITHUB_REPOSITORY\/actions\/artifacts\/\$ARTIFACT_ID"/,
146164
);
147165
assert.doesNotMatch(jobText.cleanup, /secrets\./);
148166

@@ -213,6 +231,95 @@ for (const { file, agentStep, context } of updaters) {
213231
}
214232
});
215233

234+
test(`${file}: retries keep the context until generate succeeds and tolerate a deleted context`, () => {
235+
// "Re-run failed jobs" does not rerun a successful context job, so generate
236+
// needs the context from the first attempt. Publish never downloads it.
237+
assert.doesNotMatch(jobText.publish, new RegExp(context));
238+
const condition = jobText.cleanup.match(/^ if: (.*)$/m)[1];
239+
const script = runScript(step('cleanup', 'Delete update context'));
240+
const dir = mkdtempSync(join(tmpdir(), 'cleanup-'));
241+
try {
242+
const store = join(dir, 'artifacts');
243+
mkdirSync(join(dir, 'bin'));
244+
writeFileSync(
245+
join(dir, 'bin/gh'),
246+
`#!/usr/bin/env bash
247+
id="\${@: -1}"
248+
id="\${id##*/}"
249+
if grep -qx "$id" "$STORE"; then
250+
grep -vx "$id" "$STORE" > "$STORE.new" || true
251+
mv "$STORE.new" "$STORE"
252+
exit 0
253+
fi
254+
if [[ -n "$SERVER_ERROR" ]]; then
255+
echo "gh: Server Error (HTTP 500)" >&2
256+
exit 1
257+
fi
258+
echo '{"message":"Not Found","status":"404"}'
259+
echo "gh: Not Found (HTTP 404)" >&2
260+
exit 1
261+
`,
262+
{ mode: 0o755 },
263+
);
264+
const stored = () => readFileSync(store, 'utf8').split('\n').includes('42');
265+
const attempt = (generate, publish, env = {}, artifactId = '42') => {
266+
const needs = {
267+
context: { result: 'success', outputs: { context_artifact_id: artifactId } },
268+
generate: { result: generate },
269+
publish: { result: publish },
270+
};
271+
if (!evaluate(condition, needs)) return null;
272+
return spawnSync('bash', ['-e', '-c', script], {
273+
encoding: 'utf8',
274+
env: {
275+
...process.env,
276+
PATH: `${join(dir, 'bin')}:${process.env.PATH}`,
277+
STORE: store,
278+
ARTIFACT_ID: artifactId,
279+
GITHUB_REPOSITORY: 'kernel/example',
280+
...env,
281+
},
282+
});
283+
};
284+
285+
// Generation fails or is cancelled: the context stays for the retry, which then deletes it.
286+
writeFileSync(store, '42\n');
287+
assert.equal(attempt('failure', 'skipped'), null);
288+
assert.equal(attempt('cancelled', 'skipped'), null);
289+
assert.ok(stored());
290+
let run = attempt('success', 'success');
291+
assert.equal(run.status, 0, run.stderr);
292+
assert.ok(!stored());
293+
294+
// Publish fails after generate used the context: it is deleted, and the retry tolerates that.
295+
writeFileSync(store, '42\n');
296+
run = attempt('success', 'failure');
297+
assert.equal(run.status, 0, run.stderr);
298+
assert.ok(!stored());
299+
run = attempt('success', 'success');
300+
assert.equal(run.status, 0, run.stderr);
301+
assert.match(run.stdout, /already deleted/);
302+
303+
run = attempt('success', 'success', { SERVER_ERROR: '1' });
304+
assert.notEqual(run.status, 0);
305+
assert.match(run.stderr, /HTTP 500/);
306+
assert.equal(attempt('success', 'skipped', {}, ''), null);
307+
} finally {
308+
rmSync(dir, { recursive: true, force: true });
309+
}
310+
});
311+
312+
test(`${file}: the agent compares against the main commit the branch was rebuilt on`, () => {
313+
const text = readFileSync(new URL(`../prompts/${prompt}`, import.meta.url), 'utf8');
314+
assert.match(
315+
step('generate', agentStep),
316+
new RegExp(`@\\.\\./\\.github/prompts/${prompt.replace('.', '\\.')}`),
317+
);
318+
assert.match(text, /diff against `\$MAIN_SHA`/);
319+
assert.doesNotMatch(text.replaceAll('`origin/main` may have moved since', ''), /origin\/main/);
320+
assert.match(jobText.generate, /\n MAIN_SHA: \$\{\{ needs\.context\.outputs\.main_sha \}\}\n/);
321+
});
322+
216323
test(`${file}: generate rebuilds exactly the branch the context job prepared`, () => {
217324
const script = runScript(step('generate', 'Rebuild update branch'));
218325
const dir = mkdtempSync(join(tmpdir(), 'rebuild-'));
@@ -259,6 +366,9 @@ for (const { file, agentStep, context } of updaters) {
259366
assert.equal(git('rev-parse', 'HEAD^{tree}'), rebasedTree);
260367
assert.equal(rebased.output, `agent_base=${git('rev-parse', 'HEAD')}\n`);
261368

369+
// origin/main can move after the context job; the agent diffs against MAIN_SHA.
370+
git('switch', '-q', '--detach', main);
371+
git('update-ref', 'refs/remotes/origin/main', commit('d.mdx', 'd\n', 'main moved again'));
262372
const fromMain = run({
263373
REBASED: '',
264374
UPDATE_SHA: update,
@@ -267,6 +377,8 @@ for (const { file, agentStep, context } of updaters) {
267377
});
268378
assert.equal(fromMain.status, 0, fromMain.stderr);
269379
assert.equal(git('rev-parse', 'HEAD'), main);
380+
assert.equal(git('diff', '--name-only', main), '');
381+
assert.equal(git('diff', '--name-only', 'origin/main'), 'd.mdx');
270382

271383
const moved = run({
272384
REBASED: 'true',

‎.github/workflows/update-docs.yml‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -521,10 +521,12 @@ jobs:
521521
--title "$title" --body-file "$body"
522522
fi
523523
524-
# Runs whether or not publish does, so the context never outlives the run.
524+
# Deletes the context once generate has used it, whether or not publish runs.
525+
# After a failed or cancelled generate the context is kept until it expires, so
526+
# re-running failed jobs can still download it.
525527
cleanup:
526528
needs: [context, generate, publish]
527-
if: always() && needs.context.outputs.context_artifact_id != ''
529+
if: always() && needs.generate.result == 'success' && needs.context.outputs.context_artifact_id != ''
528530
runs-on: ubuntu-latest
529531
timeout-minutes: 5
530532
permissions:
@@ -534,4 +536,12 @@ jobs:
534536
env:
535537
GH_TOKEN: ${{ github.token }}
536538
ARTIFACT_ID: ${{ needs.context.outputs.context_artifact_id }}
537-
run: gh api --method DELETE "repos/$GITHUB_REPOSITORY/actions/artifacts/$ARTIFACT_ID"
539+
run: |
540+
# A re-run after a failed publish finds the context already deleted.
541+
if ! error=$(gh api --method DELETE "repos/$GITHUB_REPOSITORY/actions/artifacts/$ARTIFACT_ID" 2>&1); then
542+
if [[ "$error" != *"(HTTP 404)"* ]]; then
543+
echo "$error" >&2
544+
exit 1
545+
fi
546+
echo "The update context was already deleted."
547+
fi

‎.github/workflows/update-mcp-coverage.yml‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,10 +483,12 @@ jobs:
483483
--title "$title" --body-file "$body"
484484
fi
485485
486-
# Runs whether or not publish does, so the context never outlives the run.
486+
# Deletes the context once generate has used it, whether or not publish runs.
487+
# After a failed or cancelled generate the context is kept until it expires, so
488+
# re-running failed jobs can still download it.
487489
cleanup:
488490
needs: [context, generate, publish]
489-
if: always() && needs.context.outputs.context_artifact_id != ''
491+
if: always() && needs.generate.result == 'success' && needs.context.outputs.context_artifact_id != ''
490492
runs-on: ubuntu-latest
491493
timeout-minutes: 5
492494
permissions:
@@ -496,4 +498,12 @@ jobs:
496498
env:
497499
GH_TOKEN: ${{ github.token }}
498500
ARTIFACT_ID: ${{ needs.context.outputs.context_artifact_id }}
499-
run: gh api --method DELETE "repos/$GITHUB_REPOSITORY/actions/artifacts/$ARTIFACT_ID"
501+
run: |
502+
# A re-run after a failed publish finds the context already deleted.
503+
if ! error=$(gh api --method DELETE "repos/$GITHUB_REPOSITORY/actions/artifacts/$ARTIFACT_ID" 2>&1); then
504+
if [[ "$error" != *"(HTTP 404)"* ]]; then
505+
echo "$error" >&2
506+
exit 1
507+
fi
508+
echo "The update context was already deleted."
509+
fi

0 commit comments

Comments
 (0)