skip if there are no non-.md changes vs base branch before checking if there's a PR#18
Merged
Merged
Conversation
The orchestrator's "skip when on the base branch / no diff / .md-only" logic lived in Step 6, *after* the Step 5 ensure-pr gate. On the base branch, ensure-pr called `gh pr view <base>`, found no PR, and exited 2 before the base-branch skip could ever run -- so stopping on `main` nagged with "No PR found for branch main. Please create a draft PR". Move the informational-session detection ahead of ensure-pr. It has no dependency on the PR step (PR_NUMBER is only consumed later in the gate phase), so base-branch / no-diff / .md-only sessions now exit 0 cleanly while real code changes still hit the PR requirement unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The step also skips on the base branch and when there's no diff vs base yet -- not just .md-only sessions -- so "informational" undersold it. Rename the section + README to "gate-skip detection" and enumerate all three cases. Also reorder the README step list to match the code: the skip check now runs before the Push + PR check. Config key (stop_hook.skip_informational) and the IS_INFORMATIONAL_ONLY variable are left unchanged -- the key is a settings contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
No changes vs base subsumes it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the historical 'previously ran after ensure-pr' note (that's commit history, not current behavior) and tighten the case list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
evgunter
changed the title
evgunter
changed the title
…test An empty diff trivially has no non-.md files, so the two were the same skip decision split only to log different strings. Fold them into a single check vs the base branch. Skip decision and exit codes unchanged (verified across base / no-diff / .py / .md-only / mixed cases); only the info log message changes. Base-branch check stays separate -- it needs no diff and so does not depend on the fetch in Step 4. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…in/<base> The diff is vs origin/\$BASE_BRANCH, not the local base branch; "vs it" blurred that. Name the ref explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Listing "base branch" alongside it in a one-line summary reads as a subset (the origin/<base> distinction that makes it not one is invisible there). The detailed Step 5 comment keeps both cases + names the ref. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
"Gate-skip detection" named the action, not the detected condition, and read as jargon. Name the condition instead. Also drop the double-negative "no non-.md changes" from comments + log in favor of "only .md / empty" phrasing. Behavior unchanged; verified across base/no-diff/.py/.md cases. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
lgtm, thanks for this! |
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
2 participants
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
Stopping on the base branch (
main) nags with:even though
stop_hook_orchestrator.shhas explicit "no PR needed on the base branch" logic.Root cause
Ordering bug. The informational-session skip (base branch / no diff vs base /
.md-only) was Step 6, after the Step 5ensure-prgate. On the base branch,ensure-prrunsgh pr view <base>, finds nothing, andexit 2s — the orchestrator propagates that and never reaches the Step 6 skip. The base-branch guard was effectively dead code on that path.(Same ordering also defeated the no-diff /
.md-only skip on a feature branch whose PR isn't open yet.)Fix
Move the informational-session detection block ahead of
ensure-pr. It has no dependency on the PR step (PR_NUMBERis only consumed later, in the parallel gate phase), and the fetch/merge step still runs first, so the "CI starts early" optimization is preserved for real PR sessions. Pure reorder + renumber; no logic change inside either block.Verification
shellcheck -xoutput is identical tomain(no new warnings). Functional test in throwaway repos:main, clean.pydiff.md-only diff🤖 Generated with Claude Code