refactor(skills): consolidate CLI guidance - #36
Merged
Conversation
Signed-off-by: Jingxiang Zhang <jingzhang@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe changes add synchronized ChangesSkill workflows and validation
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/fleet-health-report/references/cli-contract.md`:
- Around line 22-24: Update the authentication guidance in cli-contract.md so it
no longer passes the API key through the --api-key command-line argument. Use an
interactive prompt or standard-input mechanism instead, and revise the
referenced auth command example while preserving the instruction not to request
or expose API keys.
In `@skills/fleet-health-report/SKILL.md`:
- Around line 63-69: Update the boundary initialization block around end,
cur_start, and prev_start to execute platform-appropriate date syntax: use
BSD/macOS `-v` flags on BSD systems and GNU/Linux `-d` expressions on Linux.
Ensure both cur_start and prev_start are assigned valid UTC timestamps before
trend queries run.
In `@skills/node-rca-rcca/SKILL.md`:
- Around line 30-39: Update the date initialization block in the node/event
query guidance to use an executable platform-specific GNU/Linux versus BSD/macOS
branch. Ensure both branches assign valid UTC end and seven-day start
timestamps, while preserving the 168h duration only for optional APIs and
avoiding the invalid 7d Go duration.
In `@skills/nvfleetint/references/auth.md`:
- Around line 26-29: Update the auth instructions around auth list and auth add
to clarify that listing profiles does not prevent overwriting an existing
profile. Tell users to verify the positional target name before running auth
add, and document overwrite confirmation only if the CLI actually enforces it;
preserve the existing guidance that --yes is used only for explicitly requested
key replacement.
- Around line 19-23: Update the auth command documentation in auth.md and every
duplicated CLI contract to remove API-key command-line argument examples, and
document a secure API-key input method such as an interactive prompt, stdin, or
file descriptor. Also state that auth add updates an exact existing profile and
prompts for confirmation before replacing its key, including the --yes behavior
for explicit noninteractive replacement.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 99040898-05b3-4fcc-80cc-5e15131ea5a1
📒 Files selected for processing (15)
scripts/sync-skill-cli-contract.shscripts/validate-skills.shskills/fleet-health-report/SKILL.mdskills/fleet-health-report/references/cli-contract.mdskills/fleet-health-report/references/html-report-template.mdskills/fleet-health-report/references/workspace.mdskills/node-rca-rcca/SKILL.mdskills/node-rca-rcca/references/cli-contract.mdskills/node-rca-rcca/references/example.mdskills/node-rca-rcca/references/html-report-template.mdskills/node-rca-rcca/references/report-writing.mdskills/node-rca-rcca/references/scratch-workspace.mdskills/nvfleetint/SKILL.mdskills/nvfleetint/references/auth.mdskills/nvfleetint/references/cli-contract.md
💤 Files with no reviewable changes (1)
- skills/node-rca-rcca/references/scratch-workspace.md
Signed-off-by: Jingxiang Zhang <jingzhang@nvidia.com>
Signed-off-by: Jingxiang Zhang <jingzhang@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
skills/fleet-health-report/SKILL.md (2)
91-95: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winApply unknown-total handling to every cardinality probe.
Line 91 uses a non-
--allresponse, so the alert array is.alerts, not normalized.items. The shared contract also treats a nonempty response with a missing or zerototalas unreported for nodes, node groups, and compute zones.Use the exact backend fields and mark all such probes as unknown. Otherwise, the workflow can miss conservative collection and timeout handling for large inventory results.
Proposed clarification
-If an alert probe returns items but total is zero/absent, - treat cardinality as unknown and use the large-fleet path. +If any single-page probe returns a nonempty backend array +(`alerts`, `nodes`, `nodeGroups`, or `computezones`) while `total` +is zero or absent, treat cardinality as unknown. Use the large-fleet +path for alerts and conservative collection handling for inventory.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/fleet-health-report/SKILL.md` around lines 91 - 95, Update the cardinality-probe guidance to apply unknown-total handling consistently across alerts, nodes, node groups, and compute zones. Use the exact backend response arrays, including .alerts for the non-—all alert response rather than normalized .items, and treat any nonempty array with a missing or zero top-level total as unknown. Ensure unknown cardinality selects the large-fleet path with conservative collection and timeout handling.
38-46: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSecurity Misconfiguration (CWE-16)
Reachability: External · Exploitability: Moderate
Bind every
nvfleetintcommand to the selected profile.The commands in this section omit
--profile <name>. Add the same explicit profile to every command, or document a wrapper that injects and validates it. Without this binding, queries can use different defaults and combine data from the wrong tenant.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/fleet-health-report/SKILL.md` around lines 38 - 46, Update the commands in the profile rules section to explicitly include the selected profile via --profile <name> on every nvfleetint invocation. Apply the same profile consistently to both computezone and nodegroup commands, or document and use a validated wrapper that injects it for all commands.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@skills/fleet-health-report/SKILL.md`:
- Around line 91-95: Update the cardinality-probe guidance to apply
unknown-total handling consistently across alerts, nodes, node groups, and
compute zones. Use the exact backend response arrays, including .alerts for the
non-—all alert response rather than normalized .items, and treat any nonempty
array with a missing or zero top-level total as unknown. Ensure unknown
cardinality selects the large-fleet path with conservative collection and
timeout handling.
- Around line 38-46: Update the commands in the profile rules section to
explicitly include the selected profile via --profile <name> on every nvfleetint
invocation. Apply the same profile consistently to both computezone and
nodegroup commands, or document and use a validated wrapper that injects it for
all commands.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 0314effe-f9c2-4eb7-82af-49fafb6d516a
📒 Files selected for processing (3)
scripts/validate-skills.shskills/fleet-health-report/SKILL.mdskills/node-rca-rcca/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/node-rca-rcca/SKILL.md
4 tasks
Signed-off-by: Jingxiang Zhang <jingzhang@nvidia.com>
Signed-off-by: Jingxiang Zhang <jingzhang@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/validate-skills.sh`:
- Around line 117-142: Update the validation wording in the skill-check
expectations to constrain only the alert-collection phase to at most 12
nvfleetint command invocations, rather than the entire workflow. Explicitly
state that paginated invocations may fetch multiple underlying requests, while
retaining the existing alert-phase guidance in the validation strings array.
In `@skills/fleet-health-report/references/cli-contract.md`:
- Around line 21-37: Update the CLI contract’s list classification to state that
alert summary supports --page-size but does not support --all, while preserving
its existing response-field and bounded partial-page behavior.
In `@skills/fleet-health-report/SKILL.md`:
- Around line 80-88: Replace the claim that the workflow makes at most 12 calls
with a statement scoped specifically to alert collection, clarifying that this
phase invokes one options call, one summary call, and up to 10 node calls; do
not imply a whole-workflow limit or account for pagination beyond command
invocations.
In `@skills/node-rca-rcca/references/cli-contract.md`:
- Around line 33-35: Update the --all response validation rules in the CLI
contract so nonempty items with an explicit total of 0 are rejected as
inconsistent, rather than labeled as an unreported total. Treat only missing or
null total values as unreported, and apply the existing stop or allowed fallback
workflow for inconsistent results.
- Around line 21-25: Clarify the --all normalization rule so it applies only to
paginated list commands, excluding non-paginated commands such as tag list.
Preserve tag list’s native tags response and ensure the documented pagination
flags and {items, pagination:{total,hasMore,pagesFetched}} shape are not implied
for it.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ce1adc2d-81ce-4fcf-b039-8b501d66c4ce
📒 Files selected for processing (16)
scripts/sync-skill-cli-contract.shscripts/validate-skills.shskills/fleet-health-report/SKILL.mdskills/fleet-health-report/references/cli-contract.mdskills/fleet-health-report/references/html-report-template.mdskills/fleet-health-report/references/html-theme.mdskills/fleet-health-report/references/workspace.mdskills/node-rca-rcca/SKILL.mdskills/node-rca-rcca/references/cli-contract.mdskills/node-rca-rcca/references/example.mdskills/node-rca-rcca/references/html-report-template.mdskills/node-rca-rcca/references/html-theme.mdskills/node-rca-rcca/references/workspace.mdskills/nvfleetint/SKILL.mdskills/nvfleetint/references/auth.mdskills/nvfleetint/references/cli-contract.md
💤 Files with no reviewable changes (3)
- skills/fleet-health-report/references/html-report-template.md
- skills/node-rca-rcca/references/example.md
- skills/node-rca-rcca/references/html-report-template.md
🚧 Files skipped from review as they are similar to previous changes (4)
- skills/nvfleetint/references/auth.md
- scripts/sync-skill-cli-contract.sh
- skills/nvfleetint/SKILL.md
- skills/node-rca-rcca/SKILL.md
Signed-off-by: Jingxiang Zhang <jingzhang@nvidia.com>
emilyzhangbg
approved these changes
Aug 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Checklist
git commit -s) per the Contributing Guidelines.Summary by CodeRabbit
Documentation
Chores