Skip to content

feat(command): Add BareGroupAction support - #76

Closed
mguerrero3-godaddy wants to merge 4 commits into
mainfrom
DEVEX-717
Closed

feat(command): Add BareGroupAction support#76
mguerrero3-godaddy wants to merge 4 commits into
mainfrom
DEVEX-717

Conversation

@mguerrero3-godaddy

Copy link
Copy Markdown
Collaborator

Summary

Adds support for bare group actions, groups can now register a callback that runs when invoked with no subcommand, rendering its return value as the JSON envelope instead of default group help text. This as part of the AC on DEVEX-717

Test plan

  • cargo fmt --all --check
  • cargo clippy --all-targets -- -D warnings
  • cargo test --all-targets

Manual verification

Setup:

# In the cli repo, temporarily override the engine dependency:
cd cli/rust
# Edit Cargo.toml → cli-engine = { features = ["pkce-auth"], path = "../../cli-engine" }
cargo build --release && cp target/release/gddy ~/.local/bin/gddy

Test WITHOUT the fix (baseline):

cd cli-engine && git checkout main
cd cli/rust && cargo build --release && cp target/release/gddy ~/.local/bin/gddy
# <command to reproduce the issue>
# Expected: <describe broken behavior>

Test WITH the fix:

cd cli-engine && git checkout <this-branch>
cd cli/rust && cargo build --release && cp target/release/gddy ~/.local/bin/gddy
# <same command>
# Expected: <describe fixed behavior>

Cleanup:

# Revert cli/rust/Cargo.toml back to:
# cli-engine = { features = ["pkce-auth"], version = "<published-version>" }

@mguerrero3-godaddy mguerrero3-godaddy self-assigned this Aug 3, 2026
@jpage-godaddy

jpage-godaddy commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

I'm not so sure about this. Do you have a concrete example of how you would use this feature? It looks like you plan on doing something custom for specific groups, but I wonder if what we really are looking for is something more general for groups. @wcole1-godaddy do you recall the specific feedback that caused the line in DEVEX-717 that says:

Parent/group discovery output depends on cli-engine help instead of explicit JSON command tree.

I assume that's why this is being created, @mguerrero3-godaddy? I also wonder if gddy tree --json was noticed since that does do a JSON command tree.

@mguerrero3-godaddy

mguerrero3-godaddy commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator Author

I'm not so sure about this. Do you have a concrete example of how you would use this feature? It looks like you plan on doing something custom for specific groups, but I wonder if what we really are looking for is something more general for groups. @wcole1-godaddy do you recall the specific feedback that caused the line in DEVEX-717 that says:

Parent/group discovery output depends on cli-engine help instead of explicit JSON command tree.

I assume that's why this is being created, @mguerrero3-godaddy? I also wonder if gddy tree --json was noticed since that does do a JSON command tree.

Yes that's exactly why. Unsure If this intended changes only on cli side but this is for being able to define an action for the parent group instead of falling to the help text, added to the actual engine so cli just would need to define the action. e.g. {{cli}} actions can have a custom behavior instead. I also based on the Command.make("...", {}, ...) pattern from the TS part where there's an effect handler as the default action.

When you mean more general is like pre-defined instead of customizable ?

@jpage-godaddy

jpage-godaddy commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Well what specifically would the group action we'd be adding to gddy do? "Parent/group discovery output" sounds like something we'd want to be consistent across every command, so if we have a feature gap somewhere, I'd rather bake it in to the CLI engine's group functionality, but I do wonder if this is an actual gap or if gddy tree already supplies the discovery we need.

Sorry for the vagueness of these tickets; they were created in reference to @wcole1-godaddy's feedback, but the intent was for us to first plan out the feature rather than jumping right into exactly duplicating the TS CLI's functionality in all cases. Let's make sure the same capabilities exist even if they don't work exactly the same way. If we have a capability gap, let's add it universally if it's a cross-cutting concern.

@mguerrero3-godaddy

Copy link
Copy Markdown
Collaborator Author

Well what specifically would the group action we'd be adding to gddy do? "Parent/group discovery output" sounds like something we'd want to be consistent across every command, so if we have a feature gap somewhere, I'd rather bake it in to the CLI engine's group functionality, but I do wonder if this is an actual gap or if gddy tree already supplies the discovery we need.

Sorry for the vagueness of these tickets; they were created in reference to @wcole1-godaddy's feedback, but the intent was for us to first plan out the feature rather than jumping right into exactly duplicating the TS CLI's functionality in all cases. Let's make sure the same capabilities exist even if they don't work exactly the same way. If we have a capability gap, let's add it universally if it's a cross-cutting concern.

No problem! Good point on gddy tree --json, seems like I missed that entirely.

Rethinking this with your thoughts... instead of a per-group opt-in closure, change the default bare-group fallback from clap's plain-text help to the same JSON tree node tree already builds build_tree_from_clap_with_path, scoped to that group. Like this evert group gets consistent JSON discovery, and no consumer needs to write the custom action per command

Sound right, @jpage-godaddy? If so I'll rework the PR that way.

@jpage-godaddy

Copy link
Copy Markdown
Collaborator

Well what specifically would the group action we'd be adding to gddy do? "Parent/group discovery output" sounds like something we'd want to be consistent across every command, so if we have a feature gap somewhere, I'd rather bake it in to the CLI engine's group functionality, but I do wonder if this is an actual gap or if gddy tree already supplies the discovery we need.
Sorry for the vagueness of these tickets; they were created in reference to @wcole1-godaddy's feedback, but the intent was for us to first plan out the feature rather than jumping right into exactly duplicating the TS CLI's functionality in all cases. Let's make sure the same capabilities exist even if they don't work exactly the same way. If we have a capability gap, let's add it universally if it's a cross-cutting concern.

No problem! Good point on gddy tree --json, seems like I missed that entirely.

Rethinking this with your thoughts... instead of a per-group opt-in closure, change the default bare-group fallback from clap's plain-text help to the same JSON tree node tree already builds build_tree_from_clap_with_path, scoped to that group. Like this evert group gets consistent JSON discovery, and no consumer needs to write the custom action per command

Sound right, @jpage-godaddy? If so I'll rework the PR that way.

I like the idea, but one of the design deviations with the original godaddy CLI is that we want human-friendly output as well as JSON output. For regular commands, we check (if the --output, --json, --toon, or --human flags aren't used) if the command is running inside of an interactive terminal (a TTY) and default to JSON if it's not a TTY, human if it is. Maybe we can support JSON-based help output when a user explicitly runs --help --json, if you "run" a group with --json, (which I think just display the same as the help text), or default to JSON output if we're not in an interactive terminal. That way, if an agent runs gddy platform, it can receive a JSON tree subset or something more similar to what the godaddy CLI did. I'd keep the current help text in the case of human output.

@mguerrero3-godaddy

Copy link
Copy Markdown
Collaborator Author

Reimplemented on #78 (Also to rename branch following proper guidelines)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants