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
40 changes: 30 additions & 10 deletions .github/workflows/castiron-custom-code-comment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ permissions: {}
concurrency:
group: castiron-custom-code-comment-${{ github.event.workflow_run.head_repository.id }}-${{ github.event.workflow_run.head_branch }}
cancel-in-progress: false
queue: max # Preserve newer pending evaluations if older runs arrive out of order.
Comment thread
apcha-oai marked this conversation as resolved.

jobs:
compute:
Expand All @@ -31,13 +32,15 @@ jobs:
isolation: ${{ steps.budget.outputs.isolation }}
budget: ${{ steps.budget.outputs.budget }}
steps:
# Selecting main here pins both the executable checker and PR policy base.
# Subsequent steps use this checkout's SHA even if main advances.
- name: Check out the trusted reporter
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
ref: main
persist-credentials: false

- name: Compute from the current pull request Git objects
- name: Compute from the selected main and pull request Git objects
id: report
if: github.event.workflow_run.event == 'pull_request'
env:
Expand All @@ -53,6 +56,7 @@ jobs:
run: |
python3 -I scripts/castiron/custom_code_report.py trusted-report \
--repo "$RUNNER_TEMP/castiron-objects.git" \
--base "$(git rev-parse HEAD)" \
--repository "$REPOSITORY" --run-id "$RUN_ID" --run-attempt "$RUN_ATTEMPT" \
--out "$RUNNER_TEMP/castiron-custom-code"
if test -f "$RUNNER_TEMP/castiron-custom-code/context.json"; then
Expand Down Expand Up @@ -133,17 +137,18 @@ jobs:
HEAD_SHA: ${{ needs.compute.outputs.head-sha }}
ISOLATION_RESULT: ${{ needs.compute.outputs.isolation }}
BUDGET_RESULT: ${{ needs.compute.outputs.budget }}
PUBLISH_ATTEMPT: ${{ github.run_attempt }}
with:
script: |
const event = context.payload.workflow_run;
const {data: run} = await github.rest.actions.getWorkflowRun({...context.repo, run_id: event.id});
if (run.head_sha !== event.head_sha || run.run_attempt !== event.run_attempt ||
if (run.event !== event.event || run.head_sha !== event.head_sha || run.run_attempt !== event.run_attempt ||
run.status !== 'completed' || run.path.split('@', 1)[0] !== '.github/workflows/castiron-custom-code.yml' ||
run.repository.full_name !== `${context.repo.owner}/${context.repo.repo}`) return;
const head = run.head_sha;
if (!/^[0-9a-f]{40}$/.test(head)) throw new Error('Invalid candidate SHA');
const {data: main} = await github.rest.git.getRef({...context.repo, ref: 'heads/main'});
const base = main.object.sha;
const base = process.env.BASE_SHA;
let fresh = /^[0-9a-f]{40}$/.test(base) && head === process.env.HEAD_SHA;
if (run.event === 'pull_request') {
let pulls = run.pull_requests.length ? run.pull_requests : await github.paginate(
github.rest.repos.listPullRequestsAssociatedWithCommit, {...context.repo, commit_sha: head});
Expand All @@ -153,25 +158,40 @@ jobs:
const current = [];
for (const pull of pulls) {
const {data: pr} = await github.rest.pulls.get({...context.repo, pull_number: pull.number});
if (pr.state === 'open' && pr.head.sha === head && pr.base.sha === base &&
if (pr.state === 'open' && pr.head.sha === head &&
pr.base.ref === 'main' && pr.base.repo.full_name === `${context.repo.owner}/${context.repo.repo}`) current.push(pr);
}
if (current.length !== 1) return;
} else if (run.event !== 'merge_group' || !run.head_branch.startsWith('gh-readonly-queue/main/')) {
return;
} else {
const {data: main} = await github.rest.git.getRef({...context.repo, ref: 'heads/main'});
fresh = fresh && base === main.object.sha;
}
const fresh = base === process.env.BASE_SHA && head === process.env.HEAD_SHA;
const url = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
// Concurrency serializes writes, but does not order runs or retries.
// Keep an older evaluation from overwriting a newer result for this head.
const order = [run.id, run.run_attempt, Number(context.runId), Number(process.env.PUBLISH_ATTEMPT)];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Order reruns by the new evaluation rather than source run ID

When the same PR head has source runs 123 and 124, rerunning run 123 after main advances produces attempt 2 and a new trusted evaluation, but this tuple still ranks it below run 124 because run.id is compared first. The guard therefore returns on run 124's existing marker and leaves its older-base statuses in place, even though the rerun just evaluated the current policy. Use the downstream evaluation/publication ordering (or otherwise compare the captured evaluations) before the original source-run ID so a later rerun of an older source run can publish its result.

Useful? React with 👍 / 👎.

const marker = `[evaluation ${order.join(':')}]`;
const statuses = await github.paginate(github.rest.repos.listCommitStatusesForRef, {...context.repo, ref: head});
const names = ['Castiron / budget-only change', 'Castiron / custom-code budget'];
for (const status of statuses) {
if (!names.includes(status.context) || status.creator?.login !== 'github-actions[bot]') continue;
const prior = status.description?.match(/\[evaluation (\d+):(\d+):(\d+):(\d+)\]$/);
if (!prior || status.target_url !== `${url.slice(0, url.lastIndexOf('/') + 1)}${prior[3]}`) continue;
const previous = prior.slice(1).map(Number);
const different = previous.findIndex((value, index) => value !== order[index]);
if (different !== -1 && previous[different] > order[different]) return;
}
for (const [name, result] of [
['Castiron / budget-only change', process.env.ISOLATION_RESULT],
['Castiron / custom-code budget', process.env.BUDGET_RESULT],
]) {
const state = fresh && result === 'success' ? 'success' : 'failure';
const description = !fresh ? 'Evaluation unavailable or base changed; rerun against current main.'
: state === 'success' ? 'Passed against main policy. See the trusted run summary.'
: 'Budget check failed. See the trusted run summary.';
const description = !fresh ? 'Evaluation unavailable or queue base changed; inspect the trusted run and rerun.'
: `${state === 'success' ? 'Passed' : 'Failed'} against main ${base.slice(0, 12)}. See the trusted run summary.`;
await github.rest.repos.createCommitStatus({...context.repo, sha: head, context: name,
state, description, target_url: url});
state, description: `${description} ${marker}`, target_url: url});
}
comment:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/castiron-custom-code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ concurrency:
cancel-in-progress: false

env:
REPORTER_SHA256: be56ac91a1fc757d5d842586ae364354f11e8f0ec4cc458c06af1d7bd54a12cf
REPORTER_SHA256: d92cc331cea526bb3230ea4b0c2430ad974338a8373134e0b07b3a1ad3535e2a

jobs:
queue-signal:
Expand Down
14 changes: 9 additions & 5 deletions scripts/castiron/CUSTOM_CODE.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ The custom-code workflow pair separates candidate execution from trusted checks:
- `castiron-custom-code.yml` runs proposed offline tests and the advisory report
on `pull_request` with read-only permissions.
- `castiron-custom-code-comment.yml` handles `workflow_run` from **main**. Its
read-only compute job runs main's reporter against candidate Git objects in a
read-only compute job captures the main checkout SHA and the source run's PR
head as one immutable evaluation pair. It runs that main reporter against Git objects in a
new bare repository, then reuses that verified report to check main's budget.
It never checks out, imports, installs, or executes candidate code.
- An unprivileged `merge_group` job in the first workflow only signals that a candidate
Expand All @@ -50,16 +51,19 @@ The custom-code workflow pair separates candidate execution from trusted checks:
grant the SDK PR a higher limit.

A separate publisher with no checkout attaches these statuses to the exact PR
head or merge-group SHA, after rechecking head/base freshness:
head or merge-group SHA, after rechecking the PR head/target or queue base freshness:

- `Castiron / budget-only change`
- `Castiron / custom-code budget`

The policy is read from the current base commit, not the PR or its merge base.
The policy is read from the captured main commit, not the PR or its merge base.
Reporter changes in a PR cannot change the checker executing on that PR. Missing
snapshots, invalid hashes, unavailable queue membership, and policy errors fail
closed. If main moves during evaluation, rerun the workflow; reruns check out the
new main. PR-head statuses are feedback at a point in time: a main update alone
closed. PR base metadata may lag main; it does not select the evaluation base.
If main moves during a PR evaluation, the result still publishes for the captured
pair, with the evaluated base recorded in the report and status. Merge-group checks
still require current main and independently evaluate the combined candidate.
PR-head statuses are feedback at a point in time: a main update alone
does not rerun them. The checker therefore fails unless main has an effective
**require merge queue** rule. The queue must recheck combined usage against current
main before merging; do not replace that protection with PR-head statuses alone.
Expand Down
16 changes: 11 additions & 5 deletions scripts/castiron/custom_code_budget.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,9 +282,9 @@ def github_evaluate(
if event["repository"]["full_name"] != repository:
raise ValueError("event repository mismatch")
branch = metadata["default_branch"]
main = report.require_sha(report.api("GET", f"{root}/git/ref/heads/{branch}")["object"]["sha"])
if report.require_sha(trusted_sha) != main:
raise ValueError("trusted checkout is stale; rerun against current main")
if branch != "main":
raise ValueError("budget gate requires main as the default branch")
main = report.require_sha(trusted_sha)
signal = event["workflow_run"]
run_id = signal["id"]
if type(run_id) is not int or run_id <= 0:
Expand All @@ -311,12 +311,18 @@ def github_evaluate(
and pull["head"]["sha"] == head
and pull["base"]["repo"]["full_name"] == repository
and pull["base"]["ref"] == branch
and pull["base"]["sha"] == main
):
current.append(number)
if len(current) != 1:
raise ValueError("source run must identify exactly one current PR targeting main")
elif run["event"] == "merge_group":
# Queue candidates independently validate actual current main. A PR's
# captured snapshot must never authorize a different merged candidate.
current_main = report.require_sha(
report.api("GET", f"{root}/git/ref/heads/{branch}")["object"]["sha"]
)
if main != current_main:
raise ValueError("trusted checkout is stale; rerun against current main")
if not run["head_branch"].startswith(f"gh-readonly-queue/{branch}/"):
raise ValueError("queue signal does not target main")
else:
Expand All @@ -335,7 +341,7 @@ def github_evaluate(
raise ValueError("only PR runs can reuse the trusted report")
measured = json.loads((trusted_report_dir / "report.json").read_text())
if measured["target_base_sha"] != main or measured["head_sha"] != head:
raise ValueError("trusted report is stale; rerun against current main")
raise ValueError("trusted report does not match the captured base/head")
if report.git(repo, "rev-parse", "--is-bare-repository").strip() != b"true":
raise ValueError("trusted report must use a bare object store")
measurement = (measured, (trusted_report_dir / "custom-code.patch").read_bytes())
Expand Down
28 changes: 18 additions & 10 deletions scripts/castiron/custom_code_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,7 @@ def render_report(
) -> str:
head = require_sha(report["head_sha"])
lines = [MARKER, "", "## Castiron custom code", ""]
lines.extend([f"Evaluated main: `{require_sha(report['target_base_sha'])}`.", ""])
if report.get("status") != "ok":
reason = html.escape(str(report.get("error", "Report could not be computed")))
lines.extend([f"⚠️ Report unavailable for `{head[:12]}`.", "", reason])
Expand Down Expand Up @@ -803,7 +804,8 @@ def publish_comment(
if (
pull["state"] != "open"
or pull["head"]["sha"] != report["head_sha"]
or pull["base"]["sha"] != report["target_base_sha"]
or pull["base"]["ref"] != "main"
or pull["base"]["repo"]["full_name"] != repository
):
return "Skipped stale report"
run = api("GET", f"{root}/actions/runs/{run_id}")
Expand Down Expand Up @@ -850,7 +852,8 @@ def publish_comment(
if (
pull["state"] != "open"
or pull["head"]["sha"] != report["head_sha"]
or pull["base"]["sha"] != report["target_base_sha"]
or pull["base"]["ref"] != "main"
or pull["base"]["repo"]["full_name"] != repository
):
return "Skipped stale report"
if found is not None:
Expand Down Expand Up @@ -893,9 +896,12 @@ def write_report(


def trusted_report(
repo: Path, repository: str, run_id: int, run_attempt: int, out: Path
repo: Path, repository: str, run_id: int, run_attempt: int, out: Path, *, base: str
) -> None:
"""Recompute from GitHub-associated Git objects, never from PR-produced artifacts."""
# The caller pins this to the trusted checkout selected from main. PR base
# metadata may lag behind main, and main may move again during computation.
base = require_sha(base)
if not REPOSITORY.fullmatch(repository) or min(run_id, run_attempt) <= 0:
raise ReportError("invalid GitHub report target")
root = f"repos/{repository}"
Expand All @@ -910,7 +916,7 @@ def trusted_report(
return
head = require_sha(run["head_sha"])
associated = associated_pulls(repository, run)
current: list[tuple[int, str]] = []
current: list[int] = []
for number in sorted({int(pr["number"]) for pr in associated}):
if number <= 0:
raise ReportError("invalid associated pull request")
Expand All @@ -919,13 +925,14 @@ def trusted_report(
pull["state"] == "open"
and pull["head"]["sha"] == head
and pull["base"]["repo"]["full_name"] == repository
and pull["base"]["ref"] == "main"
):
current.append((number, require_sha(pull["base"]["sha"])))
current.append(number)
if not current:
return
if len(current) != 1:
raise ReportError("workflow run has multiple current pull requests")
number, base = current[0]
number = current[0]
public = not api("GET", root)["private"]
# This must be a new, bare repository: no PR worktree, hooks, configuration,
# submodules, or Python imports can affect the trusted reporter.
Expand All @@ -935,9 +942,7 @@ def trusted_report(
git(repo, "fetch", "--quiet", "--no-tags", "origin", base, head)
write_report(repo, base, head, out, fetch=True, require_head_hash=True, public=public)
(out / "context.json").write_text(
json.dumps(
{"pr": number, "repository": repository, "run": run_id, "attempt": run_attempt}
)
json.dumps({"pr": number, "repository": repository, "run": run_id, "attempt": run_attempt})
+ "\n"
)

Expand All @@ -962,6 +967,7 @@ def main() -> int:
trusted.add_argument("--run-id", type=int, required=True)
trusted.add_argument("--run-attempt", type=int, required=True)
trusted.add_argument("--out", type=Path, required=True)
trusted.add_argument("--base", required=True, help="immutable main SHA of the trusted checkout")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep trusted-report compatible with queued workflow definitions

If a workflow_run created from the predecessor workflow is still queued when this commit reaches main, its YAML checks out ref: main at execution time but invokes trusted-report without --base. It will therefore execute this new parser and fail immediately because the newly required argument is absent, publishing unavailable/failure results for otherwise valid pending evaluations. Derive the checkout SHA when the option is omitted, or otherwise retain CLI compatibility until all runs using the old workflow definition have drained.

Useful? React with 👍 / 👎.

preparing = commands.add_parser("prepare-public")
preparing.add_argument("--source-repo", type=Path, required=True)
preparing.add_argument("--source-base", required=True)
Expand Down Expand Up @@ -996,7 +1002,9 @@ def main() -> int:
+ "\n"
)
elif args.command == "trusted-report":
trusted_report(args.repo, args.repository, args.run_id, args.run_attempt, args.out)
trusted_report(
args.repo, args.repository, args.run_id, args.run_attempt, args.out, base=args.base
)
elif args.command == "comment":
if args.report.stat().st_size > 5_000_000:
raise ReportError("report artifact is too large")
Expand Down
73 changes: 73 additions & 0 deletions scripts/castiron/custom_code_test_support.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
"""Small Git/checkpoint fixture shared by the offline Castiron tests."""

from __future__ import annotations

import base64
import json
import subprocess
import tempfile
import unittest
from pathlib import Path
from typing import Any

import custom_code_report as report

GENERATION = "550e8400-e29b-41d4-a716-446655440000"


class GitTestCase(unittest.TestCase):
def setUp(self) -> None: # pyright: ignore[reportImplicitOverride]
self.temporary = tempfile.TemporaryDirectory()
self.addCleanup(self.temporary.cleanup)
self.repo = Path(self.temporary.name)
self.git("init", "-q", "-b", "main")
self.git("config", "user.name", "Castiron test")
self.git("config", "user.email", "castiron@example.test")

def git(self, *args: str) -> str:
return subprocess.run(
["git", "-C", str(self.repo), *args], check=True, capture_output=True, text=True
).stdout.strip()

def write(self, path: str, body: str) -> None:
target = self.repo / path
target.parent.mkdir(parents=True, exist_ok=True)
target.write_text(body)

def commit(self, message: str = "fixture") -> str:
self.git("add", "-A")
self.git("commit", "-q", "--allow-empty", "-m", message)
return self.git("rev-parse", "HEAD")

def baseline(self) -> tuple[str, str]:
self.write("generated.py", "generated\n")
metadata = {
"generation_id": GENERATION,
"source_branch": "test",
"target": "openai-python",
"language": "python",
}
encoded = base64.b64encode(json.dumps(metadata).encode()).decode()
generated = self.commit(f"codegen\n\nGeneration metadata: {encoded}")
self.git("update-ref", "refs/remotes/origin/codegen/test", generated)
self.write(
".castiron.stats.yml",
f"schema_version: 1\ngeneration_id: {GENERATION}\ncodegen_sha: {generated}\ncodegen_hash: {report.hash_codegen_commit(self.repo, generated)}\n",
)
return generated, self.commit("integrated")


def source_run(head: str, event: str = "pull_request") -> dict[str, Any]:
return {
"id": 123,
"event": event,
"head_sha": head,
"head_branch": "gh-readonly-queue/main/pr-7-example" if event == "merge_group" else "sdk",
"repository": {"full_name": "openai/example"},
"head_repository": {"owner": {"login": "contributor"}},
"path": ".github/workflows/castiron-custom-code.yml",
"status": "completed",
"run_attempt": 1,
"pull_requests": [{"number": 3}] if event == "pull_request" else [],
"conclusion": "failure", # The candidate's result is deliberately ignored.
}
Loading
Loading