fix(claude-lanes): treat an already-reviewed new-SHA contract as satisfying lockstep

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));
 }

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);