fix(installer): OpenCode integration: replace name frontmatter with mode: all and update directory names#1764
Merged
bmadcode merged 4 commits intobmad-code-org:mainfrom Feb 25, 2026
Merged
Conversation
…er, fix directory names
- Replace name: '{{name}}' with mode: all in opencode-agent.md
mode: all enables both Tab-key agent switching in the TUI and @subagent
invocation via the Task tool (mode: primary blocked subagent use)
- Remove name: '{{name}}' from opencode-task/tool/workflow/workflow-yaml templates
OpenCode derives command name from filename, not from a name frontmatter field;
the bare {{name}} value was overriding the bmad- prefixed filename causing
name collisions with built-in OpenCode commands (fixes bmad-code-org#1762)
- Fix deprecated singular directory names in platform-codes.yaml:
.opencode/agent -> .opencode/agents, .opencode/command -> .opencode/commands
- Add legacy_targets migration: cleanup() now removes stale bmad-* files from
old singular directories on reinstall so existing users don't get duplicates
- Fix removeEmptyParents to continue walking up to parent when starting dir is
already absent instead of breaking early
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🤖 Augment PR SummarySummary: Fixes OpenCode installer templates to avoid command/agent name collisions by removing invalid 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThe PR updates the OpenCode installer to use pluralized target directories ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tools/cli/installers/lib/ide/_config-driven.js (3)
337-362:⚠️ Potential issue | 🔴 Critical
getDefaultTemplatefallback re-introduces thename:frontmatter bug this PR claims to fix.The hardcoded fallback at line 339 still contains
name: '{{name}}', and line 341 usesdisable-model-invocation: true— neither of which matches the updatedopencode-agent.mdtemplate (mode: all+description). IfloadTemplateexhausts all candidates and falls through to this code path (e.g., missing template file in an unexpected install), the deployed agent file will have the invalidname:frontmatter and be missingmode: all, silently reintroducing issue#1762.The non-agent fallback (lines 353–362) similarly still emits
name: '{{name}}', which is equally irrelevant for OpenCode commands.🐛 Proposed fix for agent fallback
getDefaultTemplate(artifactType) { if (artifactType === 'agent') { return `--- -name: '{{name}}' -description: '{{description}}' -disable-model-invocation: true +mode: all +description: '{{description}}' --- You must fully embody this agent's persona and follow all activation instructions exactly as specified. <agent-activation CRITICAL="TRUE"> 1. LOAD the FULL agent file from {project-root}/{{bmadFolderName}}/{{path}} 2. READ its entire contents - this contains the complete agent persona, menu, and instructions 3. FOLLOW every step in the <activation> section precisely </agent-activation> `; } return `--- -name: '{{name}}' -description: '{{description}}' +description: '{{description}}' --- -# {{name}} - LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}} `; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 337 - 362, The agent fallback in getDefaultTemplate still emits invalid frontmatter (e.g., "name: '{{name}}'" and "disable-model-invocation: true"); update the agent branch (when artifactType === 'agent') to match opencode-agent.md by removing the hardcoded name, adding "mode: all" and keeping "description" (and any other fields from opencode-agent.md), and remove "disable-model-invocation"; likewise update the non-agent fallback to stop emitting "name: '{{name}}'" and instead emit the correct open-code frontmatter (include "mode: all" and "description" as appropriate) so both fallbacks match the new template format used by loadTemplate and avoid reintroducing the original bug.
489-540:⚠️ Potential issue | 🟠 Major
cleanupTargetonly removes files starting withbmad— case-sensitive, no glob; non-bmadfiles in legacy dirs silently block directory removal.Line 515 uses
entry.startsWith('bmad'). This is case-sensitive, so a file namedBmad-foo.md(unlikely but possible from a Windows install) is skipped. More practically: if a user placed any non-bmad file in.opencode/agent, the directory is not empty after cleanup (remaining.length > 0), so the directory is never removed bycleanupTarget.removeEmptyParentslikewise breaks at that level (line 560). The legacy directory persists, and OpenCode may still scan it, potentially producing duplicate command names — the precise problem this PR aims to solve.This is arguably by design (preserve user content), but there is no warning to the user that the legacy directory was not fully migrated due to non-BMAD files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 489 - 540, The cleanupTarget function currently only deletes entries where entry.startsWith('bmad') (case-sensitive) and silently leaves directories containing non-BMAD files, preventing the directory removal and downstream removeEmptyParents logic; change the matching to a case-insensitive check (e.g. test entry against /^bmad/i or use entry.toLowerCase().startsWith('bmad')) so names like "Bmad-foo.md" are removed, and after the removal pass if the directory is not empty and removedCount > 0 (use the existing remaining variable) emit a non-silent warning/log (unless options.silent) that the legacy directory could not be fully cleaned due to non-BMAD files so the caller/user is informed and removeEmptyParents behavior is predictable.
548-567:⚠️ Potential issue | 🟡 MinorJSDoc terminology is misleading — "recursively" implies downward traversal, not upward ancestor walking.
The method removes empty directories while walking up the path chain toward
projectDir, not recursively into subdirectories. The doc should say "walks up" or "traverses ancestor directories."Additionally, line 561 uses
fs.rmdir(fullPath)while the rest of the file usesfs.remove()for removals (lines 518, 535). For consistency and clarity, usefs.remove()here as well — it's safe for empty directories and aligns with thefs-extrapatterns used throughout the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 548 - 567, Update the removeEmptyParents function documentation and implementation: change any JSDoc text that says "recursively" to clarify it "walks up" or "traverses ancestor directories toward projectDir" to reflect upward traversal, and replace the fs.rmdir(fullPath) call inside removeEmptyParents with fs.remove(fullPath) to match the rest of the file's fs-extra usage (refer to function removeEmptyParents and the fs.rmdir call).
🧹 Nitpick comments (2)
tools/cli/installers/lib/ide/platform-codes.yaml (1)
134-143:legacy_targetsis undocumented in the schema comment block.The schema comment at lines 188–203 documents every installer field (
target_dir,template_type,header_template,body_template,targets,artifact_types,skip_existing) but omitslegacy_targets. Any developer adding a new platform with a migration need will have no schema guidance.📄 Proposed schema addition
# targets: array (optional) # For multi-target installations # - target_dir: string # template_type: string # artifact_types: [agents, workflows, tasks, tools] +# legacy_targets: array (optional) # Old target dirs to clean up on reinstall (migration) # artifact_types: array (optional) # Filter which artifacts to install (default: all)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/platform-codes.yaml` around lines 134 - 143, Add documentation for the legacy_targets field to the schema comment block alongside the existing fields (target_dir, template_type, header_template, body_template, targets, artifact_types, skip_existing); describe that legacy_targets is an optional list of string paths representing deprecated installer target locations used for migration/compatibility, specify expected value type (array of strings), intended usage (lookup/remapping during migration), and include a short example (e.g., [" .opencode/agent", ".opencode/command"]) and note it is ignored for new installs. Update the comment block where installer fields are documented so developers see legacy_targets next to targets and artifact_types.tools/cli/installers/lib/ide/_config-driven.js (1)
456-462: No user-visible signal for the legacy migration step.
cleanupTargetat line 527 logs individual file removals (Cleaned N BMAD files from ${targetDir}), but there is no higher-level message indicating a migration is in progress. Users reinstalling OpenCode will see files deleted from.opencode/agentand.opencode/commandwith no contextual explanation that this is an intentional one-time migration, which may cause alarm.🔍 Suggested addition
if (this.installerConfig?.legacy_targets) { + if (!options.silent) await prompts.log.message(' Migrating legacy OpenCode directories...'); for (const legacyDir of this.installerConfig.legacy_targets) { await this.cleanupTarget(projectDir, legacyDir, options); await this.removeEmptyParents(projectDir, legacyDir); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 456 - 462, The legacy migration loop using installerConfig.legacy_targets calls cleanupTarget and removeEmptyParents but emits no high-level user-visible message; add a clear informational log before processing (e.g., via this.logger.info or processLogger) that states a one-time legacy-directory migration is starting and lists the legacy targets (reference installerConfig.legacy_targets, projectDir, cleanupTarget, removeEmptyParents) so users know deletions from directories like .opencode/agent and .opencode/command are intentional; keep the message concise and only emitted when installerConfig.legacy_targets is non-empty.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/cli/installers/lib/ide/_config-driven.js`:
- Around line 457-461: The legacy_targets cleanup runs unconditionally and can
execute when the current installer config is invalid; change the cleanup flow so
legacy migration only runs if a valid current installer config exists (e.g.,
require this.installerConfig.targets or this.installerConfig.target_dir or an
isValidInstallerConfig() check) before iterating
this.installerConfig.legacy_targets; move the for-loop that calls cleanupTarget
and removeEmptyParents behind that guard (or add an early return when the
installer config is invalid) so legacy cleanup cannot run when installation
would later fail.
- Around line 555-558: The loop advances current with current =
path.dirname(current) when fullPath is missing but doesn't update last, which is
confusing and can cause an extra iteration; update last = current immediately
when you advance current (and add a brief comment explaining the current/last
invariant), then proceed. To reduce the TOCTOU window between
fs.readdir(fullPath) and fs.rmdir(fullPath), don't bail out of the entire upward
walk on any rmdir error: after readdir returns empty, attempt fs.rmdir(fullPath)
and on error inspect the code—if it's ENOENT or ENOTEMPTY (or another transient
concurrency-related error), treat it as “skip this level and continue upward”
rather than breaking the loop; only break/throw for unexpected/fatal errors. Use
the existing symbols fullPath, current, last, fs.pathExists, fs.readdir,
fs.rmdir to locate and modify the logic.
---
Outside diff comments:
In `@tools/cli/installers/lib/ide/_config-driven.js`:
- Around line 337-362: The agent fallback in getDefaultTemplate still emits
invalid frontmatter (e.g., "name: '{{name}}'" and "disable-model-invocation:
true"); update the agent branch (when artifactType === 'agent') to match
opencode-agent.md by removing the hardcoded name, adding "mode: all" and keeping
"description" (and any other fields from opencode-agent.md), and remove
"disable-model-invocation"; likewise update the non-agent fallback to stop
emitting "name: '{{name}}'" and instead emit the correct open-code frontmatter
(include "mode: all" and "description" as appropriate) so both fallbacks match
the new template format used by loadTemplate and avoid reintroducing the
original bug.
- Around line 489-540: The cleanupTarget function currently only deletes entries
where entry.startsWith('bmad') (case-sensitive) and silently leaves directories
containing non-BMAD files, preventing the directory removal and downstream
removeEmptyParents logic; change the matching to a case-insensitive check (e.g.
test entry against /^bmad/i or use entry.toLowerCase().startsWith('bmad')) so
names like "Bmad-foo.md" are removed, and after the removal pass if the
directory is not empty and removedCount > 0 (use the existing remaining
variable) emit a non-silent warning/log (unless options.silent) that the legacy
directory could not be fully cleaned due to non-BMAD files so the caller/user is
informed and removeEmptyParents behavior is predictable.
- Around line 548-567: Update the removeEmptyParents function documentation and
implementation: change any JSDoc text that says "recursively" to clarify it
"walks up" or "traverses ancestor directories toward projectDir" to reflect
upward traversal, and replace the fs.rmdir(fullPath) call inside
removeEmptyParents with fs.remove(fullPath) to match the rest of the file's
fs-extra usage (refer to function removeEmptyParents and the fs.rmdir call).
---
Nitpick comments:
In `@tools/cli/installers/lib/ide/_config-driven.js`:
- Around line 456-462: The legacy migration loop using
installerConfig.legacy_targets calls cleanupTarget and removeEmptyParents but
emits no high-level user-visible message; add a clear informational log before
processing (e.g., via this.logger.info or processLogger) that states a one-time
legacy-directory migration is starting and lists the legacy targets (reference
installerConfig.legacy_targets, projectDir, cleanupTarget, removeEmptyParents)
so users know deletions from directories like .opencode/agent and
.opencode/command are intentional; keep the message concise and only emitted
when installerConfig.legacy_targets is non-empty.
In `@tools/cli/installers/lib/ide/platform-codes.yaml`:
- Around line 134-143: Add documentation for the legacy_targets field to the
schema comment block alongside the existing fields (target_dir, template_type,
header_template, body_template, targets, artifact_types, skip_existing);
describe that legacy_targets is an optional list of string paths representing
deprecated installer target locations used for migration/compatibility, specify
expected value type (array of strings), intended usage (lookup/remapping during
migration), and include a short example (e.g., [" .opencode/agent",
".opencode/command"]) and note it is ignored for new installs. Update the
comment block where installer fields are documented so developers see
legacy_targets next to targets and artifact_types.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
tools/cli/installers/lib/ide/_config-driven.jstools/cli/installers/lib/ide/platform-codes.yamltools/cli/installers/lib/ide/templates/combined/opencode-agent.mdtools/cli/installers/lib/ide/templates/combined/opencode-task.mdtools/cli/installers/lib/ide/templates/combined/opencode-tool.mdtools/cli/installers/lib/ide/templates/combined/opencode-workflow-yaml.mdtools/cli/installers/lib/ide/templates/combined/opencode-workflow.md
💤 Files with no reviewable changes (4)
- tools/cli/installers/lib/ide/templates/combined/opencode-workflow-yaml.md
- tools/cli/installers/lib/ide/templates/combined/opencode-task.md
- tools/cli/installers/lib/ide/templates/combined/opencode-workflow.md
- tools/cli/installers/lib/ide/templates/combined/opencode-tool.md
- Add project boundary guard to removeEmptyParents() using path.resolve and startsWith check to prevent traversal outside projectDir (Augment) - Fix JSDoc: "Recursively remove" -> "Walk up ancestor directories" - Add user-visible migration log message when processing legacy_targets - Document legacy_targets field in Installer Config Schema comment block in platform-codes.yaml (CodeRabbit + Augment) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rity - Distinguish recoverable errors (ENOTEMPTY, ENOENT) from fatal errors in removeEmptyParents() catch block — skip level and continue upward on TOCTOU races or concurrent removal, break only on fatal errors (EACCES) - Add comment clarifying loop invariant for missing-path continue branch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced Feb 25, 2026
2 tasks
This was referenced Mar 14, 2026
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.
Summary
Closes #1762
Partial revert of #1644 (see notes below)
What
name: '{{name}}'fromopencode-task.md,opencode-tool.md,opencode-workflow.md,opencode-workflow-yaml.mdplatform-codes.yaml:.opencode/agent→.opencode/agents,.opencode/command→.opencode/commandslegacy_targetsmigration inplatform-codes.yaml+cleanup()in_config-driven.jsso existing users with old singular dirs get them cleaned up on reinstallOn
name:in frontmattername:is not a valid OpenCode frontmatter field. Per the OpenCode commands documentation:The supported options are
description,agent,subtask, andmodel. There is nonamefield. The same applies to agents — the agent name is derived from the filename, not from aname:frontmatter key.The presence of
name: '{{name}}'(where{{name}}resolves to e.g.pm,analyst) was overriding thebmad--prefixed filename-based name with a bare, non-prefixed name — causing the name collisions reported in #1762.This PR removes
name:from all five OpenCode templates.Migration note
Users who installed BMAD with a previous version of the OpenCode installer will have
bmad-*files in.opencode/agent/and.opencode/command/(singular). The oldcleanup()logic only cleaned directories listed in the current config, so those stale files would have persisted alongside the new plural-directory files — causing duplicates in OpenCode's command discovery.This PR adds
legacy_targetsto the OpenCode platform config and updatescleanup()in_config-driven.jsto clean the old singular directories on the next reinstall.Testing
opencode-*.mdtemplates have noname:field after this changeopencode-agent.mdfrontmatter is exactlymode: all+description:platform-codes.yamltargets are.opencode/agentsand.opencode/commands