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
14 changes: 7 additions & 7 deletions .castiron.stats.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
schema_version: 1
generation_id: 13866b42-3537-41d4-9c12-f53214868c28
openapi_spec_hash: 4a02922518092801c43f2526a9ced0f5
openapi_transformed_spec_hash: b577364c156fa7f501df6fef87ae11ed
config_hash: 07f8531f9ff20bc9f5156b5a258c76ff
codegen_sha: e92d3226b583127c9519d4b89e52c20d6c596d0b
codegen_hash: 7afac60a97214c8b2cd5a6684367ee3e19efa7bd30856d197a78779278534669
public_codegen_sha: cf08e5bb49e8c3d1f4eadee5d4b517dd2a06cabc
generation_id: ecf0c665-7b5c-432c-8ca0-d26784190472
openapi_spec_hash: 23f0bfa58ad53fef090bea6a72a05dda
openapi_transformed_spec_hash: c631189d638b72705db583893fa22bb2
config_hash: 249b5f41fe1b916955299cea75cddfbf
codegen_sha: a42b98f1e77a430e69488201ad31243f9a938996
codegen_hash: 36d13008ea00041d76a078918a4b3cac9d4206d4f79cbdd222e65912a347ef33
public_codegen_sha: 4720178859e015fec0cbd3af1e1a7a4244a11434
99 changes: 37 additions & 62 deletions .github/workflows/castiron-custom-code-comment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,21 @@ jobs:
pull-requests: read
statuses: write
steps:
# Trusted main definition: no candidate checkout, artifacts, or executable inputs.
# Execute only the helper belonging to this immutable workflow revision.
- name: Check out the trusted review policy
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
ref: ${{ github.workflow_sha }}
persist-credentials: false

- name: Re-evaluate current PR and queue after review changes
env:
GH_TOKEN: ${{ github.token }}
GIT_CONFIG_COUNT: '2'
GIT_CONFIG_KEY_0: credential.helper
GIT_CONFIG_VALUE_0: ''
GIT_CONFIG_KEY_1: credential.https://github.com.helper
GIT_CONFIG_VALUE_1: '!gh auth git-credential'
uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
with:
script: |
Expand All @@ -56,58 +69,21 @@ jobs:
if (!/^[0-9a-f]{40}$/.test(sha || '')) throw new Error('Invalid policy merge base');
return sha;
}
async function readPolicy(ref) {
const {data: file} = await github.rest.repos.getContent({
...context.repo, path: '.castiron-ratchet.json', ref,
});
if (file.type !== 'file' || file.encoding !== 'base64' || !Number.isSafeInteger(file.size) ||
file.size > 4096 || typeof file.content !== 'string') throw new Error('Invalid policy file');
const content = Buffer.from(file.content, 'base64');
if (content.length !== file.size) throw new Error('Invalid policy content');
const policy = JSON.parse(content.toString('utf8'), (key, value, source) =>
key === 'max_custom_patch_lines' && typeof value === 'number' ? BigInt(source.source) : value);
if (!policy || typeof policy !== 'object' || typeof policy.max_custom_patch_lines !== 'bigint' ||
policy.max_custom_patch_lines < 0n) throw new Error('Invalid policy budget');
return policy;
}
async function supportsEffectivePinReview(ref) {
let file;
async function requiresHuman(pr, mainSha) {
const path = require('node:path');
const fs = require('node:fs');
const root = fs.mkdtempSync(path.join(process.env.RUNNER_TEMP, 'castiron-review-'));
try {
({data: file} = await github.rest.repos.getContent({
...context.repo, path: 'scripts/castiron/custom_code_policy.py', ref,
}));
} catch (error) {
if (error.status === 404) return false;
throw error;
const result = await exec.getExecOutput('python3', ['-I',
'scripts/castiron/custom_code_policy.py', 'review-required',
'--repository', repo, '--repo', path.join(root, 'objects.git'),
'--base', pr.base.sha, '--head', pr.head.sha, '--main', mainSha], {silent: true});
const decision = JSON.parse(result.stdout);
if (typeof decision.requires_human !== 'boolean') throw new Error('Invalid review decision');
return decision.requires_human;
} finally {
fs.rmSync(root, {recursive: true, force: true});
}
if (file.type !== 'file' || file.encoding !== 'base64' || !Number.isSafeInteger(file.size) ||
file.size < 0 || typeof file.content !== 'string') throw new Error('Invalid checker contract');
const content = Buffer.from(file.content, 'base64');
if (content.length !== file.size) throw new Error('Invalid checker content');
// Read data only from the effective checker, never import or execute it.
const declarations = content.toString('utf8').match(/^PIN_REVIEW_POLICY = .*$/gm) || [];
return declarations.length === 1 && declarations[0] === 'PIN_REVIEW_POLICY = "effective-policy-v1"';
}
async function pinRequiresHuman(pr, headPolicy, mainSha) {
const targetPolicy = await readPolicy(pr.base.sha);
const pinSha = pin => pin?.repository === repo && /^[0-9a-f]{40}$/.test(pin.main_sha || '')
? pin.main_sha : null;
const proposedChecker = pinSha(headPolicy.next_policy);
const previousChecker = targetPolicy.next_policy ? pinSha(targetPolicy.next_policy) : mainSha;
if (!proposedChecker || !/^[0-9a-f]{40}$/.test(previousChecker || '') ||
!/^[0-9a-f]{40}$/.test(mainSha || '')) return true;
// Older checkers require human review for every pin edit. Retain that
// requirement until the protected checker explicitly supports the exemption.
if (!await supportsEffectivePinReview(previousChecker)) return true;
const [previous, proposed, previousBase, proposedBase] = await Promise.all([
readPolicy(previousChecker),
readPolicy(proposedChecker), mergeBase(previousChecker, proposedChecker),
mergeBase(proposedChecker, mainSha),
]);
return previousBase !== previousChecker || proposedBase !== proposedChecker ||
proposed.max_custom_patch_lines > previous.max_custom_patch_lines ||
proposed.schema_version !== previous.schema_version ||
proposed.generated_files !== previous.generated_files || proposed.ratchet !== previous.ratchet;
}
const dirtyTargets = new Set();
// Retry discovery within this job timeout while targets are changing.
Expand Down Expand Up @@ -146,18 +122,17 @@ jobs:
const pr = matching[0];
const files = await github.paginate(github.rest.pulls.listFiles, {...context.repo, pull_number: pr.number});
if (!files.some(file => file.filename === '.castiron-ratchet.json')) return;
const baseline = await mergeBase(pr.base.sha, pr.head.sha);
const [basePolicy, headPolicy] = await Promise.all([readPolicy(baseline), readPolicy(pr.head.sha)]);
const pinChanged = !require('node:util').isDeepStrictEqual(basePolicy.next_policy ?? null, headPolicy.next_policy ?? null);
let requiresHuman = headPolicy.max_custom_patch_lines > basePolicy.max_custom_patch_lines ||
(headPolicy.ratchet === 'go' && basePolicy.ratchet !== 'go');
let mainSha = null;
if (!requiresHuman && pinChanged) {
const {data: main} = await github.rest.git.getRef({...context.repo, ref: 'heads/main'});
mainSha = main.object.sha;
requiresHuman = await pinRequiresHuman(pr, headPolicy, mainSha);
const {data: main} = await github.rest.git.getRef({...context.repo, ref: 'heads/main'});
const mainSha = main.object.sha;
let requiresReview;
try {
requiresReview = await requiresHuman(pr, mainSha);
} catch (error) {
const {data: currentMain} = await github.rest.git.getRef({...context.repo, ref: 'heads/main'});
if (currentMain.object.sha !== mainSha) continue;
throw error;
Comment on lines +129 to +133

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 Recheck main after a successful policy decision

If main advances after the Python helper fetches the old main ref but before it returns requires_human: true, this catch block never runs and the only subsequent main-freshness check is inside the !requiresReview branch. The handler therefore invalidates successful PR and queue statuses using the stale policy decision and may leave them pending if the requested rerun fails. Recheck heads/main after every successful evaluation and retry when it differs from mainSha, regardless of the decision.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I checked this against requires_human_policy_review and local Git-history probes. With fixed PR/base commits, advancing main preserves a true decision: budget and ratchet edits are unchanged; existing pins compare immutable policies; bootstrap pins become older than main and still require review. A false decision can become true, but that branch already rechecks main. I don't see a stale-decision defect here under normal main advancement.

}
if (!requiresHuman) {
if (!requiresReview) {
const [{data: current}, {data: target}, main] = await Promise.all([
github.rest.pulls.get({...context.repo, pull_number: pr.number}),
github.rest.git.getRef({...context.repo, ref: 'heads/next'}),
Expand Down
Loading
Loading