diff --git a/components/claude-lanes/repin-policy-lockstep.mjs b/components/claude-lanes/repin-policy-lockstep.mjs index b3b38c99..dfd83042 100755 --- a/components/claude-lanes/repin-policy-lockstep.mjs +++ b/components/claude-lanes/repin-policy-lockstep.mjs @@ -242,13 +242,20 @@ export function renamedPathReason(pin) { ); } +/** `policy` is a validatePolicy result, whose contracts are a Map. */ +export function policyHasContract(policy, workflowPath, sha) { + return policy.approvedReusableWorkflowContracts.has(`${workflowPath}@${sha}`); +} + /** * Splits re-pinned callers into contract copy-forwards and declines. A pin * whose path changed is always declined: a contract for a renamed path is - * reviewed by hand, never copied, however its surface compares. Every other - * pin copies forward only when `surfaceOf(pin)` reports it unchanged. + * reviewed by hand, never copied, however its surface compares. A pin whose + * new-SHA contract `hasContract(path, sha)` already reports is satisfied: it + * is neither declined nor copied. Every other pin copies forward only when + * `surfaceOf(pin)` reports it unchanged. */ -export async function planLockstep(pins, newSha, surfaceOf) { +export async function planLockstep(pins, newSha, surfaceOf, hasContract = () => false) { const reasons = []; const copyForwards = []; const unique = new Map( @@ -261,6 +268,7 @@ export async function planLockstep(pins, newSha, surfaceOf) { reasons.push(renamedPathReason(pin)); continue; } + if (hasContract(pin.newWorkflowPath, newSha)) continue; const surface = await surfaceOf(pin); if (!surface.unchanged) { reasons.push(`${pin.newWorkflowPath} ${surface.reason} (from ${pin.oldSha.slice(0, 7)})`); @@ -350,24 +358,29 @@ async function main() { // The workflow step exports GH_TOKEN for gh; runner-policy sends the token // only on its api.github.com tree requests. const githubToken = process.env.GITHUB_TOKEN || process.env.GH_TOKEN; - const { reasons, copyForwards } = await planLockstep(pins, newSha, (pin) => { - const repoPath = repoPathOf(pin.newWorkflowPath); - let newSource = newSourceByRepoPath.get(repoPath); - if (newSource === undefined) { - newSource = fetchUpstreamFile(repoPath, newSha); - newSourceByRepoPath.set(repoPath, newSource); - } - const oldSource = fetchUpstreamFile(repoPathOf(pin.oldWorkflowPath), pin.oldSha); - return laneSecuritySurfacesMatch({ - oldSource, - newSource, - workflowPath: pin.newWorkflowPath, - oldSha: pin.oldSha, - newSha, - policy, - githubToken, - }); - }); + const { reasons, copyForwards } = await planLockstep( + pins, + newSha, + (pin) => { + const repoPath = repoPathOf(pin.newWorkflowPath); + let newSource = newSourceByRepoPath.get(repoPath); + if (newSource === undefined) { + newSource = fetchUpstreamFile(repoPath, newSha); + newSourceByRepoPath.set(repoPath, newSource); + } + const oldSource = fetchUpstreamFile(repoPathOf(pin.oldWorkflowPath), pin.oldSha); + return laneSecuritySurfacesMatch({ + oldSource, + newSource, + workflowPath: pin.newWorkflowPath, + oldSha: pin.oldSha, + newSha, + policy, + githubToken, + }); + }, + (workflowPath, sha) => policyHasContract(policy, workflowPath, sha), + ); await appendOutput(outputFile, await settleLockstep({ reasons, copyForwards }, tag)); } diff --git a/components/claude-lanes/repin-policy-lockstep.test.mjs b/components/claude-lanes/repin-policy-lockstep.test.mjs index cd31f416..1f030247 100644 --- a/components/claude-lanes/repin-policy-lockstep.test.mjs +++ b/components/claude-lanes/repin-policy-lockstep.test.mjs @@ -13,6 +13,7 @@ import { manualPolicyNote, pairCallerPins, planLockstep, + policyHasContract, settleLockstep, } from "./repin-policy-lockstep.mjs"; @@ -279,6 +280,36 @@ test("a same-path pin whose surface changed is declined, not copied forward", as assert.deepEqual(plan.reasons, [`${REVIEW_WORKFLOW} x (from 91d06c9)`]); }); +const changedSurface = () => ({ unchanged: false, reason: "x" }); +const registered = (workflowPath, sha) => workflowPath === REVIEW_WORKFLOW && sha === FRESH_SHA; + +test("a pin whose surface changed but whose new-SHA contract is registered is satisfied", async () => { + const plan = await planLockstep([samePathPin()], FRESH_SHA, changedSurface, registered); + assert.deepEqual(plan, { reasons: [], copyForwards: [] }); +}); + +test("policyHasContract finds a contract in a validated policy and not an absent one", async () => { + const policy = validatePolicy(await policyWithHistory()); + assert.equal(policyHasContract(policy, REVIEW_WORKFLOW, CURRENT_REVIEW_SHA), true); + assert.equal(policyHasContract(policy, REVIEW_WORKFLOW, FRESH_SHA), false); +}); + +test("a pin whose surface changed and has no new-SHA contract is still declined", async () => { + const plan = await planLockstep([samePathPin()], FRESH_SHA, changedSurface, () => false); + assert.deepEqual(plan.reasons, [`${REVIEW_WORKFLOW} x (from 91d06c9)`]); +}); + +test("a renamed pin is declined even when a contract exists at the new path", async () => { + const plan = await planLockstep( + [{ ...samePathPin(), newWorkflowPath: RENAMED_REVIEW_WORKFLOW }], + FRESH_SHA, + () => ({ unchanged: true }), + (workflowPath, sha) => workflowPath === RENAMED_REVIEW_WORKFLOW && sha === FRESH_SHA, + ); + assert.equal(plan.reasons.length, 1); + assert.deepEqual(plan.copyForwards, []); +}); + async function policyRoot() { const root = await mkdtemp(path.join(tmpdir(), "repin-settle-")); temporaryRoots.push(root);