Remove redundant nil-client fallback in skills publish by SamMorrowDrums · Pull Request #13168 · cli/cli

SamMorrowDrums · 2026-04-15T20:49:44Z

Stacked on #13165.

Problem

The publishRun function contained a block (lines 363-376) that silently swallowed errors from opts.HttpClient() and opts.Config() when creating an API client. If either factory function failed, the code would simply skip client creation and continue with client == nil, eventually producing a vague "could not create API client" message instead of surfacing the actual error.

This block was an artifact from migrating away from custom HTTP clients.

Changes

Removed the error-swallowing client == nil fallback block — factory errors are now properly returned
Moved client creation into the remote-checks block where it's actually needed, after local validation completes
Resolved host from repoInfo first, then falls back to config (with proper error returns)
Removed the unreachable client == nil early exit before the publish flow — the client is now guaranteed to exist at that point

Testing

All existing tests pass. No new tests needed — this is a pure refactor that makes errors visible rather than silently swallowing them.

Copilot

Pull request overview

Introduces the new preview “skills” feature set in the GitHub CLI by adding a top-level gh skill / gh skills command group with supporting subcommands and underlying skills infrastructure (discovery, install metadata, lockfile, and cross-platform file locking), plus wiring into root and acceptance tests.

Changes:

Add gh skill command group and new search, preview, and update commands (plus tests).
Add internal skills infrastructure (discovery, registry, metadata parsing/injection, lockfile with cross-platform flocking).
Wire commands into the root command and extend acceptance coverage; add small git client helpers.

Show a summary per file

File	Description
pkg/cmd/skills/update/update.go	Implements `gh skill update` by scanning installed skills, checking remote SHAs, and reinstalling updates.
pkg/cmd/skills/skills.go	Adds the `gh skill`/`gh skills` command group and attaches subcommands.
pkg/cmd/skills/search/search.go	Implements `gh skill search` using GitHub Code Search with relevance ranking and optional interactive install.
pkg/cmd/skills/search/search_test.go	Unit tests for search behavior, pagination, relevance ranking, and rate-limit messaging.
pkg/cmd/skills/preview/preview.go	Implements `gh skill preview` including tree display, pager rendering, and interactive file browsing.
pkg/cmd/skills/preview/preview_test.go	Unit tests for preview flows, interactive selection, and render limits.
pkg/cmd/root/root.go	Registers the new `skills` command group on the root command.
internal/skills/source/source.go	Adds helpers to build/parse repo metadata and enforce supported host restrictions.
internal/skills/source/source_test.go	Tests repo URL building/parsing and supported-host validation.
internal/skills/registry/registry.go	Defines supported agent hosts and resolves install locations by scope.
internal/skills/registry/registry_test.go	Tests agent lookup, install dir resolution, scope labels, and remote URL parsing.
internal/skills/lockfile/lockfile.go	Adds `.skill-lock.json` management with file locking to avoid concurrent write races.
internal/skills/lockfile/lockfile_test.go	Tests lockfile creation, updates, recovery from corruption/version mismatch, and lock contention.
internal/skills/installer/installer.go	Implements remote/local install logic with metadata injection and path traversal protections.
internal/skills/installer/installer_test.go	Tests local/remote install behaviors, symlink handling, traversal blocking, lockfile writing, and progress callbacks.
internal/skills/frontmatter/frontmatter.go	Parses/serializes frontmatter and injects GitHub/local metadata into SKILL.md.
internal/skills/frontmatter/frontmatter_test.go	Tests parsing and metadata injection/serialization behaviors.
internal/skills/discovery/discovery.go	Implements skill discovery (remote + local), ref resolution, file enumeration, and blob fetching.
internal/skills/discovery/collisions.go	Adds detection/formatting of install-name collisions.
internal/skills/discovery/collisions_test.go	Tests collision detection and formatting.
internal/flock/flock.go	Adds OS-agnostic sentinel error for lock contention.
internal/flock/flock_unix.go	Unix implementation of non-blocking exclusive file locks.
internal/flock/flock_windows.go	Windows implementation of non-blocking exclusive file locks using Win32 APIs.
internal/flock/flock_test.go	Tests lock acquisition, contention, and basic file usability while locked.
go.mod	Promotes `golang.org/x/sys` to a direct dependency (needed for Windows locking).
git/client.go	Adds `RemoteURL`, `IsIgnored`, and `ShortSHA` helpers on the git client.
git/client_test.go	Tests for the new git client helper methods.
acceptance/acceptance_test.go	Adds a `skills` acceptance test suite runner.
acceptance/testdata/skills/skills-update.txtar	Acceptance coverage for update flow in custom directory mode.
acceptance/testdata/skills/skills-update-noinstalled.txtar	Acceptance coverage for update behavior when no installed skills exist.
acceptance/testdata/skills/skills-search.txtar	Acceptance coverage for basic search and JSON output flags.
acceptance/testdata/skills/skills-search-page.txtar	Acceptance coverage for search pagination.
acceptance/testdata/skills/skills-search-noresults.txtar	Acceptance coverage for no-results behavior in non-TTY mode.
acceptance/testdata/skills/skills-preview.txtar	Acceptance coverage for preview rendering and not-found errors.
acceptance/testdata/skills/skills-preview-noninteractive.txtar	Acceptance coverage for non-interactive preview requiring a skill name.
acceptance/testdata/skills/skills-install.txtar	Acceptance coverage for install, metadata injection, lockfile creation, and `--dir`.
acceptance/testdata/skills/skills-install-scope.txtar	Acceptance coverage for project vs user scope install locations.
acceptance/testdata/skills/skills-install-pin.txtar	Acceptance coverage for `--pin` behavior.
acceptance/testdata/skills/skills-install-nonexistent-skill.txtar	Acceptance coverage for install failure when skill doesn’t exist.
acceptance/testdata/skills/skills-install-nested-files.txtar	Acceptance coverage for installing skills with nested file structures.
acceptance/testdata/skills/skills-install-invalid-repo.txtar	Acceptance coverage for install failure when repo doesn’t exist.
acceptance/testdata/skills/skills-install-invalid-agent.txtar	Acceptance coverage for invalid `--agent` validation output.
acceptance/testdata/skills/skills-install-from-local.txtar	Acceptance coverage for `--from-local` install and local metadata injection.
acceptance/testdata/skills/skills-install-force.txtar	Acceptance coverage for `--force` overwrite behavior.
acceptance/testdata/skills/skills-publish-lifecycle.txtar	Acceptance coverage for publish/install/preview/update lifecycle using a real repo.
acceptance/testdata/skills/skills-publish-dry-run.txtar	Acceptance coverage for publish dry-run validation behavior.
.gitignore	Ignores a local `gh` binary/artifact.

Copilot's findings

Tip

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

Files reviewed: 52/53 changed files
Comments generated: 2

babakks

LGTM, but I recommend you apply the suggested change to keep it consistent with other gh commands.

babakks · 2026-04-16T09:07:55Z

+	// simple errors (missing skills/, bad SKILL.md, etc.) are reported
+	// without requiring an HTTP client.
 	client := opts.client
 	host := opts.host


We don't inject HTTP clients for testing in this way. Basically, the test function creates a stubbed HTTP client and make the factory function (opts.HttpClient) return that (example)

That way you never need to involve the test-aware mindset in the command code.

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>

- 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>

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cli/cli](https://github.com/cli/cli) | minor | `v2.89.0` → `v2.90.0` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>cli/cli (cli/cli)</summary> ### [`v2.90.0`](https://github.com/cli/cli/releases/tag/v2.90.0): GitHub CLI 2.90.0 [Compare Source](cli/cli@v2.89.0...v2.90.0) #### Manage agent skills with `gh skill` (Public Preview) [Agent skills](https://agentskills.io) are portable sets of instructions, scripts, and resources that teach AI coding agents how to perform specific tasks. The new `gh skill` command makes it easy to discover, install, manage, and publish agent skills from GitHub repositories - right from the CLI. ``` # Discover skills gh skill search copilot # Preview a skill without installing gh skill preview github/awesome-copilot documentation-writer # Install a skill gh skill install github/awesome-copilot documentation-writer # Pin to a specific version gh skill install github/awesome-copilot documentation-writer --pin v1.2.0 # Check installed skills for updates gh skill update --all # Validate and publish your own skills gh skill publish --dry-run ``` Skills are automatically installed to the correct directory for your agent host. `gh skill` supports GitHub Copilot, Claude Code, Cursor, Codex, Gemini CLI, and Antigravity. Target a specific agent and scope with `--agent` and `--scope` flags. `gh skill publish` validates skills against the [Agent Skills specification](https://agentskills.io/specification) and checks remote settings like tag protection and immutable releases to improve supply chain security. Read the full announcement on the [GitHub Blog](https://github.blog/changelog/2026-04-16-manage-agent-skills-with-github-cli/). `gh skill` is launching in public preview and is subject to change without notice. #### Official extension suggestions When you run a command that matches a known official extension that isn't installed (e.g. `gh stack`), the CLI now offers to install it instead of showing a generic "unknown command" error. This feature is available for [github/gh-aw](https://github.com/github/gh-aw) and [github/gh-stack](https://github.com/github/gh-stack). When possible, you'll be prompted to install immediately. When prompting isn't possible, the CLI prints the `gh extension install` command to run. #### `gh extension install` no longer requires authentication `gh extension install` previously required a valid auth token even though it only needs to download a public release asset. The auth check has been removed, so you can install extensions without being logged in. #### What's Changed ##### ✨ Features - Add `gh skill` command group: install, preview, search, update, publish by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13165](cli/cli#13165) - Suggest and install official extensions for unknown commands by [@BagToad](https://github.com/BagToad) in [#13175](cli/cli#13175) - `gh skill publish`: auto-push unpushed commits before publish by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13171](cli/cli#13171) - Disable auth check for `gh extension install` by [@BagToad](https://github.com/BagToad) in [#13176](cli/cli#13176) ##### 🐛 Fixes - Fix infinite loop in `gh release list --limit 0` by [@Bahtya](https://github.com/Bahtya) in [#13097](cli/cli#13097) - Ensure `api` and `auth` commands record agentic invocations by [@williammartin](https://github.com/williammartin) in [#13046](cli/cli#13046) - Disable auth check for local-only skill flags by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13173](cli/cli#13173) - URL-encode parentPath in skills discovery API call by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13172](cli/cli#13172) - Fix: use target directory remotes in skills publish by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13169](cli/cli#13169) - Fix: preserve namespace in skills search deduplication by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13170](cli/cli#13170) ##### 📚 Docs & Chores - docs: include PGP key fingerprints by [@babakks](https://github.com/babakks) in [#13112](cli/cli#13112) - docs: add sha/md5 checksums of keyring files by [@babakks](https://github.com/babakks) in [#13150](cli/cli#13150) - docs: fix SHA512 checksum for GPG key by [@timsu92](https://github.com/timsu92) in [#13157](cli/cli#13157) - docs(skill): polish skill commandset docs by [@babakks](https://github.com/babakks) in [#13183](cli/cli#13183) - Document dependency CVE policy in SECURITY.md by [@BagToad](https://github.com/BagToad) in [#13119](cli/cli#13119) - Replace github.com/golang/snappy with klauspost/compress/snappy by [@thaJeztah](https://github.com/thaJeztah) in [#13048](cli/cli#13048) - chore: bump to go1.26.2 by [@babakks](https://github.com/babakks) in [#13116](cli/cli#13116) - chore: delete experimental script/debian-devel by [@babakks](https://github.com/babakks) in [#13127](cli/cli#13127) - Suggest first party extensions by [@williammartin](https://github.com/williammartin) in [#13182](cli/cli#13182) - Add cli/skill-reviewers as CODEOWNERS for skills packages by [@BagToad](https://github.com/BagToad) in [#13189](cli/cli#13189) - Add [@cli/code-reviewers](https://github.com/cli/code-reviewers) to all CODEOWNERS rules by [@BagToad](https://github.com/BagToad) in [#13190](cli/cli#13190) - Address post-merge review feedback for skills commands by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13185](cli/cli#13185) - Fix skills-publish-dry-run acceptance test error message mismatch by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13187](cli/cli#13187) - Skills: replace real git in publish tests with CommandStubber by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13188](cli/cli#13188) - Remove redundant nil-client fallback in skills publish by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13168](cli/cli#13168) - Publish: use shared discovery logic instead of requiring skills/ directory by [@SamMorrowDrums](https://github.com/SamMorrowDrums) in [#13167](cli/cli#13167) #####

Dependencies - chore(deps): bump github.com/klauspost/compress from 1.18.4 to 1.18.5 by [@dependabot](https://github.com/dependabot)\[bot] in [#13071](cli/cli#13071) - chore(deps): bump github.com/yuin/goldmark from 1.7.16 to 1.8.2 by [@dependabot](https://github.com/dependabot)\[bot] in [#13045](cli/cli#13045) - chore(deps): bump charm.land/bubbles/v2 from 2.0.0 to 2.1.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#13051](cli/cli#13051) - chore(deps): bump github.com/sigstore/timestamp-authority/v2 from 2.0.3 to 2.0.6 by [@dependabot](https://github.com/dependabot)\[bot] in [#13152](cli/cli#13152) - chore(deps): bump github.com/google/go-containerregistry from 0.21.3 to 0.21.4 by [@dependabot](https://github.com/dependabot)\[bot] in [#13129](cli/cli#13129) - chore(deps): bump github.com/sigstore/protobuf-specs from 0.5.0 to 0.5.1 by [@dependabot](https://github.com/dependabot)\[bot] in [#13128](cli/cli#13128) - chore(deps): bump github.com/in-toto/attestation from 1.1.2 to 1.2.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#13044](cli/cli#13044) - chore(deps): bump advanced-security/filter-sarif from 1.0.1 to 1.1 by [@dependabot](https://github.com/dependabot)\[bot] in [#12918](cli/cli#12918) - chore(deps): bump google.golang.org/grpc from 1.79.3 to 1.80.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#13076](cli/cli#13076) - chore(deps): bump github.com/hashicorp/go-version from 1.8.0 to 1.9.0 by [@dependabot](https://github.com/dependabot)\[bot] in [#13065](cli/cli#13065) #### New Contributors - [@thaJeztah](https://github.com/thaJeztah) made their first contribution in [#13048](cli/cli#13048) - [@Bahtya](https://github.com/Bahtya) made their first contribution in [#13097](cli/cli#13097) - [@timsu92](https://github.com/timsu92) made their first contribution in [#13157](cli/cli#13157) - [@SamMorrowDrums](https://github.com/SamMorrowDrums) made their first contribution in [#13173](cli/cli#13173) **Full Changelog**: <cli/cli@v2.89.0...v2.90.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

SamMorrowDrums requested a review from a team as a code owner April 15, 2026 20:49

SamMorrowDrums requested review from Copilot and williammartin April 15, 2026 20:49

Copilot started reviewing on behalf of SamMorrowDrums April 15, 2026 20:50 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread pkg/cmd/root/root.go

Comment thread pkg/cmd/skills/update/update.go

SamMorrowDrums changed the base branch from trunk to sm/add-skills-command April 15, 2026 21:41

babakks approved these changes Apr 16, 2026

View reviewed changes

SamMorrowDrums mentioned this pull request Apr 16, 2026

Skills: post-merge review feedback follow-ups #13184

Closed

8 tasks

SamMorrowDrums force-pushed the sammorrowdrums/cleanup-skills-publish-nil-client branch from 74034e2 to 768470d Compare April 16, 2026 13:52

SamMorrowDrums merged commit 29e55fe into sm/add-skills-command Apr 16, 2026
8 checks passed

SamMorrowDrums deleted the sammorrowdrums/cleanup-skills-publish-nil-client branch April 16, 2026 13:52

SamMorrowDrums mentioned this pull request Apr 16, 2026

Address post-merge review feedback for skills commands #13185

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove redundant nil-client fallback in skills publish#13168

Remove redundant nil-client fallback in skills publish#13168
SamMorrowDrums merged 1 commit into
sm/add-skills-commandfrom
sammorrowdrums/cleanup-skills-publish-nil-client

SamMorrowDrums commented Apr 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

babakks left a comment

Uh oh!

babakks Apr 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

SamMorrowDrums commented Apr 15, 2026

Problem

Changes

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Copilot's findings

Uh oh!

Uh oh!

Uh oh!

babakks left a comment

Choose a reason for hiding this comment

Uh oh!

babakks Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants