From a912262d6e5e495e85f5cbf4e5b028301a483220 Mon Sep 17 00:00:00 2001 From: a3chron Date: Wed, 26 Aug 2026 14:49:28 +0200 Subject: [PATCH 1/8] style: gofmt struct field alignment Whitespace only, no behavioural change. These two files were left unformatted by earlier commits and `gofmt -l` flagged them. Co-Authored-By: Claude Opus 5 (1M context) --- internal/config/config.go | 4 ++-- internal/testutil/testutil.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 76134a7..2007685 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -9,8 +9,8 @@ import ( ) type Config struct { - CurrentTheme string `json:"current_theme"` // "alice/rainbow@1.2" - CurrentPath string `json:"current_path"` // Full path to .toml + CurrentTheme string `json:"current_theme"` // "alice/rainbow@1.2" + CurrentPath string `json:"current_path"` // Full path to .toml PreviousTheme string `json:"previous_theme,omitempty"` PreviousPath string `json:"previous_path,omitempty"` DownloadedThemes []string `json:"downloaded_themes,omitempty"` // ["alice/rainbow", "bob/sunset"] diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go index 3da23b6..6976660 100644 --- a/internal/testutil/testutil.go +++ b/internal/testutil/testutil.go @@ -16,11 +16,11 @@ import ( // TestEnv holds the test environment configuration type TestEnv struct { - t *testing.T - RootDir string // Root temp directory - StellarDir string // ~/.config/stellar equivalent + t *testing.T + RootDir string // Root temp directory + StellarDir string // ~/.config/stellar equivalent StarshipPath string // ~/.config/starship.toml equivalent - TmpDir string // /tmp/stellar equivalent + TmpDir string // /tmp/stellar equivalent // Original env values for restoration origEnv map[string]string From b051cf6e6e4f4ba288294245e499e62a1129713e Mon Sep 17 00:00:00 2001 From: a3chron Date: Wed, 26 Aug 2026 14:49:28 +0200 Subject: [PATCH 2/8] feat(completion): add shell tab-completion for theme identifiers Completes `author/slug@version` in three stages (author, slug, version) for apply, preview, info and remove, sourced from the local theme cache so TAB is instant and works offline. Set STELLAR_COMPLETION_ONLINE=1 to additionally query the hub, via a 2s-timeout client that degrades silently to local-only results when the hub is slow or unreachable. Supporting changes: - internal/cache: ListAuthors, ListAuthorThemes and ListThemeVersions, all returning (nil, nil) for a missing directory so completion never fails on a cache that doesn't exist yet. - internal/api: NewCompletionClient (2s instead of 30s) and SearchThemesByAuthorName over GET /api/themes?authorName=. - internal/theme: IsValidSegment and IsValidVersion, derived from the same character class and grammar as ParseIdentifier, so a name that apply would reject is never suggested. Every candidate is gated on them, local ones included: the cache is a plain directory that a synced dotfiles checkout can put anything in, and candidates are printed straight into the user's terminal. - Hub candidates must prefix-match the typed word case-sensitively. bash's compgen and zsh's compadd filter that way themselves, so emitting the hub's canonical casing worked in fish and PowerShell only. - No-argument commands get cobra.NoFileCompletions; without a ValidArgsFunction cobra returns ShellCompDirectiveDefault and the shell offers the user's filenames after e.g. `stellar list`. - The root PersistentPreRunE is skipped for completion requests. It ran a directory cleanup and MkdirAll on every keystroke, and an error there suppresses the directive line entirely - which every shell reads as "offer filenames", silently breaking completion on a read-only HOME. - internal/testutil: mock GET /api/themes, per-path request counters, a distinct opaque author id (the hub's id and name are unrelated, and routes resolve authors by name), limit/offset handling, and a snapshot-under-mutex fix for a data race in the theme handlers. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 22 ++ cmd/completion_args.go | 85 +++++ cmd/completion_test.go | 513 ++++++++++++++++++++++++++++++ cmd/root.go | 21 ++ internal/api/client.go | 69 +++- internal/api/client_test.go | 48 +++ internal/cache/manager.go | 96 ++++++ internal/cache/manager_test.go | 60 ++++ internal/completion/completion.go | 333 +++++++++++++++++++ internal/testutil/mockapi.go | 188 ++++++++++- internal/theme/parser.go | 39 ++- internal/theme/parser_test.go | 110 +++---- 12 files changed, 1509 insertions(+), 75 deletions(-) create mode 100644 cmd/completion_args.go create mode 100644 cmd/completion_test.go create mode 100644 internal/completion/completion.go diff --git a/README.md b/README.md index e0a98d2..362c86a 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,28 @@ Some [basic usage](#basic-usage) covered here, for more info, run `stellar --hel > > You can force copy mode anywhere (e.g. for testing) by setting `STELLAR_APPLY_MODE=copy`. +### Shell completion + +stellar ships tab completion for commands, flags, and theme identifiers +(`author/slug@version`). Candidates come from your local theme cache, so +completion is always instant and works offline. If you want hub themes +suggested too, set `STELLAR_COMPLETION_ONLINE=1` (adds up to ~2s of network +lookup per completion; degrades silently to local-only when offline). + +```bash +# bash +stellar completion bash > ~/.local/share/bash-completion/completions/stellar + +# zsh +stellar completion zsh > "${fpath[1]}/_stellar" + +# fish +stellar completion fish > ~/.config/fish/completions/stellar.fish + +# powershell (add to $PROFILE) +stellar completion powershell | Out-String | Invoke-Expression +``` + ## Why use **Before:** Getting good starship configs so far was mostly random, from someones github dotfiles, searching for something entirely else... diff --git a/cmd/completion_args.go b/cmd/completion_args.go new file mode 100644 index 0000000..0774406 --- /dev/null +++ b/cmd/completion_args.go @@ -0,0 +1,85 @@ +package cmd + +import ( + "os" + "strings" + + "github.com/a3chron/stellar/internal/completion" + "github.com/spf13/cobra" +) + +// init wires up shell tab-completion for the commands that take theme +// identifiers ("author/slug@version"). It lives in its own file rather than +// touching apply.go/preview.go/info.go/remove.go directly so those files' +// existing structure stays untouched. +func init() { + applyCmd.ValidArgsFunction = themeIdentifierArgs + previewCmd.ValidArgsFunction = themeIdentifierArgs + infoCmd.ValidArgsFunction = themeIdentifierArgs + removeCmd.ValidArgsFunction = removeIdentifierArgs + + // Commands that take no arguments still need this: with no + // ValidArgsFunction cobra returns ShellCompDirectiveDefault and the shell + // falls back to offering filenames, so `stellar list ` would list the + // user's working directory. + for _, c := range []*cobra.Command{ + listCmd, cleanCmd, currentCmd, rollbackCmd, updateCmd, versionCmd, + } { + c.ValidArgsFunction = cobra.NoFileCompletions + } +} + +// themeCompletionMode returns the candidate sources for apply/preview/info +// completion. Local-only by default: even a 2s hub round trip makes TAB feel +// broken, and most users complete themes they already have cached. Setting +// STELLAR_COMPLETION_ONLINE=1 opts in to hub suggestions. +// +// Read per invocation (not in init) because every shell completion request +// is its own process and tests toggle the variable at runtime. +func themeCompletionMode() completion.Mode { + if os.Getenv(completion.EnvOnline) == "1" || os.Getenv(completion.EnvOnline) == "true" { + return completion.LocalAndRemote + } + return completion.LocalOnly +} + +// themeIdentifierArgs is the ValidArgsFunction for commands that accept +// exactly one identifier (apply, preview, info): once that argument is +// already typed, there's nothing left to complete. +func themeIdentifierArgs(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + if len(args) > 0 { + return nil, cobra.ShellCompDirectiveNoFileComp + } + return completion.ThemeIdentifier(toComplete, themeCompletionMode()) +} + +// removeIdentifierArgs completes "stellar remove" arguments from the local +// cache only - matching remove's own semantics, it never touches the +// network - and, since remove accepts several identifiers, filters out any +// candidate that's already been typed on the command line so repeated +// completion doesn't keep re-suggesting the same theme. +func removeIdentifierArgs(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + candidates, directive := completion.ThemeIdentifier(toComplete, completion.LocalOnly) + if len(candidates) == 0 || len(args) == 0 { + return candidates, directive + } + + already := make(map[string]bool, len(args)) + for _, a := range args { + already[a] = true + } + + filtered := make([]string, 0, len(candidates)) + for _, c := range candidates { + value := c + if idx := strings.IndexByte(c, '\t'); idx != -1 { + value = c[:idx] + } + if already[value] { + continue + } + filtered = append(filtered, c) + } + + return filtered, directive +} diff --git a/cmd/completion_test.go b/cmd/completion_test.go new file mode 100644 index 0000000..c53d67c --- /dev/null +++ b/cmd/completion_test.go @@ -0,0 +1,513 @@ +// Package cmd contains E2E tests for stellar CLI commands, including shell +// tab-completion (see internal/completion for the implementation these +// tests exercise end-to-end via the hidden cobra "__complete" command). +package cmd + +import ( + "bytes" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/a3chron/stellar/internal/completion" + "github.com/a3chron/stellar/internal/testutil" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// enableOnlineCompletion opts the test in to hub-backed completion for +// apply/preview/info - the default is local-only for speed. +func enableOnlineCompletion(t *testing.T) { + t.Helper() + t.Setenv(completion.EnvOnline, "1") +} + +// runComplete invokes the hidden "__complete" command with args and returns +// the output split into lines: zero or more candidate lines, followed by a +// trailing ":" line (see cobra's completions.go - the directive +// integer is always the last line, following a single colon). +func runComplete(t *testing.T, args ...string) []string { + t.Helper() + + cmd := NewRootCmd() + var out, errOut bytes.Buffer + cmd.SetOut(&out) + cmd.SetErr(&errOut) + cmd.SetArgs(append([]string{cobra.ShellCompRequestCmd}, args...)) + + err := cmd.Execute() + require.NoError(t, err, "stderr: %s", errOut.String()) + + trimmed := strings.TrimRight(out.String(), "\n") + if trimmed == "" { + return nil + } + return strings.Split(trimmed, "\n") +} + +// directiveLine returns the last line of a runComplete result (the +// ":" line). +func directiveLine(lines []string) string { + if len(lines) == 0 { + return "" + } + return lines[len(lines)-1] +} + +// candidateLines returns every line except the trailing directive line. +func candidateLines(lines []string) []string { + if len(lines) == 0 { + return nil + } + return lines[:len(lines)-1] +} + +func TestCompletion_EmptyInput_LocalAuthorsOnly(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("local", "mytheme", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("testuser", "sample-theme", "1.2", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "") + + assert.Equal(t, ":6", directiveLine(lines)) + assert.Equal(t, []string{"local/\tlocal", "testuser/\tlocal"}, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_LocalAuthorPrefix_SuppressesRemote(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("local", "mytheme", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("testuser", "sample-theme", "1.2", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "lo") + + assert.Equal(t, []string{"local/\tlocal"}, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_UnknownAuthorPrefix_FallsBackToHub(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("local", "mytheme", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "other") + + assert.Equal(t, []string{"otheruser/\thub"}, candidateLines(lines)) + assert.GreaterOrEqual(t, mockAPI.Requests("/api/themes"), 1) +} + +func TestCompletion_AuthorSlash_LocalThenRemoteDeduped(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + // "sample-theme" exists both locally and on the hub (should be deduped, + // kept as the local entry); "local-only" only exists locally; + // "custom-theme" only exists on the hub. + env.CreateThemeFile("testuser", "sample-theme", "1.2", testutil.SampleTOML()) + env.CreateThemeFile("testuser", "local-only", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "testuser/") + + assert.Equal(t, ":36", directiveLine(lines)) + assert.Equal(t, []string{ + "testuser/local-only\tlocal", + "testuser/sample-theme\tlocal", + "testuser/custom-theme\thub", + }, candidateLines(lines)) + assert.GreaterOrEqual(t, mockAPI.Requests("/api/themes"), 1) +} + +func TestCompletion_RemoteOnlyAuthor_Slash(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + lines := runComplete(t, "apply", "otheruser/") + + assert.Equal(t, []string{"otheruser/ocean-theme\thub"}, candidateLines(lines)) +} + +func TestCompletion_VersionStage_LocalThenRemoteThenLatest(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + // Only 1.0 is cached locally; the hub (via CreateDefaultMockAPI) has + // 1.2, 1.1 and 1.0 for testuser/sample-theme. + env.CreateThemeFile("testuser", "sample-theme", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "testuser/sample-theme@") + + assert.Equal(t, ":36", directiveLine(lines)) + assert.Equal(t, []string{ + "testuser/sample-theme@1.0\tlocal", + "testuser/sample-theme@1.2\thub", + "testuser/sample-theme@1.1\thub", + "testuser/sample-theme@latest", + }, candidateLines(lines)) +} + +func TestCompletion_Offline_DegradesToLocalOnly(t *testing.T) { + testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + // Nothing is listening on this port: connection should fail fast rather + // than hang for the full 2s completion-client timeout. + t.Setenv("STELLAR_API_URL", "http://127.0.0.1:1") + + lines := runComplete(t, "apply", "anything") + + assert.Empty(t, candidateLines(lines)) + assert.Equal(t, ":6", directiveLine(lines)) +} + +func TestCompletion_Remove_LocalOnly_NeverHitsAPI(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("testuser", "sample-theme", "1.2", testutil.SampleTOML()) + + lines := runComplete(t, "remove", "testuser/") + + assert.Equal(t, []string{"testuser/sample-theme\tlocal"}, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_Remove_ExcludesAlreadyTypedArgs(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + env.CreateThemeFile("testuser", "sample-theme", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("testuser", "other-theme", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "remove", "testuser/sample-theme", "testuser/") + + assert.Equal(t, []string{"testuser/other-theme\tlocal"}, candidateLines(lines)) +} + +func TestCompletion_MissingStellarDir_NoErrorEmptyOutput(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + require.NoError(t, os.RemoveAll(env.StellarDir)) + + lines := runComplete(t, "apply", "") + + assert.Empty(t, candidateLines(lines)) +} + +func TestCompletion_BackupTheme_SkipsRemoteLookup(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("someauthor", "backup", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "someauthor/backup@") + + assert.Contains(t, candidateLines(lines), "someauthor/backup@1.0\tlocal") + assert.Equal(t, 0, mockAPI.Requests("/api/someauthor/backup")) +} + +func TestCompletion_EmptyAuthorSegment_NoCandidates(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + // "/x" would otherwise list author dirs as slugs of an empty author and + // fire an unfiltered hub query - both must be suppressed. + env.CreateThemeFile("xylo", "mytheme", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "/x") + + assert.Empty(t, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_EmptySlugSegment_NoCandidates(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + + // "alice/@latest" would not parse, so "alice/@" must complete to nothing. + lines := runComplete(t, "apply", "alice/@") + + assert.Empty(t, candidateLines(lines)) +} + +func TestCompletion_VersionStage_VPrefix(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("testuser", "sample-theme", "1.0", testutil.SampleTOML()) + + // The parser accepts "@v1.0", so "@v" should offer v-prefixed versions + // (and never a nonsensical "vlatest"). + lines := runComplete(t, "apply", "testuser/sample-theme@v") + + assert.Equal(t, []string{ + "testuser/sample-theme@v1.0\tlocal", + "testuser/sample-theme@v1.2\thub", + "testuser/sample-theme@v1.1\thub", + }, candidateLines(lines)) +} + +func TestCompletion_HubCanonicalAuthorCasing(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + mockAPI.AddTheme(testutil.MockTheme{ + ID: "cased-id", + Author: "CasedUser", + Slug: "neon-theme", + Name: "Neon Theme", + Versions: []testutil.MockVersion{ + {Version: "1.0", ConfigContent: testutil.SampleTOML(), CreatedAt: "2024-03-01T00:00:00Z"}, + }, + }) + env.SetupMockAPI(mockAPI) + + // The hub's /api/{author}/{slug} routes match author names exactly, and + // every shell filters candidates against the typed word (bash's compgen + // and zsh's compadd case-sensitively). So the hub's canonical casing + // completes... + lines := runComplete(t, "apply", "CasedUser/") + assert.Equal(t, []string{"CasedUser/neon-theme\thub"}, candidateLines(lines)) + + // ...while a differently-cased prefix suggests nothing, rather than + // emitting a candidate that bash and zsh would silently discard. + lines = runComplete(t, "apply", "caseduser/") + assert.Empty(t, candidateLines(lines)) +} + +func TestCompletion_Default_UnknownAuthor_LocalOnlyNoNetwork(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + // Without STELLAR_COMPLETION_ONLINE, an unknown author prefix must NOT + // fall back to the hub - completion stays instant and offline. + lines := runComplete(t, "apply", "other") + + assert.Empty(t, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_Default_SlugStage_LocalOnlyNoNetwork(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("testuser", "sample-theme", "1.2", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "testuser/") + + assert.Equal(t, []string{"testuser/sample-theme\tlocal"}, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_Default_VersionStage_LocalPlusLatestNoNetwork(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + mockAPI := testutil.CreateDefaultMockAPI() + env.SetupMockAPI(mockAPI) + + env.CreateThemeFile("testuser", "sample-theme", "1.0", testutil.SampleTOML()) + + lines := runComplete(t, "apply", "testuser/sample-theme@") + + assert.Equal(t, []string{ + "testuser/sample-theme@1.0\tlocal", + "testuser/sample-theme@latest", + }, candidateLines(lines)) + assert.Equal(t, 0, mockAPI.TotalRequests()) +} + +func TestCompletion_HostileHubValues_Filtered(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + mockAPI := testutil.CreateDefaultMockAPI() + // Author and slug outside the identifier character class (ANSI escape, + // colon, space) must never reach the user's terminal as candidates. + mockAPI.AddTheme(testutil.MockTheme{ + ID: "evil-id", + Author: "evil\x1b[31muser", + Slug: "bad:theme name", + Name: "Evil", + Versions: []testutil.MockVersion{ + {Version: "1.0", ConfigContent: testutil.SampleTOML(), CreatedAt: "2024-03-01T00:00:00Z"}, + }, + }) + env.SetupMockAPI(mockAPI) + + lines := runComplete(t, "apply", "evil") + + assert.Empty(t, candidateLines(lines)) + // Guard against a vacuous pass: the hub must actually have been queried, + // proving the empty result came from filtering, not from a dead network path. + assert.GreaterOrEqual(t, mockAPI.Requests("/api/themes"), 1) +} + +func TestCompletion_MalformedLocalCacheEntries_Filtered(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + env.CreateThemeFile("good", "mytheme", "1.0", testutil.SampleTOML()) + // The cache is a plain directory a synced dotfiles checkout or an + // extracted tarball can write to, so it gets the same treatment as an + // untrusted hub response: names outside the identifier character class + // must never be suggested - an ANSI escape especially, since candidates + // are printed straight into the user's terminal. + for _, name := range []string{".git", "my author", "evil\x1b[31muser"} { + require.NoError(t, os.MkdirAll(filepath.Join(env.StellarDir, name), 0o755)) + } + + lines := runComplete(t, "apply", "") + + assert.Equal(t, []string{"good/\tlocal"}, candidateLines(lines)) +} + +func TestCompletion_MalformedLocalSlugsAndVersions_Filtered(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + require.NoError(t, os.MkdirAll(filepath.Join(env.StellarDir, "alice", "bad slug"), 0o755)) + // A theme directory can hold any *.toml name; only versions apply can + // actually parse ("1.2", "latest") may be suggested. + themeDir := filepath.Join(env.StellarDir, "alice", "rainbow") + for _, name := range []string{"1.0.1.toml", "notes.toml", "latest.toml"} { + require.NoError(t, os.WriteFile(filepath.Join(themeDir, name), []byte(testutil.SampleTOML()), 0o644)) + } + + lines := runComplete(t, "apply", "alice/") + assert.Equal(t, []string{"alice/rainbow\tlocal"}, candidateLines(lines)) + + lines = runComplete(t, "apply", "alice/rainbow@") + assert.Equal(t, []string{"alice/rainbow@1.0\tlocal", "alice/rainbow@latest\tlocal"}, candidateLines(lines)) +} + +func TestCompletion_NoArgCommands_NoFileCompletion(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + + // Without a ValidArgsFunction these would return ShellCompDirectiveDefault + // (":0") and the shell would offer the user's filenames instead. + // ("version" is wired up the same way but registered on the package-level + // rootCmd rather than in NewRootCmd, so it isn't reachable from here.) + for _, name := range []string{"list", "clean", "current", "rollback", "update"} { + lines := runComplete(t, name, "") + assert.Equal(t, ":4", directiveLine(lines), "command %q should suppress file completion", name) + assert.Empty(t, candidateLines(lines), "command %q should offer no candidates", name) + } +} + +func TestCompletion_HangingHub_BoundedByTimeout(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + enableOnlineCompletion(t) + + // The 2s cap in api.NewCompletionClient is the property the whole feature + // rests on: TAB must never hang on a slow hub. A refused connection (see + // TestCompletion_Offline_DegradesToLocalOnly) fails instantly and so + // doesn't exercise it - this server accepts and then never answers. + released := make(chan struct{}) + server := httptest.NewServer( + http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { + <-released + }), + ) + // Cleanups run LIFO, so this releases the blocked handler *before* + // server.Close() starts waiting for it - registering them the other way + // round deadlocks the test. + t.Cleanup(server.Close) + t.Cleanup(func() { close(released) }) + t.Setenv("STELLAR_API_URL", server.URL) + + env.CreateThemeFile("local", "mytheme", "1.0", testutil.SampleTOML()) + + start := time.Now() + lines := runComplete(t, "apply", "unknown-author") + elapsed := time.Since(start) + + assert.Less(t, elapsed, 10*time.Second, "completion must not wait on a hanging hub") + // Local candidates survive the failed lookup, and nothing leaks onto + // stdout - stray output there corrupts what the shell parses. + assert.Empty(t, candidateLines(lines)) + assert.Equal(t, ":6", directiveLine(lines)) +} + +func TestCompletion_AllIdentifierCommands_CompleteThemes(t *testing.T) { + env := testutil.SetupTestEnv(t) + resetFlags() + + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + + // apply, preview and info all take one identifier; remove takes several. + // Each needs its own ValidArgsFunction, and only apply/remove were + // covered before. + for _, name := range []string{"apply", "preview", "info", "remove"} { + lines := runComplete(t, name, "ali") + assert.Equal(t, []string{"alice/\tlocal"}, candidateLines(lines), + "command %q should complete theme identifiers", name) + } +} diff --git a/cmd/root.go b/cmd/root.go index 4d821db..6728b18 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -19,6 +19,14 @@ func NewRootCmd() *cobra.Command { Short: "Starship theme manager", Long: `Stellar - Discover, preview, and apply Starship themes from the community`, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + // Shell completion runs this on every keystroke, so it must not + // touch the filesystem - and must not fail: an error here prints + // no directive line at all, which every shell reads as "offer + // filenames", silently breaking completion (e.g. on a read-only + // HOME) rather than degrading to no candidates. + if isCompletionRequest(cmd) { + return nil + } // Best-effort cleanup of files left behind by a previous self-update cleanupUpdateLeftovers() // Initialize stellar directory structure before any command runs @@ -42,6 +50,19 @@ func NewRootCmd() *cobra.Command { return cmd } +// isCompletionRequest reports whether cmd is (or sits under) one of cobra's +// hidden completion-request commands, i.e. whether this process was spawned by +// the user pressing TAB rather than running stellar themselves. +func isCompletionRequest(cmd *cobra.Command) bool { + for c := cmd; c != nil; c = c.Parent() { + if c.Name() == cobra.ShellCompRequestCmd || + c.Name() == cobra.ShellCompNoDescRequestCmd { + return true + } + } + return false +} + func Execute() error { return rootCmd.Execute() } diff --git a/internal/api/client.go b/internal/api/client.go index 2048a90..b69f724 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "net/url" "time" "github.com/a3chron/stellar/internal/paths" @@ -17,23 +18,35 @@ type Client struct { httpClient *http.Client } -func NewClient() *Client { +// newClient builds a Client pointed at baseURL with the given request +// timeout. It's the single place that assembles the http.Client so +// NewClient, NewClientWithURL and NewCompletionClient can't drift apart on +// anything but the two knobs that actually differ between them. +func newClient(baseURL string, timeout time.Duration) *Client { return &Client{ - baseURL: paths.APIURL(BaseURL), + baseURL: baseURL, httpClient: &http.Client{ - Timeout: 30 * time.Second, + Timeout: timeout, }, } } +func NewClient() *Client { + return newClient(paths.APIURL(BaseURL), 30*time.Second) +} + // NewClientWithURL creates a client with a specific base URL (for testing) func NewClientWithURL(baseURL string) *Client { - return &Client{ - baseURL: baseURL, - httpClient: &http.Client{ - Timeout: 30 * time.Second, - }, - } + return newClient(baseURL, 30*time.Second) +} + +// NewCompletionClient creates a client tuned for shell completion requests. +// Shell completion runs synchronously on every keystroke in the user's +// shell, so it must never block waiting on a slow or unreachable +// stellar-hub: callers are expected to treat any error from this client as +// "degrade to local-only completions" rather than surfacing it. +func NewCompletionClient() *Client { + return newClient(paths.APIURL(BaseURL), 2*time.Second) } // Author info nested in theme response @@ -67,6 +80,16 @@ type VersionInfo struct { CreatedAt string `json:"createdAt"` } +// ThemeSummary is the lightweight theme shape returned by GET /api/themes, +// used by shell completion (internal/completion) to look up hub authors and +// theme slugs without downloading a full ThemeInfo per candidate. +type ThemeSummary struct { + Author AuthorInfo `json:"author"` + Name string `json:"name"` + Slug string `json:"slug"` + LatestVersion string `json:"latestVersion"` +} + func (c *Client) FetchThemeConfig(author, name, version string) (string, error) { url := fmt.Sprintf("%s/api/%s/%s/%s", c.baseURL, author, name, version) @@ -116,6 +139,34 @@ func (c *Client) GetThemeInfo(author, name string) (*ThemeInfo, error) { return &info, nil } +// SearchThemesByAuthorName queries GET /api/themes for themes whose author +// name matches authorName as a prefix (server-side match), used by shell +// completion to suggest hub authors/themes that aren't in the local cache. +func (c *Client) SearchThemesByAuthorName(authorName string) ([]ThemeSummary, error) { + reqURL := fmt.Sprintf("%s/api/themes?authorName=%s&limit=100&sort=name", c.baseURL, url.QueryEscape(authorName)) + + resp, err := c.httpClient.Get(reqURL) + if err != nil { + return nil, fmt.Errorf("failed to search themes: %w", err) + } + defer func() { + _ = resp.Body.Close() + }() + + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("server returned %d", resp.StatusCode) + } + + var result struct { + Themes []ThemeSummary `json:"themes"` + } + if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { + return nil, err + } + + return result.Themes, nil +} + func (c *Client) IncrementDownloadCount(author, name string) error { url := fmt.Sprintf("%s/api/%s/%s", c.baseURL, author, name) diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 2849dd9..9bd0b23 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -3,7 +3,9 @@ package api import ( "net/http" "net/http/httptest" + "net/url" "testing" + "time" "github.com/a3chron/stellar/internal/testutil" "github.com/stretchr/testify/assert" @@ -184,6 +186,52 @@ func TestClient_StatusCodes(t *testing.T) { } } +func TestNewCompletionClient(t *testing.T) { + client := NewCompletionClient() + assert.Equal(t, BaseURL, client.baseURL) + assert.Equal(t, 2*time.Second, client.httpClient.Timeout) +} + +func TestClient_SearchThemesByAuthorName(t *testing.T) { + var gotQuery url.Values + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotQuery = r.URL.Query() + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"themes":[ + {"author":{"id":"a1","name":"alice","image":null},"name":"Rainbow","slug":"rainbow","latestVersion":"1.2"}, + {"author":{"id":"a1","name":"alice","image":null},"name":"Sunset","slug":"sunset","latestVersion":"2.0"} + ]}`)) + })) + defer server.Close() + + client := NewClientWithURL(server.URL) + + summaries, err := client.SearchThemesByAuthorName("ali") + require.NoError(t, err) + require.Len(t, summaries, 2) + + assert.Equal(t, "ali", gotQuery.Get("authorName")) + assert.Equal(t, "100", gotQuery.Get("limit")) + assert.Equal(t, "name", gotQuery.Get("sort")) + + assert.Equal(t, "alice", summaries[0].Author.Name) + assert.Equal(t, "rainbow", summaries[0].Slug) + assert.Equal(t, "1.2", summaries[0].LatestVersion) + assert.Equal(t, "sunset", summaries[1].Slug) +} + +func TestClient_SearchThemesByAuthorName_NonOK(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + client := NewClientWithURL(server.URL) + + _, err := client.SearchThemesByAuthorName("ali") + assert.Error(t, err) +} + func TestClientWithEnvOverride(t *testing.T) { env := testutil.SetupTestEnv(t) mockAPI := testutil.CreateDefaultMockAPI() diff --git a/internal/cache/manager.go b/internal/cache/manager.go index c44c1fc..c671963 100644 --- a/internal/cache/manager.go +++ b/internal/cache/manager.go @@ -5,6 +5,7 @@ import ( "log" "os" "path/filepath" + "sort" "strings" "github.com/a3chron/stellar/internal/paths" @@ -164,6 +165,101 @@ func CleanCache(excludeCurrentPath string) error { return nil } +// ListAuthors returns the names of author directories under the stellar +// cache (e.g. ["alice", "bob"]), sorted, skipping non-directories and +// config.json. Returns (nil, nil) if the stellar home directory doesn't +// exist yet: shell completion (internal/completion) calls this on every +// keystroke and cobra doesn't guarantee the directory has been created by +// the time "__complete" runs, so a missing cache must read as "no authors" +// rather than an error. +func ListAuthors() ([]string, error) { + cacheDir, err := paths.StellarHome() + if err != nil { + return nil, err + } + + entries, err := os.ReadDir(cacheDir) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + + var authors []string + for _, e := range entries { + if !e.IsDir() || e.Name() == "config.json" { + continue + } + authors = append(authors, e.Name()) + } + + sort.Strings(authors) + return authors, nil +} + +// ListAuthorThemes returns the theme slug directories cached under author +// (e.g. ["rainbow", "sunset"]), sorted. Returns (nil, nil) if the author +// directory doesn't exist (see ListAuthors for why that's not an error). +func ListAuthorThemes(author string) ([]string, error) { + cacheDir, err := paths.StellarHome() + if err != nil { + return nil, err + } + + entries, err := os.ReadDir(filepath.Join(cacheDir, author)) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + + var slugs []string + for _, e := range entries { + if !e.IsDir() { + continue + } + slugs = append(slugs, e.Name()) + } + + sort.Strings(slugs) + return slugs, nil +} + +// ListThemeVersions returns the cached version strings for author/name, +// newest first (theme.CompareSemver, descending), e.g. ["1.2", "1.1", "1.0"]. +// Returns (nil, nil) if the theme directory doesn't exist (see ListAuthors +// for why that's not an error). +func ListThemeVersions(author, name string) ([]string, error) { + cacheDir, err := paths.StellarHome() + if err != nil { + return nil, err + } + + entries, err := os.ReadDir(filepath.Join(cacheDir, author, name)) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + + var versions []string + for _, e := range entries { + if e.IsDir() || filepath.Ext(e.Name()) != ".toml" { + continue + } + versions = append(versions, strings.TrimSuffix(e.Name(), ".toml")) + } + + sort.Slice(versions, func(i, j int) bool { + return theme.CompareSemver(versions[i], versions[j]) > 0 + }) + + return versions, nil +} + func TmpCachePath(t *theme.Theme) string { return paths.TmpThemePath(t.Author, t.Name, t.Version) } diff --git a/internal/cache/manager_test.go b/internal/cache/manager_test.go index e56ac7f..0e8b839 100644 --- a/internal/cache/manager_test.go +++ b/internal/cache/manager_test.go @@ -1,6 +1,7 @@ package cache import ( + "os" "path/filepath" "testing" @@ -73,6 +74,65 @@ func TestListCachedThemes_IgnoresConfigJSON(t *testing.T) { assert.Equal(t, "alice/rainbow@1.0", themes[0]) } +func TestListAuthors(t *testing.T) { + env := testutil.SetupTestEnv(t) + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("bob", "sunset", "2.0", testutil.SampleTOML()) + env.CreateConfig(`{"current_theme": ""}`) + + authors, err := ListAuthors() + require.NoError(t, err) + + assert.Equal(t, []string{"alice", "bob"}, authors) +} + +func TestListAuthors_MissingDir(t *testing.T) { + env := testutil.SetupTestEnv(t) + require.NoError(t, os.RemoveAll(env.StellarDir)) + + authors, err := ListAuthors() + require.NoError(t, err) + assert.Nil(t, authors) +} + +func TestListAuthorThemes(t *testing.T) { + env := testutil.SetupTestEnv(t) + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("alice", "sunset", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("bob", "other", "1.0", testutil.SampleTOML()) + + themes, err := ListAuthorThemes("alice") + require.NoError(t, err) + assert.Equal(t, []string{"rainbow", "sunset"}, themes) +} + +func TestListAuthorThemes_MissingAuthor(t *testing.T) { + testutil.SetupTestEnv(t) + + themes, err := ListAuthorThemes("nobody") + require.NoError(t, err) + assert.Nil(t, themes) +} + +func TestListThemeVersions(t *testing.T) { + env := testutil.SetupTestEnv(t) + env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) + env.CreateThemeFile("alice", "rainbow", "1.5", testutil.SampleTOML()) + env.CreateThemeFile("alice", "rainbow", "1.10", testutil.SampleTOML()) + + versions, err := ListThemeVersions("alice", "rainbow") + require.NoError(t, err) + assert.Equal(t, []string{"1.10", "1.5", "1.0"}, versions) +} + +func TestListThemeVersions_MissingTheme(t *testing.T) { + testutil.SetupTestEnv(t) + + versions, err := ListThemeVersions("alice", "nonexistent") + require.NoError(t, err) + assert.Nil(t, versions) +} + func TestTmpCacheFunctions(t *testing.T) { env := testutil.SetupTestEnv(t) diff --git a/internal/completion/completion.go b/internal/completion/completion.go new file mode 100644 index 0000000..c1e4bed --- /dev/null +++ b/internal/completion/completion.go @@ -0,0 +1,333 @@ +// Package completion implements shell tab-completion for stellar theme +// identifiers ("author/slug@version"), shared by the commands that accept +// one (apply, preview, info) or several (remove) of them. +package completion + +import ( + "fmt" + "strings" + + "github.com/a3chron/stellar/internal/api" + "github.com/a3chron/stellar/internal/cache" + "github.com/a3chron/stellar/internal/theme" + "github.com/spf13/cobra" +) + +// Mode controls whether ThemeIdentifier is allowed to reach out to the +// stellar-hub API in addition to the local cache. +type Mode int + +const ( + // LocalOnly restricts completion to the local cache (~/.config/stellar). + // This is the default for every command: remote lookups (even with a 2s + // cap) make TAB feel broken, and `stellar remove` only ever operates on + // cached themes anyway. + LocalOnly Mode = iota + // LocalAndRemote additionally queries the stellar-hub API when the local + // cache alone doesn't have enough to complete usefully. Opt-in via the + // EnvOnline environment variable. + LocalAndRemote +) + +// EnvOnline opts apply/preview/info completion in to hub suggestions +// ("1" or "true"). Off by default: completion must never feel slow. +const EnvOnline = "STELLAR_COMPLETION_ONLINE" + +const ( + descLocal = "local" + descHub = "hub" +) + +// ThemeIdentifier completes an "author/slug@version" identifier in three +// stages, splitting on the first "/" and then the first "@": +// +// - Stage A (no "/" yet): author names, emitted as "author/". +// - Stage B ("author/" typed, no "@" yet): slugs for that author, emitted +// as "author/slug". +// - Stage C ("@" typed): versions for that author/slug, emitted as +// "author/slug@version". +// +// It never blocks on the network for long: any remote lookup goes through +// api.NewCompletionClient (2s timeout), and any error from it degrades +// silently to whatever local results were already gathered - logged only via +// cobra.CompDebugln, since stray text on stdout would corrupt what the +// user's shell parses as completion candidates. +// +// It also tolerates a local cache that doesn't exist yet: every cache +// listing helper it calls returns (nil, nil) rather than an error for a +// missing directory, so completion always degrades to "no candidates" +// instead of failing. +func ThemeIdentifier(toComplete string, mode Mode) ([]string, cobra.ShellCompDirective) { + slashIdx := strings.IndexByte(toComplete, '/') + if slashIdx == -1 { + return completeAuthor(toComplete, mode) + } + + author := toComplete[:slashIdx] + rest := toComplete[slashIdx+1:] + + // An empty author ("/x") can never form a valid identifier, and letting + // it through would list the cache root's author dirs as slugs of "". + if !theme.IsValidSegment(author) { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + atIdx := strings.IndexByte(rest, '@') + if atIdx == -1 { + return completeSlug(author, rest, mode) + } + + slug := rest[:atIdx] + versionPrefix := rest[atIdx+1:] + + // Same for an empty slug ("author/@"): nothing valid can complete it. + if !theme.IsValidSegment(slug) { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + return completeVersion(author, slug, versionPrefix, mode) +} + +// completeAuthor implements Stage A: local author directories first; if (and +// only if) toComplete is non-empty and none of them match, fall back to +// querying the hub for authors by prefix. +func completeAuthor(toComplete string, mode Mode) ([]string, cobra.ShellCompDirective) { + directive := cobra.ShellCompDirectiveNoFileComp | cobra.ShellCompDirectiveNoSpace + + if !validSegment(toComplete) { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + authors, _ := cache.ListAuthors() + + var candidates []string + for _, a := range authors { + // The cache is a plain directory: it can hold anything a synced + // dotfiles checkout or an extracted tarball put there, so its entries + // get the same grammar check as the hub's (see below). + if !theme.IsValidSegment(a) { + continue + } + if strings.HasPrefix(a, toComplete) { + candidates = append(candidates, withDesc(a+"/", descLocal)) + } + } + + // Empty input never triggers a network call: it's a legitimate case + // (user just typed the bare command) and local-only is a good enough + // answer. Same if a local author already matched, or the caller is + // LocalOnly. + if toComplete == "" || len(candidates) > 0 || mode == LocalOnly { + return candidates, directive + } + + client := api.NewCompletionClient() + summaries, err := client.SearchThemesByAuthorName(toComplete) + if err != nil { + cobra.CompDebugln(err.Error(), false) + return candidates, directive + } + + seen := make(map[string]bool) + for _, s := range summaries { + // The hub response is untrusted input: never emit a name that + // couldn't have come from ParseIdentifier's character class, or a + // hostile hub could inject control sequences into the user's shell. + if !theme.IsValidSegment(s.Author.Name) { + continue + } + // The hub matches authorName case-insensitively, but every shell + // filters candidates against the typed word itself - bash's compgen + // and zsh's compadd case-sensitively. A candidate that doesn't + // prefix-match what the user typed is silently dropped there, so + // emitting one would make TAB work in fish and PowerShell only. + // Filtering here keeps behaviour identical across shells (and doesn't + // rely on the server having honoured authorName at all). + if !strings.HasPrefix(s.Author.Name, toComplete) { + continue + } + if seen[s.Author.Name] { + continue + } + seen[s.Author.Name] = true + candidates = append(candidates, withDesc(s.Author.Name+"/", descHub)) + } + + return candidates, directive +} + +// completeSlug implements Stage B: local slugs for author first, then (in +// LocalAndRemote mode) that author's hub themes appended, deduplicated on +// slug. Every candidate keeps the author exactly as the user typed it: the +// hub's /api/{author}/{slug} routes resolve authors with an exact match, so a +// hub theme is only suggested when the typed casing is the hub's casing - +// otherwise the completion would either 404 on apply or (see completeAuthor) +// be dropped by the shell's own prefix filter anyway. +func completeSlug(author, slugPrefix string, mode Mode) ([]string, cobra.ShellCompDirective) { + directive := cobra.ShellCompDirectiveNoFileComp | cobra.ShellCompDirectiveKeepOrder + + if !validSegment(slugPrefix) { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + localSlugs, _ := cache.ListAuthorThemes(author) + + seen := make(map[string]bool) + var candidates []string + for _, slug := range localSlugs { + // Cache entries get the same grammar check as hub ones - a stray + // directory must not become a suggestion (see completeAuthor). + if !theme.IsValidSegment(slug) { + continue + } + if !strings.HasPrefix(slug, slugPrefix) { + continue + } + seen[slug] = true + candidates = append(candidates, withDesc(author+"/"+slug, descLocal)) + } + + if mode == LocalOnly { + return candidates, directive + } + + client := api.NewCompletionClient() + summaries, err := client.SearchThemesByAuthorName(author) + if err != nil { + cobra.CompDebugln(err.Error(), false) + return candidates, directive + } + + for _, s := range summaries { + // Exact, not EqualFold: the hub resolves authors exactly, and the + // user's shell filters candidates against the typed word. + if s.Author.Name != author { + continue + } + // Untrusted hub response: only emit values matching the identifier + // character class (see completeAuthor). + if !theme.IsValidSegment(s.Slug) { + continue + } + if seen[s.Slug] || !strings.HasPrefix(s.Slug, slugPrefix) { + continue + } + seen[s.Slug] = true + candidates = append(candidates, withDesc(s.Author.Name+"/"+s.Slug, descHub)) + } + + return candidates, directive +} + +// completeVersion implements Stage C: local versions newest-first, then (in +// LocalAndRemote mode) remote versions from GetThemeInfo not already +// present, then the "latest" keyword last. Remote is skipped entirely for +// the reserved backup theme (theme.BackupThemeName): it's never published on +// the hub, so looking it up there would only cost a doomed round trip. +func completeVersion(author, slug, versionPrefix string, mode Mode) ([]string, cobra.ShellCompDirective) { + directive := cobra.ShellCompDirectiveNoFileComp | cobra.ShellCompDirectiveKeepOrder + + if !validVersionSegment(versionPrefix) { + return nil, cobra.ShellCompDirectiveNoFileComp + } + + // The parser accepts an optional "v" before the version ("@v1.0"), but + // versions are stored without it. Strip it for matching and re-prepend + // it on emission so the shell's own prefix filter keeps the candidates. + // No prefix of "latest" starts with "v", so a leading "v" is unambiguous. + emitV := strings.HasPrefix(versionPrefix, "v") + if emitV { + versionPrefix = versionPrefix[1:] + } + emit := func(version string) string { + if emitV { + version = "v" + version + } + return identifierAt(author, slug, version) + } + // suggestable gates every candidate, local or remote, on the grammar + // apply will later hold it to: "@1.0.1" or "@notes" would only complete + // to an "invalid theme identifier" error. "@vlatest" parses but is silly. + suggestable := func(version string) bool { + if !theme.IsValidVersion(version) { + return false + } + if emitV && version == "latest" { + return false + } + return strings.HasPrefix(version, versionPrefix) + } + + localVersions, _ := cache.ListThemeVersions(author, slug) // already newest-first + + seen := make(map[string]bool) + var candidates []string + for _, v := range localVersions { + // A theme directory can hold any *.toml name ("1.0.1.toml", + // "notes.toml", an editor backup), and ListThemeVersions reports the + // filename verbatim - hence the same gate the hub values get. + if !suggestable(v) { + continue + } + seen[v] = true + candidates = append(candidates, withDesc(emit(v), descLocal)) + } + + if mode == LocalAndRemote && slug != theme.BackupThemeName { + client := api.NewCompletionClient() + info, err := client.GetThemeInfo(author, slug) + if err != nil { + cobra.CompDebugln(err.Error(), false) + } else { + for _, v := range info.Versions { + // Untrusted hub response (see completeAuthor). + if !suggestable(v.Version) { + continue + } + if seen[v.Version] { + continue + } + seen[v.Version] = true + candidates = append(candidates, withDesc(emit(v.Version), descHub)) + } + } + } + + if !seen["latest"] && suggestable("latest") { + candidates = append(candidates, identifierAt(author, slug, "latest")) + } + + return candidates, directive +} + +func identifierAt(author, slug, version string) string { + return fmt.Sprintf("%s/%s@%s", author, slug, version) +} + +func withDesc(value, desc string) string { + return value + "\t" + desc +} + +// validSegment reports whether s is a viable *partial* author or slug - i.e. +// what the user has typed so far, so the empty string qualifies. Candidates +// about to be emitted are held to the stricter theme.IsValidSegment instead. +func validSegment(s string) bool { + for _, r := range s { + if !theme.IsValidIdentifierRune(r) { + return false + } + } + return true +} + +// validVersionSegment is like validSegment but additionally allows '.', since +// a partially typed version looks like "1", "1." or "lat". Emitted versions +// are held to theme.IsValidVersion instead. +func validVersionSegment(s string) bool { + for _, r := range s { + if r != '.' && !theme.IsValidIdentifierRune(r) { + return false + } + } + return true +} diff --git a/internal/testutil/mockapi.go b/internal/testutil/mockapi.go index d00c79f..d79a132 100644 --- a/internal/testutil/mockapi.go +++ b/internal/testutil/mockapi.go @@ -3,6 +3,8 @@ package testutil import ( "encoding/json" "net/http" + "sort" + "strconv" "strings" "sync" ) @@ -24,11 +26,11 @@ type MockTheme struct { // MockVersion represents a theme version type MockVersion struct { - Version string + Version string ConfigContent string // The actual TOML content - VersionNotes string - Dependencies []string - CreatedAt string + VersionNotes string + Dependencies []string + CreatedAt string } // MockAPIHandler implements http.Handler for testing @@ -36,6 +38,7 @@ type MockAPIHandler struct { mu sync.Mutex themes map[string]*MockTheme // key: "author/slug" DownloadCounts map[string]int // Track download increments for verification + RequestCounts map[string]int // Track requests received, keyed by r.URL.Path } // NewMockAPIHandler creates a new mock API handler @@ -43,6 +46,7 @@ func NewMockAPIHandler() *MockAPIHandler { return &MockAPIHandler{ themes: make(map[string]*MockTheme), DownloadCounts: make(map[string]int), + RequestCounts: make(map[string]int), } } @@ -58,10 +62,19 @@ func (h *MockAPIHandler) AddTheme(theme MockTheme) { // ServeHTTP implements http.Handler func (h *MockAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + h.mu.Lock() + h.RequestCounts[r.URL.Path]++ + h.mu.Unlock() + // Parse path: /api/{author}/{slug} or /api/{author}/{slug}/{version} path := strings.TrimPrefix(r.URL.Path, "/api/") parts := strings.Split(path, "/") + if len(parts) == 1 && parts[0] == "themes" { + h.handleSearchThemes(w, r) + return + } + if len(parts) < 2 { http.Error(w, "Not found", http.StatusNotFound) return @@ -71,8 +84,15 @@ func (h *MockAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { slug := parts[1] key := author + "/" + slug + // Snapshot the theme under the mutex: handleIncrementDownload writes + // theme.Downloads concurrently, so handlers must not read the shared + // struct after unlocking (would race under go test -race). h.mu.Lock() - theme, exists := h.themes[key] + stored, exists := h.themes[key] + var theme MockTheme + if exists { + theme = *stored + } h.mu.Unlock() if !exists { @@ -83,14 +103,14 @@ func (h *MockAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Handle version endpoint: GET /api/{author}/{slug}/{version} if len(parts) == 3 { version := parts[2] - h.handleVersionRequest(w, r, theme, version) + h.handleVersionRequest(w, r, &theme, version) return } // Handle theme endpoint: GET/POST /api/{author}/{slug} switch r.Method { case http.MethodGet: - h.handleGetTheme(w, theme) + h.handleGetTheme(w, &theme) case http.MethodPost: h.handleIncrementDownload(w, key) default: @@ -111,7 +131,7 @@ func (h *MockAPIHandler) handleGetTheme(w http.ResponseWriter, theme *MockTheme) "createdAt": theme.CreatedAt, "updatedAt": theme.UpdatedAt, "author": map[string]interface{}{ - "id": theme.Author, + "id": authorID(theme.Author), "name": theme.Author, "image": nil, "bio": nil, @@ -161,6 +181,108 @@ func (h *MockAPIHandler) handleVersionRequest(w http.ResponseWriter, r *http.Req http.Error(w, "Version not found", http.StatusNotFound) } +// clampQueryInt reads an integer query parameter, falling back to def for +// anything unparseable and clamping the rest into [min, max] - mirroring the +// hub's own handling of limit/offset. +func clampQueryInt(r *http.Request, key string, def, minVal, maxVal int) int { + raw := r.URL.Query().Get(key) + if raw == "" { + return def + } + parsed, err := strconv.Atoi(raw) + if err != nil { + return def + } + return max(minVal, min(parsed, maxVal)) +} + +// authorID derives an opaque author id from an author name. On the real hub +// these are unrelated - the id is a better-auth row id, while every +// /api/{author}/{slug} route resolves the author by exact name - so the mock +// must not let them be used interchangeably, or code that reads the wrong one +// would pass every test here and 404 in production. +func authorID(author string) string { + return "user-" + strings.ToLower(author) +} + +// handleSearchThemes implements GET /api/themes?authorName=, used by +// shell completion (internal/completion) to look up hub authors/themes +// without downloading a full ThemeInfo per candidate. It filters registered +// themes by a case-insensitive prefix match on author name, and orders the +// result by theme name (matching the hub's sort=name, which is what the +// client requests), tie-broken by author so map iteration order can't leak +// into a test. +func (h *MockAPIHandler) handleSearchThemes(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + return + } + + authorPrefix := strings.ToLower(r.URL.Query().Get("authorName")) + + // Copy matching themes under the mutex (see ServeHTTP for why). + h.mu.Lock() + var matches []MockTheme + for _, theme := range h.themes { + if authorPrefix != "" && !strings.HasPrefix(strings.ToLower(theme.Author), authorPrefix) { + continue + } + matches = append(matches, *theme) + } + h.mu.Unlock() + + sort.Slice(matches, func(i, j int) bool { + if matches[i].Name != matches[j].Name { + return matches[i].Name < matches[j].Name + } + return matches[i].Author < matches[j].Author + }) + + themesJSON := make([]map[string]interface{}, len(matches)) + for i, t := range matches { + // The hub omits latestVersion entirely for a theme with no versions + // (JSON.stringify drops an undefined value). + theme := map[string]interface{}{ + "id": t.ID, + "author": map[string]interface{}{ + "id": authorID(t.Author), + "name": t.Author, + "image": nil, + }, + "name": t.Name, + "slug": t.Slug, + "description": t.Description, + "screenshotUrl": "https://example.com/" + t.Slug + ".png", + "downloads": t.Downloads, + "colorScheme": t.ColorScheme, + "createdAt": t.CreatedAt, + "updatedAt": t.UpdatedAt, + } + if len(t.Versions) > 0 { + theme["latestVersion"] = t.Versions[0].Version + } + themesJSON[i] = theme + } + + // Honour limit/offset the way the hub does (default 20, capped at 100), + // so a test can't get a page size the real API could never return. + limit := clampQueryInt(r, "limit", 20, 1, 100) + offset := clampQueryInt(r, "offset", 0, 0, len(themesJSON)) + themesJSON = themesJSON[offset:min(offset+limit, len(themesJSON))] + + response := map[string]interface{}{ + "themes": themesJSON, + "pagination": map[string]interface{}{ + "total": len(themesJSON), + "limit": limit, + "offset": offset, + }, + } + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(response) +} + // handleIncrementDownload increments the download count func (h *MockAPIHandler) handleIncrementDownload(w http.ResponseWriter, key string) { h.mu.Lock() @@ -181,6 +303,26 @@ func (h *MockAPIHandler) GetDownloadCount(author, slug string) int { return h.DownloadCounts[author+"/"+slug] } +// Requests returns how many requests the handler has received for path +// (matched against r.URL.Path exactly, e.g. "/api/testuser/sample-theme"). +func (h *MockAPIHandler) Requests(path string) int { + h.mu.Lock() + defer h.mu.Unlock() + return h.RequestCounts[path] +} + +// TotalRequests returns the total number of requests received across all paths. +func (h *MockAPIHandler) TotalRequests() int { + h.mu.Lock() + defer h.mu.Unlock() + + total := 0 + for _, count := range h.RequestCounts { + total += count + } + return total +} + // CreateDefaultMockAPI creates a mock API with sample themes for testing func CreateDefaultMockAPI() *MockAPIHandler { handler := NewMockAPIHandler() @@ -243,5 +385,35 @@ func CreateDefaultMockAPI() *MockAPIHandler { }, }) + // Add a theme from a second author, used by shell completion tests to + // exercise "unknown author prefix -> hub-only suggestions" behavior. + handler.AddTheme(MockTheme{ + ID: "test-id-3", + Author: "otheruser", + Slug: "ocean-theme", + Name: "Ocean Theme", + Description: "A calming ocean theme", + Downloads: 5, + Group: "nature", + CreatedAt: "2024-02-01T00:00:00Z", + UpdatedAt: "2024-02-05T00:00:00Z", + Versions: []MockVersion{ + { + Version: "2.1", + ConfigContent: SampleTOML(), + VersionNotes: "Latest version", + Dependencies: []string{}, + CreatedAt: "2024-02-05T00:00:00Z", + }, + { + Version: "2.0", + ConfigContent: SampleTOML(), + VersionNotes: "Initial release", + Dependencies: []string{}, + CreatedAt: "2024-02-01T00:00:00Z", + }, + }, + }) + return handler } diff --git a/internal/theme/parser.go b/internal/theme/parser.go index a00e6a0..f719084 100644 --- a/internal/theme/parser.go +++ b/internal/theme/parser.go @@ -36,6 +36,39 @@ func IsValidIdentifierRune(r rune) bool { r == '_' || r == '-' } +// versionRe matches a complete version as ParseIdentifier accepts it, minus +// the optional "v" prefix (which the parser strips). Kept next to the +// identifier regex below so the two can't drift. +var versionRe = regexp.MustCompile(`^([0-9]+\.[0-9]+|latest)$`) + +// IsValidSegment reports whether s is a complete, usable author or theme +// segment: non-empty and made up entirely of IsValidIdentifierRune runes. +// +// Callers that emit segments they didn't parse themselves (e.g. shell +// completion listing cache directories, or reading an API response) must gate +// on this, so a name that ParseIdentifier would reject is never handed back to +// the user as a suggestion - and so a hostile name can't smuggle control +// characters into a terminal. +func IsValidSegment(s string) bool { + if s == "" { + return false + } + for _, r := range s { + if !IsValidIdentifierRune(r) { + return false + } + } + return true +} + +// IsValidVersion reports whether s is a complete version ParseIdentifier +// accepts ("1.2" or "latest"), without the optional "v" prefix. Same contract +// as IsValidSegment: gate emitted versions on it, since a cache directory can +// hold arbitrary *.toml filenames. +func IsValidVersion(s string) bool { + return versionRe.MatchString(s) +} + // ParseIdentifier parses "alice/rainbow@1.2", "alice/rainbow@latest", or "alice/rainbow" func ParseIdentifier(identifier string) (*Theme, error) { // Normalize: remove leading/trailing whitespace @@ -105,16 +138,16 @@ func FindLatestLocalVersion(themeDir string) (string, error) { // Sort by semver descending, "latest" goes last as fallback sort.Slice(versions, func(i, j int) bool { - return compareSemver(versions[i], versions[j]) > 0 + return CompareSemver(versions[i], versions[j]) > 0 }) return versions[0], nil } -// compareSemver compares two version strings. +// CompareSemver compares two version strings. // Returns >0 if a > b, <0 if a < b, 0 if equal. // Non-numeric versions (like "latest") are sorted to the end. -func compareSemver(a, b string) int { +func CompareSemver(a, b string) int { aMajor, aMinor, aOk := parseSemver(a) bMajor, bMinor, bOk := parseSemver(b) diff --git a/internal/theme/parser_test.go b/internal/theme/parser_test.go index 7b22ff4..de1c4a8 100644 --- a/internal/theme/parser_test.go +++ b/internal/theme/parser_test.go @@ -11,85 +11,85 @@ import ( func TestParseIdentifier(t *testing.T) { tests := []struct { - name string - identifier string - wantAuthor string - wantName string - wantVersion string + name string + identifier string + wantAuthor string + wantName string + wantVersion string wantExplicit bool - wantErr bool + wantErr bool }{ { - name: "simple identifier", - identifier: "alice/rainbow", - wantAuthor: "alice", - wantName: "rainbow", - wantVersion: "latest", + name: "simple identifier", + identifier: "alice/rainbow", + wantAuthor: "alice", + wantName: "rainbow", + wantVersion: "latest", wantExplicit: false, - wantErr: false, + wantErr: false, }, { - name: "with version", - identifier: "alice/rainbow@1.2", - wantAuthor: "alice", - wantName: "rainbow", - wantVersion: "1.2", + name: "with version", + identifier: "alice/rainbow@1.2", + wantAuthor: "alice", + wantName: "rainbow", + wantVersion: "1.2", wantExplicit: true, - wantErr: false, + wantErr: false, }, { - name: "with v prefix version", - identifier: "alice/rainbow@v1.2", - wantAuthor: "alice", - wantName: "rainbow", - wantVersion: "1.2", + name: "with v prefix version", + identifier: "alice/rainbow@v1.2", + wantAuthor: "alice", + wantName: "rainbow", + wantVersion: "1.2", wantExplicit: true, - wantErr: false, + wantErr: false, }, { - name: "with latest version", - identifier: "alice/rainbow@latest", - wantAuthor: "alice", - wantName: "rainbow", - wantVersion: "latest", + name: "with latest version", + identifier: "alice/rainbow@latest", + wantAuthor: "alice", + wantName: "rainbow", + wantVersion: "latest", wantExplicit: true, - wantErr: false, + wantErr: false, }, { - name: "with underscores", - identifier: "some_user/my_theme", - wantAuthor: "some_user", - wantName: "my_theme", - wantVersion: "latest", + name: "with underscores", + identifier: "some_user/my_theme", + wantAuthor: "some_user", + wantName: "my_theme", + wantVersion: "latest", wantExplicit: false, - wantErr: false, + wantErr: false, }, { - name: "with hyphens", - identifier: "some-user/my-theme", - wantAuthor: "some-user", - wantName: "my-theme", - wantVersion: "latest", + name: "with hyphens", + identifier: "some-user/my-theme", + wantAuthor: "some-user", + wantName: "my-theme", + wantVersion: "latest", wantExplicit: false, - wantErr: false, + wantErr: false, }, { - name: "with numbers", - identifier: "user123/theme456", - wantAuthor: "user123", - wantName: "theme456", - wantVersion: "latest", + name: "with numbers", + identifier: "user123/theme456", + wantAuthor: "user123", + wantName: "theme456", + wantVersion: "latest", wantExplicit: false, - wantErr: false, + wantErr: false, }, { - name: "with whitespace", - identifier: " alice/rainbow ", - wantAuthor: "alice", - wantName: "rainbow", - wantVersion: "latest", + name: "with whitespace", + identifier: " alice/rainbow ", + wantAuthor: "alice", + wantName: "rainbow", + wantVersion: "latest", wantExplicit: false, - wantErr: false, + wantErr: false, }, { name: "invalid - no slash", @@ -249,7 +249,7 @@ func TestCompareSemver(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := compareSemver(tt.a, tt.b) + result := CompareSemver(tt.a, tt.b) if tt.expect > 0 { assert.Greater(t, result, 0, "expected %s > %s", tt.a, tt.b) } else if tt.expect < 0 { From a7993c78cbabaca513d0470e535bceb2dc9aa3a3 Mon Sep 17 00:00:00 2001 From: a3chron Date: Wed, 26 Aug 2026 14:51:19 +0200 Subject: [PATCH 3/8] fix(tests): stop run-tests.sh aborting on the first passing test `((passed++))` evaluates to the pre-increment value, so the arithmetic command exits 1 the first time it runs with passed=0 - and under `set -e` that killed the script right after printing the first PASS. The suite itself was fine; only the runner bailed out. Co-Authored-By: Claude Opus 5 (1M context) --- run-tests.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/run-tests.sh b/run-tests.sh index a5a3888..da2bfcb 100755 --- a/run-tests.sh +++ b/run-tests.sh @@ -82,11 +82,11 @@ run_go_tests() { case "$action" in pass) echo -e " ${GREEN}PASS${NC} $test" - ((passed++)) + passed=$((passed + 1)) ;; fail) echo -e " ${RED}FAIL${NC} $test" - ((failed++)) + failed=$((failed + 1)) failed_tests+=("$pkg $test") ;; esac From c1847cc46442133dfcfa730e35884322f87becc2 Mon Sep 17 00:00:00 2001 From: a3chron Date: Wed, 26 Aug 2026 16:25:48 +0200 Subject: [PATCH 4/8] ci: gate releases on go vet and go test The release workflow built and published binaries without ever running the test suite, so a tag was the first thing to find out whether the code worked. goreleaser now needs a test job as well as lint. Linux runs with -race and is blocking. Windows runs too, since it's the platform whose copy-vs-symlink path most needs native coverage, but is marked continue-on-error: the suite has never been run on a Windows runner, and a release is the wrong moment to discover that. Drop that line once it's been green once. Also switched all three jobs from a pinned go-version: 1.24 to go-version-file: go.mod. The module requires 1.25.5, so the pin made every job download a newer toolchain before doing any work, and it can now no longer drift from the module. Contract tests stay out of CI - they're behind the `contract` build tag and hit the live hub. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release.yaml | 39 +++++++++++++++++++++++++++++++--- README.md | 3 ++- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index cfc04d6..b46b996 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -16,13 +16,46 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 with: - go-version: 1.24 + # Read from go.mod so CI can never drift from what the module + # requires (it says 1.25.5, so a pinned 1.24 silently made every + # job download a newer toolchain first). + go-version-file: go.mod - name: golangci-lint uses: golangci/golangci-lint-action@v8 + test: + name: test (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + # Windows is advisory for now: the suite has never been run on a native + # Windows runner, so a failure there must not be discovered while cutting + # a release. Once it has been green once, delete this line so the copy + # path is genuinely guarded. + continue-on-error: ${{ matrix.os == 'windows-latest' }} + strategy: + fail-fast: false + matrix: + include: + # -race needs a C toolchain, which the Windows runner doesn't have + # set up by default. + - os: ubuntu-latest + raceflag: "-race" + - os: windows-latest + raceflag: "" + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: go vet + run: go vet ./... + # Contract tests are behind the `contract` build tag and hit the live + # hub, so they stay out of CI. + - name: go test + run: go test ${{ matrix.raceflag }} ./... + goreleaser: runs-on: ubuntu-latest - needs: lint + needs: [lint, test] steps: - name: Checkout uses: actions/checkout@v4 @@ -32,7 +65,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: 1.24 + go-version-file: go.mod - name: Check mod files run: | diff --git a/README.md b/README.md index 362c86a..bf843e9 100644 --- a/README.md +++ b/README.md @@ -278,7 +278,8 @@ When adding new CLI features, please add corresponding E2E tests in `cmd/e2e_tes - [ ] **Preview: fix bash formatting** - [ ] **`stellar preview` on Windows**: `cmd/preview.go` only spawns terminals on macOS/Linux and returns "unsupported platform" on Windows. Needs a Windows Terminal / PowerShell branch that opens a shell with `STARSHIP_CONFIG` set. - [ ] **Windows packaging**: consider scoop/winget packaging (leftover `stellar.exe.old` from self-update is already cleaned up on the next run). -- [ ] **CI test job**: the release workflow runs no `go test` today; add one (ideally with a `windows-latest` runner) to guard the copy path natively. +- [x] **CI test job**: the release workflow now runs `go vet` and `go test` (with `-race` on Linux) before goreleaser, so a tag can't publish a failing build. + - Still open: the `windows-latest` leg is marked `continue-on-error` because the suite has never been run on a native Windows runner. Once it's been green once, drop that line so the copy path is genuinely guarded. - [ ] **`stellar publish` command**: Upload local themes directly to stellar-hub - Challenge: Need to implement CLI authentication (OAuth flow with browser redirect or API keys) - Would read from `~/.config/stellar///.toml` From 995c4841a2d4bdaf9a2262467dd1afaf75c41054 Mon Sep 17 00:00:00 2001 From: a3chron Date: Wed, 26 Aug 2026 16:30:02 +0200 Subject: [PATCH 5/8] ci: run the release checks on pull requests too Tests only ran on a tag push, so anything broken on main stayed invisible until release time - which is exactly when you don't want to find out. lint, test and the go.mod checks move into a reusable checks.yaml that both ci.yaml (pull requests, and pushes to main) and release.yaml call. Defining them once means the checks gating a release are by construction the same ones the PR had to pass, rather than two lists that drift. The module checks used to live inside the goreleaser job, where they only ever ran during a release; they now run on every PR as well. A PR from a branch in this repo fires only the pull_request event, since its push doesn't match main, so this doesn't double up. In-flight runs are cancelled when a PR gets a new push. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/checks.yaml | 70 ++++++++++++++++++++++++++++++++++ .github/workflows/ci.yaml | 22 +++++++++++ .github/workflows/release.yaml | 59 +++------------------------- 3 files changed, 98 insertions(+), 53 deletions(-) create mode 100644 .github/workflows/checks.yaml create mode 100644 .github/workflows/ci.yaml diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml new file mode 100644 index 0000000..2a5faf9 --- /dev/null +++ b/.github/workflows/checks.yaml @@ -0,0 +1,70 @@ +name: Checks + +# Reusable: called by ci.yaml on pushes and pull requests, and by +# release.yaml before goreleaser publishes anything. Defined once here so the +# checks that gate a release are exactly the ones a PR had to pass. +on: + workflow_call: + +permissions: + contents: read + +jobs: + lint: + name: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + # Read from go.mod so CI can never drift from what the module + # requires (it says 1.25.5, so a pinned 1.24 silently made every + # job download a newer toolchain first). + go-version-file: go.mod + - name: golangci-lint + uses: golangci/golangci-lint-action@v8 + + test: + name: test (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + # Windows is advisory for now: the suite has never been run on a native + # Windows runner, so a failure there must not be discovered while cutting + # a release. Once it has been green once, delete this line so the copy + # path is genuinely guarded. + continue-on-error: ${{ matrix.os == 'windows-latest' }} + strategy: + fail-fast: false + matrix: + include: + # -race needs a C toolchain, which the Windows runner doesn't have + # set up by default. + - os: ubuntu-latest + raceflag: "-race" + - os: windows-latest + raceflag: "" + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: go vet + run: go vet ./... + # Contract tests are behind the `contract` build tag and hit the live + # hub, so they stay out of CI. + - name: go test + run: go test ${{ matrix.raceflag }} ./... + + modules: + name: modules + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: Check mod files + run: | + go mod tidy + git diff --exit-code go.mod go.sum + - name: Verify module hashes + run: go mod verify diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml new file mode 100644 index 0000000..dd282a8 --- /dev/null +++ b/.github/workflows/ci.yaml @@ -0,0 +1,22 @@ +name: CI + +# A PR from a branch in this repo only fires the pull_request event (its push +# doesn't match main), so this doesn't double up. Tag pushes don't match +# either - those go to release.yaml. +on: + pull_request: + push: + branches: + - main + +permissions: + contents: read + +# A new push to a PR makes the in-flight run pointless. +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + checks: + uses: ./.github/workflows/checks.yaml diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index b46b996..b3f96f9 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -9,53 +9,14 @@ permissions: contents: write jobs: - lint: - name: lint - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 - with: - # Read from go.mod so CI can never drift from what the module - # requires (it says 1.25.5, so a pinned 1.24 silently made every - # job download a newer toolchain first). - go-version-file: go.mod - - name: golangci-lint - uses: golangci/golangci-lint-action@v8 - - test: - name: test (${{ matrix.os }}) - runs-on: ${{ matrix.os }} - # Windows is advisory for now: the suite has never been run on a native - # Windows runner, so a failure there must not be discovered while cutting - # a release. Once it has been green once, delete this line so the copy - # path is genuinely guarded. - continue-on-error: ${{ matrix.os == 'windows-latest' }} - strategy: - fail-fast: false - matrix: - include: - # -race needs a C toolchain, which the Windows runner doesn't have - # set up by default. - - os: ubuntu-latest - raceflag: "-race" - - os: windows-latest - raceflag: "" - steps: - - uses: actions/checkout@v4 - - uses: actions/setup-go@v5 - with: - go-version-file: go.mod - - name: go vet - run: go vet ./... - # Contract tests are behind the `contract` build tag and hit the live - # hub, so they stay out of CI. - - name: go test - run: go test ${{ matrix.raceflag }} ./... + # Same lint/test/module checks a pull request has to pass, so a tag can't + # publish something CI would have rejected. + checks: + uses: ./.github/workflows/checks.yaml goreleaser: runs-on: ubuntu-latest - needs: [lint, test] + needs: checks steps: - name: Checkout uses: actions/checkout@v4 @@ -67,14 +28,6 @@ jobs: with: go-version-file: go.mod - - name: Check mod files - run: | - go mod tidy - git diff --exit-code go.mod go.sum - - - name: Verify module hashes - run: go mod verify - - name: Run GoReleaser uses: goreleaser/goreleaser-action@v6 with: @@ -82,4 +35,4 @@ jobs: version: "~> v2" args: release --clean env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} \ No newline at end of file + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} From ef2f612a3b47946be05053a8ae5fe83211f09e44 Mon Sep 17 00:00:00 2001 From: a3chron Date: Wed, 26 Aug 2026 16:40:26 +0200 Subject: [PATCH 6/8] test: fix Unix assumptions that broke the suite on Windows The first native Windows CI run failed in ~25 places. All of it is the tests assuming a Unix filesystem - product code is unaffected, and nothing here changes behaviour for Windows users. - testutil.CreateConfig: call sites build config.json by concatenating a path into a JSON string, and "C:\Users\..." makes that invalid JSON - \U is not a valid escape, so every affected test failed with "invalid character 'U' in string escape code". This was the bulk of the failures (rollback, current, remove, clean). Backslashes are now escaped in the helper rather than at all ~30 call sites; on Unix it's a no-op. Config writing in internal/config already used json.MarshalIndent and was never affected. - e2e apply: asserted the config file contains the raw theme path, but JSON stores it with escaped separators. Compares the encoded form now. - theme parser: two expectations built paths with "/" instead of filepath.Join, so they could only ever match on Unix. - completion: the hostile-cache-entry test staged a directory with an ANSI escape in its name. Windows rejects control characters in filenames outright, so that case is now Unix-only - it's also the only platform where the threat exists. - update: staging a leftover next to the *running* test binary hits a Windows sharing violation, since every executed command runs cleanupUpdateLeftovers and a pending delete blocks re-creating the name. It skips with the reason instead of failing; the removal logic is covered against a temp dir by TestRemoveUpdateLeftovers. Verified on Linux only - the Windows leg is still continue-on-error, so the next run reports without blocking. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/completion_test.go | 10 +++++++++- cmd/e2e_test.go | 4 +++- cmd/update_test.go | 14 +++++++++++++- internal/testutil/testutil.go | 11 ++++++++++- internal/theme/parser_test.go | 8 +++++--- 5 files changed, 40 insertions(+), 7 deletions(-) diff --git a/cmd/completion_test.go b/cmd/completion_test.go index c53d67c..f82278d 100644 --- a/cmd/completion_test.go +++ b/cmd/completion_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -416,7 +417,14 @@ func TestCompletion_MalformedLocalCacheEntries_Filtered(t *testing.T) { // untrusted hub response: names outside the identifier character class // must never be suggested - an ANSI escape especially, since candidates // are printed straight into the user's terminal. - for _, name := range []string{".git", "my author", "evil\x1b[31muser"} { + hostile := []string{".git", "my author"} + if runtime.GOOS != "windows" { + // Windows rejects control characters in filenames outright, so the + // escape-sequence case can only be set up (and can only arise) on a + // Unix filesystem. + hostile = append(hostile, "evil\x1b[31muser") + } + for _, name := range hostile { require.NoError(t, os.MkdirAll(filepath.Join(env.StellarDir, name), 0o755)) } diff --git a/cmd/e2e_test.go b/cmd/e2e_test.go index 5dbee28..2d604d4 100644 --- a/cmd/e2e_test.go +++ b/cmd/e2e_test.go @@ -402,7 +402,9 @@ func TestE2E_Apply(t *testing.T) { configContent := env.ReadFile(filepath.Join(env.StellarDir, "config.json")) assert.Contains(t, configContent, "local/mytheme@1.0") - assert.Contains(t, configContent, themePath) + // The config is JSON, so a Windows path is stored with its separators + // escaped - compare against the encoded form, not the raw path. + assert.Contains(t, configContent, strings.ReplaceAll(themePath, `\`, `\\`)) assert.Contains(t, configContent, "applied_hash", "config should record the hash of the applied theme") }) diff --git a/cmd/update_test.go b/cmd/update_test.go index 72839e2..6f16be2 100644 --- a/cmd/update_test.go +++ b/cmd/update_test.go @@ -172,7 +172,19 @@ func TestCleanupUpdateLeftovers_RemovesRealArtifacts(t *testing.T) { oldBinary := execPath + ".old" tmpUpdate := filepath.Join(filepath.Dir(execPath), ".stellar-update-test123") - require.NoError(t, os.WriteFile(oldBinary, []byte("old binary"), 0644)) + // Setting up next to the *running* binary is inherently at the mercy of + // the platform. On Windows this fails with a sharing violation ("the + // process cannot access the file because it is being used by another + // process"): every command the E2E tests execute runs + // cleanupUpdateLeftovers via PersistentPreRunE, and a delete Windows still + // has pending blocks re-creating the same name. That's an artefact of + // testing against the live executable, not a defect in the cleanup being + // tested, so treat it as "can't run here" rather than a failure. The + // removal logic itself is covered against a temp dir elsewhere in this + // file. + if err := os.WriteFile(oldBinary, []byte("old binary"), 0644); err != nil { + t.Skipf("cannot stage a leftover next to the running test binary: %v", err) + } require.NoError(t, os.WriteFile(tmpUpdate, []byte("partial download"), 0644)) ageFile(t, tmpUpdate) t.Cleanup(func() { diff --git a/internal/testutil/testutil.go b/internal/testutil/testutil.go index 6976660..0152824 100644 --- a/internal/testutil/testutil.go +++ b/internal/testutil/testutil.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/a3chron/stellar/internal/paths" @@ -117,9 +118,17 @@ func (e *TestEnv) CreateThemeFile(author, name, version, content string) string return themePath } -// CreateConfig creates a config.json file in the test stellar directory +// CreateConfig creates a config.json file in the test stellar directory. +// +// Callers build the JSON by concatenating filesystem paths into it, which is +// invalid JSON on Windows: a path like C:\Users\... contains \U, and encoding/ +// json rejects it as a bad escape ("invalid character 'U' in string escape +// code"). Backslashes are therefore escaped here rather than at all ~30 call +// sites. No test wants a real JSON escape sequence in this content, so this +// is safe - but if one ever does, it needs to pass \\ itself. func (e *TestEnv) CreateConfig(content string) string { configPath := filepath.Join(e.StellarDir, "config.json") + content = strings.ReplaceAll(content, `\`, `\\`) if err := os.WriteFile(configPath, []byte(content), 0644); err != nil { e.t.Fatalf("failed to write config file: %v", err) } diff --git a/internal/theme/parser_test.go b/internal/theme/parser_test.go index de1c4a8..e440336 100644 --- a/internal/theme/parser_test.go +++ b/internal/theme/parser_test.go @@ -2,6 +2,7 @@ package theme import ( "os" + "path/filepath" "testing" "github.com/a3chron/stellar/internal/testutil" @@ -337,8 +338,9 @@ func TestTheme_CachePath(t *testing.T) { path, err := theme.CachePath() require.NoError(t, err) - // Should be relative to the test stellar home - expected := env.StellarDir + "/alice/rainbow/1.2.toml" + // Should be relative to the test stellar home. filepath.Join, not "/", so + // the expectation uses the platform's separator like CachePath does. + expected := filepath.Join(env.StellarDir, "alice", "rainbow", "1.2.toml") assert.Equal(t, expected, path) } @@ -354,7 +356,7 @@ func TestTheme_CacheDir(t *testing.T) { dir, err := theme.CacheDir() require.NoError(t, err) - expected := env.StellarDir + "/alice/rainbow" + expected := filepath.Join(env.StellarDir, "alice", "rainbow") assert.Equal(t, expected, dir) } From bda54592066c733fdf5838d77a3ed2795490f0ce Mon Sep 17 00:00:00 2001 From: a3chron Date: Wed, 26 Aug 2026 16:45:57 +0200 Subject: [PATCH 7/8] test: build the rollback symlink expectation with filepath.Join Last Windows holdout. previousPath was assembled with "/" and then compared against the symlink target stellar writes, which uses the platform separator - so the assertion could only ever match on Unix. Swept the rest of the suite for the same shape: the other occurrences are paths handed to file APIs (Windows accepts "/" there) or fed into config JSON for reading back, and every other assert.Equal against a symlink target already uses a CreateThemeFile path, which is filepath.Join-built. This was the only comparison that broke. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/e2e_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/e2e_test.go b/cmd/e2e_test.go index 2d604d4..f15288b 100644 --- a/cmd/e2e_test.go +++ b/cmd/e2e_test.go @@ -1076,7 +1076,9 @@ func TestE2E_Rollback(t *testing.T) { currentPath := env.CreateThemeFile("alice", "rainbow", "1.0", testutil.SampleTOML()) require.NoError(t, os.Symlink(currentPath, env.StarshipPath)) - previousPath := env.StellarDir + "/testuser/sample-theme/1.2.toml" + // filepath.Join, not "/": this is compared against the symlink target + // stellar itself writes, which uses the platform separator. + previousPath := filepath.Join(env.StellarDir, "testuser", "sample-theme", "1.2.toml") config := `{ "current_theme": "alice/rainbow@1.0", From 2cb88de373510c2035f44828fcc9afee06cf5950 Mon Sep 17 00:00:00 2001 From: a3chron Date: Wed, 26 Aug 2026 16:53:21 +0200 Subject: [PATCH 8/8] ci: make the Windows test leg blocking It's been green on a native runner, which is what the advisory continue-on-error was waiting for. Both legs now gate pull requests and releases, so the copy-vs-symlink path can't regress unnoticed. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/checks.yaml | 5 ----- README.md | 3 +-- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/.github/workflows/checks.yaml b/.github/workflows/checks.yaml index 2a5faf9..94efe48 100644 --- a/.github/workflows/checks.yaml +++ b/.github/workflows/checks.yaml @@ -27,11 +27,6 @@ jobs: test: name: test (${{ matrix.os }}) runs-on: ${{ matrix.os }} - # Windows is advisory for now: the suite has never been run on a native - # Windows runner, so a failure there must not be discovered while cutting - # a release. Once it has been green once, delete this line so the copy - # path is genuinely guarded. - continue-on-error: ${{ matrix.os == 'windows-latest' }} strategy: fail-fast: false matrix: diff --git a/README.md b/README.md index bf843e9..07323a1 100644 --- a/README.md +++ b/README.md @@ -278,8 +278,7 @@ When adding new CLI features, please add corresponding E2E tests in `cmd/e2e_tes - [ ] **Preview: fix bash formatting** - [ ] **`stellar preview` on Windows**: `cmd/preview.go` only spawns terminals on macOS/Linux and returns "unsupported platform" on Windows. Needs a Windows Terminal / PowerShell branch that opens a shell with `STARSHIP_CONFIG` set. - [ ] **Windows packaging**: consider scoop/winget packaging (leftover `stellar.exe.old` from self-update is already cleaned up on the next run). -- [x] **CI test job**: the release workflow now runs `go vet` and `go test` (with `-race` on Linux) before goreleaser, so a tag can't publish a failing build. - - Still open: the `windows-latest` leg is marked `continue-on-error` because the suite has never been run on a native Windows runner. Once it's been green once, drop that line so the copy path is genuinely guarded. +- [x] **CI test job**: `go vet` and `go test` (with `-race` on Linux) run on every pull request and again before goreleaser, on both `ubuntu-latest` and `windows-latest`, so a tag can't publish a failing build and the copy path is guarded natively. - [ ] **`stellar publish` command**: Upload local themes directly to stellar-hub - Challenge: Need to implement CLI authentication (OAuth flow with browser redirect or API keys) - Would read from `~/.config/stellar///.toml`