ci: check custom-code budgets against main's policy - #2447
Conversation
apcha-oai
added
the
custom-code-burndown
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Castiron custom code✅ No new custom-code files detected. 32 mixed files remain; 2 existing customizations changed. Compared
30 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 32506622536 --repo openai/openai-node \
--name castiron-custom-code-32506622536-1 --dir /tmp/castiron-custom-code-32506622536-1
git apply --stat /tmp/castiron-custom-code-32506622536-1/custom-code.patch
cat /tmp/castiron-custom-code-32506622536-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin d1410d190d047be6aa3969a86d043648fdf378fe c87c1d95dc4ab3355912808158425568f1f2d2d8
python3 scripts/castiron/custom_code_report.py report \
--base d1410d190d047be6aa3969a86d043648fdf378fe \
--head c87c1d95dc4ab3355912808158425568f1f2d2d8 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-c87c1d95dc4a
cat /tmp/castiron-custom-code-c87c1d95dc4a/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Please address the inline P2: enforce monotonic source workflow run/attempt ordering before publishing commit statuses so older reruns cannot replace newer successful merge-gate results. Trusted policy isolation and budget calculation otherwise look sound.
| 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.'; | ||
| await github.rest.repos.createCommitStatus({...context.repo, sha: head, context: name, |
There was a problem hiding this comment.
P2 — Prevent older source workflows from overwriting newer commit statuses. This publisher unconditionally calls createCommitStatus for the same SHA/context without comparing the source workflow run ID and run attempt with the most recent published result. If an older source run is rerun or handlers complete out of order, its transient failure can replace a newer successful status and incorrectly block the PR or merge queue; branch-level concurrency serializes handlers but does not establish source-run freshness. Apply the same monotonic (run_id, run_attempt) guard used by the comment publisher before writing either status, and add an out-of-order/rerun regression.
There was a problem hiding this comment.
We're accepting this fail-closed availability edge case for the initial rollout and leaving the implementation unchanged.
Older source reruns recompute against current trusted inputs; they don't replay stale budget results. A later transient evaluation failure can replace green statuses with red and bounce an otherwise valid candidate, but this is recoverable by rerunning the latest workflow or requeuing. The merge queue does not prevent that false block; it remains the boundary for checking the combined candidate against current main once the budget statuses are required.
We don't think source-run ordering state is justified without evidence of recurring disruption in normal queue operation. In particular, source run ID alone is not policy freshness: an older source run can legitimately be rerun against a newer main/policy revision. This also keeps the rollout aligned with the approved Python workflow.
We'll revisit if this becomes a recurring operational problem. No code, budget, ruleset, or review-state changes are being made as part of this response.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at the reviewer's request.
972b8dc to
c87c1d9
Compare
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| import subprocess | ||
| import sys | ||
| import tempfile | ||
| import unittest |
## Summary Set a 3,000-line ceiling on additions **plus** deletions in the remaining custom patch against verified generated output. This is the budget-only foundation for the same rollout approved in [Python #3714](openai/openai-python#3714) and [Python #3715](openai/openai-python#3715). The verified current patch is **+1,589 / −181 = 1,770 lines**, leaving **1,230 lines** of headroom. This is a generous bound on accidental growth, not a target or permission to add custom code. Only `.castiron-ratchet.json` changes here. The stacked tooling PR will use the base-branch allowance, so a PR cannot raise its own budget. Future increases require a separate budget-only PR, explicit justification, and a human approving review; agents cannot approve them. No workflow, CODEOWNERS, or ruleset changes are included. Ruleset enforcement is a separate, later decision. Tooling companion: openai#2447
2 participants
Summary
Catch accidental growth in the SDK's remaining generated-file customization before it becomes a large patch to maintain. This ports the same layout and tooling approved in Python #3714 and Python #3715, including Python's trusted report computation.
The existing workflow pair measures additions plus deletions against verified generated output, using the checker and budget from main. A PR cannot increase its own allowance; budget changes must be isolated and explicitly justified, and increases require a human approving review. Agents cannot approve or bypass increases.
Technical details live in
scripts/castiron/CUSTOM_CODE.md, with references from AGENTS and CONTRIBUTING. No extra workflow files, SDK API changes, checkpoint edits, exclusion changes, or CODEOWNERS changes. Ruleset enforcement is intentionally deferred.The integrated tooling measures +1,955 / −208 = 2,163 custom lines, below the 3,000-line ceiling. This includes the reporter changes themselves.
Stack
The budget-only foundation has merged. This tooling commit is now restacked onto
mainatd1410d190d04; the tooling patch is unchanged and the PR contains no budget-file change. CI is rerunning against main. The trusted statuses become active after the tooling lands, but will not be required until a separately authorized ruleset rollout.