Visitar URL original
fix discovery support in nested dirs by tommaso-moro · Pull Request #13459 · cli/cli · GitHub
Skip to content

fix discovery support in nested dirs - #13459

Merged
babakks merged 5 commits into
trunkfrom
tommy/nested-dirs-bug-fix
Jun 2, 2026
Merged

fix discovery support in nested dirs#13459
babakks merged 5 commits into
trunkfrom
tommy/nested-dirs-bug-fix

Conversation

@tommaso-moro

@tommaso-moro tommaso-moro commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #13372

Summary

Fix gh skill install and gh skill preview handling for nested skill paths.

What changed

  • Allow --allow-hidden-dirs to discover hidden skill directories at any repository depth, not only at the root.
  • Treat deeply nested skill arguments as exact repo-relative paths in both install and preview.
    • Example: packages/agent-skills/my-skill
  • Preserve one-slash selectors like owner/skill as namespaced skill names, not paths.
  • Share path-detection logic between install and preview.
  • Add regression tests for nested hidden directories, arbitrary nested path installs, and arbitrary nested path previews.

Product note

This does not change the recommended Agent Skills publishing conventions or broaden search/publish behavior. Non-standard layouts are only supported when the user explicitly provides an exact path.

Testing

  • go test ./internal/skills/discovery ./pkg/cmd/skills/install ./pkg/cmd/skills/preview -run 'TestIsSkillPath|Test_isSkillPath|TestPreviewRun|TestMatchHiddenDirConventions|TestDiscoverSkillsWithOptions|TestDiscoverLocalSkillsWithOptions|TestInstallRun' -count=1
  • go test ./internal/skills/discovery ./pkg/cmd/skills/... -count=1
  • git diff --check

@tommaso-moro
tommaso-moro marked this pull request as ready for review May 20, 2026 09:20
@tommaso-moro
tommaso-moro requested a review from a team as a code owner May 20, 2026 09:20
Copilot AI review requested due to automatic review settings May 20, 2026 09:20
@tommaso-moro
tommaso-moro requested a review from a team as a code owner May 20, 2026 09:20
Copilot AI reviewed May 20, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes gh skill install and gh skill preview behavior for nested skill layouts by improving hidden-directory discovery and by treating deeply nested selectors as repo-relative paths.

Changes:

  • Extend hidden-dir discovery so --allow-hidden-dirs finds skills under hidden directories at any repository depth.
  • Add shared path-detection logic (discovery.IsSkillPath) and use it to support exact nested-path installs/previews (skipping full discovery).
  • Add regression tests covering nested hidden directories and arbitrary nested path selectors.
Show a summary per file
File Description
pkg/cmd/skills/preview/preview.go Add path-based preview flow that looks up a single skill by repo-relative path.
pkg/cmd/skills/preview/preview_test.go Add test ensuring nested path preview skips full discovery.
pkg/cmd/skills/install/install.go Centralize path detection via discovery.IsSkillPath.
pkg/cmd/skills/install/install_test.go Add install test for arbitrary nested path, plus path-detection cases.
internal/skills/discovery/discovery.go Add IsSkillPath helper and broaden hidden-dir convention matching to any depth.
internal/skills/discovery/discovery_test.go Add coverage for nested hidden-dir matching and IsSkillPath.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 6/6 changed files
  • Comments generated: 3

Comment thread internal/skills/discovery/discovery.go Resolved
Comment thread internal/skills/discovery/discovery.go Resolved
Comment thread pkg/cmd/skills/preview/preview.go Resolved
@tommaso-moro
tommaso-moro marked this pull request as draft May 20, 2026 09:34
@tommaso-moro
tommaso-moro marked this pull request as ready for review May 22, 2026 14:56
babakks
babakks requested changes Jun 1, 2026
View reviewed changes

@babakks babakks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. The implementation seems solid. Just some comments (mostly nitpicks) on tests.

Comment thread internal/skills/discovery/discovery.go Resolved
Comment thread internal/skills/discovery/discovery.go Resolved
Comment thread internal/skills/discovery/discovery.go Resolved
Comment thread internal/skills/discovery/discovery.go Resolved
Comment thread internal/skills/discovery/discovery.go Resolved
Comment thread pkg/cmd/skills/install/install_test.go Resolved
Comment thread pkg/cmd/skills/install/install_test.go Resolved
Comment thread pkg/cmd/skills/preview/preview.go Resolved
Comment thread pkg/cmd/skills/preview/preview_test.go Resolved
Comment thread pkg/cmd/skills/preview/preview_test.go Resolved
babakks
babakks approved these changes Jun 2, 2026
View reviewed changes

@babakks babakks left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@babakks
babakks merged commit 7f88572 into trunk Jun 2, 2026
11 checks passed
@babakks
babakks deleted the tommy/nested-dirs-bug-fix branch June 2, 2026 09:07
@BrewTestBot BrewTestBot mentioned this pull request Jun 10, 2026
Merged
@JoseAlcerreca JoseAlcerreca mentioned this pull request Jul 14, 2026
Open
@ntsk ntsk mentioned this pull request Jul 14, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@babakks babakks babakks approved these changes
@BagToad BagToad Awaiting requested review from BagToad BagToad is a code owner automatically assigned from cli/code-reviewers
@SamMorrowDrums SamMorrowDrums Awaiting requested review from SamMorrowDrums

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Skill discovery issues

3 participants

@tommaso-moro @babakks