Commit graph

915 commits

Author SHA1 Message Date
Kynan Ware
eff9d48f6e Replace em-dashes with hyphens in code comments
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-05-06 17:33:29 -06:00
Kynan Ware
c00257386e feat: add Issues 2.0 API types, mutations, and feature detection
Add API infrastructure for issue types, sub-issues, and issue
relationships (blocked-by/blocking):

- New types: IssueType, LinkedIssue, SubIssues, SubIssuesSummary,
  LinkedIssueConnection
- New Issue struct fields for all Issues 2.0 data
- GraphQL query builder cases for new fields
- ExportData cases for JSON output
- Mutation functions: UpdateIssueIssueType, AddSubIssue,
  RemoveSubIssue, AddBlockedBy, RemoveBlockedBy
- Helper functions: RepoIssueTypes, IssueNodeID
- Feature detection: IssueRelationshipsSupported for GHES 3.19+
  (issue types and sub-issues are GA on GHES 3.17+, no detection needed)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-30 08:49:43 -06:00
Pavel Dostál
6d6ea5f371 Fix flaky Password test by increasing echo mode setup timeout
The beforePasswordSendTimeout was set to 100 microseconds, which is
insufficient for huh to disable echo mode on the PTY in slow or
constrained environments (e.g. network-isolated build containers).
Increase to 100 milliseconds to avoid the race condition.
2026-04-28 18:25:51 +02:00
Sam Morrow
2c1f5b2f72
Merge pull request #13264 from SamMorrowDrums/sammorrowdrums/skill-ghec-data-residency
feat(skills): support GHEC with data residency hosts
2026-04-24 11:45:28 +02:00
Sam Morrow
96b9af3443
Merge pull request #13266 from cli/sammorrowdrums/fix-skill-install-flat-path
Install skills flat by Name, not namespaced InstallName
2026-04-24 11:41:03 +02:00
Copilot
2e93afc272 Install skills flat by Name, not namespaced InstallName
Most agent clients (Claude Code, Copilot, etc.) only discover immediate
subdirectories of their skills folder. When a skill repository used
namespaced paths like skills/author/my-skill/, the installer created
nested directories (e.g. .claude/skills/author/my-skill/) that clients
could not find.

This separates the skill's identity (InstallName, used for lockfile keys,
search, filtering, display) from the filesystem path (Name, used for the
install directory). Skills are now always installed flat:

  .claude/skills/my-skill/SKILL.md  (not .claude/skills/author/my-skill/)

Changes:
- installer: use skill.Name for directory paths instead of InstallName
- install.go: use skill.Name for overwrite checks and prompts
- collisions: detect conflicts by Name since flat install means two
  skills with the same Name but different Namespace values will collide
- update: clean up old namespaced directories when migrating to flat

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-23 01:26:31 +02:00
Copilot
8498bdf435 feat(skills): add --allow-hidden-dirs flag to preview command
Add support for the --allow-hidden-dirs flag in `gh skill preview`,
matching the existing pattern in `gh skill install`. This allows users
to preview skills located in hidden directories (e.g. .claude/skills/,
.agents/skills/).

Changes:
- Add AllowHiddenDirs field to PreviewOptions
- Register --allow-hidden-dirs flag on the preview command
- Switch from DiscoverSkills to DiscoverSkillsWithOptions to get all
  skills including hidden-dir ones
- Add filterHiddenDirSkills to exclude hidden-dir skills by default,
  showing a hint when they are found but excluded
- Print a warning when --allow-hidden-dirs is used and hidden skills
  are present
- Return an error when only hidden-dir skills exist without the flag

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-22 23:41:25 +02:00
sammorrowdrums
63262dce8b feat(skills): support GHEC with data residency hosts
Widen ValidateSupportedHost to accept tenancy hosts (*.ghe.com) alongside
github.com. GHEC with data residency uses these domains, and all skill
subcommands (search, install, preview, publish, update) now allow them.

GitHub Enterprise Server remains unsupported and is explicitly rejected
with a clear error message.

Also fix the lockfile writer to use the actual host when constructing
SourceURL instead of hardcoding github.com.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-22 23:27:38 +02:00
William Martin
7095e2a4fc
Fix SetSampleRate not updating sample_rate dimension
The sample_rate common dimension was set once at service creation
time and never updated when SetSampleRate was called later. This
caused commands like 'gh skill publish' that override the sample
rate via PersistentPreRunE to report the wrong sample_rate in
telemetry events (e.g. 1 instead of 100).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-22 13:04:57 +01:00
William Martin
90ef03ea38
Enable telemetry without env var 2026-04-21 18:40:02 +01:00
Babak K. Shandiz
0467ed499a
test(telemetry): assert ANSI escape chars for color codes
Signed-off-by: Babak K. Shandiz <babakks@github.com>
2026-04-21 18:05:28 +01:00
Babak K. Shandiz
ec4a3ed6bd
fix(telemetry): lower bias in sample bucket calc
Signed-off-by: Babak K. Shandiz <babakks@github.com>
2026-04-21 17:49:28 +01:00
William Martin
571bb1c923 Log when there is no telemetry 2026-04-21 18:15:04 +02:00
Sam Morrow
a67f4f7303
Merge pull request #13235 from cli/sammorrowdrums/fix-skill-install-discovery
Make skill discovery less strict: support nested `skills/` directories
2026-04-21 10:01:53 +02:00
Sam Morrow
9a368f45e9
feat(skills): support nested skills/ directories in discovery
Relax skill discovery to recognize skills/ directories at any depth
in the repository tree, not just at the root. This enables repos like
hashicorp/agent-skills that organize skills under prefixes such as
terraform/code-generation/skills/*/SKILL.md.

Changes:
- matchSkillConventions: add checks for <prefix>/skills/<name>/SKILL.md
  and <prefix>/skills/<ns>/<name>/SKILL.md at any depth
- isSkillPath: also match paths containing /skills/ for direct
  path-based install
- DiscoverSkillByPath: fix namespace detection to find the skills
  segment anywhere in the path
- Update error messages and help text to mention nested conventions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-20 21:07:15 +02:00
William Martin
78f1ad537c Include CI context in telemetry 2026-04-20 12:02:56 +02:00
Sam Morrow
eaa018545a
refactor: decouple hidden-dir filtering from discovery layer
Move --allow-hidden-dirs filtering logic from the discovery package to
the install command, addressing review feedback. Discovery functions now
always return all skills (including hidden-dir), and callers decide how
to handle them.

Changes:
- DiscoverSkillsWithOptions/DiscoverLocalSkillsWithOptions always return
  hidden-dir skills; callers filter using IsHiddenDirConvention()
- DiscoverSkills/DiscoverLocalSkills (convenience wrappers) auto-filter
  hidden-dir skills for backward compatibility with preview/update/publish
- Remove --allow-hidden-dirs reference from discovery error messages
- Add filterHiddenDirSkills in install.go with caller-side flag logic
- Inline warning using heredoc.Docf, remove printHiddenDirWarning
- Add inline comments in matchHiddenDirConventions (babakks nitpicks)
- Add non-hidden-namespaced dir and no-skills-at-all test cases
- Add --allow-hidden-dirs tests in TestNewCmdInstall, TestInstallRun,
  and TestRunLocalInstall

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-20 11:14:39 +02:00
Tommaso Moro
082f15a8fd
Add support for installation in multiple agent hosts in gh skills install (#13209)
* add support for installation in multiple agent host

* print correct dir in warning

* remove dir as it depends on user vs project installation scope

* Move comment closer to assertion in registry test

Move the explanatory comment from above the map initialization to
directly above the assertions it describes, per review feedback.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* List supported agent names and IDs in help text

Replace the self-referencing "run --help" sentence with an inline list
of all supported --agent values showing Name (id) pairs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Use heredoc.Docf for Kiro CLI post-install hint

Replace individual fmt.Fprintln calls with a single heredoc.Docf block
for the Kiro CLI post-install guidance, per review feedback. Also
shorten the --agent flag usage line by overriding the auto-generated
enum list with a reference to the supported values in the help text.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Kynan Ware <47394200+BagToad@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-18 15:22:09 -06:00
William Martin
998b6212b3
Add skills specific telemetry
* Add skills specific telemetry

* Remove VisibilityFuture, inline goroutine at call sites

The VisibilityFuture/FetchRepoVisibilityAsync/Wait wrapper was an
unidiomatic async abstraction built for a single pattern used in
exactly two call sites. In Go the channel is already the future;
wrapping it in a struct with a Wait(timeout) method adds no value.

Delete the abstraction and inline a local visResult struct,
buffered channel, goroutine, and select at each call site. Behavior
is preserved exactly: err -> "unknown", timeout -> "unknown",
success+public -> include skill_names.

FetchRepoVisibility (synchronous) is kept as-is.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix nonsense copilot tests

* Update telemetry tests for public-only dims and search event removal

Production telemetry emission changed:
- preview: skill_owner/skill_repo/skill_name (renamed from skill_names)
  are now emitted only when repo_visibility=public.
- install: skill_owner/skill_repo/skill_names are now emitted only
  when repo_visibility=public.
- search: the initial skill_search event was removed entirely; the
  skill_search_install event no longer carries query/owner dims.

Update tests to match: rename skill_names -> skill_name in preview,
make owner/repo assertions conditional on public visibility in both
preview and install, and reduce the search test to a single event
with explicit Empty assertions for the removed query/owner dims so a
privacy regression cannot pass silently.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Test CategorizeHost and switch telemetry to skill_host_type

Add TestCategorizeHost covering all four classification branches
(github.com, ghes, tenancy, uncategorized) with cases verified
against the real ghauth implementation rather than guessed.

Update install and preview unit tests to assert the new
skill_host_type dimension name, and fix a typo in the preview
acceptance txtar (skill_hos_type -> skill_host_type).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Shrink visibility wait and test unknown visibility

The 2s visibilityWaitTimeout was wildly overprovisioned: by the time
telemetry emission reaches the select, the command has already done
several serial GitHub REST calls (and for install, a git sparse-checkout
plus possibly interactive prompts), so the one-call visibility fetch
has almost always completed. Drop the timeout to 200ms — a short safety
net for a stalled REST call, not a wait budget for a healthy one.

Also adds a table-driven case to TestFetchRepoVisibility covering an
unknown/future visibility value from the API, addressing @babakks'
review nitpick.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-17 19:58:59 +02:00
William Martin
17776cafc1 Apply review feedback
- Harden SpawnSendTelemetry against relative executable paths
- Use io.Copy for telemetry subprocess stdin write
- Clean up GH_TELEMETRY/DO_NOT_TRACK help text
- Fall back to built-in defaults (NoOp telemetry) on config load failure

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-17 12:28:52 +02:00
William Martin
3ed389d664 Disable telemetry for GHES 2026-04-17 11:50:24 +02:00
William Martin
18dc5e77f0 Add sampled command telemetry 2026-04-16 21:42:46 +02:00
Sam Morrow
8b115d2c23
fix: address post-merge review feedback for skills commands
- Remove direct opts.client injection in publish; use HttpClient factory
  pattern (PR #13168 feedback)
- Rename testName to name in discovery test struct (PR #13170 feedback)
- Use typed struct keys for dedup map with case-insensitive comparison
  in deduplicateResults (PR #13170 feedback)
- Simplify remote selection to use Remotes() ordering instead of manual
  origin-first logic (PR #13171 feedback)
- Fix push icon timing: show no icon before push, SuccessIcon after
  success (PR #13171 feedback)

Closes #13184

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-16 16:19:59 +02:00
Sam Morrow
963a1438a9
Merge pull request #13170 from cli/sammorrowdrums/fix-skills-namespace-dedup
fix: preserve namespace in skills search deduplication
2026-04-16 15:55:41 +02:00
Sam Morrow
9f9b93aa6a
URL-encode parentPath in skills discovery API call
The parentPath parameter in the contents API path was not URL-encoded,
which would cause failures when paths contain spaces or other special
characters. Apply url.PathEscape() to parentPath, consistent with
the rest of the file. commitSHA is left unescaped since SHAs are
hex-only and never need encoding.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-16 00:06:48 +02:00
Sam Morrow
92e40eabea
fix: preserve namespace in skills search deduplication
Skills with the same name but different namespaces (e.g.
skills/kynan/commit and skills/will/commit) were being collapsed
into a single search result because extractSkillName discarded the
namespace. This also caused deduplicateByName to cap results
across different namespaces as if they were the same skill.

Changes:
- Add MatchSkillPath to discovery package returning both name and
  namespace (the existing MatchesSkillPath is kept for compat)
- Add Namespace field to skillResult in search
- Fix deduplicateResults to use repo/namespace/name as the dedup key
- Fix deduplicateByName to cap by namespace-qualified name
- Update table, prompt, and JSON output to show qualified names
- Use skill path for install subprocess when namespace is present,
  ensuring unambiguous install of namespaced skills
- Add namespace to --json fields and relevance scoring/filtering
- Add unit tests for namespace dedup, qualified names, and filtering
- Add acceptance test for namespaced skill search and install

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-15 23:01:09 +02:00
Sam Morrow
a6f6ab330f
fix: enforce size cap on first preview file, surface corrupted skills, fail on path traversal
- preview: remove 'fetched > 0' guard so the 512KB size cap applies
  uniformly to all files including the first
- update: return skills with corrupted YAML frontmatter with metadataErr
  set instead of silently dropping them from scan results
- installer: fail installation when a path traversal is detected in
  remote or local skill files instead of silently skipping

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-15 16:53:27 +02:00
tommaso-moro
45d0ec0b51
address review comments
Co-authored-by: Sam Morrow <info@sam-morrow.com>
2026-04-15 16:01:26 +02:00
tommaso-moro
1f5a6b8396
clean up interface and fix a few bugs
support specifying a sha in gh skills preview command
2026-04-15 15:50:53 +02:00
tommaso-moro
663df07fcf
cleanup frontmatter fields
remove git sha because we only need git tree sha

remove github-owner from frontmatter, and make github-repo support full url. Only support github.com as host, error out otherwise
2026-04-15 15:47:34 +02:00
tommaso-moro
3b50bbbf16
add .agents/skills as default installation path for hosts that support it: cursor, codex, gemini CLI, github copilot, antigravity.
excluded: claude code
2026-04-15 15:47:34 +02:00
tommaso-moro
b26256a10d
show loading spinner during installation, even for multi-file skills 2026-04-15 15:47:34 +02:00
Kynan Ware
8ea84d0dee
Expand test coverage and fix invariants/bugs
Cover the three primary discovery entry points with httpmock-based tests.
DiscoverSkills: happy path, truncated tree, no skills, API error, dedup.
DiscoverSkillByPath: path resolution, namespaces, invalid name, missing
directory, missing SKILL.md. DiscoverLocalSkills: convention matching,
root skill, no skills, nonexistent directory.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Test InstallLocal public API instead of private installLocalSkill

Replace tests that called installLocalSkill directly with tests through
InstallLocal. Adds coverage for AgentHost+Scope resolution path,
multiple skills, and missing Dir/AgentHost error. Fixes symlink test
to require.NoError on os.Symlink.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Test partial failure in concurrent Install

Add test where one of two skills fails (500 on tree fetch). Asserts
that result.Installed contains the successful skill and err wraps the
failed skill name. Fixes test loop to not clear Dir for partial failure
cases.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Refactor update tests to table-driven pattern

Consolidate 16 individual test functions into 3 standalone + 3 table
tests matching cli/cli conventions. Fix ArgsPassedToOptions to use
iostreams.Test() instead of os.Stdout/os.Stderr. Use GitHub-branded
test data. No coverage lost.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add update execution test that verifies SKILL.md is rewritten

All prior update tests used DryRun or hit early exits. New test
exercises the full fetch-and-rewrite path: stale treeSHA triggers
re-download, SKILL.md is overwritten with new content and metadata.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use heredoc.Doc for multiline SKILL.md strings in update tests

Replace escaped newline strings with heredoc.Doc backtick literals
for readability, matching cli/cli conventions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add interactive update path tests

Cover confirm-and-apply, confirm-cancelled, and no-metadata prompt
paths in TestUpdateRun. These interactive branches were previously
untested since all prior tests used non-TTY or DryRun.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Test no-metadata prompt enrichment through full update path

Add test where a skill with no GitHub metadata is prompted for origin,
user provides owner/repo, skill gets enriched and proceeds through
version resolution and file rewrite. Covers lines 222-224 in update.go.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Replace deprecated cs.Gray with cs.Muted

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Test namespaced skill update with --dir base resolution

Cover the filepath.Dir double-up path for namespaced skills (name
contains '/') when using --dir. Verifies the install base is resolved
correctly so the update writes to the right directory.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Test install failure during update reports error and preserves file

Cover the path where version resolution succeeds but blob fetch fails
during the actual install. Verifies stderr error message, SilentError
return, and that the original SKILL.md is not modified.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Dedupe resolveGitRoot/resolveHomeDir into installer, rename scanAllHosts

Move ResolveGitRoot and ResolveHomeDir to the installer package to
eliminate duplication between install and update commands. Fix
ResolveGitRoot to check RepoDir before calling ToplevelDir.

Rename scanAllHosts to scanAllAgents to match registry naming. Add
test exercising scanAllAgents via updateRun without --dir.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use heredoc.Doc for multiline YAML strings across all test files

Convert 13 escaped-newline frontmatter strings to heredoc.Doc for
readability. Applies to discovery, frontmatter, install, update,
publish, and preview test files. Preserves edge-case test strings
and fmt.Sprintf interpolations as-is.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use git.Client.Copy() instead of struct copy to avoid mutex copy

Fixes go vet 'copies lock value' warnings in publish command where
*git.Client was copied by value to set a different RepoDir. Rename
terse variable names (bc/ic/dc) to branchGit/ignoreGit/dirGit.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Rewrite publish tests: table-driven through publishRun

Consolidate 35 test functions into 2: TestNewCmdPublish (4 cases for
CLI arg parsing) and TestPublishRun (22 cases exercising all behavior
through the command's run function). No individual helper function
tests — every codepath tested through publishRun scenarios.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Remove .gitkeep from acceptance/testdata/skills

Delete the placeholder .gitkeep file from acceptance/testdata/skills. The directory no longer needs a placeholder file to be tracked in the repository.

Rename testPublishGitClient to newTestGitClient

Rename the test helper function testPublishGitClient to newTestGitClient in pkg/cmd/skills/publish/publish_test.go and update all call sites accordingly. This is a purely refactor/name-change with no behavioral changes to tests.

Fix Windows CI: set USERPROFILE alongside HOME in tests

os.UserHomeDir() uses USERPROFILE on Windows, not HOME. All tests
that redirect HOME for lockfile isolation now also set USERPROFILE
to the same temp directory.

Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>

Use range-over-int in acquireLock retry loop

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Test lock acquisition edge cases through RecordInstall

Make lockRetries and lockRetryInterval configurable (package-level vars)
so tests can avoid the 3s retry wait. Add two RecordInstall cases:
- Stale lock (>30s old) is broken and install succeeds
- Fresh lock exhausts retries, proceeds best-effort without lock

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Rename test helpers for lockfile tests

Rename setupHome to setupTestHome and readLockfile to readTestLockfile in internal/skills/lockfile tests, and update all call sites and comments accordingly. This is a refactor-only change to clarify test helper names with no behavior change.

Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>

Test read() degradation through RecordInstall, delete TestRead

Move corrupt JSON and wrong version cases into TestRecordInstall table.
RecordInstall calls read() internally, so these exercise the same
degradation paths through the public API. Verifies the lockfile is
rewritten with correct version and new data after recovery.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Fix InstalledAt preservation test to actually prove preservation

Move the update-preserves-InstalledAt case out of the table into a
standalone subtest that reads InstalledAt between two RecordInstall
calls and asserts exact equality. The table version only checked
NotEmpty which couldn't detect if InstalledAt was overwritten.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Merge duplicate plugin test into TestMatchSkillConventions table

The standalone TestDuplicatePluginSkills_DifferentAuthors re-implemented
dedup logic that belongs in DiscoverSkills. Replace with a table case
that tests convention matching only. Dedup is already covered by
TestDiscoverSkills.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Fix broken validateName max-length test case

Replace make([]byte, N) (which produces null bytes) with
strings.Repeat to actually test the 64-character boundary. Add
positive test for valid 64-char name.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Replace name-matching hack with createDir field in TestDiscoverLocalSkills

Use a struct field instead of comparing tt.name to control whether
the test directory is created. Prevents silent breakage if someone
renames the test case.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Improve collisions tests: table-driven FormatCollisions, exercise DisplayName

Convert TestFormatCollisions to table test with nil-input case. Update
single collision case to use different conventions (plugins vs skills)
so DisplayName() logic is actually exercised in the assertion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add tests for MatchesSkillPath, DiscoverSkillFiles, ListSkillFiles, FetchDescriptionsConcurrent

Also cover previously untested branches: root convention matching,
annotated tag dereference failure, empty tag_name/default_branch
fallbacks, recursive walkTree with subtrees, and skill directory
deduplication.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Test full GitHub key stripping in InjectLocalMetadata

Add all 7 github-* keys to the input metadata and assert all are
absent after injection. Previously only tested github-owner and
github-repo removal.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Test Serialize trailing-newline addition for body without newline

Add case where body doesn't end in newline and assert the output
has one appended. Previously this branch was uncovered.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Test InjectGitHubMetadata with no existing frontmatter

Add case where content has no --- delimiters, exercising the
RawYAML == nil branch that creates frontmatter from scratch.
Also fix test data to use GitHub-branded names.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Convert TestInjectLocalMetadata to table-driven with no-metadata case

Add case for content with no frontmatter, exercising the meta == nil
branch. Aligns with table-driven pattern used throughout.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Replace name-matching hack with useAgentHost field in TestInstallLocal

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add tests for ResolveGitRoot

Cover RepoDir shortcut, nil client fallback, and empty RepoDir
fallback. Skip ResolveHomeDir — it's a thin os.UserHomeDir wrapper
with no logic to test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Test OnProgress callback in both single and multi-skill Install paths

Cover the progress reporting branches in Install for both the
single-skill fast path (len==1) and the concurrent multi-skill path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Cover missing InstallDir error branches and malformed URL in registry

Add user-scope-without-homeDir and invalid-scope cases to TestInstallDir.
Add malformed URL case to TestRepoNameFromRemote. Coverage 80.5% → 87.8%.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Rewrite install tests: table-driven through installRun and runLocalInstall

Consolidate 48 individual test functions into 6: TestNewCmdInstall (10
cases for CLI parsing), TestInstallRun (21 cases for remote install
flow), TestRunLocalInstall (10 cases for local install flow), plus
TestIsLocalPath, TestIsSkillPath, TestFriendlyDir for pure input
classification. Delete zero-value Help test. All behavior tested
through public functions instead of calling internal helpers directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Fix data race in OnProgress test with atomic counter

The OnProgress callback was appending to a shared slice from concurrent
goroutines. Replace with sync/atomic counter to avoid the race.

Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>

Add interactive install tests for skill selection, scope, host, and overwrite

Exercise the interactive TTY paths in installRun: MultiSelectWithSearch
for skill selection, Select for scope prompt, MultiSelect for host
selection, and Confirm for overwrite declined.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Exercise skillSearchFunc fully through interactive mock

Update the interactive skill selection test to use 31 skills (exceeding
maxSearchResults cap), include a skill without a description, and have
the mock call searchFunc with both empty and filtered queries. Verifies
the MoreResults count, label formatting, and truncation branches.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Fill remaining install coverage gaps

Add local path detection cases to TestNewCmdInstall. Add interactive
repo prompt, user scope selection, overwrite without metadata, and
single exact match cases to TestInstallRun. Add bare tilde expansion
to TestRunLocalInstall.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Move HOME/USERPROFILE setenv to test loops, remove per-case duplication

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add isTTY field to install test tables, centralize TTY setup

Move TTY configuration from individual opts funcs into the test loops.
Each table case declares isTTY: true/false and the loop sets all three
streams accordingly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Remove INSTALL_TARGET env var hack from install test

Metadata injection is already proven by installer package tests.
This test only needs to verify installRun orchestrates correctly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add ScopeChanged: true to all install tests with explicit Scope

Ensures tests simulate the same state cobra produces when --scope is
explicitly provided, preventing silent codepath divergence if the
default scope behavior changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Fix assert.Error → require.Error in TestNewCmdSearch

Prevents nil panic on err.Error() if the command unexpectedly returns nil.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Improve preview test quality and coverage

- Fix assert.Error/assert.NoError → require.Error/require.NoError to
  prevent nil panics in TestNewCmdPreview and TestPreviewRun
- Add renderAllFiles edge case tests: maxFiles cap (20 files), maxBytes
  cap (512KB), and FetchBlob error fallback message
- Replace custom discardWriter with io.Discard
- Use GitHub-branded names (monalisa) in new tests

Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>

Add search test coverage: rate limits, owner scope, blob enrichment

- Add HTTP 429 and 403+Retry-After rate limit test cases
- Add owner-scoped no-results test (exercises noResultsMessage branch)
- Add blob description enrichment test (exercises fetchDescriptions path)
- Replace custom splitOnSpaces with strings.Fields
- Replace custom discardWriter with io.Discard

Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>

Remove low-value alias test for preview command

The test only asserts a string literal matches another string literal.
Alias presence is already visible in the command definition.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Replace local pluralize with text.Pluralize

The internal/text package already provides this function via go-gh.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Inline collapseWhitespace — just strings.Fields + Join

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Doc: suggest using go-humanize for star formatting

Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>

Return cmdutil.CancelError on user cancellation in publish and update

Both commands returned nil (success exit) when the user declined
confirmation. The core CLI pattern is to return CancelError so the
process exits with a non-zero status.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add interactive publish prompt tests and isTTY field

Cover all prompt branches in runPublishRelease:
- Topic confirm + semver tag selection + final confirm (happy path)
- Custom tag input path (select idx=1)
- Final confirm declined (CancelError)
- Immutable releases prompt (enable via PATCH)

Add isTTY field to test table struct for centralized TTY setup,
matching the pattern used in install tests. Add auto-confirm
prompters to existing TTY tests that now need them.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Remove duplicate giturl import alias in publish

The git package was imported twice — once as 'git' and again as
'giturl'. Use git.ParseURL directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Fix data race in search enrichment

fetchDescriptions and fetchRepoStars run concurrently but both wrote
to fields of the same skillResult slice elements, triggering the race
detector. Refactor both functions to return index-keyed maps instead
of mutating the slice directly. enrichSkills merges the maps into the
slice after both goroutines complete.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

refactor: remove Claude plugin branding, align with Open Plugin Spec

Replace all 'Claude plugin' references with generic 'plugin' terminology
to align with the vendor-neutral Open Plugin Spec
(https://github.com/vercel-labs/open-plugin-spec).

Changes:
- Rename .claude-plugin/ to .plugin/ (spec §5.1 vendor-neutral manifest)
- Rename claudePluginJSON/claudeAuthor types to pluginJSON/pluginAuthor
- Rename claudeMarketplaceJSON to marketplaceJSON
- Rename generateClaudePlugin to generatePlugin
- Remove 'Claude Code' from plugin-related comments, help text, and flags
- Update install.go plugins/ convention message

Factual host references (Claude Code as an agent name, .claude/skills
directories) are intentionally preserved — those are product names, not
plugin branding.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Remove --plugins flag from publish command

Remove the --plugins flag and all associated plugin generation code from
the publish flow. This was scope creep — the publish command should focus
on validating and publishing skills, not generating plugin
manifests.

Removed:
- --plugins flag and Plugins option field
- generatePlugin, generateMarketplace, buildPluginDescription functions
- pluginJSON, marketplaceJSON, marketplacePlugin types
- Related tests and help text

The install command's ability to discover and pluck skills from plugin-
structured repositories (plugins/ convention) is preserved.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

don't fall back on default branch if you can't fetch latest release

improve search algo by using square rot instead of log for stars, and reduce weight for exact name match

add support for --unpin flag when updating a skill
2026-04-15 15:46:58 +02:00
tommaso-moro
40b2a784e3
add core logic and improve test coverage 2026-04-15 15:45:49 +02:00
tommaso-moro
758785b8f4
improve test coverage/cleanup 2026-04-15 15:45:29 +02:00
tommaso-moro
e57fb436fa
add skills command scaffold 2026-04-15 15:43:43 +02:00
Babak K. Shandiz
2bf528ccc7
test(internal/authflow): assert user-agent header is not modified/added
Signed-off-by: Babak K. Shandiz <babakks@github.com>
2026-03-27 11:48:17 +00:00
William Martin
fb8e22a767 fix(auth): preserve User-Agent in authflow getViewer
getViewer was building a new HTTP client from scratch, losing
AppVersion and InvokingAgent from the plain client already passed
into AuthFlow. Reuse the existing client by shallow-copying it and
wrapping its transport with AddAuthTokenHeader for the new token.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 17:26:27 +01:00
William Martin
cb2b50576f Ensure huh prompter cleans up 2026-03-26 14:26:57 +01:00
Kynan Ware
84a3ba83e4 fix(huh prompter): remove unused fields and imports
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:26:57 +01:00
Kynan Ware
13e47d0078 feat(huh prompter): clear search input after submitting query
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:26:17 +01:00
Kynan Ware
cfb2224176 refactor(huh prompter): custom Field for MultiSelectWithSearch
Replace the OptionsFunc-based MultiSelectWithSearch with a custom huh
Field implementation. huh's OptionsFunc runs in a goroutine, causing
data races with selection state and stale cache issues that made
selections disappear on toggle or search changes.

The custom field (multiSelectSearchField) combines a text input and
multi-select list in a single field with full control over the update
loop. Search runs asynchronously via tea.Cmd when the user presses
Enter, with a themed spinner during loading. Selections are stored in
a simple map — no goroutine races, no Eval cache, no syncAccessor.

Also adds defensive validation for mismatched Keys/Labels slices from
searchFunc.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:26:17 +01:00
Kynan Ware
f38abbe1ca feat(huh prompter): add placeholder to search input
Add 'Type to search, Ctrl+U to clear' placeholder to the
MultiSelectWithSearch search input. Set WithWidth(80) in the test
harness to prevent textinput placeholder rendering panics when
there is no terminal.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:26:17 +01:00
Kynan Ware
38e10d5ebf fix(huh prompter): use synchronized accessors to eliminate data race
Replace Value() pointer bindings with syncAccessor in
MultiSelectWithSearch. huh's OptionsFunc runs in a goroutine while
the main event loop writes field values, causing a data race on
shared variables. syncAccessor implements huh's Accessor interface
with a shared mutex, ensuring all reads and writes are synchronized.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:26:17 +01:00
Kynan Ware
95a59f4431 fix(accessible prompter): update test expectations for huh v2
Fix accessible prompter tests that broke with the huh v2 upgrade:
- Replace 'Input a number' with 'Enter a number' (huh v2 changed text)
- Remove trailing CRLF from ExpectString calls that now fail due to
  ANSI color codes wrapping the title text
- Allow ANSI escape codes in password masking regex assertions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:26:16 +01:00
Kynan Ware
4d74e057f2 refactor(huh prompter): pipe-based test harness with full coverage
Replace manual model updates with an io.Pipe-based test harness that
drives forms through bubbletea's real event loop. Interaction helpers
(tab(), toggle(), typeKeys(), enter(), etc.) send raw terminal bytes
through io.Pipe to form.Run() in a goroutine.

Add tests for AuthToken, ConfirmDeletion, and InputHostname including
validation rejection paths. Add MultiSelectWithSearch coverage for
persistent options and empty search results.

30 tests, ~1s, all build*Form methods at 94-100% coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:26:16 +01:00
Kynan Ware
86d876fd34 test(huh prompter): add table-driven tests for all prompt types
Extract build*Form() methods from each huhPrompter method, separating
form construction from form.Run(). This enables testing the real form
construction code by driving it with direct model updates, adapted
from huh's own test patterns.

Tests cover Input, Select, MultiSelect, Confirm, Password,
MarkdownEditor, and MultiSelectWithSearch including a persistence
test that verifies selections survive across search query changes.

Also fixes a search cache initialization bug where the first
buildOptions("") call would skip the searchFunc due to
cachedSearchQuery defaulting to "".

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:26:16 +01:00
Kynan Ware
f294831e7d Upgrade to huh/v2 and fix selection persistence in MultiSelectWithSearch
Migrate from github.com/charmbracelet/huh v1 to charm.land/huh/v2,
updating ThemeBase16 to the new ThemeFunc API.

Fix selected options being lost across searches in the huhPrompter's
MultiSelectWithSearch. The root cause was huh's internal Eval cache:
when the user returned to a previously-seen search query, cached
options with stale .selected state overwrote the current selections
via updateValue(). The fix includes selectedValues in the OptionsFunc
binding hash (via searchOptionsBinding) so the cache key changes
whenever selections change, preventing stale cache hits. A local
searchFunc result cache avoids redundant API calls when only the
selection state (not the query) has changed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:26:15 +01:00
Kynan Ware
726714d1a7 Use LayoutStack for huhPrompter MultiSelectWithSearch
Implement a huh-native MultiSelectWithSearch that renders the search
input and multi-select list simultaneously using LayoutStack. The
search input is in Group 0 and the multi-select in Group 1, with
OptionsFunc bound to the search query so results update when the
user presses Enter to advance focus. Users can Shift+Tab back to
refine their search, and selections persist across queries.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:24:56 +01:00
Kynan Ware
87426ee236 Add experimental huh-only prompter gated by GH_EXPERIMENTAL_PROMPTER
Introduce a new Prompter implementation (huhPrompter) that uses the
charmbracelet/huh library in its standard interactive mode, as an
alternative to the survey-based default prompter. The new implementation
is gated behind the GH_EXPERIMENTAL_PROMPTER environment variable,
following the same truthy/falsey pattern as GH_ACCESSIBLE_PROMPTER.

Key differences from the accessible prompter:
- No WithAccessible(true) flag (full interactive TUI)
- Uses EchoModePassword (masked with *) instead of EchoModeNone
- No default value annotations appended to prompt text

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-03-26 14:24:56 +01:00