Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: minor
---

### Changed

- A pr-verify FAIL first uploads one index artifact per tie the merge gate counts (`.github/actions/upload-fail-index`), and uploads only once every index did; a job without the index step writes no FAIL. The gate reads those exact names instead of the whole artifact listing once the legacy FAILs file is committed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
bump: minor
---

### Added

- The pr-merge verdict reader has an index mode: once `verify-legacy-fails.json` sits beside it, a judgment finds every FAIL by exact-name reads of the FAIL index (`pr-verify-index-pr-<n>`, `-sha-<head>`, `-ref-<digest>`, `-issue-<i>`) and the runs they name, instead of paging the whole artifact listing. Any read that fails or misses its own count refuses.
- `babysit_verify_evidence.py legacy-fails --merge-sha <sha>` builds that file. It reads every pr-verify run in one-day `created` windows up to the cutover the merge commit sets, and each run's artifacts, each read holding its own `total_count`. It then adds every run a whole artifact listing names, and refuses when a listed FAIL is still missing. Its calls are paced, capped per rate-limit window and resumable from a checkpoint.
136 changes: 136 additions & 0 deletions .github/actions/upload-fail-index/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
name: Upload a FAIL's index artifacts
description: >
Upload one small artifact per name in `names` (write-verdict.mjs
failIndexNames: the pull request, the head, the head branch's digest and each
issue the FAIL records), each holding `path`, so the pr-merge lane's reader
finds every FAIL by exact-name lookups instead of the repository's whole
artifact listing (babysit_verify_evidence.py `Reader._targeted`). The FAIL's
upload must require this step's success: a FAIL with no index is one that
reader cannot see, and a failed index upload leaves no verdict at all. More
names than slots fail the step. A name the run already holds (an earlier
attempt's upload) fails the step too, so a re-run attempt writes no FAIL:
replacing the name would delete the earlier attempt's pointer first, and
keeping it would leave a new FAIL that outlives its index.

inputs:
names:
description: The index artifact names, one per line (at most 13).
required: true
path:
description: The file each index artifact holds.
required: true
retention-days:
description: The FAIL's retention, which each index keeps.
required: true
github-token:
description: A token that can read this run's artifacts (actions read).
required: true

runs:
using: composite
steps:
- name: Split the names
id: slots
shell: bash
env:
NAMES: ${{ inputs.names }}
RETENTION: ${{ inputs.retention-days }}
GH_TOKEN: ${{ inputs.github-token }}
REPO: ${{ github.repository }}
RUN_ID: ${{ github.run_id }}
run: |
case "$RETENTION" in
[1-9] | [1-8][0-9] | 90) echo "retention-days=$RETENTION" >>"$GITHUB_OUTPUT" ;;
*)
echo "::error::retention-days is not 1 to 90"
exit 1
;;
esac
mapfile -t names < <(printf '%s\n' "$NAMES" | sed '/^$/d')
if [ "${#names[@]}" -lt 3 ] || [ "${#names[@]}" -gt 13 ]; then
echo "::error::${#names[@]} FAIL index names, not 3 to 13"
exit 1
fi
# The names this run holds already, every attempt's, in one read
# that must hold its own count.
if ! body=$(gh api "repos/$REPO/actions/runs/$RUN_ID/artifacts?per_page=100"); then
echo "::error::the run's artifacts could not be read"
exit 1
fi
if ! jq -e '.total_count == (.artifacts | length) and .total_count < 100' <<<"$body" >/dev/null; then
echo "::error::the run's artifacts do not hold their count"
exit 1
fi
mapfile -t held < <(jq -r '.artifacts[].name' <<<"$body")
slot=0
for name in "${names[@]}"; do
case "$name" in
pr-verify-index-[a-z]*-[0-9a-f]*) ;;
*)
echo "::error::a FAIL index name no reader takes"
exit 1
;;
esac
found=
for have in "${held[@]}"; do
if [ "$have" = "$name" ]; then found=1; fi
done
if [ -n "$found" ]; then
echo "::error::$name is held by an earlier attempt of this run, so this attempt writes no FAIL"
exit 1
fi
Comment thread
kyle-sexton marked this conversation as resolved.
Resolved
slot=$((slot + 1))
echo "n$slot=$name" >>"$GITHUB_OUTPUT"
done
- name: Upload index 1
if: ${{ steps.slots.outputs.n1 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n1 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 2
if: ${{ steps.slots.outputs.n2 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n2 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 3
if: ${{ steps.slots.outputs.n3 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n3 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 4
if: ${{ steps.slots.outputs.n4 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n4 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 5
if: ${{ steps.slots.outputs.n5 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n5 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 6
if: ${{ steps.slots.outputs.n6 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n6 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 7
if: ${{ steps.slots.outputs.n7 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n7 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 8
if: ${{ steps.slots.outputs.n8 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n8 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 9
if: ${{ steps.slots.outputs.n9 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n9 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 10
if: ${{ steps.slots.outputs.n10 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n10 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 11
if: ${{ steps.slots.outputs.n11 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n11 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 12
if: ${{ steps.slots.outputs.n12 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n12 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
- name: Upload index 13
if: ${{ steps.slots.outputs.n13 != '' }}
uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with: { name: "${{ steps.slots.outputs.n13 }}", path: "${{ inputs.path }}", retention-days: "${{ steps.slots.outputs.retention-days }}", if-no-files-found: error }
11 changes: 11 additions & 0 deletions .github/artifact-retention.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
{
"$schema": "./artifact-retention.schema.json",
"families": {
"pr-verify-index-*": {
"retention-days": "max",
"readers": [
"plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py Reader._targeted finds every FAIL by these exact names instead of the whole artifact listing"
],
"max-reader": "plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py",
"reason": "Each index artifact is how the gate finds its FAIL, so it keeps the FAIL's own retention (write-verdict.mjs retentionDays, max for a FAIL): an index that expired first would hide a FAIL that still counts. It holds only the FAIL's artifact name, built from numbers and a SHA, and no model text, so no leak scan runs over it.",
"scan": "none",
"computed-by": ".github/scripts/pr-verify/write-verdict.mjs",
"uploads": [".github/actions/upload-fail-index/action.yml#runs"]
},
"pr-verify-*": {
"retention-days": "max",
"readers": [
Expand Down
60 changes: 58 additions & 2 deletions .github/scripts/pr-verify/workflow.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,44 @@ const upstreamRun = (overrides = {}) => ({
},
});

// upload-fail-index's "Split the names" step, run with a stand-in gh that
// answers the run's artifacts read with `held` (or fails), returning its exit
// status and the outputs it wrote.
const ACTION = readFileSync(new URL("../../actions/upload-fail-index/action.yml", import.meta.url), "utf8");
const splitNames = (names, held, { total, fail = false } = {}) => {
const script = /\n {6}run: \|\n([\s\S]*?)\n {4}- name: /.exec(ACTION)[1].replace(/^ {8}/gm, "");
const dir = tempDir("upload-fail-index-");
const bin = path.join(dir, "bin");
mkdirSync(bin);
const body = JSON.stringify({ total_count: total ?? held.length, artifacts: held.map((name) => ({ name })) });
writeFileSync(path.join(bin, "gh"), fail ? "#!/bin/sh\nexit 1\n" : `#!/bin/sh\ncat <<'JSON'\n${body}\nJSON\n`, { mode: 0o755 });
const out = path.join(dir, "out");
writeFileSync(out, "");
const run = spawnSync("bash", ["--noprofile", "--norc", "-eo", "pipefail", "-c", script], {
env: { PATH: `${bin}:${process.env.PATH}`, NAMES: names.join("\n"), RETENTION: "90", GITHUB_OUTPUT: out, REPO: "o/r", RUN_ID: "1", GH_TOKEN: "t" },
encoding: "utf8",
});
return { status: run.status, outputs: readFileSync(out, "utf8") };
};

test("a FAIL index never replaces or keeps a name its run already holds", () => {
const names = ["pr-verify-index-pr-41", `pr-verify-index-sha-${"a".repeat(40)}`, "pr-verify-index-ref-0123456789abcdef"];
assert.equal(splitNames(names, []).outputs, `retention-days=90\n${names.map((n, i) => `n${i + 1}=${n}`).join("\n")}\n`);
// A re-run attempt: an earlier attempt's index name fails the step, so no
// new FAIL outlives that index, and nothing replaces it.
const rerun = splitNames(names, [names[0], "pr-verify-41-x-fail"]);
assert.notEqual(rerun.status, 0);
assert.doesNotMatch(rerun.outputs, /^n\d+=/m);
// Another name the run holds (the verdict's own) uploads every index.
assert.equal(splitNames(names, ["pr-verify-41-x-fail"]).outputs, `retention-days=90\n${names.map((n, i) => `n${i + 1}=${n}`).join("\n")}\n`);
// A read that fails or does not hold its count uploads nothing.
assert.notEqual(splitNames(names, [], { fail: true }).status, 0);
assert.notEqual(splitNames(names, [names[0]], { total: 2 }).status, 0);
assert.doesNotMatch(ACTION, /overwrite/);
assert.equal((ACTION.match(/uses: actions\/upload-artifact@/g) ?? []).length, 13);
assert.equal((ACTION.match(/if: \$\{\{ steps\.slots\.outputs\.n\d+ != '' \}\}/g) ?? []).length, 13);
});

test("the upload is each job's only artifact step and follows the written verdict", () => {
for (const job of JOBS) {
const uploads = STEPS[job].filter((s) => /uses: actions\/upload-artifact@/.test(s.block));
Expand All @@ -125,19 +163,37 @@ test("the upload is each job's only artifact step and follows the written verdic
assert.ok(index("Judge the pull request", job) >= 0 && index("Write the verdict", job) > index("Judge the pull request", job), job);
// The written verdict is scanned just before it uploads, and uploads only on the scan's clean output.
assert.ok(index("Scan the upload for credentials", job) > index("Write the verdict", job), job);
// A FAIL's index uploads before the scan, and the verdict is scanned and
// uploads only when it is no FAIL or every index uploaded
// (write-verdict.mjs failIndexNames).
assert.equal(index("Upload the FAIL index", job), index("Write the verdict", job) + 1, job);
assert.equal(index("Scan the upload for credentials", job), index("Upload the FAIL index", job) + 1, job);
assert.equal(index("Upload the verdict", job), index("Scan the upload for credentials", job) + 1, job);
const indexed = "steps\\.verdict\\.outcome == 'success' && \\(steps\\.verdict\\.outputs\\.verdict != 'fail' \\|\\| steps\\.fail-index\\.outcome == 'success'\\)";
const scan = step("Scan the upload for credentials", job);
assert.match(scan, /^ {8}id: scan-upload\n {8}if: >-\n {10}\$\{\{ !cancelled\(\) && steps\.verdict\.outcome == 'success' \}\}$/m, job);
assert.match(scan, new RegExp(`^ {8}id: scan-upload\\n {8}if: >-\\n {10}\\$\\{\\{ !cancelled\\(\\) && ${indexed} \\}\\}$`, "m"), job);
assert.match(scan, /^ {10}path: \$\{\{ runner\.temp \}\}\/pr-verify-verdict\/verdict\.json$/m, job);
const failIndex = step("Upload the FAIL index", job);
assert.match(
failIndex,
/^ {8}id: fail-index\n {8}if: >-\n {10}\$\{\{ !cancelled\(\) && steps\.verdict\.outcome == 'success' && steps\.verdict\.outputs\.verdict == 'fail' \}\}$/m,
job,
);
assert.match(failIndex, /^ {8}uses: \.\/\.base\/\.github\/actions\/upload-fail-index /m, job);
assert.equal(input(failIndex, "names"), "${{ steps.verdict.outputs.index-names }}", job);
assert.equal(input(failIndex, "retention-days"), "${{ steps.verdict.outputs.retention-days }}", job);
assert.equal(input(failIndex, "github-token"), "${{ github.token }}", job);
assert.match(
uploads[0].block,
/^ {8}if: >-\n {10}\(!cancelled\(\) && steps\.verdict\.outcome == 'success'\) && steps\.scan-upload\.outputs\.clean == 'true'$/m,
new RegExp(`^ {8}if: >-\\n {10}\\(!cancelled\\(\\) && ${indexed}\\) && steps\\.scan-upload\\.outputs\\.clean == 'true'$`, "m"),
job,
);
// The verdict step names the retention (write-verdict.mjs retentionDays).
assert.equal(input(uploads[0].block, "retention-days"), "${{ steps.verdict.outputs.retention-days }}", job);
// A pass's retention follows this job's resolver (resolve-pr.mjs relinkedSince).
assert.match(step("Write the verdict", job), /^ {10}RELINK_SINCE_HEAD: \$\{\{ steps\.pr\.outputs\.relink-since-head \}\}$/m, job);
// Only a job with the index step may write a FAIL (write-verdict.mjs FAIL_INDEX).
assert.match(step("Write the verdict", job), /^ {10}FAIL_INDEX: "1"$/m, job);
// A verdict needs a judge step that succeeded, which run-claude-cli never
// reports with a lane-class, so a credit stop writes none.
assert.match(step("Write the verdict", job), /^ {8}if: \$\{\{ !cancelled\(\) && \(/m, job);
Expand Down
34 changes: 33 additions & 1 deletion .github/scripts/pr-verify/write-verdict.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,24 @@ export function artifactName(verdict) {
return name;
}

// The exact names under which a FAIL's index artifacts upload, one per tie the
// merge gate's reader looks up (babysit_verify_evidence.INDEX_PREFIX and
// `Reader._targeted`): the pull request, the head, the head branch's digest,
// then each issue the FAIL's name carries (`artifactName`), as the reader ties
// a FAIL only to those. The API's name filter matches whole names across runs,
// so each lookup returns every FAIL so tied without the whole listing. None
// starts like a verdict name, which the reader would hold.
export const INDEX_PREFIX = "pr-verify-index-";
export function failIndexNames(verdict) {
const issues = NAME.exec(artifactName(verdict))?.[4]?.split(".") ?? [];
return [
`${INDEX_PREFIX}pr-${verdict.pr}`,
`${INDEX_PREFIX}sha-${verdict.head_sha}`,
`${INDEX_PREFIX}ref-${headRefDigest(verdict.head_ref)}`,
...issues.map((n) => `${INDEX_PREFIX}issue-${n}`),
];
}

// How many days the verdict's artifact lasts. A PASS or PASS-NOTES judged
// after no body edit or issue connect or disconnect since the head commit
// (resolve-pr.mjs relinkedSince, RELINK_SINCE_HEAD `false`) is read only at
Expand Down Expand Up @@ -802,7 +820,21 @@ export function main(env = process.env) {
mkdirSync(env.VERDICT_DIR, { recursive: true });
writeFileSync(path.join(env.VERDICT_DIR, "verdict.json"), `${JSON.stringify(verdict, null, 2)}\n`);
const name = artifactName(verdict);
appendFileSync(env.GITHUB_OUTPUT, `name=${name}\nverdict=${verdict.verdict}\nretention-days=${retentionDays(verdict.verdict, env.RELINK_SINCE_HEAD !== "false")}\n`);
// A FAIL's index (failIndexNames): each artifact holds the FAIL's name. A
// job without FAIL_INDEX (a re-run of a pr-verify.yml from before the
// index, on this script from main) has no index step, so it writes no FAIL.
if (verdict.verdict === "fail" && env.FAIL_INDEX !== "1") {
throw new Error("a FAIL needs its index step (FAIL_INDEX), which this job lacks");
}
const index = verdict.verdict === "fail" ? failIndexNames(verdict) : [];
if (index.length > 0) {
writeFileSync(path.join(env.VERDICT_DIR, "fail-index.txt"), `${name}\n`);
}
appendFileSync(
env.GITHUB_OUTPUT,
`name=${name}\nverdict=${verdict.verdict}\nretention-days=${retentionDays(verdict.verdict, env.RELINK_SINCE_HEAD !== "false")}\n` +
`index-names<<INDEX_NAMES\n${index.join("\n")}\nINDEX_NAMES\n`,
);
results.forEach(({ k, output: one }) => console.log(`::notice title=pr-verify chunk::pr=${verdict.pr} chunk=${k}/${count} verdict=${one?.verdict ?? "none"}`));
console.log(`write-verdict: ${name}`);
}
Expand Down
Loading
Loading