Skip to content

fix(canary): pin callers to merged workflow commit - #93

Merged
kyle-sexton merged 1 commit into
mainfrom
codex/h1-main-reachable-canary-pin
Jul 14, 2026
Merged

fix(canary): pin callers to merged workflow commit#93
kyle-sexton merged 1 commit into
mainfrom
codex/h1-main-reachable-canary-pin

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • repin both canonical ci-runner-canary callers from PR-only commit 169e3a4 to merged-main commit c18c3d7
  • update the executable pin-contract tests
  • remove the now-unnecessary hidden-PR-ref fetches from both full-history CI lanes

Why

The old commit is currently resolvable, but it is not reachable from ci-workflows/main; it survives only through PR #89's retained ref. PR ci-runner-canary #1 incorrectly described that pin as a main ancestor.

The two reusable workflow blobs at 169e3a4 and signed merged-main commit c18c3d7 are byte-identical, so this is a provenance and durability correction with no runtime behavior change. GitHub recommends immutable commit-SHA references for reusable workflows: official reusable-workflow documentation.

Verification

  • confirmed 169e3a4 is not an ancestor of main
  • confirmed c18c3d7 is an ancestor of main and GitHub-verified
  • confirmed both reusable workflow blob IDs are identical across the two commits
  • targeted immutable-pin tests: 2 passed
  • actionlint passed for all changed workflow/template files
  • git diff --check passed
  • canonical source at c265418 previously passed the complete Linux CI suite, including 169 selector/acceptance contract tests

After merge, the corrected canonical template will be redistributed into ci-runner-canary PR #1 before H1 acceptance.

Part of melodic-software/github-iac#88.


Note

Low Risk
Pin and CI wiring only; no application runtime, auth, or workflow logic changes beyond commit-SHA provenance.

Overview
Repins both templates/ci-runner-canary callers (local-runner-canary.yml and production-ha-proof.yml) from PR-only commit 169e3a4 to merged-main commit c18c3d7 so reusable workflow refs stay reachable from ci-workflows/main (byte-identical workflow blobs; provenance fix, not a behavior change).

Updates executable pin contracts in local-runner-canary.test.cjs and production-ha-proof.test.cjs to expect c18c3d7 as currentCanarySha / implementationSha.

Removes the Fetch immutable workflow pins under test shallow git fetch steps from the selector-contract job in ci.yml and the selector-unit job in selector-conformance.yml, since full-history checkout no longer needs that extra ref for the old pin.

Reviewed by Cursor Bugbot for commit 033d3fc. Bugbot is set up for automated code reviews on this repo. Configure here.

@claude

claude Bot commented Jul 14, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 5s —— View job


PR Review

  • Gather PR diff and context
  • Review changed files
  • Post findings

Overall: LGTM — clean provenance fix, well-coordinated across all layers.

The six-file change is internally consistent and the removal of the explicit git fetch workaround is correctly paired with the SHA correction.


What changed and why it's correct

File Change Verdict
templates/.../local-runner-canary.yml 169e3a4c18c3d7
templates/.../production-ha-proof.yml 169e3a4c18c3d7
.github/scripts/local-runner-canary.test.cjs currentCanarySha updated
.github/scripts/production-ha-proof.test.cjs implementationSha updated
.github/workflows/ci.yml Removes explicit fetch of 169e3a4
.github/workflows/selector-conformance.yml Same fetch removal

The fetch removal is correctly motivated. Both CI lanes check out with fetch-depth: 0, which makes any main-ancestor commit available without a separate fetch step. The old git fetch --depth=1 origin 169e3a4... was a workaround precisely because 169e3a4 lived only in a PR ref — invisible to a full-history clone. With c18c3d7 on main, no workaround is needed.

The pin-contract tests are correctly wired. Both test files use git show ${sha}:<path> to verify the blob content at the pinned commit. These calls now resolve against a main-ancestor commit rather than a PR-only ref, so they will succeed under any normal checkout.


Observations

169e3a4 is not added to supersededCanaryShas. The test's supersededCanaryShas list guards against accidentally re-using previously canonical versions. Since 169e3a4 was never a properly canonical pin (it was a PR-only commit incorrectly described as a main ancestor in ci-runner-canary #1), omitting it from the superseded list is the right call — its doesNotMatch assertion would be misleading, and the old SHA naturally drops off once the PR ref is eventually cleaned up.

Comment convention on template uses: lines. The # reviewed canary contract and # reviewed production HA contract comments do not follow the # vX.Y.Z semver pattern required by CLAUDE.md for external actions. That's intentional and correct: these are self-referential reusable workflow calls within the same org, and the pin-contract tests explicitly assert this exact comment text (local-runner-canary.test.cjs:469, production-ha-proof.test.cjs:729-733), so this is an intentional and enforced deviation from the semver-comment rule.

Blob identity. The PR claims the workflow blobs at 169e3a4 and c18c3d7 are byte-identical. This is auditable via git diff 169e3a4 c18c3d7 -- .github/workflows/local-runner-canary.yml and the same for production-ha-proof.yml. The test suite's behavioral assertions (selector SHA, secrets: inherit absence, runner label, etc.) passing against c18c3d7 provides indirect confirmation.


No security concerns. The change aligns fully with CLAUDE.md's SHA-pin-everything requirement and the durability/provenance rationale is solid.

@kyle-sexton
kyle-sexton marked this pull request as ready for review July 14, 2026 21:09
@cursor

cursor Bot commented Jul 14, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_8ffafc8a-cf5f-4728-99b5-0778ba0df0a0)

@kyle-sexton
kyle-sexton merged commit 86a8cc6 into main Jul 14, 2026
37 checks passed
@kyle-sexton
kyle-sexton deleted the codex/h1-main-reachable-canary-pin branch July 14, 2026 21:09
@claude

claude Bot commented Jul 14, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 27s —— View job


PR Review

  • Gather PR diff and context
  • Review changed files
  • Post findings

Overall: LGTM — clean provenance and durability fix, all six files are internally consistent.


What changed and why it's correct

File Change Verdict
templates/.../local-runner-canary.yml 169e3a4...c18c3d73...
templates/.../production-ha-proof.yml 169e3a4...c18c3d73...
.github/scripts/local-runner-canary.test.cjs currentCanarySha updated
.github/scripts/production-ha-proof.test.cjs implementationSha updated
.github/workflows/ci.yml Removes explicit git fetch of 169e3a4
.github/workflows/selector-conformance.yml Same fetch removal

Fetch removal is correctly motivated. Both CI lanes use fetch-depth: 0, making any main-ancestor commit reachable without a separate fetch step. The old git fetch --depth=1 origin 169e3a4... was a workaround for a PR-only ref — invisible to a full-history clone. With c18c3d7 on main, no workaround is needed.

Pin-contract tests are correctly wired. Both test files use git show ${sha}:<path> to assert blob content at the pinned commit. These now resolve against a main-ancestor commit, so they'll succeed under any normal checkout.

Old SHA fully removed. 169e3a4 does not appear anywhere in .yml or .cjs files — confirmed clean sweep.


Observations

169e3a4 correctly omitted from supersededCanaryShas. The guard list exists to prevent accidental re-use of previously canonical versions. Since 169e3a4 was never a properly canonical pin (it was a PR-only commit incorrectly described as a main ancestor), adding it there would be misleading. The omission is the right call.

Comment convention on uses: lines. The # reviewed canary contract and # reviewed production HA contract comments deviate from CLAUDE.md's # vX.Y.Z semver pattern. This is intentional: these are self-referential reusable workflow calls within the same org, and the pin-contract tests explicitly assert this comment text (local-runner-canary.test.cjs:469, production-ha-proof.test.cjs:730). Intentional and enforced deviation.

Blob identity. The PR claims the workflow blobs at 169e3a4 and c18c3d7 are byte-identical. This is auditable via git diff 169e3a4 c18c3d7 -- .github/workflows/local-runner-canary.yml. The behavioral assertions in the test suite passing against c18c3d7 provides indirect confirmation.


No security concerns. The change aligns with CLAUDE.md's SHA-pin-everything requirement and the provenance/durability rationale is solid.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Risk: low. Not approving: the Cursor Bugbot check completed as skipped, so there is no clean automated review signal. The PR is already merged; no reviewers were assigned.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant