fix(ci): break infinite SHA-bump loop + fix extract-plan nested JSON#136
Merged
Conversation
(?R) in perl recursively applies the full outer pattern, which requires
every nested {} to contain "version":"<version>" — causing extraction to
fail whenever the action plan has nested objects like params:{...} inside
an action entry (e.g. escalate action with reason+labels params).
Replace the recursive regex in Strategies B2, C (issue) and C (pr) with
a character-by-character depth tracker that:
1. Walks the input finding balanced {…} blocks (tracking strings/escapes)
2. Filters candidates by /"version"\s*:\s*"$v"/ on the full block text
3. Returns the last match so multi-attempt runs use the final output
Strategy E (issue) is fixed the same way, filtering by /"actions"\s*:\s*\[/
instead; the jq validation step that follows still checks version+type.
Adds a regression test (issue #93) that embeds a plan with nested params
objects in a markdown code-fence and confirms it is extracted correctly.
Three pre-existing issues blocked `git push` via the lefthook pre-push: 1. actionlint-full: auto-release.yml had shellcheck-reported SC2001/SC2086/ SC2129 findings (sed → param-expansion, unquoted vars, grouped redirects). 2. verify-sha: docs.yml referenced b96e013b but manifest.yml pinned 4e1ecad; commit 2c283d8 updated the workflow without updating the manifest. Fixed by reverting docs.yml to the manifest SHA. 3. shellcheck-full / shell-lint: SC2034 (warning) is a false positive in library/helper .sh files that are consumed via `source`; shellcheck cannot follow dynamic source paths so it wrongly flags library variables as unused. Also fix SC2168 (local outside function) in test-reusable- issue.sh. Both hooks updated to --severity=error so genuine errors are still caught while warning-level false positives don't block pushes.
Two bugs caused on-main-bump-sha to open PRs in an endless loop: 1. No guard against self-triggering: merging a bump PR pushes a new commit to main, which retriggered the workflow, which created another bump PR, ad infinitum. Fixed by adding a Guard step that reads commit message + author via git log and sets skip=true when the HEAD commit is itself a bot-authored bump (covers both squash and regular merge strategies). 2. Skip condition required current_sha == head_sha, which can never be true after any merge commit. Simplified to only skip when the pinned SHA is already at HEAD; bump-self-sha.sh already handles walking back to a valid ancestor. Also adds the same guard to auto-release.yml to avoid wasteful no-op runs when a bump commit lands on main (chore commits never produce a release anyway).
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdded guard steps to two release automation workflows to detect and skip bot-authored SHA-bump and merge commits, preventing recursive automation loops. Updated the pinned reference for a reusable documentation workflow to a new commit hash. Simplified SHA validation logic to check only direct HEAD equality instead of inspecting workflow file changes. ChangesRelease Automation Guard Logic
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
3 tasks
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
1 participant
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.
Summary
on-main-bump-sha.yml): merging a bump PR was re-triggering the workflow, creating another bump PR, ad infinitum. Added a Guard step that reads the commit message and author viagit logand skips when the HEAD commit is itself a bot-authored bump (covers both squash-merge and regular-merge strategies).current_sha == head_sha, which can never be true after a merge commit. Simplified to skip only when already at HEAD;bump-self-sha.shalready walks back to find a valid ancestor.auto-releaseruns on bump commits: added the same guard sochore(manifest)commits don't spin up the full commit-analysis pipeline (they always produceBUMP=noneanyway).extract-plannested JSON parsing (actions/issue/extract-plan,actions/pr/extract-plan): replaced the broken(?R)recursive regex (not supported in most shells) with a depth-tracking parser that correctly handles arbitrarily nested JSON objects.docs.ymlSHA with manifest after drift introduced in an earlier commit.batspath issue).Test plan
on-main-bump-shacreates exactly one follow-up bump PR and stops there (no second PR appears after the bump PR is merged)auto-releasedoes not run when the bump PR is mergedextract-plancorrectly parses issue bodies containing deeply nested JSON plan objectsdocs.ymlSHA matchesmanifest.ymlNeed help on this PR? Tag
@codesmithwith what you need.Summary by CodeRabbit