Skip to content

refactor(agents): derive identity from directory name#2046

Open
alexeyv wants to merge 3 commits intomainfrom
refactor/derive-agent-identity-2
Open

refactor(agents): derive identity from directory name#2046
alexeyv wants to merge 3 commits intomainfrom
refactor/derive-agent-identity-2

Conversation

@alexeyv
Copy link
Copy Markdown
Collaborator

@alexeyv alexeyv commented Mar 18, 2026

Summary

  • Remove redundant name, canonicalId, and module fields from all 9 agent sidecar manifests (bmad-skill-manifest.yaml)
  • Update manifest-generator.js, bmad-artifacts.js, path-utils.js, skill-manifest.js, and _config-driven.js to derive agent identity from directory name and module context
  • Remove the agent exemption for canonicalId warnings in collectSkills()

Test plan

  • npm run test:install — 232 passed, 0 failed
  • npm run validate:schemas && npm run test:schemas — 52 passed, 0 failed
  • Full push gate (format, lint, lint:md, docs:build, validate:schemas, test:schemas, test:install, validate:refs) — all green

🤖 Generated with Claude Code

alexeyv and others added 2 commits March 17, 2026 19:21
Remove redundant name, canonicalId, and module fields from all 9 agent
sidecar manifests. Update manifest-generator, bmad-artifacts, path-utils,
skill-manifest, and _config-driven to derive identity from directory name
and module context instead of reading from YAML.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 18, 2026

📝 Walkthrough

Walkthrough

This PR shifts identity resolution from manifest-provided fields to directory-derived names. Removes name, module, and canonicalId from agent skill manifests; updates installer code to derive these identifiers from directory paths instead of manifest entries; updates tests accordingly.

Changes

Cohort / File(s) Summary
Agent skill manifest identity removal
src/bmm/agents/bmad-agent-analyst/bmad-skill-manifest.yaml, src/bmm/agents/bmad-agent-architect/bmad-skill-manifest.yaml, src/bmm/agents/bmad-agent-dev/bmad-skill-manifest.yaml, src/bmm/agents/bmad-agent-pm/bmad-skill-manifest.yaml, src/bmm/agents/bmad-agent-qa/bmad-skill-manifest.yaml, src/bmm/agents/bmad-agent-quick-flow-solo-dev/bmad-skill-manifest.yaml, src/bmm/agents/bmad-agent-sm/bmad-skill-manifest.yaml, src/bmm/agents/bmad-agent-tech-writer/bmad-skill-manifest.yaml, src/bmm/agents/bmad-agent-ux-designer/bmad-skill-manifest.yaml
Removed public fields name, module, and canonicalId from all agent manifests. Also updated role field in analyst, tech-writer, and ux-designer agents to include additional role qualifications or expertise areas.
Manifest generator identity derivation
tools/cli/installers/lib/core/manifest-generator.js
Modified skill and agent object construction to derive name and canonicalId from directory names (derivedName/derivedCanonicalId) instead of manifest fields; updated validation to warn on any canonicalId presence in manifests.
Path-based skill name resolution
tools/cli/installers/lib/ide/shared/path-utils.js
Changed resolveSkillName() to always derive filenames from artifact.relativePath via toDashPath(), removing previous canonicalId-based short-circuit path.
Artifact directory scanning
tools/cli/installers/lib/ide/shared/bmad-artifacts.js
Removed skill manifest dependency; updated agent and task canonicalId derivation to use directory names (basename of parent directory) instead of manifest entries.
Agent manifest canonical ID handling
tools/cli/installers/lib/ide/shared/skill-manifest.js
Updated getCanonicalId() to return empty string for single-entry agent-type manifests, deferring identity to directory-derived values.
Installer documentation
tools/cli/installers/lib/ide/_config-driven.js
Updated code comments to reflect that skill names are always path-derived rather than manifest-derived.
Test expectations
test/test-installation-components.js
Updated test suite to reference bmad-agent-bmad-master instead of bmad-master in all skill paths, assertions, and frontmatter validations; removed sidecar manifest fixture usage for canonical identity.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'refactor(agents): derive identity from directory name' accurately and concisely summarizes the main objective of the pull request: refactoring agent identity derivation to use directory names instead of manifest fields.
Description check ✅ Passed The description clearly explains the core changes: removing redundant fields from agent manifests, updating code to derive identity from directory names, and removing canonicalId warnings exemption. It includes a comprehensive test plan with passing results.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/derive-agent-identity-2
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Mar 18, 2026

🤖 Augment PR Summary

Summary: This PR refactors agent/skill identity handling so IDs and names are derived from path/directory conventions rather than sidecar manifest fields.

Changes:

  • Removed redundant name, module, and canonicalId fields from the 9 BMM agent bmad-skill-manifest.yaml sidecars.
  • Updated manifest generation to derive agent identity from directory names/module context and to warn when manifests still declare canonicalId.
  • Standardized IDE installer naming by making resolveSkillName() always path-derived (no manifest preference).
  • Simplified installed-tree artifact discovery to avoid reading sidecar manifests for ID derivation.
  • Updated installation component tests to expect the new bmad-agent-* skill directory naming.

Technical Notes: The net effect is fewer redundant metadata fields and more convention-based naming across installers, manifests, and tests.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/shared/skill-manifest.js`:
- Around line 35-39: The current single-entry manifest handling (check of
manifest.__single.type) clears canonicalId for any typed single-entry manifest;
change the logic in the single-entry branch so that only types 'agent' or
'skill' cause canonicalId to be ignored: check manifest.__single.type strictly
against 'agent' and 'skill' and return '' only for those values, otherwise
return manifest.__single.canonicalId || ''. Update the condition that references
manifest.__single.type in skill-manifest.js accordingly to preserve canonicalId
for other types like 'workflow', 'task', and 'tool'.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 98276fa1-117d-45f9-ae35-a6172265e84d

📥 Commits

Reviewing files that changed from the base of the PR and between 72f6963 and c5887c4.

📒 Files selected for processing (15)
  • src/bmm/agents/bmad-agent-analyst/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-architect/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-dev/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-pm/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-qa/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-quick-flow-solo-dev/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-sm/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-tech-writer/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-ux-designer/bmad-skill-manifest.yaml
  • test/test-installation-components.js
  • tools/cli/installers/lib/core/manifest-generator.js
  • tools/cli/installers/lib/ide/_config-driven.js
  • tools/cli/installers/lib/ide/shared/bmad-artifacts.js
  • tools/cli/installers/lib/ide/shared/path-utils.js
  • tools/cli/installers/lib/ide/shared/skill-manifest.js
💤 Files with no reviewable changes (9)
  • src/bmm/agents/bmad-agent-quick-flow-solo-dev/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-sm/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-qa/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-tech-writer/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-architect/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-analyst/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-pm/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-dev/bmad-skill-manifest.yaml
  • src/bmm/agents/bmad-agent-ux-designer/bmad-skill-manifest.yaml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant