`gh auth refresh` exists to make it simpler for users to refresh their tokens on expiration/scope mismatch, but help messages only suggest using it in limited scenarios, and not in a common case of a token expiring and the user receiving a 401 error. Now, the auth flow will detect this case, and for refreshable tokens (namely, tokens created by logging in with `gh auth login` in the first place), it will suggest using `gh auth refresh` for these cases.
- --fix and --dry-run now error when combined
- "Ready to publish!" is suppressed when --fix is set since user must
commit first
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When --fix is used, return early after applying fixes instead of
continuing to the publish flow. The fixed files need to be committed
before publishing, so proceeding would fail anyway since the repo
would not be in sync.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
* 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>
* 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>
- 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>
Replace all exec.Command("git", ...), initGitRepo, runGitInDir, and
newTestGitClientWithUpstream with run.Stub()/run.CommandStubber stubs.
Changes:
- Remove os/exec and strings imports; add fmt, regexp, internal/run
- Add newTestGitClient(), stubGitRemote(), stubEnsurePushed() helpers
- Remove initGitRepo, runGitInDir, newTestGitClientWithUpstream helpers
- Add cmdStubs field to TestPublishRun table struct
- Convert all test cases to use stub-based git interactions
- Use regexp.QuoteMeta for remote name patterns
- Use %[1]s/%[2]s format args in stubGitRemote
- Initialize git.Client with explicit GitPath to avoid real git resolution
- Rewrite TestEnsurePushed with stub-based tests
- Update TestDetectGitHubRemote_UsesDir and TestPublishRun_DirArgUsesTargetRemote
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Return nil instead of real http.Client in unsupported host test
- Move skillResultKey type inside deduplicateResults function scope
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removes post-install extension dispatch to keep the stub focused on
installation. Also removes unused args parameter from the run function.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use cmdutil.DisableAuthCheck instead of raw annotation map
- Convert tests to table-driven format
- Use generic extension name in tests (gh-cool)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Register hidden stub commands for official GitHub extensions (gh-aw,
gh-stack) that offer to install the extension when invoked. This
replaces the error-string-matching approach from the original PR with
proper cobra commands that:
- Avoid false-positive matches on flag values or post-'--' args
- Eliminate conflicting cobra 'Did you mean?' suggestions
- Properly propagate prompt/install errors for correct exit codes
- Are hidden from help output and shell completions
- Use GroupID "extension" so checkValidExtension allows installing over them
- Are registered after extensions and aliases so both take priority
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 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>
Remove the dead code block that silently swallowed errors from
opts.HttpClient() and opts.Config() when creating an API client.
Instead, create the client with proper error propagation inside the
remote-checks block where it is actually needed.
Changes:
- Remove the error-swallowing client == nil fallback (lines 363-376)
- Create the API client inside the remote repo checks block with
proper error returns from HttpClient() and Config()
- Resolve host from repoInfo first, then fall back to config
- Remove the now-unreachable 'client == nil' early exit before publish
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Like gh pr create, skill publish now automatically pushes unpushed
local commits before creating a release. This prevents the footgun
where a release is created against stale remote state when the user
has local commits that haven't been pushed yet.
The ensurePushed function checks for unpushed commits using
git rev-list @{push}..HEAD. If commits exist or the branch has
never been pushed, it pushes automatically and prints a status
message. This matches the CLI's opinionated-defaults philosophy
of doing the right thing by default.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the hardcoded skills/ directory requirement in the publish
command with the shared DiscoverLocalSkills() function used by install
and other commands. This removes the opinionated restriction that skills
must live under a skills/ directory and supports all discovery
conventions:
- skills/*/SKILL.md
- skills/{scope}/*/SKILL.md
- */SKILL.md (root-level)
- plugins/{scope}/skills/*/SKILL.md
All per-skill validation (frontmatter, spec-compliant naming, metadata
stripping, etc.) is preserved.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Allow installing extensions without being authenticated. The install
command can work with public repositories and local directories without
requiring a login, so the auth gate is unnecessary.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When a user runs an unknown command that matches a known official
extension (e.g. `gh stack` or `gh aw`), show an install suggestion.
In interactive TTY sessions, prompt the user to install it on the spot.
The official extension registry is hard-coded in pkg/extensions/.
Current entries are github/gh-aw and github/gh-stack. Teams can add
their extensions via PR. Install suggestions use an explicit github.com
host prefix for GHES compatibility.
Refs github/cli#220
Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>
Add cmdutil.DisableAuthCheckFlag for --from-local on install so that
installing from a local directory does not require authentication.
This follows the same pattern used by attestation verify for its
--bundle flag.
The --dry-run flag on publish is intentionally left with auth enabled
because dry-run validation includes remote repository checks (security
settings, tag protection, topics).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Keep skillName as bare name in JSON output for backward compat;
namespace is available as a separate --json field
- Fix Namespace field comment to cover plugin namespaces too
- Trim /SKILL.md from install path arg to match comment
- Rename acceptance test to skills-install-namespaced since it
tests install disambiguation, not search
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the now-unused newTestGitClient helper that was left behind
after refactoring tests to use initGitRepo directly. This fixes the
golangci-lint 'unused' error in CI.
Co-authored-by: BagToad <47394200+BagToad@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
When a directory argument is provided to `gh skill publish`, the remote
detection now correctly uses the target directory's git remotes instead
of the current working directory's remotes.
Previously, `detectGitHubRemote` used the factory-provided git client
which pointed to the CWD. This meant that running
`gh skill publish /path/to/repo-bar` from inside repo-foo would detect
repo-foo's remotes and potentially create the release on the wrong repo.
The fix copies the git client and sets `RepoDir` to the target directory,
matching the pattern already used by `detectMissingRepoDiagnostic` and
`checkInstalledSkillDirs`.
Co-authored-by: BagToad <47394200+BagToad@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- 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>
- Update comment in relevanceScore() from '10 000 points' to '3 000 points'
to match the actual implementation value of 3_000
- Update corresponding test comment for consistency
- Fix typo: 'swaped' → 'swapped' in formatStars comment
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
