fix: guard empty plugin_namespaces array in setup.sh deploy#784
fix: guard empty plugin_namespaces array in setup.sh deploy#784marcusquinn merged 2 commits intomainfrom
Conversation
With set -u (nounset), iterating an empty array via ${arr[@]} triggers
'unbound variable' error. This broke setup.sh deploy for users without
plugins.json, causing the supervisor post-PR lifecycle to fail at the
deploy step (t132.4 failed with 'Deploy (setup.sh) failed').
Guard all three iteration sites with array length check.
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical issue in the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
WalkthroughGuards were added to skip iterating over an empty Changes
Sequence Diagram(s)sequenceDiagram
participant Supervisor
participant Cache as "Token Cache (file)"
participant GH_API as "GitHub API"
participant GH_CLI as "gh CLI"
Supervisor->>Cache: read cached GH_TOKEN?
alt cached token present
Supervisor->>GH_API: verify token via lightweight API call
GH_API-->>Supervisor: 200 OK / auth state
else no cached token
Supervisor->>GH_CLI: gh auth status
GH_CLI-->>Supervisor: auth state or prompt
alt token obtained
Supervisor->>Cache: write GH_TOKEN to cache
end
end
Supervisor->>Supervisor: return auth result to callers (dispatch / sync / verify)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Code Review
This pull request effectively resolves the 'unbound variable' error that occurs when the plugin_namespaces array is empty. The addition of guards to check the array's length before iteration is a solid and safe fix. I've added one suggestion to make the code for populating the preserved_dirs array a bit more concise and idiomatic by appending the array directly, which is possible thanks to the guard you've added. The other changes look good.
| for pns in "${plugin_namespaces[@]}"; do | ||
| preserved_dirs+=("$pns") | ||
| done |
There was a problem hiding this comment.
This for loop can be made more concise. Since you're inside the if guard, you can safely append the entire plugin_namespaces array to preserved_dirs in one go. This is a more idiomatic way to combine arrays in Bash.
| for pns in "${plugin_namespaces[@]}"; do | |
| preserved_dirs+=("$pns") | |
| done | |
| preserved_dirs+=("${plugin_namespaces[@]}") |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Mon Feb 9 17:10:16 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
gh auth status fails in cron even when GH_TOKEN is set because it tries to validate the keyring token first. Added check_gh_auth() helper that checks GH_TOKEN via API call first, falling back to gh auth status. Also: try cached token before gh auth token (cache is more reliable in cron), and guard empty plugin_namespaces array in setup.sh deploy.
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Mon Feb 9 17:17:19 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agents/scripts/supervisor-helper.sh (1)
109-146:⚠️ Potential issue | 🟠 MajorBlocker: GH_TOKEN cache stored in
.agent-workspace(disallowed location).Persisting a token under
~/.aidevops/.agent-workspaceviolates the repo’s credential storage guidance. Please remove the workspace cache and rely on~/.config/aidevops/credentials.sh(via setup-local-api-keys.sh) or a secure store like gopass/keychain. This keeps secrets out of the workspace and maintains A‑grade security posture.Suggested fix (remove workspace cache + align priority comment)
-# Priority: existing GH_TOKEN env > `gh auth token` > cached token > gopass > credentials.sh -# When `gh auth token` succeeds (interactive), cache it for cron to use later. -_gh_token_cache="$HOME/.aidevops/.agent-workspace/supervisor/.gh-token-cache" +# Priority: existing GH_TOKEN env > `gh auth token` > gopass > credentials.sh +# For cron, persist GH_TOKEN via setup-local-api-keys.sh (credentials.sh) or gopass. if [[ -z "${GH_TOKEN:-}" ]]; then - # Try cached token FIRST (most reliable for cron — written by interactive sessions) - if [[ -f "$_gh_token_cache" ]]; then - GH_TOKEN=$(cat "$_gh_token_cache" 2>/dev/null || echo "") - fi - # If no cache, try gh auth token (works interactively, may fail in cron) - if [[ -z "$GH_TOKEN" ]]; then - GH_TOKEN=$(gh auth token 2>/dev/null || echo "") - fi - # Cache the token if we got one (for future cron runs) - if [[ -n "$GH_TOKEN" ]]; then - mkdir -p "$(dirname "$_gh_token_cache")" - printf '%s' "$GH_TOKEN" > "$_gh_token_cache" 2>/dev/null || true - chmod 600 "$_gh_token_cache" 2>/dev/null || true - fi + # Try gh auth token (interactive sessions) + GH_TOKEN=$(gh auth token 2>/dev/null || echo "") if [[ -z "$GH_TOKEN" ]]; then # Try gopass (encrypted secret store) GH_TOKEN=$(gopass show -o github/token 2>/dev/null || echo "") @@ if [[ -n "$GH_TOKEN" ]]; then export GH_TOKEN fi fi -unset _gh_token_cacheBased on learnings NEVER store credentials, API keys, tokens, or secrets in ~/.aidevops/.agent-workspace/ directory; Store API Keys and Tokens in ~/.config/aidevops/credentials.sh with 600 file permissions (owner read/write only).



Summary
Summary by CodeRabbit
New Features
Chores