STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor: remove redundant nil-client fallback in skills publish (#13168)

Browse source 
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>
This commit is contained in:
Sam Morrow 2026-04-16 15:52:53 +02:00 • committed by GitHub
parent cf1afc9b88
commit 29e55fe5b9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 33 additions and 38 deletions
Download patch file Download diff file Expand all files Collapse all files

71
pkg/cmd/skills/publish/publish.go
View file

@ -178,7 +178,10 @@ func publishRun(opts *PublishOptions) error {
canPrompt := opts.IO.CanPrompt()
// Use injected client or create one from the factory HttpClient
// Use injected client or create one from the factory HttpClient.
// Initialization is deferred until after local validation so that
// simple errors (missing skills/, bad SKILL.md, etc.) are reported
// without requiring an HTTP client.
client := opts.client
host := opts.host
@ -336,52 +339,49 @@ func publishRun(opts *PublishOptions) error {
owner = repoInfo.Repo.RepoOwner()
repo = repoInfo.Repo.RepoName()
}
hasTopic := false
var existingTags []tagEntry
if owner != "" && repo != "" {
// Create API client from factory if not already injected (tests inject directly).
if client == nil {
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
client = api.NewClientFromHTTP(httpClient)
}
if host == "" && repoInfo != nil {
host = repoInfo.Repo.RepoHost()
}
if host != "" {
if err := source.ValidateSupportedHost(host); err != nil {
if host == "" {
cfg, err := opts.Config()
if err != nil {
return err
}
host, _ = cfg.Authentication().DefaultHost()
}
if err := source.ValidateSupportedHost(host); err != nil {
return err
}
// Create API client for remote checks if not already injected
if client == nil {
httpClient, httpErr := opts.HttpClient()
if httpErr == nil {
apiClient := api.NewClientFromHTTP(httpClient)
cfg, cfgErr := opts.Config()
if cfgErr == nil {
host, _ = cfg.Authentication().DefaultHost()
if err := source.ValidateSupportedHost(host); err != nil {
return err
}
client = apiClient
}
}
// Security and ruleset checks (advisory, always shown)
var skillAbsDirs []string
for _, skill := range skills {
skillAbsDirs = append(skillAbsDirs, filepath.Join(dir, filepath.FromSlash(skill.Path)))
}
securityDiags := checkSecuritySettings(client, host, owner, repo, skillAbsDirs)
diagnostics = append(diagnostics, securityDiags...)
if client != nil {
// Security and ruleset checks (advisory, always shown)
var skillAbsDirs []string
for _, skill := range skills {
skillAbsDirs = append(skillAbsDirs, filepath.Join(dir, filepath.FromSlash(skill.Path)))
}
securityDiags := checkSecuritySettings(client, host, owner, repo, skillAbsDirs)
diagnostics = append(diagnostics, securityDiags...)
rulesetDiags := checkTagProtection(client, host, owner, repo)
diagnostics = append(diagnostics, rulesetDiags...)
rulesetDiags := checkTagProtection(client, host, owner, repo)
diagnostics = append(diagnostics, rulesetDiags...)
// Check topic (needed for publish flow, not a blocking error)
hasTopic = repoHasTopic(client, host, owner, repo)
// Check topic (needed for publish flow, not a blocking error)
hasTopic = repoHasTopic(client, host, owner, repo)
// Fetch existing tags (needed for version suggestion)
existingTags = fetchTags(client, host, owner, repo)
}
// Fetch existing tags (needed for version suggestion)
existingTags = fetchTags(client, host, owner, repo)
} else {
diagnostics = append(diagnostics, detectMissingRepoDiagnostic(opts.GitClient, dir)...)
}
@ -425,11 +425,6 @@ func publishRun(opts *PublishOptions) error {
return nil
}
if client == nil {
fmt.Fprintf(opts.IO.ErrOut, "\nValidation passed but could not create API client. Check your authentication configuration.\n")
return nil
}
fmt.Fprintf(opts.IO.ErrOut, "\nPublishing to %s/%s...\n\n", owner, repo)
return runPublishRelease(opts, client, host, owner, repo, dir, repoInfo.RemoteName, hasTopic, existingTags)