Skip to content

Conversation

@osterman
Copy link
Member

@osterman osterman commented Oct 16, 2025

what

  • Introduce command registry pattern for organizing Atmos built-in commands
  • Implement cmd/internal package with CommandProvider interface and registry
  • Migrate about command as proof-of-concept using new pattern
  • Add comprehensive documentation (PRD, developer guide, test coverage report)
  • Achieve 100% test coverage for all new components (14 tests passing)

why

  • Modular organization: Each command family will live in its own package (e.g., cmd/terraform/, cmd/describe/)
  • Plugin readiness: Foundation for future external plugin support
  • Backward compatibility: Coexists seamlessly with custom commands from atmos.yaml
  • Self-registering: Commands register via init() functions - no manual command list maintenance
  • Future-proof: Enables incremental migration of 115+ existing commands in separate PRs

changes

New Infrastructure

  • cmd/internal/command.go - CommandProvider interface definition
  • cmd/internal/registry.go - Thread-safe command registry implementation
  • cmd/internal/registry_test.go - Comprehensive tests (12 test cases, 100% coverage)

Proof-of-Concept Migration

  • cmd/about/about.go - Migrated command using registry pattern
  • cmd/about/about_test.go - Tests (2 test cases, 100% coverage)
  • cmd/about/markdown_about.md - Embedded markdown content
  • cmd/about.go - Marked deprecated (for cleanup in future PR)
  • cmd/about_test.go - Marked deprecated (for cleanup in future PR)

Root Command Integration

  • cmd/root.go - Added registry registration and blank import for about command

Documentation

  • docs/prd/command-registry-pattern.md (1,192 lines) - Complete PRD with:
    • Architecture overview and design decisions
    • Integration with custom commands from atmos.yaml
    • Three nested command patterns (static, dynamic, deeply nested)
    • Migration guide for future command families
    • Testing strategy and FAQ
  • docs/developing-atmos-commands.md (540 lines) - Developer guide with:
    • Quick start tutorial for creating new commands
    • Four command patterns with code examples
    • Best practices and common issues
    • Testing guidelines and checklist
  • docs/prd/command-registry-test-coverage.md - Test coverage report
  • CLAUDE.md - Updated "Adding New CLI Command" section with registry pattern

testing

Test Coverage: 100%

$ go test ./cmd/internal/... ./cmd/about/... -cover
ok  	github.com/cloudposse/atmos/cmd/internal	0.192s	coverage: 100.0%
ok  	github.com/cloudposse/atmos/cmd/about	    0.594s	coverage: 100.0%

All Tests Passing

  • ✅ 12/12 registry tests passing
  • ✅ 2/2 about command tests passing
  • ✅ Binary builds successfully
  • atmos about command verified working

Test Categories

  • Command registration (single, multiple, override)
  • Provider retrieval and grouping
  • Batch operations and error handling
  • Nested and deeply nested command hierarchies
  • Thread safety (concurrent registration)
  • CommandProvider interface implementation
  • Command execution with output verification

backward compatibility

Zero breaking changes

  • All existing commands continue working unchanged
  • Custom commands from atmos.yaml still override built-in commands
  • Command aliases still functional
  • Execution order preserved: built-in → custom → aliases

next steps

This PR establishes the foundation. Future command migrations will follow this process:

  1. Pick a command family (list, describe, terraform, etc.)
  2. Follow migration guide in docs/prd/command-registry-pattern.md
  3. Submit independent PR per command family

Suggested migration order:

  1. Simple commands: version, support, completion
  2. Static subcommands: list, validate
  3. Complex families: describe, terraform, helmfile
  4. Cloud integrations: aws, atlantis

references

Summary by CodeRabbit

  • New Features

    • Built-in commands now load automatically at startup, improving consistency of CLI behavior and help organization.
    • About command remains available with embedded markdown content and clearer output.
  • Refactor

    • Migrated CLI commands to a registry-based pattern for more modular, maintainable command management.
  • Documentation

    • Added comprehensive guides on creating and organizing commands, including command groups, patterns, best practices, and migration notes.
    • Introduced a product requirements document outlining the command registry approach.
  • Tests

    • Expanded unit tests for the About command and command registration flows.

@osterman osterman requested review from a team as code owners October 16, 2025 02:28
@mergify
Copy link

mergify bot commented Oct 16, 2025

💥 This pull request now has conflicts. Could you fix it @osterman? 🙏

@mergify mergify bot added conflict This PR has conflicts triage Needs triage labels Oct 16, 2025
osterman and others added 2 commits October 15, 2025 21:32
This commit introduces a command registry pattern for organizing
Atmos built-in commands, enabling modular command organization and
future plugin support while maintaining full backward compatibility
with custom commands from atmos.yaml.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@osterman osterman force-pushed the osterman/cmd-package-refactor branch from e1f2a34 to a940d38 Compare October 16, 2025 02:32
@github-actions github-actions bot added the size/xl Extra large size PR label Oct 16, 2025
@mergify mergify bot removed the conflict This PR has conflicts label Oct 16, 2025
@mergify
Copy link

mergify bot commented Oct 16, 2025

Warning

This PR exceeds the recommended limit of 1,000 lines.

Large PRs are difficult to review and may be rejected due to their size.

Please verify that this PR does not address multiple issues.
Consider refactoring it into smaller, more focused PRs to facilitate a smoother review process.

@osterman osterman added the minor New features that do not break anything label Oct 16, 2025
osterman and others added 2 commits October 15, 2025 21:34
The about command has been fully migrated to cmd/about/about.go using
the new registry pattern. No need to preserve deprecated files - git
history serves that purpose.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Instead of duplicating markdown files in each command package, all markdown
is now maintained in cmd/markdown/ and exported via cmd/markdown/content.go.
This provides a single source of truth while working around Go's embed
restriction that prevents using '..' in embed paths from subpackages.

Changes:
- Created cmd/markdown/content.go with exported AboutMarkdown variable
- Updated cmd/about to import and use markdown.AboutMarkdown
- Removed duplicate cmd/about/markdown_about.md file
- Updated PRD and developer docs to document this pattern

Benefits:
- Single source of truth for all markdown content
- No duplication across command packages
- Compatible with Go embed restrictions
- Easier to maintain and update

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Oct 16, 2025

Codecov Report

❌ Patch coverage is 95.71429% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.00%. Comparing base (a97a4a8) to head (e8288ec).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/root.go 57.14% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1643      +/-   ##
==========================================
+ Coverage   64.88%   65.00%   +0.12%     
==========================================
  Files         337      338       +1     
  Lines       37472    37535      +63     
==========================================
+ Hits        24315    24401      +86     
+ Misses      11212    11186      -26     
- Partials     1945     1948       +3     
Flag Coverage Δ
unittests 65.00% <95.71%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cmd/about/about.go 100.00% <100.00%> (ø)
cmd/internal/registry.go 100.00% <100.00%> (ø)
cmd/root.go 46.76% <57.14%> (+0.03%) ⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@osterman
Copy link
Member Author

@coderabbitai full review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

✅ Actions performed

Full review triggered.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 16, 2025

📝 Walkthrough

Walkthrough

Adds a built-in command registry pattern. Implements CommandProvider interface and a thread-safe registry. Refactors the about command into its own package using the provider pattern, embeds its markdown content, wires registration in root initialization, updates tests accordingly, removes legacy about files, and adds extensive documentation.

Changes

Cohort / File(s) Summary of changes
Registry Core
cmd/internal/command.go, cmd/internal/registry.go, cmd/internal/registry_test.go
Introduces CommandProvider interface and a mutex-protected registry with register/get/list/count/reset APIs; adds comprehensive unit tests including concurrency and nesting cases.
About Command (Provider-based)
cmd/about/about.go, cmd/markdown/content.go
Implements AboutCommandProvider with GetCommand/Name/Group; defines and registers about Cobra command; embeds about.md as markdown.AboutMarkdown and prints it in RunE.
About Tests (New)
cmd/about/about_test.go
Adds tests validating command execution output and provider methods (GetCommand, GetName, GetGroup).
Legacy About Removal
cmd/about.go, cmd/about_test.go
Removes old about command implementation and its unit test tied to embedded markdown string.
Root Wiring
cmd/root.go
Blank-imports cmd/about for side-effect registration; calls internal.RegisterAll(RootCmd) during init with error handling.
Documentation Updates
docs/developing-atmos-commands.md, docs/prd/command-registry-pattern.md, CLAUDE.md
Adds/overhauls docs describing the registry pattern, migration guides, examples, groups taxonomy, testing, and updates contributor guidance.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor User
    participant main as main (process)
    participant root as cmd/root.init
    participant about as cmd/about.init
    participant reg as internal.Registry
    participant cobra as cobra.RootCmd

    rect rgba(200,220,255,0.25)
    note over main: Process start
    main->>about: Blank import triggers init()
    about->>reg: Register(&AboutCommandProvider{})
    end

    rect rgba(200,255,200,0.25)
    main->>root: init()
    root->>reg: RegisterAll(RootCmd)
    reg-->>root: Iterate providers, add subcommands
    root->>cobra: RootCmd ready with built-ins
    end

    User->>cobra: atmos about
    cobra->>about: RunE()
    about->>about: utils.PrintfMarkdown(markdown.AboutMarkdown)
    about-->>User: Rendered markdown
Loading
sequenceDiagram
    autonumber
    participant Provider as AboutCommandProvider
    participant Registry as internal.Registry
    participant Root as cobra.RootCmd

    Provider->>Registry: Register(provider) // overwrites by name if exists
    Root->>Registry: RegisterAll(Root)
    alt Provider returns nil command
        Registry-->>Root: error
    else Valid command
        Registry->>Root: Root.AddCommand(aboutCmd)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • aknysh
  • Gowiem
  • nitrocode

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.43% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "feat: Add command registry pattern foundation with 100% test coverage" directly and clearly describes the primary change introduced in this PR. The changeset predominantly focuses on establishing a new command registry pattern infrastructure (CommandProvider interface, thread-safe registry, and supporting mechanisms) with comprehensive test coverage achieving 100% across new components. The title accurately captures this core contribution, references the architectural pattern introduced, and highlights the quality commitment through test coverage. The phrasing is concise, specific, and sufficiently informative for a developer scanning commit history to understand what was accomplished.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch osterman/cmd-package-refactor

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.

Copy link
Contributor

@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: 0

🧹 Nitpick comments (3)
cmd/internal/registry.go (3)

28-50: Register implementation is thread-safe and appropriately simple.

The function correctly uses write locks for map mutation. Intentional overwrite behavior (no uniqueness check) is well-documented and supports testing flexibility and future plugin override scenarios.

For added robustness, consider optional validation:

 func Register(provider CommandProvider) {
+	if provider == nil {
+		panic("cannot register nil command provider")
+	}
+
 	registry.mu.Lock()
 	defer registry.mu.Unlock()
 
 	name := provider.GetName()
+	if name == "" {
+		panic("command provider returned empty name")
+	}

This validates at registration time (during init) rather than later during RegisterAll, catching configuration errors earlier. However, current implementation is acceptable for an internal package where callers are trusted.


52-84: RegisterAll correctly validates providers before registration.

Thread safety is appropriate with RLock for read-only iteration. Error handling includes provider name for clear diagnostics. Nil command check prevents runtime panics.

Current fail-fast behavior (returning error on first nil command) is appropriate for init-time registration. If a partial registration state is a concern, consider collecting all errors:

 func RegisterAll(root *cobra.Command) error {
 	registry.mu.RLock()
 	defer registry.mu.RUnlock()
 
+	var errs []error
 	for name, provider := range registry.providers {
 		cmd := provider.GetCommand()
 		if cmd == nil {
-			return fmt.Errorf("command provider %s returned nil command", name)
+			errs = append(errs, fmt.Errorf("command provider %s returned nil command", name))
+			continue
 		}
 
 		root.AddCommand(cmd)
 	}
 
-	return nil
+	if len(errs) > 0 {
+		return errors.Join(errs...)
+	}
+	return nil
 }

This would attempt to register all valid commands even if some providers are broken. However, current fail-fast approach is simpler and acceptable for internal built-in commands where nil commands indicate configuration errors that should halt initialization.


98-122: ListProviders correctly groups commands by category.

Thread-safe grouping with read lock. Creates a fresh map for each call, which is appropriate for diagnostic/help text generation.

Order within groups is non-deterministic due to map iteration. If deterministic ordering is needed for help text, caller can sort the results:

import "sort"

func ListProvidersOrdered() map[string][]CommandProvider {
    grouped := ListProviders()
    for _, providers := range grouped {
        sort.Slice(providers, func(i, j int) bool {
            return providers[i].GetName() < providers[j].GetName()
        })
    }
    return grouped
}

However, current implementation is sufficient for diagnostic purposes.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 032025e and bf0c81c.

📒 Files selected for processing (12)
  • CLAUDE.md (1 hunks)
  • cmd/about.go (0 hunks)
  • cmd/about/about.go (1 hunks)
  • cmd/about/about_test.go (1 hunks)
  • cmd/about_test.go (0 hunks)
  • cmd/internal/command.go (1 hunks)
  • cmd/internal/registry.go (1 hunks)
  • cmd/internal/registry_test.go (1 hunks)
  • cmd/markdown/content.go (1 hunks)
  • cmd/root.go (2 hunks)
  • docs/developing-atmos-commands.md (1 hunks)
  • docs/prd/command-registry-pattern.md (1 hunks)
💤 Files with no reviewable changes (2)
  • cmd/about_test.go
  • cmd/about.go
🧰 Additional context used
🧠 Learnings (10)
📚 Learning: 2025-10-11T23:25:09.224Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-11T23:25:09.224Z
Learning: Applies to cmd/new_command.go : When adding a new CLI command, create cmd/new_command.go with the Cobra command definition following the documented pattern.

Applied to files:

  • cmd/internal/command.go
  • CLAUDE.md
📚 Learning: 2025-10-11T23:25:09.224Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-11T23:25:09.224Z
Learning: Applies to cmd/**/*.go : Define one Cobra command per file under cmd/ and follow example pattern with embedded markdown examples.

Applied to files:

  • cmd/internal/command.go
  • docs/developing-atmos-commands.md
  • CLAUDE.md
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
PR: cloudposse/atmos#1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.

Applied to files:

  • docs/developing-atmos-commands.md
  • CLAUDE.md
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to cmd/**/*.go : Use Cobra's recommended command structure with a root command and subcommands

Applied to files:

  • docs/developing-atmos-commands.md
  • cmd/root.go
  • CLAUDE.md
📚 Learning: 2025-10-11T23:25:09.224Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-11T23:25:09.224Z
Learning: Applies to cmd/root.go : Standard commands gain telemetry via RootCmd.ExecuteC() in cmd/root.go; do not add extra telemetry for standard Cobra commands.

Applied to files:

  • cmd/root.go
📚 Learning: 2025-10-11T23:25:09.224Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-11T23:25:09.224Z
Learning: Applies to cmd/**/*_test.go : Place command tests under cmd/ as *_test.go files.

Applied to files:

  • cmd/about/about_test.go
📚 Learning: 2025-09-23T02:30:42.362Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-09-23T02:30:42.362Z
Learning: Applies to **/*_test.go : Every new feature must include comprehensive unit tests

Applied to files:

  • cmd/about/about_test.go
📚 Learning: 2025-10-11T23:25:09.224Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-11T23:25:09.224Z
Learning: Applies to website/docs/cli/commands/**/**/*.mdx : Create Docusaurus docs for new commands with required frontmatter, purpose note, usage, examples, arguments, and flags sections in consistent order.

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-10-11T23:25:09.224Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-11T23:25:09.224Z
Learning: Applies to cmd/markdown/*_usage.md : Store command examples in embedded markdown files named atmos_<command>_<subcommand>_usage.md.

Applied to files:

  • CLAUDE.md
📚 Learning: 2025-10-11T23:25:09.224Z
Learnt from: CR
PR: cloudposse/atmos#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-11T23:25:09.224Z
Learning: Applies to website/docs/cli/commands/**/**/*.mdx : All new commands/flags/parameters must have Docusaurus documentation at website/docs/cli/commands/<command>/<subcommand>.mdx using prescribed structure and definition lists.

Applied to files:

  • CLAUDE.md
🪛 LanguageTool
docs/developing-atmos-commands.md

[grammar] ~9-~9: There might be a mistake here.
Context: ...ds** - Commands register themselves via init() - ✅ Modular organization - Each comman...

(QB_NEW_EN)


[style] ~10-~10: Since ownership is already implied, this phrasing may be redundant.
Context: ...organization** - Each command family in its own package - ✅ Type-safe interfaces - ...

(PRP_OWN)


[grammar] ~10-~10: There might be a mistake here.
Context: ...- Each command family in its own package - ✅ Type-safe interfaces - CommandProv...

(QB_NEW_EN)


[grammar] ~11-~11: There might be a mistake here.
Context: ...ommandProvider interface for consistency - ✅ Plugin readiness - Foundation for ...

(QB_NEW_EN)


[grammar] ~12-~12: There might be a mistake here.
Context: ...- Foundation for future external plugins - ✅ Custom command compatibility - Wor...

(QB_NEW_EN)


[typographical] ~146-~146: Consider using typographic quotation marks here.
Context: ...o # Tests ``` Implementation: See "Quick Start" above. --- ### Pattern 2: Command wit...

(EN_QUOTES)


[grammar] ~238-~238: There might be a mistake here.
Context: ...agRequired("stack") } ``` Key Points: - Only parent command implements CommandPr...

(QB_NEW_EN)


[grammar] ~355-~355: There might be a mistake here.
Context: ...Command(varfileCmd) } ``` Key Points: - Dynamic commands from array - Nested sub...

(QB_NEW_EN)


[grammar] ~356-~356: There might be a mistake here.
Context: ... Points:** - Dynamic commands from array - Nested subcommand groups in sub-packages...

(QB_NEW_EN)


[grammar] ~357-~357: There might be a mistake here.
Context: ...Nested subcommand groups in sub-packages - Exported parent commands (uppercase) for...

(QB_NEW_EN)


[grammar] ~431-~431: There might be a mistake here.
Context: ...pdateKubeconfigCmd) } ``` Key Points: - Only top-level command registers with re...

(QB_NEW_EN)


[grammar] ~442-~442: There might be a mistake here.
Context: ... for GetGroup(): | Group | Commands | |-------|----------| | **Core Stack Comm...

(QB_NEW_EN)


[grammar] ~443-~443: There might be a mistake here.
Context: ... Group | Commands | |-------|----------| | Core Stack Commands | terraform, h...

(QB_NEW_EN)


[grammar] ~444-~444: There might be a mistake here.
Context: ... terraform, helmfile, workflow, packer | | Stack Introspection | describe, li...

(QB_NEW_EN)


[grammar] ~445-~445: There might be a mistake here.
Context: ...ospection** | describe, list, validate | | Configuration Management | vendor,...

(QB_NEW_EN)


[grammar] ~446-~446: There might be a mistake here.
Context: ...figuration Management** | vendor, docs | | Cloud Integration | aws, atlantis ...

(QB_NEW_EN)


[grammar] ~447-~447: There might be a mistake here.
Context: ... Cloud Integration | aws, atlantis | | Pro Features | auth, pro | | **Oth...

(QB_NEW_EN)


[grammar] ~448-~448: There might be a mistake here.
Context: ...antis | | Pro Features | auth, pro | | Other Commands | about, completion...

(QB_NEW_EN)


[grammar] ~471-~471: There might be a mistake here.
Context: ...ercase (package-private) unless exported - Exported commands: Uppercase (e.g., `G...

(QB_NEW_EN)


[grammar] ~472-~472: There might be a mistake here.
Context: ...ppercase (e.g., GenerateCmd, EksCmd) - CommandProvider: `CommandProvide...

(QB_NEW_EN)


[grammar] ~477-~477: There might be a mistake here.
Context: ...rt**: One-line description (50-80 chars) - Long: Detailed description with contex...

(QB_NEW_EN)


[grammar] ~478-~478: There might be a mistake here.
Context: ...ong**: Detailed description with context - Examples: Use embedded markdown files ...

(QB_NEW_EN)


[grammar] ~483-~483: There might be a mistake here.
Context: ...Flags - Common flags on parent via PersistentFlags() - Specific flags on subcommands via `Fla...

(QB_NEW_EN)


[grammar] ~484-~484: There might be a mistake here.
Context: ...- Specific flags on subcommands via Flags() - Required flags marked with `MarkFlagRe...

(QB_NEW_EN)


[grammar] ~490-~490: There might be a mistake here.
Context: ...mentation** - Test command logic in internal/exec/ - Integration tests in tests/ - **Use ...

(QB_NEW_EN)


[grammar] ~491-~491: There might be a mistake here.
Context: ...ernal/exec/- **Integration tests** intests/` - Use table-driven tests for multiple sc...

(QB_NEW_EN)


[grammar] ~547-~547: There might be a mistake here.
Context: ...f truth** - All markdown in one location - ✅ No duplicate files - Avoids copyin...

(QB_NEW_EN)


[grammar] ~548-~548: There might be a mistake here.
Context: ...voids copying markdown into each package - ✅ Works with Go embed - Subpackages ...

(QB_NEW_EN)


[grammar] ~549-~549: There might be a mistake here.
Context: ...ubpackages can't use .. in embed paths - ✅ Easy to maintain - Update markdown...

(QB_NEW_EN)


[grammar] ~558-~558: There might be a mistake here.
Context: ...s from atmos.yaml: Execution Order: 1. Built-in commands register (via registry...

(QB_NEW_EN)


[grammar] ~579-~579: There might be a mistake here.
Context: ...mmands - [ ] Create package directory: cmd/[command]/ - [ ] Implement command with RunE function...

(QB_NEW_EN)


[grammar] ~580-~580: There might be a mistake here.
Context: ...[ ] Implement command with RunE function - [ ] Implement CommandProvider interface ...

(QB_NEW_EN)


[grammar] ~581-~581: There might be a mistake here.
Context: ... [ ] Implement CommandProvider interface - [ ] Register with internal.Register() ...

(QB_NEW_EN)


[grammar] ~582-~582: There might be a mistake here.
Context: ...dProvider interface - [ ] Register with internal.Register() - [ ] Add blank import to cmd/root.go - ...

(QB_NEW_EN)


[grammar] ~583-~583: There might be a mistake here.
Context: ...l.Register()- [ ] Add blank import tocmd/root.go- [ ] Add business logic tointernal/exec...

(QB_NEW_EN)


[grammar] ~584-~584: There might be a mistake here.
Context: ...md/root.go- [ ] Add business logic tointernal/exec/` - [ ] Write unit tests - [ ] Write integra...

(QB_NEW_EN)


[grammar] ~585-~585: There might be a mistake here.
Context: ... internal/exec/ - [ ] Write unit tests - [ ] Write integration tests (in tests/...

(QB_NEW_EN)


[grammar] ~586-~586: There might be a mistake here.
Context: ... ] Write integration tests (in tests/) - [ ] Add Docusaurus documentation in `web...

(QB_NEW_EN)


[grammar] ~587-~587: There might be a mistake here.
Context: ...) - [ ] Add Docusaurus documentation in website/docs/cli/commands/ - [ ] Build website to verify docs: `cd we...

(QB_NEW_EN)


[grammar] ~588-~588: There might be a mistake here.
Context: ...s/- [ ] Build website to verify docs:cd website && npm run build- [ ] Test with custom commands fromatmo...

(QB_NEW_EN)


[grammar] ~589-~589: There might be a mistake here.
Context: ...d- [ ] Test with custom commands fromatmos.yaml` - [ ] Update this guide if introducing new...

(QB_NEW_EN)


[grammar] ~628-~628: There might be a mistake here.
Context: ...mmand registration Solution: Check that old command file doesn't call `RootCmd....

(QB_NEW_EN)


[grammar] ~642-~642: There might be a mistake here.
Context: ...`` ### Issue: Import cycle Solution: - Don't import cmd package from other pa...

(QB_NEW_EN)


[grammar] ~653-~653: There might be a mistake here.
Context: ...s for reference: - Simple command: cmd/about/ - Static subcommands: cmd/describe/ - ...

(QB_NEW_EN)


[grammar] ~654-~654: There might be a mistake here.
Context: ... cmd/about/ - Static subcommands: cmd/describe/ - Dynamic subcommands: cmd/terraform/ ...

(QB_NEW_EN)


[grammar] ~655-~655: There might be a mistake here.
Context: ...d/describe/- **Dynamic subcommands**:cmd/terraform/- **Deeply nested**:cmd/aws/` --- ## Fur...

(QB_NEW_EN)

docs/prd/command-registry-pattern.md

[grammar] ~41-~41: There might be a mistake here.
Context: ...n** - Hard to find related command files 2. No clear organization - Commands and s...

(QB_NEW_EN)


[style] ~42-~42: ‘mixed together’ might be wordy. Consider a shorter alternative.
Context: ...ganization** - Commands and subcommands mixed together 3. Scalability issues - Adding more...

(EN_WORDINESS_PREMIUM_MIXED_TOGETHER)


[grammar] ~42-~42: There might be a mistake here.
Context: ... Commands and subcommands mixed together 3. Scalability issues - Adding more comma...

(QB_NEW_EN)


[grammar] ~43-~43: There might be a mistake here.
Context: ...dding more commands increases complexity 4. Plugin readiness - No foundation for e...

(QB_NEW_EN)


[grammar] ~44-~44: There might be a mistake here.
Context: ...s** - No foundation for external plugins 5. Inconsistent patterns - No standard wa...

(QB_NEW_EN)


[grammar] ~297-~297: There might be a mistake here.
Context: ...re, we: 1. Store all markdown files in cmd/markdown/ 2. Export them via `cmd/markdown/content.go...

(QB_NEW_EN)


[grammar] ~298-~298: There might be a mistake here.
Context: ...s in cmd/markdown/ 2. Export them via cmd/markdown/content.go 3. Import them in command packages **Examp...

(QB_NEW_EN)


[grammar] ~337-~337: There might be a mistake here.
Context: ...source of truth for all markdown content - ✅ No duplicate files across command pack...

(QB_NEW_EN)


[grammar] ~338-~338: There might be a mistake here.
Context: ... duplicate files across command packages - ✅ Compatible with Go's embed restriction...

(QB_NEW_EN)


[grammar] ~339-~339: There might be a mistake here.
Context: ... Compatible with Go's embed restrictions - ✅ Easy to maintain and update content #...

(QB_NEW_EN)


[grammar] ~432-~432: There might be a mistake here.
Context: ... Behavior Scenario 1: Custom command with same name as built-in ```yaml # atmo...

(QB_NEW_EN)


[grammar] ~481-~481: There might be a mistake here.
Context: ...naturally handles the override behavior: - If command exists → replaces it - If com...

(QB_NEW_EN)


[grammar] ~491-~491: There might be a mistake here.
Context: ...erequisites Before migrating a command: - ✅ Command must have tests - ✅ Command sh...

(QB_NEW_EN)


[grammar] ~607-~607: There might be a mistake here.
Context: ... consistency: | Group Name | Commands | |-----------|----------| | **Core Stack ...

(QB_NEW_EN)


[grammar] ~608-~608: There might be a mistake here.
Context: ...me | Commands | |-----------|----------| | Core Stack Commands | terraform, h...

(QB_NEW_EN)


[grammar] ~609-~609: There might be a mistake here.
Context: ... terraform, helmfile, workflow, packer | | Stack Introspection | describe, li...

(QB_NEW_EN)


[grammar] ~610-~610: There might be a mistake here.
Context: ...ospection** | describe, list, validate | | Configuration Management | vendor,...

(QB_NEW_EN)


[grammar] ~611-~611: There might be a mistake here.
Context: ...figuration Management** | vendor, docs | | Cloud Integration | aws, atlantis ...

(QB_NEW_EN)


[grammar] ~612-~612: There might be a mistake here.
Context: ... Cloud Integration | aws, atlantis | | Pro Features | auth, pro | | **Oth...

(QB_NEW_EN)


[grammar] ~613-~613: There might be a mistake here.
Context: ...antis | | Pro Features | auth, pro | | Other Commands | about, completion...

(QB_NEW_EN)


[typographical] ~616-~616: Consider using typographic quotation marks here.
Context: ...sion, support | ### Example: Migrating "list" Command Family Before: ``` cmd/ ├─...

(EN_QUOTES)


[grammar] ~704-~704: There might be a mistake here.
Context: ...ibe, list, validate) Characteristics: - Parent command is a grouping command (no...

(QB_NEW_EN)


[grammar] ~819-~819: There might be a mistake here.
Context: ...cess Go templates") } ``` Key Points: - ✅ Only parent command registers with...

(QB_NEW_EN)


[grammar] ~820-~820: There might be a mistake here.
Context: ...* - ✅ Only parent command registers with registry - ✅ Subcommands are package-pr...

(QB_NEW_EN)


[grammar] ~822-~822: There might be a mistake here.
Context: ...ariable names) - ✅ Subcommands attached in parent's init() - ✅ All related code ...

(QB_NEW_EN)


[grammar] ~829-~829: There might be a mistake here.
Context: ...m, helmfile, packer) Characteristics: - Parent command proxies to external tool ...

(QB_NEW_EN)


[grammar] ~832-~832: Please add a punctuation mark at the end of paragraph.
Context: ... - Common execution logic shared across subcommands *Example: terraform command family...

(PUNCTUATION_PARAGRAPH_END)


[grammar] ~990-~990: There might be a mistake here.
Context: ...ommand(planfileCmd) } ``` Key Points: - ✅ Parent command registers with registry...

(QB_NEW_EN)


[grammar] ~1001-~1001: There might be a mistake here.
Context: ...ands (aws, atlantis) Characteristics: - Multiple levels of nesting (grandparent ...

(QB_NEW_EN)


[grammar] ~1002-~1002: There might be a mistake here.
Context: ...f nesting (grandparent → parent → child) - Each level may have its own flags and lo...

(QB_NEW_EN)


[style] ~1003-~1003: Since ownership is already implied, this phrasing may be redundant.
Context: ...→ parent → child) - Each level may have its own flags and logic **Example: aws comma...

(PRP_OWN)


[grammar] ~1115-~1115: There might be a mistake here.
Context: ...agRequired("stack") } ``` Key Points: - ✅ Only top-level command registers w...

(QB_NEW_EN)


[grammar] ~1117-~1117: There might be a mistake here.
Context: ...ch level (aws → eks → update-kubeconfig) - ✅ Exported parent commands for cross-pac...

(QB_NEW_EN)


[grammar] ~1118-~1118: There might be a mistake here.
Context: ...nt commands for cross-package visibility - ✅ Clear directory hierarchy mirrors comm...

(QB_NEW_EN)


[grammar] ~1127-~1127: There might be a mistake here.
Context: ...For Static Subcommands (describe, list): - [ ] Create package directory: `cmd/[comm...

(QB_NEW_EN)


[grammar] ~1136-~1136: There might be a mistake here.
Context: ...namic Subcommands (terraform, helmfile): - [ ] Create package directory: `cmd/[comm...

(QB_NEW_EN)


[grammar] ~1145-~1145: There might be a mistake here.
Context: ...y ### For Deeply Nested Commands (aws): - [ ] Create package hierarchy: `cmd/[gran...

(QB_NEW_EN)


[grammar] ~1147-~1147: There might be a mistake here.
Context: ...arent]/[parent]/` - [ ] Move each level to appropriate directory - [ ] Export inte...

(QB_NEW_EN)


[grammar] ~1285-~1285: There might be a mistake here.
Context: ...} ``` Plugin discovery flow (future): 1. Atmos starts → loads built-in commands v...

(QB_NEW_EN)


[grammar] ~1286-~1286: There might be a mistake here.
Context: ...s → loads built-in commands via registry 2. Discovers plugins in ~/.atmos/plugins/...

(QB_NEW_EN)


[grammar] ~1287-~1287: There might be a mistake here.
Context: ...ds via registry 2. Discovers plugins in ~/.atmos/plugins/ 3. Registers plugins as CommandProviders 4....

(QB_NEW_EN)


[grammar] ~1288-~1288: There might be a mistake here.
Context: ...3. Registers plugins as CommandProviders 4. Processes custom commands from atmos.yam...

(QB_NEW_EN)


[grammar] ~1289-~1289: There might be a mistake here.
Context: ...rocesses custom commands from atmos.yaml 5. All three sources coexist: - Built-in...

(QB_NEW_EN)


[grammar] ~1290-~1290: There might be a mistake here.
Context: ...atmos.yaml 5. All three sources coexist: - Built-in commands (compiled in) - Plu...

(QB_NEW_EN)


[grammar] ~1291-~1291: There might be a mistake here.
Context: ...st: - Built-in commands (compiled in) - Plugins (external executables) - Cust...

(QB_NEW_EN)


[grammar] ~1292-~1292: There might be a mistake here.
Context: ... in) - Plugins (external executables) - Custom commands (from atmos.yaml) ## Be...

(QB_NEW_EN)


[grammar] ~1299-~1299: There might be a mistake here.
Context: ...* - Related commands grouped in packages 2. ✅ Easier navigation - Clear director...

(QB_NEW_EN)


[grammar] ~1300-~1300: There might be a mistake here.
Context: ...navigation** - Clear directory structure 3. ✅ Self-documenting - Package names s...

(QB_NEW_EN)


[grammar] ~1301-~1301: There might be a mistake here.
Context: ...** - Package names show command families 4. ✅ Consistent pattern - All commands ...

(QB_NEW_EN)


[grammar] ~1302-~1302: There might be a mistake here.
Context: ...✅ Consistent pattern - All commands follow same structure 5. ✅ **Custom command co...

(QB_NEW_EN)


[grammar] ~1302-~1302: There might be a mistake here.
Context: ...n** - All commands follow same structure 5. ✅ Custom command compatibility - No ...

(QB_NEW_EN)


[grammar] ~1307-~1307: There might be a mistake here.
Context: ...port** - Foundation for external plugins 7. ✅ Command marketplace - Community ca...

(QB_NEW_EN)


[grammar] ~1308-~1308: There might be a mistake here.
Context: ...ketplace** - Community can share plugins 8. ✅ Extensibility - Users can extend A...

(QB_NEW_EN)


[grammar] ~1313-~1313: There might be a mistake here.
Context: ...sk | Probability | Impact | Mitigation | |------|------------|--------|----------...

(QB_NEW_EN)


[grammar] ~1314-~1314: There might be a mistake here.
Context: ...----|------------|--------|------------| | Custom command conflicts | Low | Mediu...

(QB_NEW_EN)


[grammar] ~1315-~1315: There might be a mistake here.
Context: ...um | Test with custom command examples | | Import path confusion | Low | Low | Cl...

(QB_NEW_EN)


[grammar] ~1316-~1316: There might be a mistake here.
Context: ... | Low | Clear documentation, examples | | Init() ordering issues | Low | Medium ...

(QB_NEW_EN)


[grammar] ~1317-~1317: There might be a mistake here.
Context: ...um | Explicit import ordering, testing | | Breaking existing workflows | Low | Hi...

(QB_NEW_EN)


[grammar] ~1324-~1324: There might be a mistake here.
Context: ... Registry pattern implemented and tested - ✅ about command migrated successfully ...

(QB_NEW_EN)


[grammar] ~1325-~1325: There might be a mistake here.
Context: ... ✅ about command migrated successfully - ✅ All existing tests pass - ✅ Custom com...

(QB_NEW_EN)


[grammar] ~1326-~1326: There might be a mistake here.
Context: ...successfully - ✅ All existing tests pass - ✅ Custom commands still work (verified w...

(QB_NEW_EN)


[grammar] ~1327-~1327: There might be a mistake here.
Context: ...ds still work (verified with test cases) - ✅ No behavior changes for users - ✅ Docu...

(QB_NEW_EN)


[grammar] ~1328-~1328: There might be a mistake here.
Context: ...cases) - ✅ No behavior changes for users - ✅ Documentation complete (this PRD) ###...

(QB_NEW_EN)


[grammar] ~1345-~1345: There might be a mistake here.
Context: ...ds? A: Yes. The execution order is: 1. Built-in commands registered via registr...

(QB_NEW_EN)


[grammar] ~1346-~1346: There might be a mistake here.
Context: ...uilt-in commands registered via registry 2. Custom commands processed from atmos.yam...

(QB_NEW_EN)


[grammar] ~1347-~1347: There might be a mistake here.
Context: ...ustom commands processed from atmos.yaml 3. If custom command has same name → overri...

(QB_NEW_EN)


[grammar] ~1348-~1348: There might be a mistake here.
Context: ...mmand has same name → overrides built-in 4. If custom command has new name → extends...

(QB_NEW_EN)


[grammar] ~1349-~1349: Please add a punctuation mark at the end of paragraph.
Context: ...ommand has new name → extends available commands ### Q: Do I need to update my atmos.ya...

(PUNCTUATION_PARAGRAPH_END)


[typographical] ~1355-~1355: Consider using typographic quotation marks here.
Context: ...appens if I have a custom command named "about"? A: Your custom command will overr...

(EN_QUOTES)


[grammar] ~1361-~1361: There might be a mistake here.
Context: ...ration (last one wins), but in practice: - Built-in commands should have unique nam...

(QB_NEW_EN)


[typographical] ~1366-~1366: In American English, use a period after an abbreviation.
Context: ...w do I know which commands are built-in vs custom? A: Use atmos --help to s...

(MISSING_PERIOD_AFTER_ABBREVIATION)


[grammar] ~1376-~1376: There might be a mistake here.
Context: ...ferences - Cobra Command Documentation - [Atmos Custom Commands](/core-concepts/cu...

(QB_NEW_EN)


[grammar] ~1377-~1377: There might be a mistake here.
Context: ...om/spf13/cobra) - Atmos Custom Commands - [Go Init Functions](https://golang.org/do...

(QB_NEW_EN)


[grammar] ~1378-~1378: There might be a mistake here.
Context: ...ts/custom-commands) - Go Init Functions - [Docker CLI Command Registry](https://git...

(QB_NEW_EN)


[grammar] ~1379-~1379: There might be a mistake here.
Context: ..._go#init) - Docker CLI Command Registry - [kubectl Plugin System](https://kubernete...

(QB_NEW_EN)


[grammar] ~1384-~1384: There might be a mistake here.
Context: ... Changelog | Version | Date | Changes | |---------|------|---------| | 1.0 | 202...

(QB_NEW_EN)


[grammar] ~1385-~1385: There might be a mistake here.
Context: ...| Changes | |---------|------|---------| | 1.0 | 2025-10-15 | Initial PRD with cu...

(QB_NEW_EN)

CLAUDE.md

[grammar] ~463-~463: There might be a mistake here.
Context: .... Create command package directory: cmd/[command]/ 2. Implement CommandProvider interface: ...

(QB_NEW_EN)


[grammar] ~502-~502: There might be a mistake here.
Context: ... Add blank import to cmd/root.go: _ "github.com/cloudposse/atmos/cmd/mycommand" 4. Implement business logic in `internal/...

(QB_NEW_EN)


[grammar] ~503-~503: There might be a mistake here.
Context: ...nd"4. **Implement business logic** ininternal/exec/mycommand.go5. **Add tests** incmd/mycommand/mycommand_...

(QB_NEW_EN)


[grammar] ~504-~504: There might be a mistake here.
Context: .../exec/mycommand.go5. **Add tests** incmd/mycommand/mycommand_test.go6. **Create Docusaurus documentation** inwe...

(QB_NEW_EN)


[grammar] ~505-~505: There might be a mistake here.
Context: ... Create Docusaurus documentation in website/docs/cli/commands/<command>/<subcommand>.mdx 7. Build website to verify: `cd website &...

(QB_NEW_EN)


[grammar] ~506-~506: There might be a mistake here.
Context: ...d>.mdx7. **Build website to verify**:cd website && npm run build` 8. **Create pull request following template f...

(QB_NEW_EN)


[grammar] ~509-~509: There might be a mistake here.
Context: ...uest following template format** See: - **[docs/developing-atmos-commands.md](docs/...

(QB_NEW_EN)


[grammar] ~510-~510: There might be a mistake here.
Context: ...omplete guide with patterns and examples - **[docs/prd/command-registry-pattern.md](do...

(QB_NEW_EN)

🪛 markdownlint-cli2 (0.18.1)
docs/prd/command-registry-pattern.md

13-13: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


74-74: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


406-406: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


432-432: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


447-447: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


619-619: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


634-634: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


711-711: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


719-719: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


730-730: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


836-836: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


844-844: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


855-855: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


1007-1007: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


1014-1014: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


1022-1022: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Summary
🔇 Additional comments (24)
cmd/markdown/content.go (1)

1-8: LGTM!

Clean implementation of the centralized markdown content pattern. The use of go:embed is correct, and exporting AboutMarkdown allows command packages to access the content without violating embed's path restrictions.

cmd/about/about_test.go (2)

14-37: LGTM!

Solid test for command execution. The stdout capture mechanism correctly verifies that the command prints the expected markdown content. Proper cleanup and error handling throughout.


39-52: LGTM!

Clean provider interface test. Validates all three methods of the CommandProvider interface with appropriate assertions.

Based on learnings

cmd/internal/command.go (1)

1-50: LGTM!

Well-designed interface with comprehensive documentation. The three methods (GetCommand, GetName, GetGroup) provide a clean contract for command registration. The inline example and standard group categories are helpful for implementation guidance.

docs/developing-atmos-commands.md (1)

1-664: LGTM!

Comprehensive developer guide that clearly explains the command registry pattern. The progression from simple to complex patterns is logical, and the practical examples make implementation straightforward. The troubleshooting section and checklists are valuable additions.

Static analysis findings are false positives—the markdown formatting follows project conventions.

cmd/root.go (2)

35-40: LGTM!

Clean integration of the registry pattern. The blank import triggers init() in the about package, and importing internal provides access to RegisterAll. The comments clearly explain the side-effect registration mechanism.


590-597: LGTM!

Proper registration sequence. Built-in commands register before custom commands (processed in Execute()), maintaining the correct override behavior. Error handling logs failures appropriately.

docs/prd/command-registry-pattern.md (1)

1-1386: LGTM!

Excellent PRD that thoroughly documents the command registry pattern. The emphasis on backward compatibility with custom commands, detailed migration patterns, and clear architecture diagrams make this a valuable reference. The FAQ and risk mitigation sections address potential concerns effectively.

Static analysis findings are false positives—the document follows standard markdown conventions.

cmd/about/about.go (3)

11-21: LGTM!

Clean command definition. Uses cobra.NoArgs to prevent spurious arguments, and the RunE function correctly prints the embedded markdown content.


23-27: LGTM!

Proper registry integration. The init() function registers the provider during package initialization when imported with a blank import in cmd/root.go.


29-45: LGTM!

Correct implementation of the CommandProvider interface. All three methods return appropriate values aligned with the command's purpose and organization.

cmd/internal/registry_test.go (7)

12-30: LGTM!

Clean mock implementation for testing. The struct and methods provide a simple, testable implementation of the CommandProvider interface.


31-104: LGTM!

Solid coverage of registration scenarios. Tests validate single registration, multiple registrations, and override behavior. The override test correctly verifies that the last registered provider wins.


105-158: LGTM!

Good coverage of RegisterAll functionality. Tests verify successful registration with multiple providers and proper error handling when a provider returns nil for its command.


160-206: LGTM!

Tests validate retrieval and grouping behavior. The ListProviders test correctly verifies that providers are grouped by category and all providers are accessible.


208-269: LGTM!

Excellent coverage of nested command scenarios. Tests verify that subcommands remain accessible through the parent command after registration, and deeply nested hierarchies work correctly.


271-297: LGTM!

Tests validate counter and reset functionality. The Count test correctly verifies that overrides don't increase the count, and Reset properly clears the registry.


299-323: LGTM!

Critical concurrency test validates thread-safety. The test launches 10 concurrent registrations and verifies all complete successfully, confirming the mutex-based synchronization works correctly.

Based on learnings

CLAUDE.md (1)

460-511: Documentation accurately reflects the new registry pattern.

The updated instructions clearly document the command registry approach with a complete, working code example. The 8-step process covers all necessary aspects: package creation, provider implementation, registration, business logic, testing, and documentation.

Code example is syntactically correct and matches the registry implementation in cmd/internal/registry.go. The provider pattern (GetCommand/GetName/GetGroup) aligns with the CommandProvider interface, and the init() registration pattern matches the registry's Register() function.

References to supporting documentation (developing-atmos-commands.md, command-registry-pattern.md) provide good additional context.

Note: Static analysis grammar warnings (QB_NEW_EN) on lines 463, 502-506, 509-510 are false positives flagging backtick-wrapped code paths - these can be safely ignored.

cmd/internal/registry.go (5)

1-8: Imports correctly organized.

Package declaration and imports follow the documented three-group organization pattern: Go native (fmt, sync), third-party (cobra), and Atmos imports. All imports are utilized in the implementation.


10-26: Registry structure well-designed for thread-safe command management.

The CommandRegistry uses sync.RWMutex appropriately for concurrent access patterns. Documentation clearly distinguishes built-in commands (managed here) from custom commands (processed elsewhere), preventing confusion about scope.

Singleton pattern with package-level registry variable is appropriate for this use case where a single global registry is needed.


86-96: GetProvider follows standard Go lookup pattern.

Thread-safe read operation with appropriate RLock. Returns (value, found) tuple is idiomatic Go for map lookups with existence checking.


124-132: Count provides thread-safe registry size.

Simple read operation with appropriate locking. Useful for testing and diagnostics as documented.


134-143: Reset appropriately isolated for testing.

Thread-safe registry cleanup with clear documentation warning against production use. Acceptable design for test isolation needs.

Since this is in an internal package, access is already restricted to this module. The prominent WARNING comment further discourages misuse.

coderabbitai[bot]
coderabbitai bot previously approved these changes Oct 16, 2025
Replace dynamic error with sentinel error ErrCommandNil for proper
error wrapping and type-safe error checking.

Changes:
- Import errors package in cmd/internal/registry.go
- Use fmt.Errorf with %w to wrap ErrCommandNil sentinel error
- Import errors package in registry_test.go
- Update test to use errors.Is() for sentinel error checking
- Verify error message includes provider name for debugging

This follows Atmos error handling conventions per CLAUDE.md:
- All errors wrapped using static errors from errors/errors.go
- Use errors.Is() for error checking
- Use fmt.Errorf with %w for adding context

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
osterman and others added 2 commits October 15, 2025 22:53
Add comprehensive test demonstrating that custom commands from atmos.yaml
can extend built-in registry commands by adding subcommands to them.

This clarifies an important capability: when a custom command has the same
name as a registry command, processCustomCommands() reuses the existing
command object, allowing custom subcommands to be added without replacing
the built-in command entirely.

Changes:
- Added TestCustomCommandCanExtendRegistryCommand test
- Updated PRD to document three custom command scenarios:
  1. Replacing behavior of existing command
  2. Extending existing command with subcommands (NEW)
  3. Adding new commands
- Clarified processCustomCommands() behavior in PRD
- Maintained 100% test coverage (13/13 tests passing)

This addresses the question: 'will our registry support custom commands
injecting new commands into existing commands in the registry?'

Answer: YES - custom commands can add subcommands to registry commands
by matching the top-level command name and defining nested commands.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions
Copy link

Warning

Changelog Entry Required

This PR is labeled minor or major but doesn't include a changelog entry.

Action needed: Add a new blog post in website/blog/ to announce this change.

Example filename: website/blog/2025-10-16-feature-name.mdx

Alternatively: If this change doesn't require a changelog entry, remove the minor or major label.

Announce the first step toward pluggable commands with a blog post
explaining the architectural evolution and future roadmap.

The post covers:
- Why we're introducing the command registry pattern
- How the package-per-command architecture works
- Backward compatibility guarantees
- Future phases: migrating core commands and external plugin support
- How developers can get involved

Key terminology clarification: Commands are organized into **packages**
(Go's term for self-contained code modules), not 'packages' in the npm sense.

This positions the PR as the foundation for future plugin extensibility,
with subsequent PRs migrating existing commands into their own packages.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
osterman and others added 2 commits October 16, 2025 08:51
The command registry pattern is an internal architectural change for
Atmos development, not a user-facing feature. Updated the blog post
to make this clear.

Changes:
- Renamed 'Try It Today' section to 'For Atmos Contributors'
- Emphasized this is internal to Atmos development with zero user impact
- Renamed 'Backward Compatibility' to 'Impact on Users'
- Clarified that users won't notice any difference in behavior
- Removed language that made it sound like a user-facing feature

This addresses the feedback that the original blog post incorrectly
positioned this as something end users of Atmos would use, when it's
strictly for Atmos contributors and developers.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Establish clear guidelines for blog post audience targeting to avoid
confusion between user-facing features and contributor-facing changes.

Changes to CLAUDE.md:
- Added comprehensive 'Blog Post Guidelines' section
- Defined two audience types:
  1. User-Facing Posts (features, enhancements, bugfixes)
  2. Contributor-Facing Posts (architecture, refactoring, dev tools)
- Introduced 'contributors' tag for internal/architectural posts
- Provided blog post template with audience-specific sections
- Created tag reference with primary audience and technical tags

Changes to blog post:
- Updated tags from [architecture, extensibility, plugins] to
  [contributors, architecture, extensibility]
- This signals the post is for Atmos contributors, not users

Tag System:
- Primary audience tags: feature, enhancement, bugfix, contributors
- Secondary technical tags: architecture, terraform, workflows, etc.
- The 'contributors' tag distinguishes internal changes from user features

This addresses the feedback: 'This is strictly a developer Atmos, this
is a blog post for Atmos developers. It has nothing to do with running
Atmos itself.'

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
osterman and others added 2 commits October 16, 2025 08:56
'Architecture' in Atmos context means CLOUD architecture (how users
architect their infrastructure), not the internal code architecture
of Atmos itself. Updated tags to be more precise.

Changes:
- Blog post tags: [contributors, architecture, extensibility] →
  [contributors, atmos-core, refactoring]
- CLAUDE.md: Split secondary tags into contributor vs user categories
  - Contributor tags: atmos-core, refactoring, testing, ci-cd, developer-experience
  - User tags: terraform, helmfile, workflows, validation, performance, cloud-architecture
- Renamed 'architecture' to 'atmos-core' for Atmos codebase changes
- Added 'cloud-architecture' tag for user-facing architecture patterns
- Updated examples to use [contributors, atmos-core, refactoring]

This avoids confusion: 'architecture' is reserved for cloud
architecture content, while 'atmos-core' refers to Atmos internals.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Create tags.yml to document official blog post tags with descriptions.
This serves as the authoritative reference for what tags mean and when
to use them.

Tag categories:
- Primary Audience: feature, enhancement, bugfix, contributors
- Contributor Tags: atmos-core, refactoring, testing, ci-cd, developer-experience
- User Tags: terraform, helmfile, workflows, validation, performance, cloud-architecture
- General: announcements, breaking-changes

Each tag includes:
- label: Display name for the tag
- description: When and why to use this tag

This provides a single source of truth for blog post categorization and
helps contributors choose appropriate tags for their posts.

Note: Docusaurus automatically generates tag pages from blog post
frontmatter. This file documents the official tags and their meanings.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@aknysh aknysh merged commit ba4a2de into main Oct 16, 2025
54 checks passed
@aknysh aknysh deleted the osterman/cmd-package-refactor branch October 16, 2025 15:16
@mergify mergify bot removed the triage Needs triage label Oct 16, 2025
@github-actions
Copy link

These changes were released in v1.195.0-test.0.

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

Labels

minor New features that do not break anything size/xl Extra large size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants