chore(os): add a task to create PRs for OS sync #29
Conversation
WalkthroughThis change updates the Possibly related PRs
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
aqua.yaml (1)
10-11: GitHub CLI addition looks good – consider pinning by digest for stronger supply-chain guaranteesThe new entry cleanly installs
cli/[email protected]and tags it for selective installs.
If you want fully reproducible builds, Aqua also supports pinning by image digest (name: cli/[email protected]#<sha>). This prevents unexpected hash changes even if the upstream retags the same version.Nothing blocking, just a heads-up.
lib/os-modules/Taskfile.yaml (2)
141-147: Skip empty commits to avoid noisy errors
git commit -m …exits with status 1 when there’s nothing to commit, which can bubble up if a futureset -eis introduced. Consider guarding the commit:- git add . - git commit -m "chore: update with the latest template state" + git add . + if ! git diff --cached --quiet; then + git commit -m "chore: update with the latest template state" + else + echo "ℹ️ No changes detected — skipping commit" + fiKeeps the log clean and future-proofs the task.
260-262: Nice orchestration task – considerset -euo pipefailwrapper
sync-and-prstitches the whole flow together neatly.
Adding strict bash flags at the top of each inline script (set -euo pipefail) would stop the chain on the first failure, saving time and avoiding inconsistent states.Not critical for merging but worth a follow-up.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
aqua.yaml(1 hunks)lib/os-modules/Taskfile.yaml(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.yaml`: You are well-versed in writing and reviewing YAML configurations for the Task tool (https://taskfile.dev/). Provide recommendations for clarity, maintainability, and a...
**/*.yaml: You are well-versed in writing and reviewing YAML configurations for the Task tool (https://taskfile.dev/).
Provide recommendations for clarity, maintainability, and adherence to Taskfile best practices, including usage of variables, environment blocks, and includes.
These configurations may also contain embedded Bash scripts or commands.
Demonstrate bash scripting best practices such as error handling, secure variable expansions, and clear documentation.
aqua.yamllib/os-modules/Taskfile.yaml
| commits_ahead=$(git rev-list --count main..{{.SYNC_BRANCH}}) | ||
| if [ "$commits_ahead" -eq 0 ]; then | ||
| echo "⏭️ No commits ahead of main, skipping PR creation for $module" | ||
| cd - | ||
| continue | ||
| fi | ||
|
|
||
| echo "📋 Creating pull request..." | ||
| gh pr create \ | ||
| --title "{{.PR_TITLE}}" \ | ||
| --body "{{.PR_BODY}}" \ | ||
| --base main \ | ||
| --head {{.SYNC_BRANCH}} \ | ||
| --repo "masterpointio/$module" | ||
|
|
||
| cd - |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle existing PRs to make the task idempotent
gh pr create exits non-zero if a PR from the same head already exists, causing the loop to abort. A quick resilience upgrade:
- gh pr create \
+# If PR exists, this will no-op; otherwise it creates one.
+gh pr view {{.SYNC_BRANCH}} --head {{.SYNC_BRANCH}} >/dev/null 2>&1 || \
+gh pr create \
--title "{{.PR_TITLE}}" \
--body "{{.PR_BODY}}" \
--base main \
--head {{.SYNC_BRANCH}} \
--repo "masterpointio/$module"Keeps the workflow repeatable without manual cleanup.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| commits_ahead=$(git rev-list --count main..{{.SYNC_BRANCH}}) | |
| if [ "$commits_ahead" -eq 0 ]; then | |
| echo "⏭️ No commits ahead of main, skipping PR creation for $module" | |
| cd - | |
| continue | |
| fi | |
| echo "📋 Creating pull request..." | |
| gh pr create \ | |
| --title "{{.PR_TITLE}}" \ | |
| --body "{{.PR_BODY}}" \ | |
| --base main \ | |
| --head {{.SYNC_BRANCH}} \ | |
| --repo "masterpointio/$module" | |
| cd - | |
| echo "📋 Creating pull request..." | |
| # If a PR for this branch already exists, no-op; otherwise create one. | |
| gh pr view {{.SYNC_BRANCH}} --head {{.SYNC_BRANCH}} >/dev/null 2>&1 || \ | |
| gh pr create \ | |
| --title "{{.PR_TITLE}}" \ | |
| --body "{{.PR_BODY}}" \ | |
| --base main \ | |
| --head {{.SYNC_BRANCH}} \ | |
| --repo "masterpointio/$module" | |
| cd - |
🤖 Prompt for AI Agents
In lib/os-modules/Taskfile.yaml around lines 190 to 205, the script uses `gh pr
create` which fails if a PR from the same head branch already exists, causing
the loop to abort. Modify the script to first check if a PR from the current
head branch exists using `gh pr list` or handle the error from `gh pr create`
gracefully. If a PR exists, skip creation and continue the loop to make the task
idempotent and prevent aborting on duplicate PRs.
| current_branch=$(git branch --show-current) | ||
| if [ "$current_branch" != "{{.SYNC_BRANCH}}" ]; then | ||
| echo "⚠️ Warning: Not on {{.SYNC_BRANCH}} branch. Current branch: $current_branch" | ||
| echo "🔄 Checking out {{.SYNC_BRANCH}}..." | ||
| git checkout {{.SYNC_BRANCH}} | ||
| fi | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Branch-safety check: bail out early instead of continuing on wrong branch
The current logic merely warns when the user happens to be on a different branch, then checks out {{.SYNC_BRANCH}}. If the checkout fails (e.g., branch missing), the script still proceeds and gh pr create will mis-behave. Prefer failing fast:
- git checkout {{.SYNC_BRANCH}}
+ if ! git checkout {{.SYNC_BRANCH}}; then
+ echo "❌ Failed to switch to {{.SYNC_BRANCH}} – skipping PR for $module"
+ cd -; continue
+ fiImproves robustness across all repos.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| current_branch=$(git branch --show-current) | |
| if [ "$current_branch" != "{{.SYNC_BRANCH}}" ]; then | |
| echo "⚠️ Warning: Not on {{.SYNC_BRANCH}} branch. Current branch: $current_branch" | |
| echo "🔄 Checking out {{.SYNC_BRANCH}}..." | |
| git checkout {{.SYNC_BRANCH}} | |
| fi | |
| current_branch=$(git branch --show-current) | |
| if [ "$current_branch" != "{{.SYNC_BRANCH}}" ]; then | |
| echo "⚠️ Warning: Not on {{.SYNC_BRANCH}} branch. Current branch: $current_branch" | |
| echo "🔄 Checking out {{.SYNC_BRANCH}}..." | |
| if ! git checkout {{.SYNC_BRANCH}}; then | |
| echo "❌ Failed to switch to {{.SYNC_BRANCH}} – skipping PR for $module" | |
| cd -; continue | |
| fi | |
| fi |
🤖 Prompt for AI Agents
In lib/os-modules/Taskfile.yaml around lines 183 to 189, the script checks out
the sync branch if the current branch differs but does not handle checkout
failures. Modify the script to immediately exit with an error if the checkout
command fails, preventing further execution on the wrong branch. This can be
done by adding a check after the git checkout command and using an exit
statement to fail fast if the checkout is unsuccessful.
| if git branch --list "{{.SYNC_BRANCH}}" | grep -q "{{.SYNC_BRANCH}}" && [ "{{.DELETE_EXISTING_SYNC_BRANCH}}" = "false" ]; then | ||
| echo "⏭️ Branch {{.SYNC_BRANCH}} already exists, skipping creation." | ||
| echo "⏭️ Branch {{.SYNC_BRANCH}} already exists, checking it out." | ||
| git checkout {{.SYNC_BRANCH}} | ||
|
|
||
| # Check if local and remote branches have diverged | ||
| if git status --porcelain -b | grep -q "ahead\|behind\|diverged"; then | ||
| echo "⚠️ Local and remote branches have diverged. Resetting to remote branch..." | ||
| git fetch origin {{.SYNC_BRANCH}} | ||
| git reset --hard origin/{{.SYNC_BRANCH}} | ||
| fi |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Divergence check risks clobbering local commits
git status --porcelain -b flags any “ahead/behind” state and then hard-resets to the remote, which will silently drop local commits if the branch is only ahead of origin.
A safer pattern is to reset only when the branch is behind or has actually diverged:
- if git status --porcelain -b | grep -q "ahead\|behind\|diverged"; then
- echo "⚠️ Local and remote branches have diverged. Resetting to remote branch..."
- git fetch origin {{.SYNC_BRANCH}}
- git reset --hard origin/{{.SYNC_BRANCH}}
+ # Count commits left (behind) and right (ahead) of remote
+ if [ "$(git rev-list --left-right --count {{.SYNC_BRANCH}}...origin/{{.SYNC_BRANCH}} | awk '{print $1}')" -gt 0 ]; then
+ echo "🔄 Local branch is behind remote. Resetting hard to remote state…"
+ git fetch origin {{.SYNC_BRANCH}}
+ git reset --hard origin/{{.SYNC_BRANCH}}
+ fiThis preserves un-pushed work while still self-healing in the “behind” scenario.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In lib/os-modules/Taskfile.yaml around lines 95 to 104, the current divergence
check resets the branch hard whenever it is ahead, behind, or diverged, which
risks losing local commits. Modify the condition to reset only if the branch is
behind or diverged, excluding the ahead-only case. Adjust the git status check
to detect only "behind" or "diverged" states before performing the hard reset to
preserve un-pushed local commits.
what
why
references
Summary by CodeRabbit
New Features
Improvements
Chores