Skip to content

fix(cli): report skipped hidden directories during skill install - #1856

Merged
brandonpelfrey merged 1 commit into
NVIDIA:mainfrom
senthilr-nv:fix/skill-install-dotdir-reporting
Apr 14, 2026
Merged

fix(cli): report skipped hidden directories during skill install#1856
brandonpelfrey merged 1 commit into
NVIDIA:mainfrom
senthilr-nv:fix/skill-install-dotdir-reporting

Conversation

@senthilr-nv

@senthilr-nv senthilr-nv commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • collectFiles() silently dropped dot-directories (e.g. .secret/) without reporting them in skippedDotfiles. Users were warned about skipped dot-files but not about entire hidden directory trees being omitted.
  • Now both dot-files and dot-directories are reported, with a trailing / to distinguish directories in the CLI output (e.g. Skipping 2 hidden path(s): .env, .secret/).
  • Fixed misleading comment in postInstall() that said "shellQuote the relative part" when the code actually uses double-quoted interpolation (safe due to validateRelativePath charset restriction).

Reported by @brandonpelfrey.

Test plan

  • Added regression test: hidden directory with nested files (.secret/token.txt) is now reported as .secret/ in skippedDotfiles
  • All 25 unit tests pass (npx vitest run src/lib/skill-install.test.ts)
  • Pre-commit hooks pass (lint, commitlint, gitleaks)

Summary by CodeRabbit

  • Bug Fixes

    • Hidden directories and dot-prefixed files inside folders are now properly recorded and reported when skipped during skill installation.
    • Status messaging updated to display "hidden path(s)" instead of "dotfile(s)" for clearer upload feedback.
  • Tests

    • Added test coverage validating collection and reporting of non-hidden files and skipped hidden paths (directories and dot-prefixed files).

@coderabbitai

coderabbitai Bot commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 242fe035-476f-4343-a0e8-7b491c184dca

📥 Commits

Reviewing files that changed from the base of the PR and between d64a19c and f099346.

📒 Files selected for processing (3)
  • src/lib/skill-install.test.ts
  • src/lib/skill-install.ts
  • src/nemoclaw.ts
✅ Files skipped from review due to trivial changes (2)
  • src/nemoclaw.ts
  • src/lib/skill-install.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/skill-install.test.ts

📝 Walkthrough

Walkthrough

collectFiles() now records hidden directories with a trailing / in skippedDotfiles and also captures hidden files inside non-hidden directories. Console messaging changed from "dotfile(s)" to "hidden path(s)". A new test verifies these behaviors.

Changes

Cohort / File(s) Summary
Collect & Skip Hidden Paths
src/lib/skill-install.ts, src/lib/skill-install.test.ts
collectFiles(): treat entries starting with . as skipped; record directory dot-entries with a trailing / and include hidden files within non-hidden dirs in skippedDotfiles. Added test collectFiles to assert returned files and skippedDotfiles.
User-Facing Message
src/nemoclaw.ts
Updated status text from "Skipping … dotfile(s)" to "Skipping … hidden path(s)" while keeping skipped list/count unchanged.
Comment Clarification
src/lib/skill-install.ts
Reworded comment in postInstall() to state values are quoted (not shell-quoted) and rely on validateRelativePath restrictions; no command construction logic changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I found the dots and placed a slash with care,
Hidden paths now shown, no secret lair,
Tests hop in line to prove what I did,
A tidy trail where shadows once slid. 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main objective of the pull request: fixing the CLI reporting of skipped hidden directories during skill install by now including dot-directories (with trailing slashes) in the skipped paths output.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@senthilr-nv senthilr-nv self-assigned this Apr 14, 2026
collectFiles() silently dropped dot-directories (e.g. .secret/) without
adding them to skippedDotfiles. Users were warned about skipped dot-files
but not about entire hidden directory trees being omitted. Now both
dot-files and dot-directories are reported, with a trailing / to
distinguish directories in the CLI output.

Reported-by: brandonpelfrey

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
@senthilr-nv
senthilr-nv force-pushed the fix/skill-install-dotdir-reporting branch from d64a19c to f099346 Compare April 14, 2026 02:00
@brandonpelfrey
brandonpelfrey merged commit 75f147b into NVIDIA:main Apr 14, 2026
11 checks passed
cv pushed a commit that referenced this pull request Apr 14, 2026
## Summary
- Document tier-based policy selector (Restricted/Balanced/Open) in
commands, network policies, and customize-network-policy pages (from
#1753)
- Document configurable port overrides via environment variables
(`NEMOCLAW_GATEWAY_PORT`, `NEMOCLAW_DASHBOARD_PORT`,
`NEMOCLAW_VLLM_PORT`, `NEMOCLAW_OLLAMA_PORT`) (from #1645)
- Document `nemoclaw <sandbox> skill install <path>` command (from
#1845, #1856)
- Document reserved sandbox name validation — CLI command collision
check (from #1773)
- Bump doc version switcher through 0.0.15
- Remove `--dangerously-skip-permissions` from onboard usage synopsis
(docs-skip violation)
- Regenerate agent skills from updated docs

## Test plan
- [x] `make docs` builds without warnings
- [x] All pre-commit hooks pass
- [ ] Verify rendered pages in docs build output
- [ ] Cross-references resolve correctly (`policy-tiers` anchor,
`environment-variables` section)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ColinM-sys pushed a commit to ColinM-sys/NemoClaw that referenced this pull request Apr 14, 2026
…DIA#1856)

## Summary

- `collectFiles()` silently dropped dot-directories (e.g. `.secret/`)
without reporting them in `skippedDotfiles`. Users were warned about
skipped dot-files but not about entire hidden directory trees being
omitted.
- Now both dot-files and dot-directories are reported, with a trailing
`/` to distinguish directories in the CLI output (e.g. `Skipping 2
hidden path(s): .env, .secret/`).
- Fixed misleading comment in `postInstall()` that said "shellQuote the
relative part" when the code actually uses double-quoted interpolation
(safe due to `validateRelativePath` charset restriction).

Reported by @brandonpelfrey.

## Test plan

- [x] Added regression test: hidden directory with nested files
(`.secret/token.txt`) is now reported as `.secret/` in `skippedDotfiles`
- [x] All 25 unit tests pass (`npx vitest run
src/lib/skill-install.test.ts`)
- [x] Pre-commit hooks pass (lint, commitlint, gitleaks)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Hidden directories and dot-prefixed files inside folders are now
properly recorded and reported when skipped during skill installation.
* Status messaging updated to display "hidden path(s)" instead of
"dotfile(s)" for clearer upload feedback.

* **Tests**
* Added test coverage validating collection and reporting of non-hidden
files and skipped hidden paths (directories and dot-prefixed files).
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Senthil Ravichandran <senthilr@nvidia.com>
Signed-off-by: ColinM-sys <cmcdonough@50words.com>
@wscurran wscurran added the VDR Linked to VDR finding label May 11, 2026
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression VDR Linked to VDR finding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants