test: Add subcommand documentation verification#1597
test: Add subcommand documentation verification#1597Crunchyman-ralph merged 5 commits intoeyaltoledano:nextfrom
Conversation
🦋 Changeset detectedLatest commit: 37a3000 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a new unit test suite that verifies legacy and modern CLI commands are represented in generated help text; updates help output in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@apps/cli/tests/unit/help-documentation-sync.spec.ts`:
- Around line 119-141: The INTENTIONALLY_UNDOCUMENTED array includes 'generate'
but the CLI help actually documents the generate command under "Task
Generation"; remove 'generate' from the INTENTIONALLY_UNDOCUMENTED constant so
the help sync test will validate it (edit the INTENTIONALLY_UNDOCUMENTED
declaration to delete the 'generate' entry).
- Around line 147-156: COMMAND_NAME_MAPPINGS currently keeps legacy tag commands
(add-tag, use-tag, delete-tag, rename-tag, copy-tag) out of validation, which
can mask whether they should be removed; either delete those legacy entries from
COMMAND_NAME_MAPPINGS when the old commands in scripts/modules/commands.js are
fully deprecated, or add a clear explanatory comment above COMMAND_NAME_MAPPINGS
stating it's "temporary during tag-migration to exempt legacy commands from help
validation" and include the list of legacy command names and a TODO with the
deprecation milestone; ensure related tests that reference COMMAND_NAME_MAPPINGS
still explicitly assert that these legacy commands are not present as primary
documented commands so intent remains clear.
In `@scripts/modules/ui.js`:
- Around line 757-761: Update the args string for the command object with name
'tags add' so it accurately reflects supported flags: change the 'args' property
to include the --from-branch option (e.g., '<name> [--description <desc>]
[--copy-from <tag>] [--from-branch <branch>]'). Locate the 'tags add' command
object in scripts/modules/ui.js and update its args value accordingly so it
matches the description.
🧹 Nitpick comments (2)
apps/cli/tests/unit/help-documentation-sync.spec.ts (2)
55-80: Error handling could be more specific.The catch block logs a warning but continues execution. While this is reasonable for optional directories, consider distinguishing between "directory doesn't exist" (expected in some configs) versus actual read errors (unexpected).
♻️ Optional: More specific error handling
} catch (error) { - // Directory might not exist in some configurations - console.warn('Could not read modern commands directory:', error); + // Directory might not exist in some configurations + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + // Expected - directory doesn't exist + } else { + console.warn('Could not read modern commands directory:', error); + } }
326-349: Tag section regex may be fragile.The regex
/Tag Management.*?(?=\n\s*\n\s*[A-Z]|\n\s*\])/srelies on specific formatting (double newline followed by uppercase or closing bracket). This could break if:
- The section heading format changes
- There's additional whitespace
- The next section starts differently
Consider a more robust approach or add a fallback with a helpful error message.
♻️ Suggested improvement
// Check within the Tag Management section specifically const tagSectionMatch = helpContent.match( /Tag Management.*?(?=\n\s*\n\s*[A-Z]|\n\s*\])/s ); - if (tagSectionMatch) { + if (!tagSectionMatch) { + console.warn( + 'Could not isolate Tag Management section - checking entire help content instead' + ); + } + + const sectionToCheck = tagSectionMatch ? tagSectionMatch[0] : helpContent; + { - const tagSection = tagSectionMatch[0]; + const tagSection = sectionToCheck; const foundDeprecated = deprecatedCommands.filter((pattern) => pattern.test(tagSection) );
Merge Order NoteThis PR is branched from #1593 and includes its commits. Please merge in this order:
Alternatively, merging this PR alone will include #1593's changes, but merging #1593 first keeps the history cleaner. |
Crunchyman-ralph
left a comment
Crunchyman-ralph
left a comment
There was a problem hiding this comment.
love this! I think it needs a changeset, don't you ?
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Enhances help sync test to verify: - tags subcommands use new unified structure (tags add, tags use, etc.) - deprecated standalone tag commands are not documented - list command options are documented (--watch, --ready, --blocking, etc.) - list format options are documented (--json, -f, -c) Closes eyaltoledano#1596
- Add clarifying comment for COMMAND_NAME_MAPPINGS to explain legacy command exemption during migration - Update 'tags add' command args to include missing --from-branch option - Add changeset documenting test enhancements and minor fixes
Changeset validation only allows public package names (task-master-ai, extension). Changed from internal package name @tm/cli to public package name task-master-ai.
Addresses Cursor Bugbot feedback: The test "should not document deprecated standalone tag commands" now falls back to checking the entire help content if the Tag Management section regex doesn't match, instead of silently passing without making any assertions. This ensures the test always validates that deprecated commands are not documented, even if the help formatting changes.
Crunchyman-ralph
force-pushed
the
1596-enhanced-help-test
branch
from
5a9df57 to
37a3000
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| while ((match = superRegex.exec(content)) !== null) { | ||
| commands.add(match[1]); | ||
| } | ||
| } |
There was a problem hiding this comment.
Non-recursive scan silently misses subdirectory commands
Medium Severity
extractCommandsFromModernTs uses readdirSync (non-recursive) and only matches *.command.ts files, so it silently misses commands defined in subdirectories like autopilot/index.ts. The AutopilotCommand is registered in CommandRegistry and is a real user-facing CLI command, but it's neither found by this extraction function nor listed in INTENTIONALLY_UNDOCUMENTED. This creates a blind spot where the test gives false confidence that all CLI commands are accounted for — autopilot could lose or gain documentation without the sync test catching it.
| const allListSections = listMatches.join('\n'); | ||
| const missingOptions = expectedListOptions.filter( | ||
| (opt) => !allListSections.includes(opt) | ||
| ); |
There was a problem hiding this comment.
Substring match makes -w watch option check ineffective
Medium Severity
The check for '-w' (short for --watch) using allListSections.includes('-w') is unreliable because '-w' is a substring of '--with-subtasks', which appears in a different list entry. If the -w watch option were removed from the help documentation, the test would still pass due to the accidental substring match against --with-subtasks, silently defeating the test's purpose.
2 participants
Summary
Enhances the help documentation sync test to verify that subcommand documentation stays in sync with CLI implementations.
Changes
tagssubcommands use the new unified structure (tags add,tags use, etc.)add-tag,use-tag) are not documentedlistcommand options are documented (--watch,--ready,--blocking)listformat options are documented (--json,-f,-c)Test Plan
Dependencies
This PR includes the help documentation updates from #1593 so it can verify against them. If #1593 changes, this PR will need to be rebased.
Closes #1596
Summary by CodeRabbit
New Features
Documentation
Tests
Note
Low Risk
Test- and help-text-only changes; main risk is increased CI strictness causing failures if command/help strings drift or parsing heuristics miss edge cases.
Overview
Adds a new
help-documentation-sync.spec.tsunit test that parses legacycommands.js, modern*.command.ts, andui.jshelp entries to fail CI when commands are missing from help or when help documents commands that no longer exist, with explicit allowlists/mappings for intentionally-undocumented and migrating commands.Extends coverage to subcommand correctness: asserts help documents the unified
tags add/use/remove/rename/copystructure, ensures deprecated standalone tag commands aren’t documented as primary entries, and verifieslisthelp includes key option variants (watch/ready/blocking and format flags).Updates the help text for
tags addinui.jsto include the missing--from-branchargument, and adds a patch changeset describing the test/documentation improvements.Written by Cursor Bugbot for commit 37a3000. This will update automatically on new commits. Configure here.