GH#3793: refactor command generator to eliminate code duplication#3971
GH#3793: refactor command generator to eliminate code duplication#3971marcusquinn merged 4 commits intomainfrom
Conversation
…ommand generator The script had 41 manually-defined commands, each repeating the same boilerplate: cat heredoc, frontmatter generation, counter increment, and success message. This created ~316 lines of pure duplication. Introduced create_command() function that accepts name, description, agent, subtask flag, and body via stdin. Each command definition is now a single function call with a heredoc body, eliminating all repeated boilerplate. Result: 1617 -> 1301 lines (-20%), same 41 commands generated, identical output verified by running the script. Addresses critical finding from PR #70 review (Gemini Code Assist): 'the script contains significant code duplication for command creation that could be refactored into a function to improve maintainability' Closes #3793
Summary of ChangesHello, 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 significantly refactors the command generation script by introducing a reusable helper function. This change streamlines the creation of new commands, improves maintainability, and reduces the overall codebase size by eliminating redundant code blocks previously used for each command definition. The refactoring also addresses a critical code duplication finding and sets up a follow-up for another identified code pattern. 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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughIntroduces a parameterized Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as Runner / CI
participant Generator as generate-opencode-commands.sh
participant Source as scripts/commands/*.md
participant Target as ${OPENCODE_COMMAND_DIR}
Runner->>Generator: execute script
Generator->>Generator: define create_command(), AGENT_* constants
Generator->>Generator: for each inline definition: call create_command(name, desc, agent, subtask)
create_command-->>Target: write `${name}.md` (frontmatter + body)
create_command-->>Generator: ++command_count, log success
Generator->>Source: discover files
Source-->>Generator: return .md files
Generator->>Target: copy missing `.md` files (auto-discovery)
Generator-->>Runner: print final summary (command_count, list)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
🔍 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 Mar 9 07:32:23 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is an excellent refactoring that significantly reduces code duplication by introducing the create_command helper function. The script is now much shorter and more maintainable. I have one suggestion to further improve the new helper function for clarity and efficiency.
…ect in create_command
Apply Gemini Code Assist review suggestion: stream frontmatter and body
directly to file using a command group {...} redirect instead of building
a multi-line string in a variable. Clearer logic and more idiomatic shell.
🔍 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 Mar 9 08:11:15 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.agents/scripts/generate-opencode-commands.sh (3)
46-68: Well-structured helper function with correct increment pattern.The
create_command()helper cleanly eliminates the 41-instance boilerplate duplication. Key observations:
Prefix increment
((++command_count))— correctly addresses the criticalset -eissue from PR#70where postfix((var++))returns 1 when incrementing from 0, causing premature script exit.Command-group redirect
{...} > file— idiomatic shell streaming per the review suggestion from commit 2.
body=$(cat)— efficient stdin capture for heredoc content.For additional robustness, consider validating
nameis non-empty to prevent writing to${OPENCODE_COMMAND_DIR}/.md:🛡️ Optional: Add parameter guard
create_command() { local name="$1" + [[ -n "$name" ]] || { echo "Error: command name required" >&2; return 1; } local description="$2",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/generate-opencode-commands.sh around lines 46 - 68, The create_command() function should guard against an empty name to avoid creating a file named "${OPENCODE_COMMAND_DIR}/.md": add a check at the start of create_command() that verifies name is non-empty (e.g., [[ -n "$name" ]]) and if empty emit an error via the existing logger/echo and return a non-zero status; reference the create_command function, the name parameter, OPENCODE_COMMAND_DIR, and command_count so the guard runs before any file write or increment occurs.
1203-1224: Auto-discovery logic is sound; consider explicit error handling forcp.The deduplication logic and SKILL.md skip are correct. The
[[ -f "$cmd_file" ]] || continueproperly handles empty glob expansion.One reliability consideration: if
cpfails (permissions, disk full), the script continues silently but counts it as created:🛡️ Optional: Handle cp failure
# Copy command file directly (it already has proper frontmatter) - cp "$cmd_file" "$OPENCODE_COMMAND_DIR/$cmd_name.md" - ((++command_count)) - echo -e " ${GREEN}✓${NC} Auto-discovered /$cmd_name command" + if cp "$cmd_file" "$OPENCODE_COMMAND_DIR/$cmd_name.md"; then + ((++command_count)) + echo -e " ${GREEN}✓${NC} Auto-discovered /$cmd_name command" + else + echo -e " ${RED}✗${NC} Failed to copy /$cmd_name command" >&2 + fiNote: Per the relevant code snippets, auto-discovered commands from
scripts/commands/contain extended frontmatter fields (mode: subagent,tools:) thatcreate_command()does not generate. This schema divergence appears intentional — manually-defined commands use the simpler schema while auto-discovered commands retain their richer metadata.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/generate-opencode-commands.sh around lines 1203 - 1224, The script currently runs cp "$cmd_file" "$OPENCODE_COMMAND_DIR/$cmd_name.md" without checking its exit status, then increments command_count and logs success even if the copy failed; update the auto-discovery loop to check the result of cp (e.g., test its exit status) and only ++command_count and echo the success message when cp succeeds, while on failure emit a clear error (including $cmd_file and $OPENCODE_COMMAND_DIR/$cmd_name.md) and continue without incrementing; reference COMMANDS_DIR, OPENCODE_COMMAND_DIR, cmd_name, cp, and command_count when making the change.
75-77: Consider extracting repeated agent name literals to constants.SonarCloud flags
'Build+'used 36 times. For maintainability, define constants near the top of the script:♻️ Proposed refactor
command_count=0 + +# Agent name constants +readonly AGENT_BUILD="Build+" +readonly AGENT_SEO="SEO" # ============================================================================= # COMMAND CREATION HELPERThen use throughout:
-create_command "agent-review" \ - "Systematic review and improvement of agent instructions" \ - "Build+" "true" <<'BODY' +create_command "agent-review" \ + "Systematic review and improvement of agent instructions" \ + "$AGENT_BUILD" "true" <<'BODY'This aligns with the Zero Technical Debt goal and makes future agent renames a single-line change.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/generate-opencode-commands.sh around lines 75 - 77, The literal "Build+" is repeated many times (e.g., in create_command calls such as create_command "agent-review" ... "Build+" ...) — define a single constant near the top of the script (e.g., BUILD_PLUS="Build+") and replace every hard-coded "Build+" occurrence with that constant (use the variable name in all create_command invocations and other places), ensuring quoting/expansion is correct for shell context so the behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.agents/scripts/generate-opencode-commands.sh:
- Around line 46-68: The create_command() function should guard against an empty
name to avoid creating a file named "${OPENCODE_COMMAND_DIR}/.md": add a check
at the start of create_command() that verifies name is non-empty (e.g., [[ -n
"$name" ]]) and if empty emit an error via the existing logger/echo and return a
non-zero status; reference the create_command function, the name parameter,
OPENCODE_COMMAND_DIR, and command_count so the guard runs before any file write
or increment occurs.
- Around line 1203-1224: The script currently runs cp "$cmd_file"
"$OPENCODE_COMMAND_DIR/$cmd_name.md" without checking its exit status, then
increments command_count and logs success even if the copy failed; update the
auto-discovery loop to check the result of cp (e.g., test its exit status) and
only ++command_count and echo the success message when cp succeeds, while on
failure emit a clear error (including $cmd_file and
$OPENCODE_COMMAND_DIR/$cmd_name.md) and continue without incrementing; reference
COMMANDS_DIR, OPENCODE_COMMAND_DIR, cmd_name, cp, and command_count when making
the change.
- Around line 75-77: The literal "Build+" is repeated many times (e.g., in
create_command calls such as create_command "agent-review" ... "Build+" ...) —
define a single constant near the top of the script (e.g., BUILD_PLUS="Build+")
and replace every hard-coded "Build+" occurrence with that constant (use the
variable name in all create_command invocations and other places), ensuring
quoting/expansion is correct for shell context so the behavior remains
identical.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 80da831b-7628-4911-ad50-bd02244cba10
📒 Files selected for processing (1)
.agents/scripts/generate-opencode-commands.sh
…ndling, agent constants Address CodeRabbit review suggestions: - Add parameter guard for empty name in create_command() to prevent writing to .md - Wrap cp in auto-discovery loop with if/else for graceful error handling - Extract AGENT_BUILD and AGENT_SEO constants to eliminate 40+ repeated literals Verified: ShellCheck clean (SC1091 info only), 41 commands generated, frontmatter correct.
|
All three CodeRabbit nitpick suggestions applied in commit b6f792c:
Verified: ShellCheck clean, 41 commands generated (unchanged), frontmatter output correct. |
🔍 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 Mar 9 09:17:38 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/generate-opencode-commands.sh:
- Around line 1228-1233: The loop that copies discovered command files uses cp
on "$cmd_file" to "$OPENCODE_COMMAND_DIR/$cmd_name.md" and currently only prints
an error on failure; modify the logic in the loop (around the cp, cmd_file,
cmd_name, and command_count handling) to propagate failures: either exit
immediately with a non-zero code when cp fails (fail-fast) or set a
boolean/exit_code flag when any cp fails and after the loop check that flag and
exit with a non-zero status; ensure messaging remains but that the script does
not continue to the final "Done!" success path when any copy failed.
- Around line 50-58: The create_command() function currently dereferences $2–$4
without ensuring they exist, which under set -u can cause an unbound variable
error; add an explicit arity check at the top of create_command (e.g., verify
that $# -ge 4) and return a clear diagnostic (using the existing error
formatting) before any use of $2, $3, or $4 so the function fails predictably on
insufficient arguments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4a6d7147-5383-4297-9bde-c1e6e83893e9
📒 Files selected for processing (1)
.agents/scripts/generate-opencode-commands.sh
|
Dispatching worker to address CodeRabbit CHANGES_REQUESTED feedback.
|
Address CodeRabbit CHANGES_REQUESTED review on PR #3971: 1. Add (($# == 4)) arity check at top of create_command() — prevents unhelpful 'unbound variable' errors under set -u when called with fewer than 4 arguments. 2. Add exit 1 after cp failure in auto-discovery loop — ensures the script exits non-zero on partial generation instead of reporting success with an incomplete command set.
🔍 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 Mar 9 18:12:22 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Review Bot Suggestions - All AddressedAll 6 review bot suggestions have been applied across 4 commits. Summary:
Verification:
No invalid suggestions found — all 6 were valid improvements. Removed |
1 participant
Summary
create_command()helper function to eliminate repeated boilerplate across 41 manually-defined command definitions((var++))post-increment patterns across 50+ other scriptsDetails
Each command definition previously repeated the same 5-line pattern:
cat >"$OPENCODE_COMMAND_DIR/name.md" <<'EOF'with manually-written frontmatterEOF((++command_count))echo -e " ${GREEN}✓${NC} Created /name command"The new
create_command()function accepts name, description, agent, subtask flag, and body via stdin heredoc. Frontmatter is generated programmatically based on which fields are non-empty.Verification
Review Findings Addressed
((var++))across scriptsCloses #3793
Summary by CodeRabbit