Skip to content

fix(templates): repin canary callers to the cache-invariant contract - #97

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/repin-canary-callers-cache-contract
Jul 14, 2026
Merged

fix(templates): repin canary callers to the cache-invariant contract#97
kyle-sexton merged 1 commit into
mainfrom
fix/repin-canary-callers-cache-contract

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Lockstep repin after #95: template callers now pin the merged cache-invariant contract commit 4083c8d82bc4e7a79a9866544101bf8a5261fb50. The seeded ci-runner-canary repo gets the matching repin via bootstrap re-run once this merges.

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

🤖 Generated with Claude Code

https://claude.ai/code/session_01D5wqdo4adEZmWgjpY9ZjVx

@claude

claude Bot commented Jul 14, 2026

Copy link
Copy Markdown

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


Review

  • Gather context and read changed files
  • Verify target SHA against referenced commit
  • Check consistency across both template files
  • Validate against CLAUDE.md guidelines

LGTM. This is a clean, minimal lockstep repin following #95's merge. Here's what I verified:

SHA verification4083c8d82bc4e7a79a9866544101bf8a5261fb50 is confirmed as the merge commit for #95 ("fix(canary): make cross-environment cache proofs environment-invariant"). The old pin c18c3d7… was the #89 liveness-selector adoption; this correctly advances it by one reviewed commit.

Consistency — Both template callers are updated to the same SHA with no other modifications (inputs, secrets, permission blocks are untouched):

CLAUDE.md compliance — The repo requires every uses: to be SHA-pinned with a trailing comment. These use descriptive markers (# reviewed canary contract, # reviewed production HA contract) rather than # vX.Y.Z semver tags. That's appropriate here: these are internal first-party workflow references with no semver tags for Dependabot to track; the descriptive comment signals the pin was manually reviewed. This was the pre-existing convention and isn't introduced by this PR.

External step — The PR description correctly notes the ci-runner-canary seed repo needs a matching repin via bootstrap re-run post-merge. That's out of scope for this PR and documented.

No issues found.


@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b16c1cc63

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread templates/ci-runner-canary/.github/workflows/local-runner-canary.yml Outdated
Comment thread templates/ci-runner-canary/.github/workflows/production-ha-proof.yml Outdated
…contract

Lockstep repin: #95 made the cross-environment cache proofs
environment-invariant and #92 hardened the reusable execution, so the
canonical callers pin the merged commit 9eb22ac carrying both.

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

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D5wqdo4adEZmWgjpY9ZjVx
@kyle-sexton
kyle-sexton force-pushed the fix/repin-canary-callers-cache-contract branch from 4b1e709 to fe3c7e7 Compare July 14, 2026 22:31
@claude

claude Bot commented Jul 14, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 12s —— View job


Review

  • Read changed template files
  • Inspect test fixtures flagged by Codex
  • Verify SHA consistency (PR description vs actual diff)
  • Post findings

One issue to resolve before merging.


Codex findings — false positives

Both Codex P2 comments are incorrect. The test fixtures do not hardcode the canary SHA:

workflowSha() (fetch-immutable-workflow-pins.cjs:64) parses the uses: line and returns the embedded SHA. There is no stale fixture — the tests will self-update to the new pin without any changes. Those tests will not fail.


SHA discrepancy — PR description vs actual diff

The PR description states:

template callers now pin the merged cache-invariant contract commit 4083c8d82bc4e7a79a9866544101bf8a5261fb50

But the actual diff pins to 9eb22acf1a9235c352779325d24615b7b492d44d, which is the merge commit for PR #92 ("ci: harden reusable workflow execution"), not a cache-invariant related commit:

- uses: melodic-software/ci-workflows/.github/workflows/local-runner-canary.yml@c18c3d73b996081729d1955b256d9f7d31626b0f
+ uses: melodic-software/ci-workflows/.github/workflows/local-runner-canary.yml@9eb22acf1a9235c352779325d24615b7b492d44d

Both template files now pin 9eb22acf... (local-runner-canary.yml:23, production-ha-proof.yml:32).

Before this merges, please confirm:

  1. Is 9eb22acf... the correct intended target? The PR was synchronized after the original commit, and the description was not updated to reflect the new SHA.
  2. Is 9eb22acf... the squash-merge result of PR fix(canary): make cross-environment cache proofs environment-invariant #95 on main? If so, the description should be updated to reference that SHA explicitly (and clarify its relation to fix(canary): make cross-environment cache proofs environment-invariant #95 vs ci: harden reusable workflow execution #92).

The pin is internally consistent (both files match), but the stale description makes it hard to verify the intent without author confirmation.


Minor observation (not blocking)

c18c3d73b996081729d1955b256d9f7d31626b0f (the previous pin) was not added to supersededCanaryShas in local-runner-canary.test.cjs. Whether to add it follows your existing convention — the three SHAs already in that list appear to be intentionally blacklisted rather than just outdated, so omitting it may be fine. Worth a conscious decision either way.

@kyle-sexton
kyle-sexton merged commit 7e58b9d into main Jul 14, 2026
34 checks passed
@kyle-sexton
kyle-sexton deleted the fix/repin-canary-callers-cache-contract branch July 14, 2026 22:37
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