From 537b8359ed512d48abf00058769c5f680b1424e2 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 26 Oct 2026 06:09:44 -0400 Subject: [PATCH 1/2] fix(claude-lanes): treat an already-reviewed new-SHA contract as satisfying lockstep planLockstep declined a re-pinned caller whose surface changed without checking whether policy.json already holds a reviewed contract for the new SHA, so the repin lane PR kept "Runner-policy lockstep requires a human" after the contracts were registered. A same-path pin with an existing new-SHA contract is now satisfied (neither declined nor copied). The renamed-path decline is unchanged. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01DpzEJAE4EMndcWQXiKSZYn --- .../claude-lanes/repin-policy-lockstep.mjs | 52 +++++++++++-------- .../repin-policy-lockstep.test.mjs | 24 +++++++++ 2 files changed, 55 insertions(+), 21 deletions(-) diff --git a/components/claude-lanes/repin-policy-lockstep.mjs b/components/claude-lanes/repin-policy-lockstep.mjs index b3b38c99..fb5e5425 100755 --- a/components/claude-lanes/repin-policy-lockstep.mjs +++ b/components/claude-lanes/repin-policy-lockstep.mjs @@ -245,10 +245,12 @@ export function renamedPathReason(pin) { /** * 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 +263,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 +353,31 @@ 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 hasContract = (workflowPath, sha) => + Boolean(policy.approvedReusableWorkflowContracts[`${workflowPath}@${sha}`]); + 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, + }); + }, + hasContract, + ); 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..aa022c31 100644 --- a/components/claude-lanes/repin-policy-lockstep.test.mjs +++ b/components/claude-lanes/repin-policy-lockstep.test.mjs @@ -279,6 +279,30 @@ 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("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); From b9bca366e3207ac654419c867604881911d7a83c Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Mon, 26 Oct 2026 06:19:16 -0400 Subject: [PATCH 2/2] fix(claude-lanes): look up the new-SHA contract in the validated policy's Map validatePolicy returns approvedReusableWorkflowContracts as a Map, so the bracket lookup never matched. policyHasContract uses Map.has and is tested against a validatePolicy result. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01DpzEJAE4EMndcWQXiKSZYn --- components/claude-lanes/repin-policy-lockstep.mjs | 9 ++++++--- components/claude-lanes/repin-policy-lockstep.test.mjs | 7 +++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/components/claude-lanes/repin-policy-lockstep.mjs b/components/claude-lanes/repin-policy-lockstep.mjs index fb5e5425..dfd83042 100755 --- a/components/claude-lanes/repin-policy-lockstep.mjs +++ b/components/claude-lanes/repin-policy-lockstep.mjs @@ -242,6 +242,11 @@ 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 @@ -353,8 +358,6 @@ 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 hasContract = (workflowPath, sha) => - Boolean(policy.approvedReusableWorkflowContracts[`${workflowPath}@${sha}`]); const { reasons, copyForwards } = await planLockstep( pins, newSha, @@ -376,7 +379,7 @@ async function main() { githubToken, }); }, - hasContract, + (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 aa022c31..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"; @@ -287,6 +288,12 @@ test("a pin whose surface changed but whose new-SHA contract is registered is sa 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)`]);