ci: require the monad-execution pin to be on execution main - #3265
Open
maxkozlovsky wants to merge 1 commit into
Open
maxkozlovsky wants to merge 1 commit into
maxkozlovsky wants to merge 1 commit into
Conversation
maxkozlovsky
requested review from
ariqchowdhury,
dshulyak,
michael-yxchen,
omegablitz and
xinyuan-dev
as code owners
September 15, 2026 23:26
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings affect the reliability and integrity of the pin-validation gate.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds CI gating to ensure the monad-execution submodule pin is reachable from execution main and does not regress.
Changes:
- Adds API-based pin validation.
- Runs checks for pull requests and merge queues.
- Documents the gating design and failure modes.
File summaries
| File | Description |
|---|---|
docs/0004_submodule_pin_gating.md |
Documents the pin-gating policy and rationale. |
.github/workflows/exec-pin.yml |
Runs validation for pull requests and merge queues. |
.github/workflows/exec-pin.sh |
Validates pin reachability, progression, and diagnostics. |
Review details
Suppressed comments (2)
.github/workflows/exec-pin.sh:165
otherincludes closed, unmerged PRs as well as PRs merged into another branch, so this message says the pin “belongs to no execution PR” while listing one. For a closed unmerged PR, that wording and the subsequent force-push/GC explanation are misleading; describe it as belonging to an execution PR that was not merged into the selected base.
fail "$path is pinned to $short, which is not on $EXEC_REPO $EXEC_BASE and was never merged there." \
"$(printf '%s\n' "It belongs to:" "$other" "" \
"Re-pin to a commit on $EXEC_BASE.")"
.github/workflows/exec-pin.sh:89
- The progression baseline is read at the current PR path. If a PR changes
.gitmodulesto move the gitlink,git ls-tree FETCH_HEAD -- "$path"finds no pin onmaster, and lines 60–62 warn and allow the PR to proceed; an older valid execution commit can therefore bypass the rollback check. Reject an unexpected path change or compare the base branch's original path instead of treating a missing baseline as success.
url=$(git config -f .gitmodules --get "submodule.$SUBMODULE.url") || url=
path=$(git config -f .gitmodules --get "submodule.$SUBMODULE.path") || path=
- Files reviewed: 3/3 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A bft PR that pins an execution commit from an unmerged PR branch loses that commit when the branch is force-pushed or deleted, and `git submodule update` then fails for everyone on master until someone re-pins by hand. A mistyped gitlink conflict resolution can also roll the pin backwards, silently reverting execution bumps already on master. Nothing catches either before merge today. Add a workflow that reads the gitlink recorded in HEAD and requires, over the GitHub API, that the pin is reachable from execution main and that it is not behind the pin already on master. See docs/0004_submodule_pin_gating.md for how pins go bad, the failure-message design, and why the off-the-shelf action cannot serve as a required check here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
maxkozlovsky
force-pushed
the
max/exec-pin-guard
branch
from
September 15, 2026 23:49
ff0f374 to
d76d724
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
monad-executiongitlink is a bare commit SHA, and nothing in git makes it follow a branch. Two ways that goes wrong today, neither caught before merge:git submodule updatefailing for everyone on master.category-labs/monadmerges by rebase, which makes this the expected path rather than an edge case: merging an execution PR rewrites its commits, so a pin taken from a PR head is permanently unreachable frommainafterwards.main.What this adds
exec-pin.sh, run byexec-pin.yml, reads the gitlink inHEADand requires both:identical/behindexecutionmain.behindthe pin already onmaster.Both go through the GitHub API, so no submodule checkout is needed — deliberate, since a recursive checkout dies on precisely the unfetchable pin the job exists to report. Failures separate "the pin is wrong" from "the check could not run": only a definitive 404 is reported as a missing commit, and
gh's stderr is kept in the log.Rationale, including why the off-the-shelf action can't serve as a required check here, is in
docs/0004_submodule_pin_gating.md.Testing
There are no automated tests — the repo has no shell-test infrastructure, and
.github/workflows/license.shis the untested-merge-gate precedent this mirrors. CI cannot exercise the classification logic either, so it was driven by hand against real SHAs in throwaway repos. Each case and the behaviour it pins down:mainmerge_commit_shato re-pin tomainexecution branchEXEC_BASE/ API errorgit ls-treefails, or gitlink moved to another path$GITHUB_STEP_SUMMARYunwritableNot verifiable locally, and the reason this is worth one careful look in review: the
merge_grouppath, and that the CIGITHUB_TOKENrather than a personal PAT can readcategory-labs/monad. Both are public repos andcontents: readshould suffice, but the first real run is the proof.Follow-up
This only has teeth once Execution pin is added as a required status check on
master— a branch-protection change, not in this PR. It is written to work in the merge queue (it never parses the event payload), which is why it carries amerge_grouptrigger.Independent of #3266 (Dependabot submodule bumps); either can merge first.
🤖 Generated with Claude Code