test(cli): add regression test for command/help sync, fix missing guardrails entry#863
Conversation
Greptile SummaryThis PR adds a regression test ( Confidence Score: 5/5Safe to merge — changes are a one-line bug fix and a well-scoped regression test with no risk to existing behavior. Only P2 findings remain (a minor defensive guard on No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[_COMMANDS\napp/cli/commands/__init__.py] -->|set of cmd.name| C{Set diff}
B[_HELP_COMMANDS\napp/cli/layout.py] -->|set of name| C
C -->|missing_from_help == ∅\nmissing_from_registry == ∅| D[✅ Test passes]
C -->|any drift detected| E[❌ Assert fails\nnames the exact out-of-sync command]
Reviews (1): Last reviewed commit: "test(cli): add regression test for comma..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
test_node_plan_actions_emits_retrieval_controls is failing because _InputStub is missing a model_copy attribute that node_plan_actions now expects — this is unrelated to my changes, which only touch tests/cli/test_command_help_sync.py and app/cli/layout.py. The failure reproduces on main independently of this PR. |
|
@rrajan94 @VaibhavUpreti kindly review |
|
please kindly review @muddlebee |
|
@Davidson3556 fix CI |
Davidson3556
force-pushed
the
test/cli-command-help-sync
branch
from
3de1e10 to
ebe51f2
Compare
|
@muddlebee kindly review |
|
@VaibhavUpreti @rrajan94 @muddlebee kindly review |
| ("remote", "Connect to remote agents and hosted service ops."), | ||
| ("tests", "Browse and run inventoried tests from the terminal."), | ||
| ("integrations", "Manage local integration credentials."), | ||
| ("guardrails", "Manage sensitive information guardrail rules."), |
There was a problem hiding this comment.
Any reason for adding this?
There was a problem hiding this comment.
guardrails is already registered in _COMMANDS in app/cli/commands/init.py but was missing from _HELP_COMMANDS in app/cli/layout.py . this is exactly the drift the regression test was added to catch. The test itself surfaced this inconsistency, so fixing it is part of the acceptance criteria for this issue.
|
nicely done @Davidson3556 🥳 |
2 participants
Fixes #835
Describe the changes you have made in this PR -
Added
tests/cli/test_command_help_sync.py— a regression test that compares the set of registered command names in_COMMANDS(app/cli/commands/__init__.py) against the set of keys in_HELP_COMMANDS(app/cli/layout.py). The test fails with a clear message naming the exact out-of-sync command if either list drifts from the other.While writing the test, it immediately caught an existing drift:
guardrailswas registered in_COMMANDSbut missing from_HELP_COMMANDS. This has been fixed inapp/cli/layout.py.Demo/Screenshot for feature changes and bug fixes -
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
The problem:
_COMMANDSand_HELP_COMMANDSare two separate tuples that must stay in sync but have no enforced relationship — a developer can add a command to one and forget the other.The fix is a set-comparison test. It extracts
cmd.namefrom eachclick.Commandin_COMMANDS(all commands use explicitname=in their decorator so.nameis reliable), and the first element of each tuple in_HELP_COMMANDS. It then diffs the two sets and fails with a targeted message if anything is missing on either side. No formatting details are checked — only the names — so cosmetic changes to the help copy won't break it.Alternative considered: checking the Click group's registered commands at runtime via
cli.commands. Rejected because that requires booting the full CLI, while this test touches only the two data structures directly.Checklist before requesting a review
Note: Please check Allow edits from maintainers if you would like us to assist in the PR.