Skip to content

Add shell completion command to cli-processor#551

Merged
david-waltermire merged 8 commits intometaschema-framework:developfrom
david-waltermire:feature/shell-completion
Dec 19, 2025
Merged

Add shell completion command to cli-processor#551
david-waltermire merged 8 commits intometaschema-framework:developfrom
david-waltermire:feature/shell-completion

Conversation

@david-waltermire
Copy link
Contributor

@david-waltermire david-waltermire commented Dec 15, 2025

Summary

Adds a shell-completion command to the CLI that generates tab-completion scripts for Bash and Zsh shells. This improves the user experience by providing intelligent command and option completion.

Key Features

  • Shell completion command: metaschema-cli shell-completion bash|zsh [--to FILE]
  • Registry-based completion types: Decoupled type-to-completion mapping via CompletionTypeRegistry
  • Type-aware completions: Uses Option.type() and extended ExtraArgument for completion hints
  • Fallback support: Works on systems without bash-completion package installed
  • SPI registration: Command auto-discovered via ServiceLoader

Usage

# Generate and source bash completion
source <(metaschema-cli shell-completion bash)

# Or save to file
metaschema-cli shell-completion bash --to ~/.metaschema-completion.bash

# Generate zsh completion
metaschema-cli shell-completion zsh > ~/.zsh/completions/_metaschema-cli

Completion Types Supported

Type Bash Zsh
File.class _filedir _files
URI.class / URL.class _filedir _urls
Enum types compgen -W "values..." (value1 value2 ...)

Files Changed

cli-processor module:

  • ShellCompletionCommand.java - Main command implementation
  • CompletionScriptGenerator.java - Bash/Zsh script generation
  • CompletionTypeRegistry.java - Type-to-completion mapping
  • ICompletionType.java - Completion behavior interface
  • ExtraArgument.java - Extended with type hints
  • SPI registration file

metaschema-cli module:

  • Added .type() to options for completion support
  • Registered Format and SchemaFormat enums

Test Plan

  • 40 unit tests passing in cli-processor module
  • Manual testing of bash completion generation
  • Manual testing of zsh completion generation
  • Tested --to file output option
  • End-to-end testing in actual bash/zsh shells

Related Issues

Related

PRD: PRDs/20251215-shell-completion/

Summary by CodeRabbit

  • New Features

    • Shell completion for Bash and Zsh with a generator to produce installable completion scripts.
    • Type-aware completions so arguments/options (files, URIs, enums) provide intelligent suggestions.
    • New CLI command to generate or write completion scripts.
  • Documentation

    • README updated with usage and installation instructions for Bash/Zsh completions.
  • Chores

    • .gitignore updated to ignore Git worktrees.

✏️ Tip: You can customize this high-level summary in your review settings.

This PRD proposes adding a shell-completion command to the cli-processor
module that generates Bash and Zsh completion scripts by introspecting
registered CLI commands.

Key features:
- Dynamic command introspection via CLIProcessor API
- IArgumentTypeResolver interface for extensible argument completion
- Built-in resolvers for FILE, FORMAT, URL, EXPRESSION types
- CompletionScriptGenerator for Bash and Zsh output

Addresses metaschema-framework/oscal-cli#85
@coderabbitai
Copy link

coderabbitai bot commented Dec 15, 2025

📝 Walkthrough

Walkthrough

Adds a type-aware shell-completion subsystem (Bash/Zsh): new completion type interface and registry, a script generator, a shell-completion command, tests and docs; extends ExtraArgument with optional type, annotates many options/arguments with types, exposes CLIProcessor.getTopLevelCommands() publicly, and updates .gitignore for Git worktrees.

Changes

Cohort / File(s) Change Summary
Git configuration
\.gitignore``
Add .worktrees/ ignore entry under Claude section.
Design & plan docs
\PRDs/20251215-shell-completion/PRD.md`, `PRDs/20251215-shell-completion/implementation-plan.md``
Add PRD and implementation plan describing type-based shell completion, API surface, registry, generator, command, and migration guidance.
Completion API & registry
\cli-processor/src/main/java/.../completion/ICompletionType.java`, `cli-processor/src/main/java/.../completion/CompletionTypeRegistry.java``
New ICompletionType (bash/zsh snippet API) and CompletionTypeRegistry (static registry with built-ins, register/lookup, enum support).
Script generator
\cli-processor/src/main/java/.../completion/CompletionScriptGenerator.java`, `cli-processor/src/test/java/.../completion/CompletionScriptGeneratorTest.java``
New CompletionScriptGenerator producing Bash/Zsh scripts; tests validate generated content, file and enum completions.
Shell completion command & wiring
\cli-processor/src/main/java/.../command/ShellCompletionCommand.java`, `cli-processor/src/main/resources/META-INF/services/gov.nist.secauto.metaschema.cli.processor.command.ICommand``
New ShellCompletionCommand (positional shell arg, optional --to file), writes generated script to file or stdout; registered via SPI.
CLI processor visibility
\cli-processor/src/main/java/.../CLIProcessor.java``
getTopLevelCommands() visibility changed from protected to public (signature exposed, behavior unchanged).
ExtraArgument typing
\cli-processor/src/main/java/.../command/ExtraArgument.java`, `cli-processor/src/main/java/.../command/impl/DefaultExtraArgument.java``
Added @Nullable Class<?> getType() default API and newInstance(name, required, type) factory; DefaultExtraArgument stores/returns optional type via new constructor.
Completion registry tests
\cli-processor/src/test/java/.../completion/CompletionTypeRegistryTest.java` ``
Tests for registry lookups, built-in File/URI/URL entries, custom registrations, and enum completions.
Command tests
\cli-processor/src/test/java/.../command/ShellCompletionCommandTest.java` ``
Unit tests for command metadata and Shell enum parsing/behavior.
metaschema-cli: register types & annotate options/args
\metaschema-cli/src/main/java/.../CLI.java`, `metaschema-cli/src/main/java/.../commands/*.java`, `metaschema-cli/src/main/java/.../commands/metapath/EvaluateMetapathCommand.java` `
Register enum completion types at startup and add type hints (.type(...) or ExtraArgument types) across commands/options (URI, File, Format, SchemaFormat).
Minor / imports & README
\metaschema-cli/src/main/distro/README.txt`, `... (imports across multiple files)``
Documentation section for shell completion added; imports and constructor updates to support typed arguments (e.g., java.io.File, URI).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User
  participant CLI as CLI/CLIProcessor
  participant Cmd as ShellCompletionCommand
  participant Gen as CompletionScriptGenerator
  participant Reg as CompletionTypeRegistry
  participant FS as FileSystem/Stdout

  User->>CLI: run "shell-completion <bash|zsh> [--to FILE]"
  CLI->>Cmd: dispatch command
  Cmd->>CLI: getTopLevelCommands()
  Cmd->>Gen: new(programName, commands)
  Gen->>Reg: lookup types for options/extra-args
  Reg-->>Gen: ICompletionType snippets (file, uri, enum, etc.)
  Gen->>Gen: compose Bash/Zsh completion script
  Gen-->>Cmd: return generated script
  Cmd->>FS: write (--to FILE) or stdout
  FS-->>User: script content / confirmation
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Focus review on:
    • Public API additions: ICompletionType, CompletionTypeRegistry, CompletionScriptGenerator, ShellCompletionCommand, ExtraArgument.getType().
    • CompletionScriptGenerator: quoting, sanitization, and correctness of generated Bash/Zsh fragments.
    • SPI registration entry and packaging.
    • Places adding .type(...) or typed ExtraArguments to confirm CLI parsing behavior.

Possibly related PRs

Suggested reviewers

  • aj-stein-gsa

Poem

🐰
I hopped through code and tucked leaves neat,
Bash and Zsh now sing when commands and options meet.
I hid worktrees under mossy ground,
Completions hop out with a happy sound. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.78% 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 pull request title clearly and concisely summarizes the main change: adding a shell completion command to the cli-processor module.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dfa2792 and 6eb35ca.

📒 Files selected for processing (1)
  • PRDs/20251215-shell-completion/PRD.md (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-13T21:16:12.281Z
Learnt from: CR
Repo: metaschema-framework/metaschema PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T21:16:12.281Z
Learning: Applies to website/content/specification/**/*.md : Use RFC 2119 keywords (MUST, SHOULD, MAY) consistently in uppercase in specification documentation

Applied to files:

  • PRDs/20251215-shell-completion/PRD.md
⏰ 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). (2)
  • GitHub Check: Website
  • GitHub Check: Code

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

Copy link

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

🧹 Nitpick comments (1)
PRDs/20251215-shell-completion/implementation-plan.md (1)

205-210: Consider test fixtures for CompletionScriptGenerator tests.

The test cases describe verification points (syntax validation, command inclusion) but don't mention how mock commands and options will be created. Plan to create test fixtures with realistic command hierarchies to validate complex scenarios (nested subcommands, options with format arguments, etc.).

Example: Consider creating a test builder to construct realistic ICommand hierarchies for testing.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 77b2cd3 and c4139e0.

📒 Files selected for processing (3)
  • .gitignore (1 hunks)
  • PRDs/20251215-shell-completion/PRD.md (1 hunks)
  • PRDs/20251215-shell-completion/implementation-plan.md (1 hunks)
⏰ 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). (2)
  • GitHub Check: Website
  • GitHub Check: Code
🔇 Additional comments (4)
.gitignore (1)

37-39: Good addition for local development hygiene.

Adding .worktrees/ to ignore local Git worktrees is a sensible practice to prevent committing development artifacts.

PRDs/20251215-shell-completion/PRD.md (1)

189-227: Generated examples are appropriately simplified for documentation.

The Bash and Zsh examples effectively illustrate the core completion generation approach. Note that actual generated scripts will be more complex with nested subcommands and deeper hierarchies, but the examples capture the essential structure and pattern.

Also applies to: 231-279

PRDs/20251215-shell-completion/implementation-plan.md (2)

83-92: Explicitly document resolver registry initialization approach.

The implementation approach states "initialize resolver registry" but doesn't clarify whether ArgumentTypeResolvers should be injected via constructor parameter or created internally. Since this differs from the PRD's three-parameter signature, make this design choice explicit in the plan.

Recommended: Add a note clarifying:

  • Will ArgumentTypeResolvers static methods be used internally?
  • Or should the resolver map be a constructor parameter?
  • How does this interact with future custom resolver plugins?

172-184: Escalate Task 2.2 investigation—resolve CLIProcessor access before phase 2 implementation.

Task 2.2 defers the critical decision on how ShellCompletionCommand accesses the CLIProcessor, yet the execution flow in Task 2.1 (line 159) assumes CallingContext provides this access. Investigate Option A/B/C early (before Task 2.1) to unblock command implementation.

Recommend: Move Task 2.2 earlier in the implementation order (before Task 2.1) to validate the access pattern.

Replace argName-based argument type resolution with a type-based
approach using Apache Commons CLI's Option.type() method:

- Use File.class for file path completion
- Use URI.class for URL completion
- Introduce ICompletionType interface for custom completion types
- Create Format enum implementing ICompletionType for format options

This provides type safety, explicit declaration of completion behavior,
and extensibility for custom completion types.

Also adds Phase 5 task for migrating existing options to use .type().
- Add CompletionTypeRegistry for decoupled type-to-completion mapping
- Pre-register built-in types (File, URI, URL)
- Add registerEnum() helper for enum types
- Add forEnum() factory method for creating enum completion
- Update CompletionScriptGenerator to use registry lookup
- Update ExtraArgument extension for completion type support
- Expand end-user documentation with detailed installation steps
  for both Bash and Zsh shell completion setup
- Fix task numbering in implementation plan
- Add ShellCompletionCommand with bash and zsh script generation
- Add CompletionTypeRegistry for type-to-completion mapping
- Add ICompletionType interface for completion behavior
- Add CompletionScriptGenerator for script output
- Extend ExtraArgument with type hints for positional args
- Register command via SPI for automatic CLI discovery
- Add fallback functions for systems without bash-completion
- Add type hints to metaschema-cli options for completion
- Register Format and SchemaFormat enums for completion
- Add comprehensive unit tests (40 tests passing)
Copy link

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

🧹 Nitpick comments (6)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistryTest.java (1)

62-83: Test modifies shared static registry state.

This test registers StringBuilder.class in the global CompletionTypeRegistry. While this won't cause issues with the current test suite, it could affect other tests if run in parallel or if test ordering changes. Consider documenting this side effect or using @AfterEach cleanup if the registry gains an unregister method in the future.

cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistry.java (1)

52-54: Consider documenting or logging when existing registrations are overwritten.

The register() method silently replaces any existing registration for a type. While this is flexible, it could lead to unexpected behavior if a type is accidentally registered twice with different completions. Consider adding a debug-level log or documenting this behavior.

cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java (1)

60-103: Consider reusing the Shell enum from CompletionScriptGenerator.

There's a duplicate Shell enum defined in CompletionScriptGenerator (see CompletionScriptGenerator.java lines 31-36). Having two identical enums violates DRY and could lead to maintenance issues if they diverge.

Consider either:

  1. Reusing CompletionScriptGenerator.Shell here
  2. Moving the enum to a shared location
-  /**
-   * Supported shell types.
-   */
-  public enum Shell {
-    /** Bash shell. */
-    BASH("bash"),
-    /** Zsh shell. */
-    ZSH("zsh");
-
-    @NonNull
-    private final String name;
-
-    Shell(@NonNull String name) {
-      this.name = name;
-    }
-
-    /**
-     * Get the shell name.
-     *
-     * @return the name
-     */
-    @NonNull
-    public String getName() {
-      return name;
-    }
-
-    /**
-     * Parse a shell type from a string.
-     *
-     * @param value
-     *          the string value to parse
-     * @return the shell type
-     * @throws IllegalArgumentException
-     *           if the value is not a recognized shell type
-     */
-    @NonNull
-    public static Shell fromString(@NonNull String value) {
-      String normalized = value.toLowerCase(Locale.ROOT);
-      for (Shell shell : values()) {
-        if (shell.name.equals(normalized)) {
-          return shell;
-        }
-      }
-      throw new IllegalArgumentException(
-          "Unknown shell: " + value + ". Supported shells: bash, zsh");
-    }
-  }

Then use CompletionScriptGenerator.Shell throughout this class. If a shared location is preferred, consider creating a completion package-level Shell type.

cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionScriptGenerator.java (1)

284-288: The trailing backslash removal logic works but is fragile.

The logic correctly removes the trailing " \\" from the last _arguments line, but it relies on the specific structure of the preceding code. If future modifications change how lines are appended, this could break silently.

Consider a more explicit approach that tracks whether content was added, though the current implementation is functional.

PRDs/20251215-shell-completion/implementation-plan.md (2)

465-482: Add blank line before closing code fence.

The static analysis flags line 481 for a fenced code block language issue. This appears to be due to the nested code blocks in the markdown example. Adding a blank line before the closing fence may resolve the linting warning.

 your-cli shell-completion zsh > ~/.zfunc/_your-cli
 # Add to .zshrc: fpath=(~/.zfunc $fpath); autoload -Uz compinit; compinit

522-532: Add blank lines around the table for proper markdown formatting.

Per static analysis, tables should be surrounded by blank lines for consistent rendering across markdown parsers.

 **Type assignments:**
+
 | Option | Type |
 |--------|------|
 | METASCHEMA_REQUIRED_OPTION | `File.class` |
 | METASCHEMA_OPTIONAL_OPTION | `File.class` |
 | TO_OPTION | `Format.class` |
 | AS_FORMAT_OPTION | `Format.class` |
 | AS_SCHEMA_FORMAT_OPTION | Custom schema format enum |
 | CONSTRAINTS_OPTION | `URI.class` |
 | SARIF_OUTPUT_FILE_OPTION | `File.class` |
+
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c4139e0 and dd99e39.

📒 Files selected for processing (19)
  • PRDs/20251215-shell-completion/PRD.md (1 hunks)
  • PRDs/20251215-shell-completion/implementation-plan.md (1 hunks)
  • cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java (1 hunks)
  • cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ExtraArgument.java (3 hunks)
  • cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java (1 hunks)
  • cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java (4 hunks)
  • cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionScriptGenerator.java (1 hunks)
  • cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistry.java (1 hunks)
  • cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/ICompletionType.java (1 hunks)
  • cli-processor/src/main/resources/META-INF/services/gov.nist.secauto.metaschema.cli.processor.command.ICommand (1 hunks)
  • cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommandTest.java (1 hunks)
  • cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionScriptGeneratorTest.java (1 hunks)
  • cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistryTest.java (1 hunks)
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/CLI.java (2 hunks)
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java (2 hunks)
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java (2 hunks)
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateSchemaCommand.java (2 hunks)
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/MetaschemaCommands.java (5 hunks)
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-15T20:00:59.203Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java:15-15
Timestamp: 2024-11-15T20:00:59.203Z
Learning: The `DefaultExtraArgument` class in `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java` is currently a stub for future exploration.

Applied to files:

  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateSchemaCommand.java
  • cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ExtraArgument.java
  • metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java
  • cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java
🧬 Code graph analysis (5)
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
  • ObjectUtils (18-135)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommandTest.java (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java (1)
  • ShellCompletionCommand (37-196)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistryTest.java (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistry.java (1)
  • CompletionTypeRegistry (30-154)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java (3)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
  • ObjectUtils (18-135)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java (2)
  • SuppressWarnings (58-803)
  • SuppressWarnings (332-802)
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/MetaschemaCommands.java (1)
  • SuppressWarnings (56-626)
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/CLI.java (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistry.java (1)
  • CompletionTypeRegistry (30-154)
🪛 markdownlint-cli2 (0.18.1)
PRDs/20251215-shell-completion/implementation-plan.md

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

(MD040, fenced-code-language)


523-523: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)

PRDs/20251215-shell-completion/PRD.md

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

(MD040, fenced-code-language)


523-523: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)

🔇 Additional comments (41)
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java (4)

39-39: LGTM!

The File import is correctly added to support the type annotation at line 78.


62-63: LGTM!

The URI.class type hint correctly identifies the expected argument type for shell completion support, aligning with the "file-or-URI-to-validate" semantic.


66-72: LGTM!

The URI.class type annotation is semantically consistent with the argName("URL") and correctly enables completion for constraint definition URIs.


74-81: LGTM!

The File.class type annotation is semantically consistent with the argName("FILE") and correctly enables file-path completion for the SARIF output destination.

cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/ICompletionType.java (1)

1-34: LGTM! Clean interface design.

The interface is well-structured with clear contracts for shell completion. The non-null return types and comprehensive Javadoc make the API easy to understand and use.

metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java (1)

77-83: LGTM! Appropriate type constraint for URI-based argument.

The addition of .type(URI.class) aligns with the type-aware completion design and is appropriate for the FILE_OR_URL argument.

metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/CLI.java (1)

71-73: LGTM! Proper initialization of shell completion types.

The enum registrations are correctly placed at the start of runCli before processor construction, ensuring completion types are available when the shell-completion command is invoked.

cli-processor/src/main/resources/META-INF/services/gov.nist.secauto.metaschema.cli.processor.command.ICommand (1)

1-1: LGTM! Proper SPI registration for command discovery.

The SPI registration correctly enables automatic discovery of the ShellCompletionCommand.

cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java (1)

271-273: LGTM! Appropriate visibility change for command introspection.

Changing getTopLevelCommands() from protected to public enables the shell-completion subsystem to introspect registered commands. The method remains final, preserving its contract.

cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ExtraArgument.java (2)

34-53: LGTM! Well-designed type-aware factory extension.

The new newInstance overload with the type parameter extends the API cleanly. The @Nullable type parameter appropriately allows for freeform arguments when no completion type is needed.


79-92: LGTM! Backward-compatible type accessor.

The getType() default method provides a clean extension point while maintaining backward compatibility by returning null for implementations that don't support type-aware completion.

metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java (1)

51-53: LGTM! Appropriate type hints for source and destination arguments.

The type hints correctly distinguish between URI (source, which can be URL or file) and File (destination, which is always a file path). This enables proper shell completion behavior for each argument.

metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateSchemaCommand.java (1)

54-56: Type hints correctly applied for shell completion support.

The type hints (URI.class for the module file/URL argument and File.class for the destination) are semantically appropriate and will enable meaningful shell completions.

cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java (2)

35-54: Clean constructor chaining pattern with proper documentation.

The implementation correctly uses constructor delegation, maintains immutability with final fields, and properly documents the optional type parameter for shell completion lookups.


65-70: LGTM - getter correctly implements the interface contract.

The @Override annotation indicates this implements the interface method, and @Nullable correctly reflects that the type may not be specified.

metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/MetaschemaCommands.java (3)

82-82: Type hint correctly applied for URI-based option.

The .type(URI.class) metadata will enable the completion system to provide appropriate completions for this option.


122-122: Verify that Format enum is registered for completion.

The .type(Format.class) is correctly added. Ensure that CompletionTypeRegistry.registerEnum(Format.class) is called during initialization so that shell completions include the enum values.

#!/bin/bash
# Search for Format.class registration in the codebase
rg -n "registerEnum.*Format\.class" --type=java

159-159: No action required. SchemaFormat enum is correctly registered for completion via CompletionTypeRegistry.registerEnum(SchemaFormat.class) during CLI initialization.

cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistryTest.java (2)

85-91: Same concern: enum registration persists across tests.

Similar to the custom type test, registerEnum(TestFormat.class) modifies global state. This is acceptable for now since the tests are self-contained.


32-60: Good coverage of edge cases and pre-registered types.

The tests properly verify null handling, unregistered type behavior, and all pre-registered built-in types.

cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommandTest.java (1)

21-77: Comprehensive test coverage for ShellCompletionCommand.

The tests thoroughly verify command metadata, shell type parsing with case insensitivity, and error handling for invalid inputs. Good use of assertThrows for exception validation.

cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionScriptGeneratorTest.java (3)

40-44: @BeforeAll correctly used for one-time enum registration.

This ensures the TestFormat enum is registered before any tests run, which is appropriate since the registry is a global singleton.


232-275: Well-designed mock commands for testing.

The mock implementations provide appropriate test data with typed options and extra arguments. Returning null from newExecutor is acceptable since these mocks are only used for script generation, not command execution.


140-168: Good validation of Bash fallback mechanisms.

Testing for _init_completion and _filedir fallbacks ensures the generated scripts work on systems without bash-completion installed.

cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistry.java (3)

30-42: Well-designed static utility class with thread-safe registry.

The use of ConcurrentHashMap ensures thread safety, and the private constructor correctly enforces the utility class pattern. Pre-registering common types (File, URI, URL) provides sensible defaults.


82-89: Enum value transformation to lowercase aligns with CLI conventions.

Converting enum names to lowercase matches typical shell command conventions where options are case-insensitive.


136-152: EnumCompletionType generates correct shell-specific syntax.

The Bash syntax (compgen -W "...") and Zsh syntax ((...)) are both correct for their respective completion systems.

cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java (5)

1-30: LGTM!

License, package declaration, and imports are appropriate for the command's functionality.


105-129: LGTM!

Command interface methods follow the established patterns correctly.


141-174: LGTM!

The execution flow is clean and handles the expected scenarios correctly: argument validation, shell type parsing, script generation, and output handling.


189-195: LGTM!

The stdout handling is correct - using explicit UTF-8 encoding and flushing without closing the stream.


176-187: No action required. The method resolveAgainstCWD is properly defined in the parent class AbstractTerminalCommand with an overloaded version accepting a Path parameter, which matches the call at line 178.

Likely an incorrect or invalid review comment.

cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionScriptGenerator.java (6)

1-56: LGTM!

The class setup, constructor, and accessors are well-structured.


78-138: LGTM!

The Bash completion script generation is well-structured with appropriate fallbacks for systems without bash-completion installed.


140-186: LGTM!

The Zsh completion script generation follows proper Zsh conventions with correct single-quote escaping for descriptions.


188-255: LGTM!

The recursive subcommand handling and option completion logic are well-implemented.


337-346: LGTM!

The type handling correctly accounts for Option.getType() returning an Object that may not always be a Class<?>.


358-372: LGTM!

The fallback to file completion for extra arguments without a registered type is a sensible default.

PRDs/20251215-shell-completion/implementation-plan.md (3)

1-47: LGTM!

The implementation plan is well-structured with clear task breakdowns and accurate code examples.


332-401: LGTM!

The command implementation plan and testing strategy are comprehensive.


553-577: LGTM!

The acceptance criteria are comprehensive and well-aligned with the implementation.

- Add shell completion section to README.txt with bash/zsh setup instructions
- Document --to option file handling behavior in PRD (paths, overwrite, errors)
@david-waltermire david-waltermire marked this pull request as ready for review December 16, 2025 23:33
Copy link

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

🧹 Nitpick comments (1)
metaschema-cli/src/main/distro/README.txt (1)

28-52: Add version and prerequisite notes to shell completion instructions.

The documentation is clear and covers both Bash and Zsh effectively, but is missing a few important details:

  1. Bash process substitution (line 36) requires Bash 4.0+. Consider adding a note for users on older systems to use the --to approach instead.
  2. Directory creation (line 45): The Zsh example assumes ~/.zsh/completions/ exists. Consider adding mkdir -p ~/.zsh/completions before the command for clarity.
  3. Bash-completion package: Per the PR description, the tool supports fallback when the bash-completion package is unavailable. Consider documenting that some distributions require sudo apt-get install bash-completion (or equivalent) for enhanced completion, but basic completion will still work.
  4. Minor inconsistency: Bash uses --to FILE while Zsh uses > FILE. While both are valid, it might help to clarify that both approaches are available for each shell (e.g., Zsh also supports --to).

Suggested revision (for lines 34–49):

For Bash:

  # Generate and source completion script
  source <(metaschema-cli shell-completion bash)
+ # Note: Requires Bash 4.0+. For older versions, use --to approach:
+ # metaschema-cli shell-completion bash --to ~/.metaschema-cli-completion.bash

  # Or save to a file for persistent use
  metaschema-cli shell-completion bash --to ~/.metaschema-cli-completion.bash
  echo 'source ~/.metaschema-cli-completion.bash' >> ~/.bashrc

For Zsh:

+ # Ensure completions directory exists
+ mkdir -p ~/.zsh/completions
  # Generate and save to completions directory
  metaschema-cli shell-completion zsh > ~/.zsh/completions/_metaschema-cli
+ # Or use --to flag
+ # metaschema-cli shell-completion zsh --to ~/.zsh/completions/_metaschema-cli
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dd99e39 and d99f0b9.

📒 Files selected for processing (2)
  • PRDs/20251215-shell-completion/PRD.md (1 hunks)
  • metaschema-cli/src/main/distro/README.txt (1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
PRDs/20251215-shell-completion/PRD.md

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

(MD040, fenced-code-language)


506-506: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


521-521: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


533-533: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


539-539: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


545-545: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


557-557: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)


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

(MD040, fenced-code-language)


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

(MD040, fenced-code-language)


641-641: 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). (2)
  • GitHub Check: Code
  • GitHub Check: Website
🔇 Additional comments (1)
PRDs/20251215-shell-completion/PRD.md (1)

1-50: Good comprehensive PRD addressing prior feedback.

The document successfully addresses the prior review comment on --to option file handling (lines 497-502) with explicit details on path resolution, directory creation behavior, overwrite semantics, and error codes. The constructor signature alignment has also been resolved. Remaining issues are markdown formatting violations, not content concerns.

Copy link
Contributor

@aj-stein aj-stein left a comment

Choose a reason for hiding this comment

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

Looks good so far, I will need to install locally and evaluate the shell completions on my computer before giving a more thorough review though. ⌛

@david-waltermire david-waltermire linked an issue Dec 18, 2025 that may be closed by this pull request
3 tasks
@david-waltermire david-waltermire self-assigned this Dec 18, 2025
Copy link
Contributor

@aj-stein aj-stein left a comment

Choose a reason for hiding this comment

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

Definitely look forward to using this one since I am lazy tab-completer type on Linux. Thanks for implementing this so quickly.

@david-waltermire david-waltermire merged commit 66640f8 into metaschema-framework:develop Dec 19, 2025
3 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Spec and Tooling Work Board Dec 19, 2025
@david-waltermire david-waltermire deleted the feature/shell-completion branch December 19, 2025 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Shell Completions

2 participants