From 9de620bdaa55cc96825343748f5470b0f90cd59f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 18:26:31 +0000 Subject: [PATCH 1/3] feat(cli): name an Outfit with alias/unalias and complete names with TAB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `outfit alias` registers an Outfit under a short name — by default its own ALIAS — and that name then stands in wherever a path goes: apply, unapply, serve, and a leading argument to harness. `outfit unalias` drops it. Resolution lives in readOutfit, the choke point all four share, so one change gives every command the shorthand. A path on disk always beats a registered name, so adding an alias cannot change a command that already worked, and the lookup is gated on the argument being name-shaped, so a path never causes a config read at all. The registry stores the absolute path of the Outfit file rather than its directory, which keeps serve's relative PRESET resolving next to the aliased Outfit. The registry shares outfit's config file with the default-harness preference, so ownership of that file moves to a new leaf package, internal/config, where every write is a read-modify-write and neither setting can clobber the other. harness keeps its LoadPreference/SavePreference API and delegates. Also adds `outfit completion bash`, which prints a completion script driven by a hidden `outfit __complete`: TAB now offers commands, each command's flags, providers, harnesses, and — the point — your registered alias names, listing every match when several remain. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01X2JtqNGP3SPhqeTzWFGf3G --- .goreleaser.yaml | 4 + AGENTS.md | 12 +- README.md | 68 ++++- cmd/outfit/alias_test.go | 498 +++++++++++++++++++++++++++++++ cmd/outfit/complete.go | 360 ++++++++++++++++++++++ cmd/outfit/complete_test.go | 301 +++++++++++++++++++ cmd/outfit/completion.bash | 52 ++++ cmd/outfit/harness_test.go | 160 ++++++++++ cmd/outfit/main.go | 339 ++++++++++++++++++++- cmd/outfit/main_test.go | 16 + docs/outfit-file.md | 34 ++- internal/config/config.go | 208 +++++++++++++ internal/config/config_test.go | 278 +++++++++++++++++ internal/harness/harness.go | 50 +--- internal/harness/harness_test.go | 32 ++ 15 files changed, 2352 insertions(+), 60 deletions(-) create mode 100644 cmd/outfit/alias_test.go create mode 100644 cmd/outfit/complete.go create mode 100644 cmd/outfit/complete_test.go create mode 100644 cmd/outfit/completion.bash create mode 100644 internal/config/config.go create mode 100644 internal/config/config_test.go diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 025cc390..5f8bf826 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -47,6 +47,10 @@ brews: commit_author: name: goreleaser-bot email: bot@goreleaser.com + # Install the bash completion the binary prints, so `brew install` wires + # TAB up with no extra step. Only bash is supported, so ask for just that. + extra_install: | + generate_completions_from_executable(bin/"outfit", "completion", shells: [:bash]) test: | system "#{bin}/outfit", "--help" diff --git a/AGENTS.md b/AGENTS.md index 30815247..3d3c817e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,9 +22,11 @@ Run a single test: `go test -run TestWriteConfig_Idempotent ./...` The binary lives under `cmd/`; the domain logic is split into `internal/` packages so each concern is isolated and independently testable. -- `cmd/outfit/main.go` — CLI: command dispatch (`add`/`remove`/`list`/`show`/`apply`/`unapply`/`serve`/`export`/`harness`/`init-providers`), flag parsing (`parseSelection` registers both long and short flags against the same vars, and returns the chosen harness name separately so it never leaks into a `Selection`), and user-facing output. `add` and `apply` share `applySelection`, the core that writes one provider selection; `remove` and `unapply` share `removeSelection`, its inverse. Both resolve the active harness (`harness.Resolve`) and route through it. `show` reports the active harness's current config via `harness.State` (the same read `export` reconstructs an Outfit from). `serve` is harness-agnostic (it launches `llama-server`, not a config write). `harness` launches the active harness (forwarding trailing args), or manages the stored default with `--get`/`--set`; its `--outfit`/`-O` routes through the same `applySelection` before launching, so a launch can dress the harness first. That flag's value is optional (`outfitPathFlag` implements `IsBoolFlag`) — bare it means the default `./Outfit`, so a path has to be attached (`--outfit=`), since positional args belong to the harness. Tests: `main_test.go` (dispatch/add/remove/list), `apply_test.go` (`apply`/`unapply`/`export`), `harness_test.go` (the `harness` command, `show`, and Pi-routed add/export/remove), `serve_test.go`. +- `cmd/outfit/main.go` — CLI: command dispatch (`add`/`remove`/`list`/`show`/`apply`/`unapply`/`alias`/`unalias`/`serve`/`export`/`harness`/`init-providers`/`completion`, plus the hidden `__complete`), flag parsing (`parseSelection` registers both long and short flags against the same vars, and returns the chosen harness name separately so it never leaks into a `Selection`), and user-facing output. `add` and `apply` share `applySelection`, the core that writes one provider selection; `remove` and `unapply` share `removeSelection`, its inverse. Both resolve the active harness (`harness.Resolve`) and route through it. `show` reports the active harness's current config via `harness.State` (the same read `export` reconstructs an Outfit from). `serve` is harness-agnostic (it launches `llama-server`, not a config write). `harness` launches the active harness (forwarding trailing args), or manages the stored default with `--get`/`--set`; its `--outfit`/`-O` routes through the same `applySelection` before launching, so a launch can dress the harness first. That flag's value is optional (`outfitPathFlag` implements `IsBoolFlag`) — bare it means the default `./Outfit`, so a path has to be attached (`--outfit=`), since positional args belong to the harness; the exception is a *leading* positional that names an Outfit, which is applied and not forwarded (see "Alias registry" below). `alias`/`unalias` manage the registry that lets a name stand in for a path, and `writeAliases` renders it for both `alias --list` and `show`. Tests: `main_test.go` (dispatch/add/remove/list), `apply_test.go` (`apply`/`unapply`/`export`), `harness_test.go` (the `harness` command, `show`, and Pi-routed add/export/remove), `serve_test.go`, `alias_test.go`, `complete_test.go`. +- `cmd/outfit/complete.go` + `completion.bash` — tab completion, and the only reason the CLI is no longer a single source file: `//go:embed` needs the script beside it. `outfit completion bash` prints the script; the script calls the hidden `outfit __complete `, which prints one candidate per line and a final `:nofile`/`:file` directive. The `commands` table there mirrors `run`'s switch and each `cmdX`'s `FlagSet` (the `flag` package cannot enumerate a `FlagSet`); `TestCompletionCoversDispatch` scans `main.go` and fails when a new command is not completable. +- `internal/config` — outfit's own config file, `${XDG_CONFIG_HOME:-~/.config}/outfit/config.json`: the default-harness preference and the Outfit alias registry. A **leaf package** — stdlib imports only, and it must stay that way (in particular it must never import `internal/outfit` to parse an Outfit), because that is what lets `readOutfit` resolve a name without dragging in the harness abstraction. Every write goes through `Update`, a read-modify-write of the whole document; unknown top-level keys round-trip through `File.extra`. - `internal/outfit` — the `Outfit` file format and the shared `Selection` type: a flat, Dockerfile-style description of one provider selection (`PROVIDER`/`FAMILY`/`MODEL`/`ALIAS`/`CONTEXT`/`OUTPUT`/`BASEURL`/`PRESET`). `Parse` reads it into a `Selection` (keywords case-insensitive via `canonicalKeyword`, UPPERCASE canonical, `#` comments); `Format` renders one back out for `export`. `apply` defaults to `./Outfit` (`outfit.DefaultFile`). The harness is deliberately **not** a keyword — it is a runtime choice, so an Outfit stays portable. -- `internal/harness` — the harness abstraction. The `Harness` interface (`Name`/`Command`/`ConfigPath`/`Apply`/`Remove`/`State`), a `registry` of the opencode and pi adapters (`adapters.go`, which wrap `catalog` + `opencode`/`pi` + `contextsize`), runtime selection via `Resolve` with precedence **`--harness`/`-H` flag > `OUTFIT_HARNESS` env > stored preference > opencode**, and preference load/save (`LoadPreference`/`SavePreference`) in `${XDG_CONFIG_HOME:-~/.config}/outfit/config.json`. **Start here when adding a third harness:** implement the interface and register it. +- `internal/harness` — the harness abstraction. The `Harness` interface (`Name`/`Command`/`ConfigPath`/`Apply`/`Remove`/`State`), a `registry` of the opencode and pi adapters (`adapters.go`, which wrap `catalog` + `opencode`/`pi` + `contextsize`), runtime selection via `Resolve` with precedence **`--harness`/`-H` flag > `OUTFIT_HARNESS` env > stored preference > opencode**, and preference load/save (`LoadPreference`/`SavePreference`), which delegate to `internal/config` — that package, not this one, owns the config file. **Start here when adding a third harness:** implement the interface and register it. - `internal/catalog` — the embedded provider catalogue (`//go:embed providers.yaml`), its types, and the two block builders: `BuildProviderBlock` turns a provider+family+model selection into an opencode provider block; `BuildPiProvider` turns the same into a Pi provider entry. `MatchFamily` does the reverse for `export` (configured models -> family name). The catalogue can be overridden at runtime via the `--providers` flag or `OUTFIT_PROVIDERS` env var (flag > env > embedded; see `ResolveCatalogPath`/`LoadFrom`). `providers.yaml` lives here. - `internal/opencode` — opencode config IO: JSONC read/merge/write, env/key resolution, JSON-Pointer helpers. `LoadConfigState` reads the config back (configured providers, their model keys, the default model, context/output limits) for `export`. - `internal/pi` — Pi config IO: resolves `~/.pi/agent/models.json` (note: `~/.pi/agent/`, **not** XDG), deep-merges one managed provider entry into it preserving everything else (other providers, unknown fields), `Remove`, and reads provider state back for `export`. Written `0600`. @@ -35,7 +37,9 @@ The binary lives under `cmd/`; the domain logic is split into `internal/` packag ## Architecture notes (the important part) -**Harness routing.** `cmd` does the harness-neutral work (load the catalogue, validate the family/model, parse sizes, pick the provider) and then hands a `Selection` to the harness resolved by `harness.Resolve`. Each adapter owns its config format end-to-end. The import graph is one-way: `harness` imports `catalog`/`opencode`/`pi`/`contextsize`/`outfit`; none of those import `harness`, so there are no cycles. A model is keyed by its `ALIAS` when given, otherwise the provider-native `MODEL` (`modelKey` in `adapters.go`). +**Harness routing.** `cmd` does the harness-neutral work (load the catalogue, validate the family/model, parse sizes, pick the provider) and then hands a `Selection` to the harness resolved by `harness.Resolve`. Each adapter owns its config format end-to-end. The import graph is one-way: `harness` imports `catalog`/`opencode`/`pi`/`contextsize`/`outfit`/`config`; none of those import `harness`, and `config` sits at the bottom importing nothing, so there are no cycles. A model is keyed by its `ALIAS` when given, otherwise the provider-native `MODEL` (`modelKey` in `adapters.go`). + +**Alias registry.** `outfit alias` registers an Outfit under a name, and `readOutfit` — the single choke point `apply`/`unapply`/`serve`/`harness --outfit` all share — resolves it, so one change gives every command the shorthand. Three rules hold it together. (1) The lookup is gated on the argument being *name-shaped* (`config.NameShaped`), so a path-shaped argument never causes a config read at all; that is what keeps `serve` working with no config of ours present, and what keeps the test suite off the developer's real config. (2) A path on disk beats a registered name (`namesAnOutfit` decides), so registering an alias can never change what an already-working command does; the shadowing is reported, not silent. (3) The registry stores the absolute path of the **file**, never the directory, so `serve`'s relative-`PRESET` resolution still lands next to the aliased Outfit. Under `harness`, the first positional is taken as an Outfit only when no `--outfit` was given, flag parsing was not terminated by an explicit `--` (`flagsTerminated`), and it names one; a `--` immediately after a *consumed* Outfit is dropped, so every other command line is forwarded byte-for-byte. **In-place JSONC merge, never overwrite (opencode).** This is the core invariant of the opencode adapter. Understand it before touching `internal/opencode`: @@ -52,7 +56,7 @@ The binary lives under `cmd/`; the domain logic is split into `internal/` packag **`serve`.** `outfit serve` launches `llama-server` for an Outfit without touching any harness config. With a `PRESET` it flattens the matching `.ini` section (plus `[*]` defaults) into flags via `internal/preset`; otherwise it derives a command from `MODEL`/`ALIAS`/`CONTEXT`/`BASEURL`. Anything the Outfit states overrides the preset. `--dry-run`/`-n` prints the command without running it. -Invariants to keep: opencode merges stay idempotent and never drop unrelated config or comments; Pi merges preserve sibling providers and unknown fields; output stays `0600`; the harness never appears in an Outfit; and every family's `defaultModel` must be one of its `models` (enforced by `TestCatalogIntegrity`). +Invariants to keep: opencode merges stay idempotent and never drop unrelated config or comments; Pi merges preserve sibling providers and unknown fields; output stays `0600`; the harness never appears in an Outfit, and neither does an alias — both are machine-local, so an Outfit stays portable; outfit's own `config.json` is only ever written read-modify-write, so a new setting cannot clobber an existing one; `__complete` never returns an error and never writes to stderr, whatever the state of the config or catalogue; and every family's `defaultModel` must be one of its `models` (enforced by `TestCatalogIntegrity`). ## Reference diff --git a/README.md b/README.md index 4e52933f..d1df6c64 100644 --- a/README.md +++ b/README.md @@ -149,15 +149,21 @@ outfit add --provider [--model-family ] [--model ] [--alia outfit remove --provider [--model-family ] [--model ] outfit apply [path] [--output ] # apply an Outfit file or directory (default ./Outfit) outfit unapply [path] # remove what an Outfit file selects +outfit alias [path] [-n ] [-l] # name an Outfit; -l lists them +outfit unalias # drop a registered name outfit serve [path] [--dry-run] # run llama-server from the Outfit's PRESET outfit export [--provider ] # print the current config as an Outfit outfit init-providers [path] # write the built-in catalogue out to edit -outfit harness [-H ] [--outfit[=]] [args...] - # launch the harness (--outfit applies one first; - # --get shows it; --set stores the default) +outfit harness [] [-H ] [--outfit[=]] [args...] + # launch the harness (a leading Outfit or alias is + # applied first; --get shows it; --set stores it) +outfit completion bash # shell tab completion ``` -Short flags: `-p` (provider), `-f` (model-family), `-m` (model), `-a` (alias), `-c` (context), `-o` (output), `-u` (base-url), `-H` (harness), `-O` (outfit). +Short flags: `-p` (provider), `-f` (model-family), `-m` (model), `-a` (alias), `-c` (context), `-o` (output), `-u` (base-url), `-H` (harness), `-O` (outfit), and under `alias`: `-n` (name), `-l` (list), `-F` (force). + +Anywhere a `[path]` appears above you can put a name registered with +[`outfit alias`](#aliases) instead. ## Harnesses @@ -176,6 +182,7 @@ outfit harness -H pi # launch a specific harness, ignoring the default outfit harness -O # apply ./Outfit, then launch the harness outfit harness --outfit=path/to/Outfit # ...applying a specific one first +outfit harness qwen3.6-27b # ...or name one positionally (see Aliases) outfit show # what the active harness has configured outfit show --harness pi # ...for a specific harness, without changing the default @@ -226,6 +233,59 @@ An `Outfit` describes one provider selection and applies exactly like the equivalent `add`. Full syntax is in [`docs/outfit-file.md`](docs/outfit-file.md), and ready-to-use examples live under [`examples/`](examples/). +## Aliases + +Keeping a directory per model soon means typing a path per command. Name one +once with `outfit alias` and the name works wherever a path does: + +```sh +$ ls +Outfit preset.ini + +$ head -2 Outfit +ALIAS qwen3.6-27b +CONTEXT 128k + +$ outfit alias +Added alias "qwen3.6-27b" for /home/me/models/qwen3.6/Outfit … + +$ outfit apply qwen3.6-27b # from anywhere, no path needed +$ outfit serve qwen3.6-27b +$ outfit harness qwen3.6-27b -- --some-agent-arg +``` + +The name defaults to the `Outfit`'s own `ALIAS`; `--name`/`-n` picks a different +one, and is required when the file has no `ALIAS` to borrow — `outfit` never +invents a name you didn't write. `--force`/`-F` re-points a name you have +already used. + +```sh +outfit alias --list # every name, its Outfit, and whether it still exists +outfit show # ...alongside what the harness has configured +outfit unalias qwen3.6-27b # drop the name (the Outfit file is left alone) +``` + +A path on disk always wins over a registered name, so adding an alias can never +change what an already-working command does. Under `outfit harness`, only a +*leading* argument is read as an Outfit and everything after it goes to the +agent — put `--` between the two, and start with `--` to opt out entirely if a +name of yours collides with one of the agent's own subcommands. + +The registry lives in `outfit`'s own config +(`${XDG_CONFIG_HOME:-~/.config}/outfit/config.json`), not in any `Outfit`, so +your `Outfit` files stay as portable and committable as they were. + +### Tab completion + +```sh +source <(outfit completion bash) # add to ~/.bashrc +``` + +TAB then completes commands, flags, providers, harnesses, and your registered +aliases — `outfit unalias ` offers exactly the names you have. Homebrew +installs it for you. (zsh can use it via `autoload -U +X bashcompinit && +bashcompinit` first.) + ## Serving a local model Running a model with llama.cpp? `outfit serve` reads an `Outfit` and launches diff --git a/cmd/outfit/alias_test.go b/cmd/outfit/alias_test.go new file mode 100644 index 00000000..fc484bea --- /dev/null +++ b/cmd/outfit/alias_test.go @@ -0,0 +1,498 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/lucinate-ai/outfit/internal/config" + "github.com/lucinate-ai/outfit/internal/outfit" +) + +// registerOutfit writes an Outfit in a fresh directory and registers it under +// the name its ALIAS gives, returning the directory and the Outfit's path. +func registerOutfit(t *testing.T, body string) (dir, path string) { + t.Helper() + dir = t.TempDir() + path = filepath.Join(dir, outfit.DefaultFile) + mustWrite(t, path, body) + captureStdout(t, func() { + if err := cmdAlias([]string{dir}); err != nil { + t.Fatalf("cmdAlias: %v", err) + } + }) + return dir, path +} + +// storedAlias returns the path registered under name, failing if there is none. +func storedAlias(t *testing.T, name string) string { + t.Helper() + f, err := config.Load() + if err != nil { + t.Fatal(err) + } + path, ok := f.Alias(name) + if !ok { + t.Fatalf("alias %q is not registered (have %v)", name, f.AliasNames()) + } + return path +} + +// TestCmdAlias_DefaultsToOutfitALIAS checks that a bare `outfit alias` names the +// Outfit after its own ALIAS, and stores the absolute path to the file itself. +func TestCmdAlias_DefaultsToOutfitALIAS(t *testing.T) { + isolateConfig(t) + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, outfit.DefaultFile), "PROVIDER llamacpp\nALIAS qwen3.6-27b\nCONTEXT 128k\n") + t.Chdir(dir) + + out := captureStdout(t, func() { + if err := cmdAlias(nil); err != nil { + t.Fatalf("cmdAlias: %v", err) + } + }) + if !strings.Contains(out, `Added alias "qwen3.6-27b"`) { + t.Errorf("unexpected output:\n%s", out) + } + + // t.Chdir resolves through any symlinks in the temp path, so compare + // against the working directory rather than dir itself. + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + want := filepath.Join(cwd, outfit.DefaultFile) + if got := storedAlias(t, "qwen3.6-27b"); got != want { + t.Errorf("stored path = %q, want the absolute Outfit file %q", got, want) + } +} + +// TestCmdAlias_NameOverrideAndDirectoryArg checks that --name wins over the +// Outfit's ALIAS, and that a directory argument registers the Outfit inside it. +func TestCmdAlias_NameOverrideAndDirectoryArg(t *testing.T) { + isolateConfig(t) + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, outfit.DefaultFile), "PROVIDER llamacpp\nALIAS qwen3.6-27b\n") + + captureStdout(t, func() { + if err := cmdAlias([]string{"-n", "q3", dir}); err != nil { + t.Fatalf("cmdAlias -n: %v", err) + } + }) + + if got, want := storedAlias(t, "q3"), filepath.Join(dir, outfit.DefaultFile); got != want { + t.Errorf("stored path = %q, want %q", got, want) + } + if f, _ := config.Load(); len(f.AliasNames()) != 1 { + t.Errorf("--name should register one alias, got %v", f.AliasNames()) + } +} + +// TestCmdAlias_RequiresANameWhenOutfitHasNoALIAS checks that outfit never +// invents a name the Outfit does not state. +func TestCmdAlias_RequiresANameWhenOutfitHasNoALIAS(t *testing.T) { + isolateConfig(t) + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, outfit.DefaultFile), "PROVIDER llamacpp\nMODEL gemma\n") + + err := cmdAlias([]string{dir}) + if err == nil { + t.Fatal("expected an error for an Outfit with no ALIAS") + } + if !strings.Contains(err.Error(), "--name") { + t.Errorf("error %q does not point at --name", err) + } +} + +// TestCmdAlias_RefusesToRePointWithoutForce checks that an established name is +// not silently moved, and that --force moves it. +func TestCmdAlias_RefusesToRePointWithoutForce(t *testing.T) { + isolateConfig(t) + + _, first := registerOutfit(t, "PROVIDER llamacpp\nALIAS q3\n") + + second := t.TempDir() + mustWrite(t, filepath.Join(second, outfit.DefaultFile), "PROVIDER llamacpp\nALIAS q3\n") + + err := cmdAlias([]string{second}) + if err == nil { + t.Fatal("expected an error re-pointing an existing alias") + } + if !strings.Contains(err.Error(), "--force") { + t.Errorf("error %q does not mention --force", err) + } + if got := storedAlias(t, "q3"); got != first { + t.Errorf("alias moved without --force: %q", got) + } + + out := captureStdout(t, func() { + if err := cmdAlias([]string{"--force", second}); err != nil { + t.Fatalf("cmdAlias --force: %v", err) + } + }) + if !strings.Contains(out, "Re-pointed") { + t.Errorf("unexpected --force output:\n%s", out) + } + if got, want := storedAlias(t, "q3"), filepath.Join(second, outfit.DefaultFile); got != want { + t.Errorf("stored path = %q, want %q", got, want) + } +} + +// TestCmdAlias_ReRegisteringSamePathIsIdempotent checks that running the command +// twice in the same directory is a no-op rather than an error. +func TestCmdAlias_ReRegisteringSamePathIsIdempotent(t *testing.T) { + isolateConfig(t) + + dir, path := registerOutfit(t, "PROVIDER llamacpp\nALIAS q3\n") + + out := captureStdout(t, func() { + if err := cmdAlias([]string{dir}); err != nil { + t.Fatalf("second cmdAlias: %v", err) + } + }) + if !strings.Contains(out, "already points here") { + t.Errorf("unexpected repeat output:\n%s", out) + } + if got := storedAlias(t, "q3"); got != path { + t.Errorf("stored path = %q, want %q", got, path) + } +} + +// TestCmdAlias_RejectsBadNames checks that a name can never be confused with a +// path or a flag. +func TestCmdAlias_RejectsBadNames(t *testing.T) { + isolateConfig(t) + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, outfit.DefaultFile), "PROVIDER llamacpp\nALIAS q3\n") + + for _, name := range []string{"a/b", ".", "..", "a b"} { + if err := cmdAlias([]string{"-n", name, dir}); err == nil { + t.Errorf("cmdAlias -n %q = nil, want an error", name) + } + } + if f, _ := config.Load(); len(f.AliasNames()) != 0 { + t.Errorf("a rejected name was registered: %v", f.AliasNames()) + } +} + +// TestCmdAlias_RejectsUnparseableOutfit checks that an Outfit is validated at +// registration, so a broken file is caught here rather than days later. +func TestCmdAlias_RejectsUnparseableOutfit(t *testing.T) { + isolateConfig(t) + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, outfit.DefaultFile), "PROVIDER llamacpp\nNONSENSE x\n") + + if err := cmdAlias([]string{"-n", "q3", dir}); err == nil { + t.Fatal("expected an error registering an Outfit that does not parse") + } + if f, _ := config.Load(); len(f.AliasNames()) != 0 { + t.Errorf("a broken Outfit was registered: %v", f.AliasNames()) + } +} + +// TestCmdAlias_List checks the listing: the empty state, sorted entries, and +// the marker on an alias whose Outfit has since gone. +func TestCmdAlias_List(t *testing.T) { + isolateConfig(t) + + out := captureStdout(t, func() { + if err := cmdAlias([]string{"--list"}); err != nil { + t.Fatalf("cmdAlias --list: %v", err) + } + }) + if !strings.Contains(out, "No aliases registered") { + t.Errorf("unexpected empty-state output:\n%s", out) + } + + registerOutfit(t, "PROVIDER llamacpp\nALIAS qwen\n") + _, gone := registerOutfit(t, "PROVIDER llamacpp\nALIAS gemma\n") + + out = captureStdout(t, func() { + if err := cmdAlias([]string{"-l"}); err != nil { + t.Fatalf("cmdAlias -l: %v", err) + } + }) + if strings.Index(out, "gemma") > strings.Index(out, "qwen") { + t.Errorf("aliases are not sorted:\n%s", out) + } + if strings.Contains(out, "(missing)") { + t.Errorf("nothing is missing yet:\n%s", out) + } + + if err := os.Remove(gone); err != nil { + t.Fatal(err) + } + out = captureStdout(t, func() { + if err := cmdAlias([]string{"--list"}); err != nil { + t.Fatalf("cmdAlias --list: %v", err) + } + }) + for _, line := range strings.Split(out, "\n") { + if strings.Contains(line, "gemma") && !strings.Contains(line, "(missing)") { + t.Errorf("a dangling alias is not marked:\n%s", out) + } + if strings.Contains(line, "qwen") && strings.Contains(line, "(missing)") { + t.Errorf("a live alias is marked missing:\n%s", out) + } + } +} + +// TestCmdUnalias checks that a name can be dropped, that the Outfit survives it, +// and that the argument is validated. +func TestCmdUnalias(t *testing.T) { + isolateConfig(t) + + _, path := registerOutfit(t, "PROVIDER llamacpp\nALIAS q3\n") + + out := captureStdout(t, func() { + if err := cmdUnalias([]string{"q3"}); err != nil { + t.Fatalf("cmdUnalias: %v", err) + } + }) + if !strings.Contains(out, `Removed alias "q3"`) { + t.Errorf("unexpected output:\n%s", out) + } + if f, _ := config.Load(); len(f.AliasNames()) != 0 { + t.Errorf("alias survived unalias: %v", f.AliasNames()) + } + if _, err := os.Stat(path); err != nil { + t.Errorf("unalias removed the Outfit file itself: %v", err) + } + + if err := cmdUnalias([]string{"q3"}); err == nil { + t.Error("expected an error for an unknown alias") + } + if err := cmdUnalias(nil); err == nil { + t.Error("expected an error with no name") + } + if err := cmdUnalias([]string{"a", "b"}); err == nil { + t.Error("expected an error with two names") + } +} + +// TestAlias_PreservesHarnessPreference is the anti-clobber test at the level the +// user meets it: outfit's config holds both settings, and writing either must +// leave the other alone. +func TestAlias_PreservesHarnessPreference(t *testing.T) { + isolateConfig(t) + + captureStdout(t, func() { + if err := cmdHarness([]string{"--set", "pi"}); err != nil { + t.Fatalf("cmdHarness --set: %v", err) + } + }) + registerOutfit(t, "PROVIDER llamacpp\nALIAS q3\n") + + out := captureStdout(t, func() { + if err := cmdHarness([]string{"--get"}); err != nil { + t.Fatalf("cmdHarness --get: %v", err) + } + }) + if !strings.Contains(out, "Stored preference: pi") { + t.Errorf("registering an alias clobbered the harness preference:\n%s", out) + } + + // ...and the other way round. + captureStdout(t, func() { + if err := cmdHarness([]string{"--set", "opencode"}); err != nil { + t.Fatalf("cmdHarness --set: %v", err) + } + }) + storedAlias(t, "q3") +} + +// TestApply_ByAlias checks the point of the feature: a registered name works +// from a directory that has nothing to do with the Outfit. +func TestApply_ByAlias(t *testing.T) { + home := isolateConfig(t) + + registerOutfit(t, "PROVIDER llamacpp\nMODEL gemma\nALIAS q3\n") + t.Chdir(t.TempDir()) // somewhere else entirely + + out := captureStdout(t, func() { + if err := cmdApply([]string{"q3"}); err != nil { + t.Fatalf("cmdApply by alias: %v", err) + } + }) + if !strings.Contains(out, `Using alias "q3"`) { + t.Errorf("the alias was not reported:\n%s", out) + } + + m := readConfigMap(t, filepath.Join(home, ".config", "opencode", "opencode.json")) + if m["model"] != "llamacpp/q3" { + t.Errorf("default model = %v, want llamacpp/q3", m["model"]) + } +} + +// TestUnapply_ByAlias checks that the inverse resolves a name too — apply and +// unapply share readOutfit, but the pair is what the user actually relies on. +func TestUnapply_ByAlias(t *testing.T) { + home := isolateConfig(t) + + registerOutfit(t, "PROVIDER llamacpp\nMODEL gemma\nALIAS q3\n") + t.Chdir(t.TempDir()) + + captureStdout(t, func() { + if err := cmdApply([]string{"q3"}); err != nil { + t.Fatalf("cmdApply: %v", err) + } + if err := cmdUnapply([]string{"q3"}); err != nil { + t.Fatalf("cmdUnapply by alias: %v", err) + } + }) + + m := readConfigMap(t, filepath.Join(home, ".config", "opencode", "opencode.json")) + models := m["provider"].(map[string]any)["llamacpp"].(map[string]any)["models"].(map[string]any) + if len(models) != 0 { + t.Errorf("models survived unapply by alias: %v", models) + } + if m["model"] != nil && m["model"] != "" { + t.Errorf("default model survived unapply by alias: %v", m["model"]) + } +} + +// TestServe_ByAliasResolvesRelativePreset checks that an aliased Outfit still +// finds the preset sitting next to it — the reason the registry stores the +// Outfit file rather than the directory. +func TestServe_ByAliasResolvesRelativePreset(t *testing.T) { + isolateConfig(t) + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, outfit.DefaultFile), "PROVIDER llamacpp\nALIAS q3\nPRESET ./preset.ini\n") + mustWrite(t, filepath.Join(dir, "preset.ini"), "[q3]\nmodel = /models/q3.gguf\nngl = 99\n") + captureStdout(t, func() { + if err := cmdAlias([]string{dir}); err != nil { + t.Fatalf("cmdAlias: %v", err) + } + }) + + t.Chdir(t.TempDir()) // the preset is not resolvable from here by name alone + + out := captureStdout(t, func() { + // The path comes last: flag parsing stops at the first positional, so + // `serve q3 --dry-run` would hand --dry-run to llama-server. + if err := cmdServe([]string{"--dry-run", "q3"}); err != nil { + t.Fatalf("cmdServe by alias: %v", err) + } + }) + if !strings.Contains(out, filepath.Join(dir, "preset.ini")) { + t.Errorf("the preset next to the aliased Outfit was not found:\n%s", out) + } + if !strings.Contains(out, "--n-gpu-layers 99") { + t.Errorf("preset flags missing from the command:\n%s", out) + } +} + +// TestReadOutfit_PathBeatsAlias checks the precedence rule: registering an alias +// can never change what an already-working command does. +func TestReadOutfit_PathBeatsAlias(t *testing.T) { + home := isolateConfig(t) + + // An alias named "dev", pointing somewhere far away. + away := t.TempDir() + mustWrite(t, filepath.Join(away, outfit.DefaultFile), "PROVIDER llamacpp\nALIAS away\n") + captureStdout(t, func() { + if err := cmdAlias([]string{"-n", "dev", away}); err != nil { + t.Fatalf("cmdAlias: %v", err) + } + }) + + // A directory of the same name, here. + here := t.TempDir() + if err := os.Mkdir(filepath.Join(here, "dev"), 0o755); err != nil { + t.Fatal(err) + } + mustWrite(t, filepath.Join(here, "dev", outfit.DefaultFile), "PROVIDER llamacpp\nALIAS local\n") + t.Chdir(here) + + out := captureStdout(t, func() { + if err := cmdApply([]string{"dev"}); err != nil { + t.Fatalf("cmdApply: %v", err) + } + }) + if !strings.Contains(out, "names both a path here and a registered alias") { + t.Errorf("the shadowing was not reported:\n%s", out) + } + + m := readConfigMap(t, filepath.Join(home, ".config", "opencode", "opencode.json")) + if m["model"] != "llamacpp/local" { + t.Errorf("default model = %v, want llamacpp/local (the path, not the alias)", m["model"]) + } +} + +// TestReadOutfit_DanglingAlias checks that an alias whose Outfit has moved says +// so, and names both ways out. +func TestReadOutfit_DanglingAlias(t *testing.T) { + isolateConfig(t) + + _, path := registerOutfit(t, "PROVIDER llamacpp\nALIAS q3\n") + if err := os.Remove(path); err != nil { + t.Fatal(err) + } + t.Chdir(t.TempDir()) + + err := cmdApply([]string{"q3"}) + if err == nil { + t.Fatal("expected an error for a dangling alias") + } + for _, want := range []string{"is gone", path, "outfit alias", "outfit unalias"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err, want) + } + } +} + +// TestReadOutfit_UnknownNameFallsThroughToPath checks that a name nobody +// registered still fails the way it always has. +func TestReadOutfit_UnknownNameFallsThroughToPath(t *testing.T) { + isolateConfig(t) + t.Chdir(t.TempDir()) + + err := cmdApply([]string{"nope"}) + if err == nil { + t.Fatal("expected an error") + } + if !strings.Contains(err.Error(), "reading nope") { + t.Errorf("error = %q, want the plain read failure", err) + } +} + +// TestCmdShow_ListsAliases checks that `outfit show` reports the registry too, +// including when no provider is configured. +func TestCmdShow_ListsAliases(t *testing.T) { + isolateConfig(t) + + registerOutfit(t, "PROVIDER llamacpp\nALIAS q3\n") + + out := captureStdout(t, func() { + if err := cmdShow(nil); err != nil { + t.Fatalf("cmdShow: %v", err) + } + }) + if !strings.Contains(out, "No providers configured") { + t.Fatalf("expected an unconfigured harness:\n%s", out) + } + if !strings.Contains(out, "Aliases:") || !strings.Contains(out, "q3") { + t.Errorf("aliases missing from an empty show:\n%s", out) + } + + captureStdout(t, func() { + if err := cmdApply([]string{"q3"}); err != nil { + t.Fatalf("cmdApply: %v", err) + } + }) + out = captureStdout(t, func() { + if err := cmdShow(nil); err != nil { + t.Fatalf("cmdShow: %v", err) + } + }) + if !strings.Contains(out, "Aliases:") || !strings.Contains(out, "q3") { + t.Errorf("aliases missing from a configured show:\n%s", out) + } +} diff --git a/cmd/outfit/complete.go b/cmd/outfit/complete.go new file mode 100644 index 00000000..62c9053a --- /dev/null +++ b/cmd/outfit/complete.go @@ -0,0 +1,360 @@ +package main + +// Tab completion. `outfit completion bash` prints the script below; the script +// calls the hidden `outfit __complete` with every word up to the cursor, and +// this file works out what could come next. +// +// The command and flag tables here mirror run()'s switch and each cmdX's +// FlagSet. That duplication is deliberate — the flag package offers no way to +// enumerate a FlagSet before it is parsed — and TestCompletionCoversDispatch +// guards the half of it that matters. + +import ( + _ "embed" + "fmt" + "sort" + "strings" + + "github.com/lucinate-ai/outfit/internal/catalog" + "github.com/lucinate-ai/outfit/internal/config" + "github.com/lucinate-ai/outfit/internal/harness" +) + +//go:embed completion.bash +var bashCompletion string + +// Directives tell the shell script whether to offer paths alongside whatever +// candidates were printed. +const ( + directiveNoFile = ":nofile" + directiveFile = ":file" +) + +// candidateKind says what belongs in a given slot — a flag's value, or a +// command's positional argument. +type candidateKind int + +const ( + kindNone candidateKind = iota // no candidates, no paths (also: bool flags) + kindFile // paths only + kindAlias // registered aliases, or a path + kindAliasOnly // registered aliases; a path would be meaningless + kindHarness + kindProvider + kindFamily + kindModel + kindShell +) + +// command describes one subcommand's completable surface. +type command struct { + // flags is every flag the command accepts, as typed. + flags []string + // values maps a value-taking flag to what its value is. A flag absent from + // this map is a boolean and consumes no following word. + values map[string]candidateKind + // positional is what the command's first positional argument accepts, and + // positionals how many arguments that applies to. + positional candidateKind + positionals int +} + +// selectionFlags are the flags add and remove share (see parseSelection). +var selectionFlags = []string{ + "--provider", "-p", "--model-family", "-f", "--model", "-m", + "--alias", "-a", "--context", "-c", "--output", "-o", + "--providers", "--base-url", "-u", "--harness", "-H", +} + +// selectionValues maps those of them that take a value. +var selectionValues = map[string]candidateKind{ + "--provider": kindProvider, "-p": kindProvider, + "--model-family": kindFamily, "-f": kindFamily, + "--model": kindModel, "-m": kindModel, + "--alias": kindNone, "-a": kindNone, + "--context": kindNone, "-c": kindNone, + "--output": kindNone, "-o": kindNone, + "--providers": kindFile, + "--base-url": kindNone, "-u": kindNone, + "--harness": kindHarness, "-H": kindHarness, +} + +// commands is the completable command surface. Keep it in step with run()'s +// switch; TestCompletionCoversDispatch fails when a command is added there and +// forgotten here. +var commands = map[string]command{ + "add": {flags: selectionFlags, values: selectionValues}, + "remove": {flags: selectionFlags, values: selectionValues}, + "list": { + flags: []string{"--providers"}, + values: map[string]candidateKind{"--providers": kindFile}, + }, + "show": { + flags: []string{"--harness", "-H"}, + values: map[string]candidateKind{"--harness": kindHarness, "-H": kindHarness}, + }, + "apply": { + flags: []string{"--providers", "--output", "-o", "--harness", "-H"}, + values: map[string]candidateKind{ + "--providers": kindFile, "--output": kindNone, "-o": kindNone, + "--harness": kindHarness, "-H": kindHarness, + }, + positional: kindAlias, positionals: 1, + }, + "unapply": { + flags: []string{"--providers", "--harness", "-H"}, + values: map[string]candidateKind{ + "--providers": kindFile, "--harness": kindHarness, "-H": kindHarness, + }, + positional: kindAlias, positionals: 1, + }, + "alias": { + flags: []string{"--name", "-n", "--force", "-F", "--list", "-l"}, + values: map[string]candidateKind{"--name": kindNone, "-n": kindNone}, + positional: kindAlias, positionals: 1, + }, + "unalias": {positional: kindAliasOnly, positionals: 1}, + "serve": { + flags: []string{"--dry-run", "-n"}, + positional: kindAlias, positionals: 1, + }, + "export": { + flags: []string{"--provider", "-p", "--providers", "--harness", "-H"}, + values: map[string]candidateKind{ + "--provider": kindProvider, "-p": kindProvider, + "--providers": kindFile, "--harness": kindHarness, "-H": kindHarness, + }, + }, + "init-providers": { + flags: []string{"--force", "-F"}, + positional: kindFile, positionals: 1, + }, + "harness": { + flags: []string{"--set", "--get", "--harness", "-H", "--outfit", "-O", "--providers"}, + values: map[string]candidateKind{ + "--set": kindHarness, "--harness": kindHarness, "-H": kindHarness, + // --outfit's value is optional, so it is not consumed as a separate + // word; naming it here is what completes `--outfit=`. + "--outfit": kindAlias, "-O": kindAlias, + "--providers": kindFile, + }, + // Only the first argument can name an Outfit; the rest are the + // harness's own, and outfit has nothing to say about them. + positional: kindAlias, positionals: 1, + }, + "completion": {positional: kindShell, positionals: 1}, + "version": {}, + "help": {}, +} + +// cmdCompletion prints the shell completion script. +func cmdCompletion(args []string) error { + if len(args) == 0 { + return fmt.Errorf("completion needs a shell (supported: bash)") + } + switch args[0] { + case "bash": + fmt.Print(bashCompletion) + return nil + default: + return fmt.Errorf("unsupported shell %q (supported: bash)", args[0]) + } +} + +// cmdComplete prints the completion candidates for a partially-typed command +// line, one per line, followed by a directive telling the shell whether paths +// belong in the list too. args is every word after `outfit` up to and including +// the one under the cursor, which may be empty. +// +// It never returns an error and never writes to stderr: a completion helper +// that failed would spew over the user's prompt. A config it cannot read, a +// catalogue it cannot load, and nonsense input all mean "no candidates". +func cmdComplete(args []string) error { + candidates, directive := completions(args) + for _, c := range candidates { + fmt.Println(c) + } + fmt.Println(directive) + return nil +} + +// completions works out what could follow the words typed so far. +func completions(args []string) ([]string, string) { + if len(args) == 0 { + return commandNames(), directiveNoFile + } + cur, words := args[len(args)-1], args[:len(args)-1] + if len(words) == 0 { + return commandNames(), directiveNoFile + } + cmd, ok := commands[words[0]] + if !ok { + return nil, directiveNoFile + } + + // bash breaks words on "=", so `--outfit=` arrives as + // [..., "--outfit", "=", ""]. Look back past the separator for the flag + // being given a value — this is the only way to complete --outfit/-O, whose + // value has to be attached. + prev := words[len(words)-1] + if prev == "=" && len(words) > 1 { + prev = words[len(words)-2] + } + if kind, ok := cmd.values[prev]; ok { + return candidatesFor(kind, words) + } + + if strings.HasPrefix(cur, "-") { + return cmd.flags, directiveNoFile + } + if positionalsUsed(cmd, words[1:]) >= cmd.positionals { + return nil, directiveNoFile + } + return candidatesFor(cmd.positional, words) +} + +// positionalsUsed counts the arguments already given to a command, skipping +// flags and the words they consume as values. +func positionalsUsed(cmd command, words []string) int { + n, skip := 0, false + for _, w := range words { + switch { + case skip: + skip = false + case w == "=": + // Part of a --flag=value triple; the value follows and is not a + // positional either. + skip = true + case strings.HasPrefix(w, "-"): + // A flag with an attached value consumes nothing further. + if _, takesValue := cmd.values[w]; takesValue && !strings.Contains(w, "=") { + skip = true + } + default: + n++ + } + } + return n +} + +// commandNames returns the commands worth completing, in stable order. The +// hidden __complete is not among them. +func commandNames() []string { + names := make([]string, 0, len(commands)) + for name := range commands { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// candidatesFor returns the candidates for one slot. words is everything typed +// before the cursor, which a few kinds consult for context. +func candidatesFor(kind candidateKind, words []string) ([]string, string) { + switch kind { + case kindFile: + return nil, directiveFile + case kindAlias: + return aliasNames(), directiveFile + case kindAliasOnly: + return aliasNames(), directiveNoFile + case kindHarness: + return harness.Names(), directiveNoFile + case kindProvider: + cat, err := loadCatalogFor(words) + if err != nil { + return nil, directiveNoFile + } + return cat.SortedProviderNames(), directiveNoFile + case kindFamily: + p := providerOn(words) + if p == nil { + return nil, directiveNoFile + } + return p.SortedFamilyNames(), directiveNoFile + case kindModel: + p := providerOn(words) + if p == nil { + return nil, directiveNoFile + } + return modelNames(p, flagValue(words, "--model-family", "-f")), directiveNoFile + case kindShell: + return []string{"bash"}, directiveNoFile + default: + return nil, directiveNoFile + } +} + +// aliasNames returns the registered alias names, or none when the config +// cannot be read. +func aliasNames() []string { + f, err := config.Load() + if err != nil { + return nil + } + return f.AliasNames() +} + +// loadCatalogFor loads the provider catalogue, honouring a --providers override +// already on the command line. +func loadCatalogFor(words []string) (*catalog.Catalog, error) { + return catalog.LoadFrom(catalog.ResolveCatalogPath(flagValue(words, "--providers"))) +} + +// providerOn returns the provider named by --provider/-p on the command line so +// far, or nil when there is none (or the catalogue will not load). +func providerOn(words []string) *catalog.Provider { + name := flagValue(words, "--provider", "-p") + if name == "" { + return nil + } + cat, err := loadCatalogFor(words) + if err != nil { + return nil + } + return cat.Providers[name] +} + +// modelNames returns the model keys of one family, or of every family when none +// is named. +func modelNames(p *catalog.Provider, family string) []string { + if fam, ok := p.Families[family]; ok { + return fam.ModelKeys() + } + var models []string + for _, name := range p.SortedFamilyNames() { + models = append(models, p.Families[name].ModelKeys()...) + } + sort.Strings(models) + return models +} + +// flagValue finds the value already given to one of names, in either the +// attached (--flag=value, which bash splits into three words) or detached +// (--flag value) form. It returns "" when the flag is not on the line. +func flagValue(words []string, names ...string) string { + matches := func(w string) bool { + for _, name := range names { + if w == name { + return true + } + } + return false + } + for i, w := range words { + if prefix, value, ok := strings.Cut(w, "="); ok && matches(prefix) { + return value + } + if !matches(w) { + continue + } + // --flag = value (bash's split), or --flag value. + if i+2 < len(words) && words[i+1] == "=" { + return words[i+2] + } + if i+1 < len(words) { + return words[i+1] + } + } + return "" +} diff --git a/cmd/outfit/complete_test.go b/cmd/outfit/complete_test.go new file mode 100644 index 00000000..cb072589 --- /dev/null +++ b/cmd/outfit/complete_test.go @@ -0,0 +1,301 @@ +package main + +import ( + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// complete runs the hidden completion helper and returns its candidate lines +// and the trailing directive. +func complete(t *testing.T, words ...string) (candidates []string, directive string) { + t.Helper() + out := captureStdout(t, func() { + if err := cmdComplete(words); err != nil { + t.Fatalf("cmdComplete(%v): %v", words, err) + } + }) + for _, line := range strings.Split(strings.TrimRight(out, "\n"), "\n") { + switch { + case line == "": + case strings.HasPrefix(line, ":"): + directive = line + default: + candidates = append(candidates, line) + } + } + return candidates, directive +} + +// hasAll reports whether every want is among got. +func hasAll(got []string, want ...string) bool { + seen := map[string]bool{} + for _, g := range got { + seen[g] = true + } + for _, w := range want { + if !seen[w] { + return false + } + } + return true +} + +// TestComplete_CommandNames checks that the first word offers the commands, and +// that the completion helper itself stays hidden. +func TestComplete_CommandNames(t *testing.T) { + isolateConfig(t) + + got, directive := complete(t, "") + if !hasAll(got, "alias", "unalias", "apply", "unapply", "serve", "harness", "show", "completion") { + t.Errorf("commands missing from %v", got) + } + for _, name := range got { + if name == "__complete" { + t.Error("the hidden __complete command should not be offered") + } + } + if directive != directiveNoFile { + t.Errorf("directive = %q, want %q", directive, directiveNoFile) + } + + // No words at all is the same question. + if got, _ := complete(t); !hasAll(got, "alias", "unalias") { + t.Errorf("commands missing with no words: %v", got) + } +} + +// TestCompletionCoversDispatch is the drift guard: a command added to run()'s +// switch has to be completable too. +func TestCompletionCoversDispatch(t *testing.T) { + src, err := os.ReadFile("main.go") + if err != nil { + t.Fatal(err) + } + // Only the dispatch switch in run() matches "case" lines holding bare + // string literals at one tab of indentation. + re := regexp.MustCompile(`(?m)^\tcase "([^"]+)"(?:, "[^"]+")*:`) + found := 0 + for _, m := range re.FindAllStringSubmatch(string(src), -1) { + name := m[1] + if name == "__complete" || strings.HasPrefix(name, "-") { + continue + } + found++ + if _, ok := commands[name]; !ok { + t.Errorf("command %q is dispatched but not completable (add it to commands in complete.go)", name) + } + } + if found < 10 { + t.Fatalf("only found %d dispatched commands; the scan is not matching run()'s switch", found) + } +} + +// TestComplete_UnaliasOffersAliasNames checks the case the feature exists for, +// including that a path makes no sense there. +func TestComplete_UnaliasOffersAliasNames(t *testing.T) { + isolateConfig(t) + + got, directive := complete(t, "unalias", "") + if len(got) != 0 { + t.Errorf("candidates with an empty registry: %v", got) + } + if directive != directiveNoFile { + t.Errorf("directive = %q, want %q", directive, directiveNoFile) + } + + registerOutfit(t, "PROVIDER llamacpp\nALIAS qwen\n") + registerOutfit(t, "PROVIDER llamacpp\nALIAS gemma\n") + + got, directive = complete(t, "unalias", "") + if !hasAll(got, "gemma", "qwen") { + t.Errorf("aliases missing from %v", got) + } + if directive != directiveNoFile { + t.Errorf("directive = %q, want %q (a path cannot be unaliased)", directive, directiveNoFile) + } + + // Only one name is taken. + if got, _ := complete(t, "unalias", "qwen", ""); len(got) != 0 { + t.Errorf("a second argument was offered candidates: %v", got) + } +} + +// TestComplete_OutfitCommandsOfferAliasesAndPaths checks the commands that take +// either. +func TestComplete_OutfitCommandsOfferAliasesAndPaths(t *testing.T) { + isolateConfig(t) + registerOutfit(t, "PROVIDER llamacpp\nALIAS qwen\n") + + for _, cmd := range []string{"apply", "unapply", "serve", "alias", "harness"} { + got, directive := complete(t, cmd, "") + if !hasAll(got, "qwen") { + t.Errorf("%s: aliases missing from %v", cmd, got) + } + if directive != directiveFile { + t.Errorf("%s: directive = %q, want %q", cmd, directive, directiveFile) + } + } +} + +// TestComplete_HarnessStopsAfterTheOutfit checks that outfit offers nothing for +// the arguments that belong to the launched agent. +func TestComplete_HarnessStopsAfterTheOutfit(t *testing.T) { + isolateConfig(t) + registerOutfit(t, "PROVIDER llamacpp\nALIAS qwen\n") + + got, directive := complete(t, "harness", "qwen", "") + if len(got) != 0 { + t.Errorf("candidates offered for the harness's own args: %v", got) + } + if directive != directiveNoFile { + t.Errorf("directive = %q, want %q", directive, directiveNoFile) + } + + // A flag and its value do not count as the Outfit. + if got, _ := complete(t, "harness", "-H", "pi", ""); !hasAll(got, "qwen") { + t.Errorf("a detached flag value was mistaken for the Outfit: %v", got) + } +} + +// TestComplete_FlagNames checks that a leading dash offers the command's own +// flags, not another command's. +func TestComplete_FlagNames(t *testing.T) { + isolateConfig(t) + + got, directive := complete(t, "alias", "-") + if !hasAll(got, "--name", "-n", "--force", "-F", "--list", "-l") { + t.Errorf("alias flags missing from %v", got) + } + if directive != directiveNoFile { + t.Errorf("directive = %q, want %q", directive, directiveNoFile) + } + + if got, _ := complete(t, "serve", "-"); !hasAll(got, "--dry-run", "-n") { + t.Errorf("serve flags missing from %v", got) + } +} + +// TestComplete_FlagValues checks the values that can be enumerated. +func TestComplete_FlagValues(t *testing.T) { + isolateConfig(t) + + for _, words := range [][]string{ + {"harness", "--set", ""}, + {"harness", "-H", ""}, + {"apply", "--harness", ""}, + } { + if got, _ := complete(t, words...); !hasAll(got, "opencode", "pi") { + t.Errorf("%v: harnesses missing from %v", words, got) + } + } + + if got, _ := complete(t, "add", "--provider", ""); !hasAll(got, "llamacpp", "openrouter") { + t.Errorf("providers missing from %v", got) + } + if got, _ := complete(t, "add", "-p", "llamacpp", "-f", ""); len(got) == 0 { + t.Error("no families offered for a named provider") + } + if got, _ := complete(t, "add", "-f", ""); len(got) != 0 { + t.Errorf("families offered with no provider named: %v", got) + } + if _, directive := complete(t, "apply", "--providers", ""); directive != directiveFile { + t.Errorf("--providers should complete paths, got %q", directive) + } + if got, _ := complete(t, "completion", ""); !hasAll(got, "bash") { + t.Errorf("shells missing from %v", got) + } +} + +// TestComplete_EqualsForm checks the attached-value form, which is how bash +// hands over `--outfit=` — the only way that flag can take a path at all. +func TestComplete_EqualsForm(t *testing.T) { + isolateConfig(t) + registerOutfit(t, "PROVIDER llamacpp\nALIAS qwen\n") + + got, directive := complete(t, "harness", "--outfit", "=", "") + if !hasAll(got, "qwen") { + t.Errorf("aliases missing from %v", got) + } + if directive != directiveFile { + t.Errorf("directive = %q, want %q", directive, directiveFile) + } + + // And the same for a flag whose value is a provider. + if got, _ := complete(t, "add", "--provider", "=", ""); !hasAll(got, "llamacpp") { + t.Errorf("providers missing from %v", got) + } + // A family still resolves against a provider given in that form. + if got, _ := complete(t, "add", "--provider", "=", "llamacpp", "-f", ""); len(got) == 0 { + t.Error("no families offered for a provider given as --provider=") + } +} + +// TestComplete_NeverErrors checks the one hard rule: whatever the state of the +// machine, completion prints candidates or nothing — never a failure. +func TestComplete_NeverErrors(t *testing.T) { + home := isolateConfig(t) + + // A corrupt config must not stop alias-less completion working. + configPath := filepath.Join(home, ".config", "outfit", "config.json") + if err := os.MkdirAll(filepath.Dir(configPath), 0o700); err != nil { + t.Fatal(err) + } + mustWrite(t, configPath, "{not json") + + for _, words := range [][]string{ + nil, + {""}, + {"-"}, + {"nonsense", ""}, + {"unalias", ""}, + {"apply", ""}, + {"harness", "--outfit", "=", ""}, + {"add", "--providers", "/nope/providers.yaml", "-p", ""}, + {"alias", "--name"}, + } { + out := captureStdout(t, func() { + if err := cmdComplete(words); err != nil { + t.Errorf("cmdComplete(%v) = %v, want nil", words, err) + } + }) + if !strings.Contains(out, ":") { + t.Errorf("cmdComplete(%v) printed no directive: %q", words, out) + } + } +} + +// TestCompletionCommand checks the script the shell actually sources. +func TestCompletionCommand(t *testing.T) { + out := captureStdout(t, func() { + if err := cmdCompletion([]string{"bash"}); err != nil { + t.Fatalf("cmdCompletion bash: %v", err) + } + }) + for _, want := range []string{"_outfit()", "complete -F _outfit outfit", "__complete"} { + if !strings.Contains(out, want) { + t.Errorf("script is missing %q:\n%s", want, out) + } + } + + if err := cmdCompletion(nil); err == nil { + t.Error("expected an error with no shell named") + } + if err := cmdCompletion([]string{"zsh"}); err == nil { + t.Error("expected an error for an unsupported shell") + } + + // The script has to be valid bash, not just plausible text. + bash, err := exec.LookPath("bash") + if err != nil { + t.Skip("bash not installed") + } + script := filepath.Join(t.TempDir(), "outfit.bash") + mustWrite(t, script, out) + if got, err := exec.Command(bash, "-n", script).CombinedOutput(); err != nil { + t.Errorf("bash -n rejected the completion script: %v\n%s", err, got) + } +} diff --git a/cmd/outfit/completion.bash b/cmd/outfit/completion.bash new file mode 100644 index 00000000..4a69e2d8 --- /dev/null +++ b/cmd/outfit/completion.bash @@ -0,0 +1,52 @@ +# bash completion for outfit. Install with: +# +# source <(outfit completion bash) +# +# or drop this file into /etc/bash_completion.d/ (or, on macOS, +# "$(brew --prefix)/etc/bash_completion.d/"). +# +# The candidates come from the binary itself, via the hidden `outfit __complete` +# command: it is handed every word up to the cursor and prints one candidate per +# line, then a directive line saying whether paths belong in the list too. + +_outfit() { + local cur out directive line + local -a candidates=() + + cur="${COMP_WORDS[COMP_CWORD]}" + # Call whichever binary is on the command line, not whatever "outfit" happens + # to resolve to now. + out="$("${COMP_WORDS[0]}" __complete "${COMP_WORDS[@]:1:COMP_CWORD}" 2>/dev/null)" + + directive=":nofile" + while IFS= read -r line; do + case "${line}" in + :*) directive="${line}" ;; + "") ;; + *) candidates+=("${line}") ;; + esac + done <<< "${out}" + + COMPREPLY=() + if ((${#candidates[@]})); then + # Alias names and flags never contain whitespace, so splitting the + # candidate list on newlines alone is safe. + local IFS=$'\n' + COMPREPLY=($(compgen -W "${candidates[*]}" -- "${cur}")) + fi + + case "${directive}" in + :file) + COMPREPLY+=($(compgen -f -- "${cur}")) + compopt -o filenames 2>/dev/null + ;; + # Nothing emits :dir yet. It is handled anyway so a later binary can start + # using it without everyone re-sourcing this script. + :dir) + COMPREPLY+=($(compgen -d -- "${cur}")) + compopt -o filenames 2>/dev/null + ;; + esac +} + +complete -F _outfit outfit diff --git a/cmd/outfit/harness_test.go b/cmd/outfit/harness_test.go index fd1bc425..6b9563ee 100644 --- a/cmd/outfit/harness_test.go +++ b/cmd/outfit/harness_test.go @@ -197,6 +197,166 @@ func TestHarness_GetDoesNotApply(t *testing.T) { } } +// launchedArgs runs cmdHarness against a stubbed binary and returns what the +// harness was launched with, plus everything outfit printed on the way. +func launchedArgs(t *testing.T, args []string) (forwarded, out string) { + t.Helper() + argsFile := filepath.Join(t.TempDir(), "args") + stubHarnessBinary(t, "opencode", argsFile) + out = captureStdout(t, func() { + if err := cmdHarness(args); err != nil { + t.Fatalf("cmdHarness %v: %v", args, err) + } + }) + got, err := os.ReadFile(argsFile) + if err != nil { + t.Fatalf("harness was not launched: %v", err) + } + return strings.TrimSpace(string(got)), out +} + +// aliasFor registers an Outfit selecting model under name, returning the +// Outfit's path. +func aliasFor(t *testing.T, name, model string) string { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "Outfit") + mustWrite(t, path, "PROVIDER llamacpp\nMODEL "+model+"\n") + captureStdout(t, func() { + if err := cmdAlias([]string{"-n", name, dir}); err != nil { + t.Fatalf("cmdAlias: %v", err) + } + }) + return path +} + +// TestHarness_LeadingAliasAppliesThenLaunches checks the shorthand the alias +// registry exists for: `outfit harness -- ` wears the Outfit and +// hands the harness only its own arguments. +func TestHarness_LeadingAliasAppliesThenLaunches(t *testing.T) { + home := isolateConfig(t) + path := aliasFor(t, "q3", "gemma") + + forwarded, out := launchedArgs(t, []string{"q3", "--", "run", "hello"}) + if !strings.Contains(out, "Applying "+path) { + t.Errorf("apply not reported:\n%s", out) + } + if forwarded != "run\nhello" { + t.Errorf("forwarded args = %q, want \"run\\nhello\" (alias and -- consumed)", forwarded) + } + + m := readConfigMap(t, filepath.Join(home, ".config", "opencode", "opencode.json")) + if m["model"] != "llamacpp/gemma" { + t.Errorf("the alias was not applied: default model = %v", m["model"]) + } +} + +// TestHarness_LeadingAliasWithoutDashDash checks that the separator is optional. +func TestHarness_LeadingAliasWithoutDashDash(t *testing.T) { + isolateConfig(t) + aliasFor(t, "q3", "gemma") + + if forwarded, _ := launchedArgs(t, []string{"q3", "run"}); forwarded != "run" { + t.Errorf("forwarded args = %q, want \"run\"", forwarded) + } +} + +// TestHarness_LeadingPositionalIsForwardedWhenNotAnOutfit is the regression +// guard for the whole feature: an ordinary harness argument must reach the +// harness untouched, `--` and all. +func TestHarness_LeadingPositionalIsForwardedWhenNotAnOutfit(t *testing.T) { + home := isolateConfig(t) + + forwarded, _ := launchedArgs(t, []string{"run", "--", "--flag"}) + if forwarded != "run\n--\n--flag" { + t.Errorf("forwarded args = %q, want them verbatim", forwarded) + } + if _, err := os.Stat(filepath.Join(home, ".config", "opencode", "opencode.json")); !os.IsNotExist(err) { + t.Error("nothing should have been applied") + } +} + +// TestHarness_DashDashProtectsAnAliasNamedLikeASubcommand checks the escape +// hatch for a name that collides with one of the harness's own commands. +func TestHarness_DashDashProtectsAnAliasNamedLikeASubcommand(t *testing.T) { + home := isolateConfig(t) + aliasFor(t, "run", "gemma") + + if forwarded, _ := launchedArgs(t, []string{"--", "run"}); forwarded != "run" { + t.Errorf("forwarded args = %q, want \"run\"", forwarded) + } + if _, err := os.Stat(filepath.Join(home, ".config", "opencode", "opencode.json")); !os.IsNotExist(err) { + t.Error("an explicit -- should have opted out of applying the alias") + } +} + +// TestHarness_TerminatorAfterDetachedFlagValue checks that the terminator is +// still recognised when a flag value precedes it — the case a naive scan for +// the first non-flag token gets wrong. +func TestHarness_TerminatorAfterDetachedFlagValue(t *testing.T) { + home := isolateConfig(t) + aliasFor(t, "run", "gemma") + + if forwarded, _ := launchedArgs(t, []string{"-H", "opencode", "--", "run"}); forwarded != "run" { + t.Errorf("forwarded args = %q, want \"run\"", forwarded) + } + if _, err := os.Stat(filepath.Join(home, ".config", "opencode", "opencode.json")); !os.IsNotExist(err) { + t.Error("the terminator after a detached flag value was missed") + } +} + +// TestHarness_LeadingOutfitPathApplies checks that a path works positionally +// too, so `harness` reads like apply and serve. +func TestHarness_LeadingOutfitPathApplies(t *testing.T) { + home := isolateConfig(t) + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "Outfit"), "PROVIDER llamacpp\nMODEL gemma\n") + + if forwarded, _ := launchedArgs(t, []string{filepath.Join(dir, "Outfit"), "--", "run"}); forwarded != "run" { + t.Errorf("forwarded args = %q, want \"run\"", forwarded) + } + m := readConfigMap(t, filepath.Join(home, ".config", "opencode", "opencode.json")) + if m["model"] != "llamacpp/gemma" { + t.Errorf("the positional Outfit was not applied: default model = %v", m["model"]) + } +} + +// TestHarness_ExplicitOutfitFlagBeatsLeadingPositional checks that --outfit +// keeps its meaning: with it, positional args are the harness's again. +func TestHarness_ExplicitOutfitFlagBeatsLeadingPositional(t *testing.T) { + home := isolateConfig(t) + aliasFor(t, "q3", "gemma") + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "Outfit"), "PROVIDER llamacpp\nMODEL other\n") + + forwarded, _ := launchedArgs(t, []string{"--outfit=" + filepath.Join(dir, "Outfit"), "q3"}) + if forwarded != "q3" { + t.Errorf("forwarded args = %q, want \"q3\"", forwarded) + } + m := readConfigMap(t, filepath.Join(home, ".config", "opencode", "opencode.json")) + if m["model"] != "llamacpp/other" { + t.Errorf("default model = %v, want llamacpp/other (the flag, not the positional)", m["model"]) + } +} + +// TestHarness_DetachedAliasIsCaught checks that a detached alias is treated like +// a detached path: a typo to point out, not an Outfit to guess at. +func TestHarness_DetachedAliasIsCaught(t *testing.T) { + isolateConfig(t) + aliasFor(t, "q3", "gemma") + + dir := t.TempDir() + mustWrite(t, filepath.Join(dir, "Outfit"), "PROVIDER llamacpp\nMODEL other\n") + t.Chdir(dir) + + err := cmdHarness([]string{"-O", "q3"}) + if err == nil || !strings.Contains(err.Error(), "--outfit=q3") { + t.Errorf("expected the detached-value hint, got %v", err) + } +} + func TestCmdShow(t *testing.T) { isolateConfig(t) t.Setenv("DEEPSEEK_API_KEY", "sk-or-v1-test") diff --git a/cmd/outfit/main.go b/cmd/outfit/main.go index 45d2a135..24a2e924 100644 --- a/cmd/outfit/main.go +++ b/cmd/outfit/main.go @@ -17,11 +17,14 @@ // outfit remove --provider [--model-family ] [--model ] // outfit apply [path] # apply an Outfit file (defaults to ./Outfit) // outfit unapply [path] # remove what an Outfit file selects +// outfit alias [path] [-n ] # name an Outfit; -l lists them +// outfit unalias # drop a registered name // outfit serve [path] # run llama-server for the Outfit (PRESET or MODEL) // outfit export [-p name] # print the current config as an Outfit // outfit init-providers [path] # write the embedded providers.yaml out // outfit harness [-H name] [-O[=path]] # launch the harness, optionally applying an // # Outfit first (--get shows it; --set stores the default) +// outfit completion bash # print the tab-completion script // // Short flags: -p (provider), -f (model-family), -m (model), -a (alias), // -c (context), -o (output), -u (base-url), -H (harness), -O (outfit). @@ -34,6 +37,11 @@ // selection, applied with `outfit apply` and reverted with `outfit unapply`; // see the internal/outfit package. The harness is deliberately not part of an // Outfit, so the same Outfit applies to any harness. +// +// `outfit alias` registers an Outfit under a short name — by default its own +// ALIAS — and every command that takes a path takes that name instead. The +// registry is machine-local state in outfit's own config, not part of any +// Outfit, so an Outfit stays as portable as it was. package main import ( @@ -49,6 +57,7 @@ import ( "strings" "github.com/lucinate-ai/outfit/internal/catalog" + "github.com/lucinate-ai/outfit/internal/config" "github.com/lucinate-ai/outfit/internal/contextsize" "github.com/lucinate-ai/outfit/internal/harness" "github.com/lucinate-ai/outfit/internal/outfit" @@ -86,6 +95,10 @@ func run(args []string) error { return cmdApply(rest) case "unapply": return cmdUnapply(rest) + case "alias": + return cmdAlias(rest) + case "unalias": + return cmdUnalias(rest) case "serve": return cmdServe(rest) case "export": @@ -94,6 +107,11 @@ func run(args []string) error { return cmdInitProviders(rest) case "harness": return cmdHarness(rest) + case "completion": + return cmdCompletion(rest) + case "__complete": + // Hidden: the completion script's way of asking what could come next. + return cmdComplete(rest) case "version", "-v", "--version": fmt.Println(version) return nil @@ -116,11 +134,14 @@ Usage: outfit remove --provider [--model-family ] [--model ] outfit apply [path] [--output ] (defaults to ./Outfit) outfit unapply [path] (remove what an Outfit selects) + outfit alias [path] [-n ] [-l] (name an Outfit; -l lists them) + outfit unalias (drop a registered name) outfit serve [path] [--dry-run] (run llama-server from the PRESET) outfit export [--provider ] outfit init-providers [path] (defaults to ./providers.yaml) - outfit harness [-H ] [--outfit[=]] [args...] + outfit harness [] [-H ] [--outfit[=]] [args...] (launch the harness; available: %s) + outfit completion bash (shell tab completion; see below) outfit version (or -v/--version) Flags: @@ -153,6 +174,12 @@ apply: applies an Outfit file — a declarative, Dockerfile-style description of one provider selection — as if you had run the equivalent add. unapply: removes what an Outfit file selects, as if you had run the equivalent remove. The inverse of apply. +alias: registers an Outfit under a short name, which then stands in wherever an + Outfit path goes (apply, unapply, serve, harness). The name defaults to + the Outfit's own ALIAS; --name/-n picks another, --force/-F re-points a + name already registered, and --list/-l shows them all. A path on disk + always wins over a name, so registering one changes nothing that works. +unalias: drops a registered name. The Outfit file itself is left alone. serve: runs llama-server for the Outfit. With a PRESET (a llama.cpp .ini) it turns the matching section into the command; otherwise it derives one from MODEL/ALIAS/CONTEXT/BASEURL. Prints the command before running it; @@ -162,13 +189,20 @@ init-providers: writes the binary's built-in providers.yaml to the working directory (or [path]) so you can customise the catalogue and point outfit at it with --providers/OUTFIT_PROVIDERS. Refuses to overwrite an existing file unless --force is given. -harness: launches the active harness, forwarding any trailing args to it. +harness: launches the active harness, forwarding any trailing args to it. A + leading argument that names an Outfit — a registered alias or a path — is + applied first and not forwarded; put -- before the harness's own args to + keep them, and a leading -- opts out of this entirely. --outfit/-O applies an Outfit first, as if you had run apply before it. --get prints the active harness instead of launching it; --set stores the default harness and exits. Honours -H/--harness and OUTFIT_HARNESS. show: lists the providers and models actually configured in the active harness's - config (where list shows the catalogue of what you could configure). + config (where list shows the catalogue of what you could configure), and + the aliases you have registered. +completion: prints a tab-completion script. Add + source <(outfit completion bash) to your ~/.bashrc and TAB then completes + commands, flags, providers, harnesses, and your registered aliases. `, strings.Join(harness.Names(), ", ")) } @@ -283,22 +317,34 @@ func applySelection(sel outfit.Selection, h harness.Harness) error { // readOutfit reads and parses the Outfit at path, defaulting to ./Outfit when // path is empty so a bare command works in a directory that holds one. When // path names a directory, the default Outfit file inside it is used, so a -// caller can pass either the file itself or the directory that holds it. usage -// shows the caller's own way of naming a path (subcommands take it positionally, -// `harness` takes it as a flag) in the not-found hint. It returns the parsed -// selection alongside the resolved path, which callers use to locate files -// referenced relative to the Outfit. +// caller can pass either the file itself or the directory that holds it. path +// may also be a name registered with `outfit alias`, which is what gives every +// Outfit command the same shorthand. usage shows the caller's own way of naming +// a path (subcommands take it positionally, `harness` takes it as a flag) in +// the not-found hint. It returns the parsed selection alongside the resolved +// path, which callers use to locate files referenced relative to the Outfit. +// +// It prints when an alias decided the path, which is against this file's +// convention that only cmdX functions write to stdout. The alternative is to +// repeat that reporting at all four call sites, where one omission would leave +// the user guessing which file was read. func readOutfit(usage, path string) (outfit.Selection, string, error) { if path == "" { path = outfit.DefaultFile } + if aliased, ok, err := resolveAlias(path); err != nil { + return outfit.Selection{}, path, err + } else if ok { + fmt.Printf("Using alias %q (%s)\n\n", path, aliased) + path = aliased + } if info, err := os.Stat(path); err == nil && info.IsDir() { path = filepath.Join(path, outfit.DefaultFile) } data, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) && path == outfit.DefaultFile { - return outfit.Selection{}, path, fmt.Errorf("no %s found in the current directory (pass a path: %s)", outfit.DefaultFile, usage) + return outfit.Selection{}, path, fmt.Errorf("no %s found in the current directory (pass a path or an alias: %s; see `outfit alias --list`)", outfit.DefaultFile, usage) } return outfit.Selection{}, path, fmt.Errorf("reading %s: %w", path, err) } @@ -309,6 +355,40 @@ func readOutfit(usage, path string) (outfit.Selection, string, error) { return sel, path, nil } +// resolveAlias looks arg up in the alias registry, returning the Outfit it +// names. It reports ok=false — leaving the caller to treat arg as a path — +// when arg is not a registered name, or when it also names something on disk. +// +// The name-shaped guard is not an optimisation: it means a path-shaped argument +// never causes a config read at all, so the commands that have nothing to do +// with outfit's own config (serve, most of all) keep working the same way when +// that config is absent, unreadable, or someone else's. +// +// A path on disk beats a registered alias, which is the opposite of how shell +// aliases work and deliberate: every existing invocation passes a path, so +// registering an alias must never change what an already-working command does. +func resolveAlias(arg string) (string, bool, error) { + if !config.NameShaped(arg) { + return "", false, nil + } + f, err := config.Load() + if err != nil { + return "", false, err + } + path, ok := f.Alias(arg) + if !ok { + return "", false, nil + } + if namesAnOutfit(arg) { + fmt.Printf("Note: %q names both a path here and a registered alias; using the path.\n\n", arg) + return "", false, nil + } + if _, err := os.Stat(path); err != nil { + return "", false, fmt.Errorf("alias %q points at %s, which is gone — re-point it with `outfit alias -n %s `, or drop it with `outfit unalias %s`", arg, path, arg, arg) + } + return path, true, nil +} + // cmdApply reads an Outfit file and applies it. The path defaults to ./Outfit // when none is given, so a bare `outfit apply` works in a directory that // holds one. @@ -375,6 +455,172 @@ func cmdUnapply(args []string) error { return removeSelection(sel, h) } +// cmdAlias registers an Outfit under a short name, so it can be used anywhere a +// path goes: `outfit apply `, `outfit serve `, `outfit harness +// `. The name defaults to the Outfit's own ALIAS instruction. +// +// Note the two senses of "alias", which are related but not the same thing: the +// ALIAS keyword inside an Outfit names the model to the harness (and to +// llama-server under `serve`), while an alias in this registry names the Outfit +// file to outfit. Taking one from the other is a convenience, not an identity — +// --name/-n decouples them. +func cmdAlias(args []string) error { + fs := flag.NewFlagSet("alias", flag.ContinueOnError) + var name string + var force, list bool + fs.StringVar(&name, "name", "", "register under this name instead of the Outfit's ALIAS") + fs.StringVar(&name, "n", "", "name to register under (shorthand)") + fs.BoolVar(&force, "force", false, "re-point a name that is already registered") + fs.BoolVar(&force, "F", false, "re-point an existing name (shorthand)") + fs.BoolVar(&list, "list", false, "list the registered aliases") + fs.BoolVar(&list, "l", false, "list the registered aliases (shorthand)") + if err := fs.Parse(args); err != nil { + return err + } + + if list { + var b strings.Builder + if err := writeAliases(&b, true); err != nil { + return err + } + fmt.Print(b.String()) + return nil + } + + var arg string + if rest := fs.Args(); len(rest) > 0 { + arg = rest[0] + } + // Parse the Outfit even when --name is given: registering a file that does + // not parse is a mistake worth catching now, not days later under `serve`. + sel, path, err := readOutfit("outfit alias [path]", arg) + if err != nil { + return err + } + if name == "" { + name = sel.Alias + } + if name == "" { + return fmt.Errorf("%s has no ALIAS to name it by — pass one with --name/-n (outfit alias -n [path])", path) + } + if err := config.ValidAliasName(name); err != nil { + return err + } + // Store an absolute path so the alias resolves from any working directory, + // and the Outfit file rather than its directory so a relative PRESET still + // resolves against the Outfit's own directory under `serve`. + abs, err := filepath.Abs(path) + if err != nil { + return err + } + + var previous string + if err := config.Update(func(f *config.File) error { + if existing, ok := f.Alias(name); ok { + if existing == abs { + previous = existing + return nil + } + if !force { + return fmt.Errorf("alias %q already points at %s; use --force to re-point it", name, existing) + } + previous = existing + } + f.SetAlias(name, abs) + return nil + }); err != nil { + return err + } + + switch { + case previous == abs: + fmt.Printf("Alias %q already points here (%s).\n", name, abs) + case previous != "": + fmt.Printf("Re-pointed alias %q to %s (was %s).\n", name, abs, previous) + default: + fmt.Printf("Added alias %q for %s (stored in %s).\n\n", name, abs, config.Path()) + fmt.Println("Use it anywhere an Outfit path goes:") + fmt.Printf(" outfit apply %s\n", name) + fmt.Printf(" outfit serve %s\n", name) + fmt.Printf(" outfit harness %s\n", name) + } + return nil +} + +// cmdUnalias drops a registered alias. The Outfit it pointed at is untouched — +// only the name goes away. +func cmdUnalias(args []string) error { + fs := flag.NewFlagSet("unalias", flag.ContinueOnError) + if err := fs.Parse(args); err != nil { + return err + } + + rest := fs.Args() + switch { + case len(rest) == 0: + return fmt.Errorf("unalias needs an alias name (see `outfit alias --list`)") + case len(rest) > 1: + return fmt.Errorf("unalias takes a single alias name, got %d", len(rest)) + } + name := rest[0] + + var previous string + if err := config.Update(func(f *config.File) error { + path, ok := f.Alias(name) + if !ok { + return fmt.Errorf("unknown alias %q (see `outfit alias --list`)", name) + } + previous = path + f.RemoveAlias(name) + return nil + }); err != nil { + return err + } + + fmt.Printf("Removed alias %q (was %s).\n", name, previous) + return nil +} + +// writeAliases renders the alias registry into b: every registered name with +// the Outfit it points at, marking any whose file has since gone. It is shared +// by `outfit alias --list` and `outfit show`. header controls the heading and +// the empty-state line, which `show` leaves out — there it is one section among +// several, and an empty registry is not worth a paragraph. +func writeAliases(b *strings.Builder, header bool) error { + f, err := config.Load() + if err != nil { + return err + } + names := f.AliasNames() + if len(names) == 0 { + if header { + b.WriteString("No aliases registered. Add one with `outfit alias` in a directory holding an Outfit.\n") + } + return nil + } + + width := 0 + for _, name := range names { + if len(name) > width { + width = len(name) + } + } + if header { + fmt.Fprintf(b, "Aliases (from %s):\n\n", config.Path()) + } else { + b.WriteString("\nAliases:\n") + } + for _, name := range names { + path, _ := f.Alias(name) + line := fmt.Sprintf(" %-*s %s", width, name, path) + if _, err := os.Stat(path); err != nil { + line += " (missing)" + } + b.WriteString(line + "\n") + } + return nil +} + // llamaServerBinary is the llama.cpp server executable that `serve` launches. // It is a package var so tests can point it at a stub instead of a real build. var llamaServerBinary = "llama-server" @@ -787,15 +1033,34 @@ func cmdHarness(args []string) error { return nil } + // Take the first positional argument as the Outfit to wear when it names + // one — a registered alias, a path, or a directory holding one. Everything + // else is forwarded to the harness untouched, so this can only claim an + // argument the harness could not have used anyway. An explicit `--` opts + // out, for an alias that collides with one of the harness's own + // subcommands. + rest := fs.Args() + if !outfitPath.set && !flagsTerminated(args, rest) && len(rest) > 0 && namesAnOutfitOrAlias(rest[0]) { + outfitPath.set, outfitPath.path = true, rest[0] + // Reslice rather than rebuild: rest shares its backing array with args, + // so appending to it would write over the caller's arguments. + rest = rest[1:] + // The `--` that separated outfit's Outfit from the harness's own args is + // ours to drop; any other `--` belongs to the harness and is forwarded. + if len(rest) > 0 && rest[0] == "--" { + rest = rest[1:] + } + } + if outfitPath.set { - if err := applyBeforeLaunch(outfitPath, providers, h, fs.Args()); err != nil { + if err := applyBeforeLaunch(outfitPath, providers, h, rest); err != nil { return err } } // Launch the harness, forwarding stdio and any trailing args. bin := h.Command() - cmd := exec.Command(bin, fs.Args()...) + cmd := exec.Command(bin, rest...) cmd.Stdin = os.Stdin cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr @@ -813,14 +1078,49 @@ func cmdHarness(args []string) error { return nil } +// flagsTerminated reports whether flag parsing stopped at an explicit bare +// `--`. The flag package consumes that terminator without reporting it, so the +// only reliable trace is the last argument it swallowed — scanning args for a +// `--` before the first non-flag token would misread a detached flag value +// (`outfit harness -H pi -- run`). +func flagsTerminated(args, rest []string) bool { + n := len(args) - len(rest) + return n > 0 && args[n-1] == "--" +} + +// namesAnOutfitOrAlias reports whether arg is a way of naming an Outfit: a path +// to one (or a directory holding one), or a registered alias. It is the +// predicate for taking `harness`'s first positional argument as an Outfit +// rather than forwarding it to the harness. +// +// A config that cannot be read is treated as "no aliases" rather than an error: +// the argument is then forwarded and the harness still launches. Someone who +// meant an alias gets the real parse error from `outfit apply `, where it +// is actionable. +func namesAnOutfitOrAlias(arg string) bool { + if namesAnOutfit(arg) { + return true + } + if !config.NameShaped(arg) { + return false + } + f, err := config.Load() + if err != nil { + return false + } + _, ok := f.Alias(arg) + return ok +} + // applyBeforeLaunch applies the Outfit named by --outfit/-O to the harness that // is about to be launched — exactly the work `outfit apply` does, so one command // can dress the harness and then run it. rest is what will be forwarded to the // harness, inspected only to catch a path that was meant for the flag. func applyBeforeLaunch(f outfitPathFlag, providers string, h harness.Harness, rest []string) error { - // The flag's value has to be attached, so `--outfit ./dev/Outfit` would - // otherwise apply ./Outfit and quietly hand the path to the harness. - if f.path == "" && len(rest) > 0 && namesAnOutfit(rest[0]) { + // The flag's value has to be attached, so `--outfit ./dev/Outfit` (or + // `--outfit q3`) would otherwise apply ./Outfit and quietly hand the path + // or alias to the harness. + if f.path == "" && len(rest) > 0 && namesAnOutfitOrAlias(rest[0]) { return fmt.Errorf("--outfit takes its path attached: --outfit=%s (a bare --outfit applies ./%s)", rest[0], outfit.DefaultFile) } sel, path, err := readOutfit("outfit harness --outfit=", f.path) @@ -839,7 +1139,10 @@ func applyBeforeLaunch(f outfitPathFlag, providers string, h harness.Harness, re } // namesAnOutfit reports whether arg points at an Outfit file, or at a directory -// holding one — the two shapes readOutfit accepts. +// holding one — the two shapes readOutfit accepts on disk. It is the shared +// "this string denotes an Outfit here" predicate: it decides whether a path +// beats a registered alias of the same name, and whether `harness` takes its +// first positional argument rather than forwarding it. func namesAnOutfit(arg string) bool { info, err := os.Stat(arg) if err != nil { @@ -889,6 +1192,9 @@ func cmdShow(args []string) error { if len(states) == 0 { b.WriteString("\nNo providers configured. Add one with `outfit add`.\n") + if err := writeAliases(&b, false); err != nil { + return err + } fmt.Print(b.String()) return nil } @@ -925,6 +1231,9 @@ func cmdShow(args []string) error { b.WriteString(line + "\n") } } + if err := writeAliases(&b, false); err != nil { + return err + } fmt.Print(b.String()) return nil } diff --git a/cmd/outfit/main_test.go b/cmd/outfit/main_test.go index 70284302..f4c5dc7d 100644 --- a/cmd/outfit/main_test.go +++ b/cmd/outfit/main_test.go @@ -71,6 +71,22 @@ func TestRunDispatch(t *testing.T) { if err := run([]string{"bogus"}); err == nil { t.Error("unknown command should error") } + + // The alias commands are routed, not swallowed by the default case. Only + // unalias is exercised: a bare `alias` would try to register whatever + // ./Outfit this test happens to run beside, and this test has no sandbox. + if err := run([]string{"unalias"}); err == nil { + t.Error("unalias with no name should error") + } else if strings.Contains(err.Error(), "unknown command") { + t.Errorf("unalias is not dispatched: %v", err) + } + + // The completion helper is dispatched and, whatever it is asked, succeeds. + captureStdout(t, func() { + if err := run([]string{"__complete", ""}); err != nil { + t.Errorf("__complete should never error, got %v", err) + } + }) } func TestVersionFlag(t *testing.T) { diff --git a/docs/outfit-file.md b/docs/outfit-file.md index 64a68e5a..e37fe515 100644 --- a/docs/outfit-file.md +++ b/docs/outfit-file.md @@ -51,7 +51,39 @@ outfit harness --outfit=path/to/dir # ...or a directory holding an Outfit Given bare, `--outfit` defaults to `./Outfit` like `apply` does; when you name a path, attach it to the flag, because anything positional is forwarded to the -agent (`outfit harness -O run --model x` passes `run --model x` on). +agent (`outfit harness -O run --model x` passes `run --model x` on). The one +exception is a *leading* argument that names an Outfit — a path, a directory +holding one, or a registered name — which is applied rather than forwarded. + +## Naming an Outfit + +`outfit alias` registers an `Outfit` under a short name, and every command above +takes that name where it takes a path: + +```sh +outfit alias # register ./Outfit under its own ALIAS +outfit alias path/to/dir # ...or the Outfit in another directory +outfit alias -n big . # ...under a name of your choosing +outfit alias --list # what is registered, and whether it still exists +outfit unalias big # drop the name; the Outfit file is untouched + +outfit apply big +outfit serve big +outfit harness big -- --agent-arg +``` + +Two senses of "alias" meet here, and they are not the same thing. The `ALIAS` +**instruction** below names the *model* — it is the key your agent shows, and +`llama-server --alias` under `serve`. A registered **alias** names the *Outfit +file* to `outfit`. `outfit alias` defaults the second to the first because you +have usually already written a good name; `--name`/`-n` separates them, and is +required when an `Outfit` states no `ALIAS` at all. + +A real path always beats a registered name, so registering one cannot change a +command that already worked. Names are stored, with absolute paths, in +`outfit`'s own config (`${XDG_CONFIG_HOME:-~/.config}/outfit/config.json`) — +they are yours and this machine's, never part of an `Outfit`, so a committed +`Outfit` stays portable. To undo it again, `outfit unapply` (same default path and arguments) removes what the Outfit selects from the active harness's config — the inverse of diff --git a/internal/config/config.go b/internal/config/config.go new file mode 100644 index 00000000..85373efd --- /dev/null +++ b/internal/config/config.go @@ -0,0 +1,208 @@ +// Package config owns outfit's own config file — the small JSON document under +// ${XDG_CONFIG_HOME:-~/.config}/outfit/config.json holding the machine-local +// state: the default-harness preference and the Outfit alias registry. +// +// It is a leaf package: it imports nothing else from this module, so both +// internal/harness (for the preference) and cmd (for aliases) can depend on it +// without a cycle, and locating an Outfit stays harness-agnostic. In particular +// it must never import internal/outfit — parsing an Outfit to find its ALIAS is +// the caller's job; this package is a dumb key/value store. +// +// Every write goes through Update, which is a read-modify-write of the whole +// document. That is the invariant: storing an alias must not clobber the +// harness preference, and vice versa. +package config + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "unicode" +) + +// fileName is outfit's own config file, inside its config directory. +const fileName = "config.json" + +// Path returns the config file's location: $XDG_CONFIG_HOME/outfit/config.json, +// falling back to ~/.config when the variable is unset. +func Path() string { + dir := os.Getenv("XDG_CONFIG_HOME") + if dir == "" { + home, _ := os.UserHomeDir() + dir = filepath.Join(home, ".config") + } + return filepath.Join(dir, "outfit", fileName) +} + +// File is outfit's config document. +// +// Unknown top-level keys are round-tripped through extra, so a read-modify-write +// by this version never drops what another version wrote — the same courtesy +// internal/pi extends to Pi's models.json. (The reverse is not possible: a +// binary predating the alias registry rewrites the whole document and will drop +// the aliases key.) +type File struct { + Harness string `json:"harness,omitempty"` + Aliases map[string]string `json:"aliases,omitempty"` + + // extra holds top-level keys this version does not know about. + extra map[string]json.RawMessage +} + +// The top-level keys this version owns; everything else lands in extra. +const ( + keyHarness = "harness" + keyAliases = "aliases" +) + +// Load reads the config file. A missing file is not an error — it yields an +// empty File, so a first run needs no special case. +func Load() (*File, error) { + f := &File{} + data, err := os.ReadFile(Path()) + if err != nil { + if os.IsNotExist(err) { + return f, nil + } + return nil, err + } + + var doc map[string]json.RawMessage + if err := json.Unmarshal(data, &doc); err != nil { + return nil, fmt.Errorf("parsing %s: %w", Path(), err) + } + for key, raw := range doc { + switch key { + case keyHarness: + if err := json.Unmarshal(raw, &f.Harness); err != nil { + return nil, fmt.Errorf("parsing %s: %s: %w", Path(), key, err) + } + case keyAliases: + if err := json.Unmarshal(raw, &f.Aliases); err != nil { + return nil, fmt.Errorf("parsing %s: %s: %w", Path(), key, err) + } + default: + if f.extra == nil { + f.extra = map[string]json.RawMessage{} + } + f.extra[key] = raw + } + } + return f, nil +} + +// Save writes f back, creating the directory 0700 and the file 0600 — it can +// sit alongside secrets, and it records where the user's projects live. Callers +// should prefer Update, which reads first. +func (f *File) Save() error { + doc := map[string]json.RawMessage{} + for key, raw := range f.extra { + doc[key] = raw + } + set := func(key string, value any, keep bool) error { + if !keep { + delete(doc, key) + return nil + } + raw, err := json.Marshal(value) + if err != nil { + return err + } + doc[key] = raw + return nil + } + if err := set(keyHarness, f.Harness, f.Harness != ""); err != nil { + return err + } + if err := set(keyAliases, f.Aliases, len(f.Aliases) > 0); err != nil { + return err + } + + data, err := json.MarshalIndent(doc, "", " ") + if err != nil { + return err + } + data = append(data, '\n') + + path := Path() + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return err + } + return os.WriteFile(path, data, 0o600) +} + +// Update loads the config, hands it to mutate, and saves the result. This is +// the only way callers should write: it guarantees an unrelated setting +// survives. +func Update(mutate func(*File) error) error { + f, err := Load() + if err != nil { + return err + } + if err := mutate(f); err != nil { + return err + } + return f.Save() +} + +// Alias returns the Outfit path registered under name. +func (f *File) Alias(name string) (string, bool) { + path, ok := f.Aliases[name] + return path, ok +} + +// SetAlias points name at path, creating the registry if this is the first one. +func (f *File) SetAlias(name, path string) { + if f.Aliases == nil { + f.Aliases = map[string]string{} + } + f.Aliases[name] = path +} + +// RemoveAlias drops name, reporting whether it was registered. +func (f *File) RemoveAlias(name string) bool { + if _, ok := f.Aliases[name]; !ok { + return false + } + delete(f.Aliases, name) + return true +} + +// AliasNames returns the registered names in stable order. +func (f *File) AliasNames() []string { + names := make([]string, 0, len(f.Aliases)) + for name := range f.Aliases { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// ValidAliasName checks that name can be typed where an Outfit path goes. It +// has to be a plain name: anything path-shaped could be confused with a file, +// and anything flag-shaped could not be passed to `outfit unalias` at all. +func ValidAliasName(name string) error { + switch { + case name == "": + return fmt.Errorf("an alias name cannot be empty") + case strings.ContainsAny(name, `/\`): + // Rejected on every platform, not just Windows: the name has to mean + // the same thing in a config file that travels between machines. + return fmt.Errorf("alias name %q cannot contain a path separator", name) + case name == "." || name == "..": + return fmt.Errorf("alias name %q looks like a path — an alias is a plain name (e.g. qwen3.6-27b)", name) + case strings.HasPrefix(name, "-"): + return fmt.Errorf(`alias name %q cannot start with "-": it would parse as a flag`, name) + case strings.IndexFunc(name, unicode.IsSpace) >= 0: + return fmt.Errorf("alias name %q cannot contain whitespace", name) + } + return nil +} + +// NameShaped reports whether s could be an alias name — the cheap test callers +// use before consulting the registry at all, so a path-shaped argument never +// causes a config read. +func NameShaped(s string) bool { return ValidAliasName(s) == nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 00000000..ed5ab223 --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,278 @@ +package config + +import ( + "encoding/json" + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// isolate points Path at a temporary directory so tests never touch the real +// config. +func isolate(t *testing.T) string { + t.Helper() + dir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", dir) + return dir +} + +// readRaw reads the config file as a generic JSON document, for assertions +// about keys this package does not model. +func readRaw(t *testing.T) map[string]any { + t.Helper() + data, err := os.ReadFile(Path()) + if err != nil { + t.Fatal(err) + } + var doc map[string]any + if err := json.Unmarshal(data, &doc); err != nil { + t.Fatal(err) + } + return doc +} + +// TestLoadMissingFile checks that a first run — no config file at all — yields +// an empty File rather than an error, and that the zero value is safe to read. +func TestLoadMissingFile(t *testing.T) { + isolate(t) + + f, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if f.Harness != "" { + t.Errorf("Harness = %q, want empty", f.Harness) + } + if _, ok := f.Alias("nope"); ok { + t.Error("Alias found something in an empty config") + } + if names := f.AliasNames(); len(names) != 0 { + t.Errorf("AliasNames = %v, want none", names) + } +} + +// TestSaveLoadRoundTrip checks that everything the document models survives a +// write and read, and that the file and its directory are written tightly. +func TestSaveLoadRoundTrip(t *testing.T) { + dir := isolate(t) + + f := &File{Harness: "pi"} + f.SetAlias("qwen3.6-27b", "/models/qwen/Outfit") + f.SetAlias("gemma4", "/models/gemma/Outfit") + if err := f.Save(); err != nil { + t.Fatalf("Save: %v", err) + } + + got, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if got.Harness != "pi" { + t.Errorf("Harness = %q, want pi", got.Harness) + } + if path, ok := got.Alias("qwen3.6-27b"); !ok || path != "/models/qwen/Outfit" { + t.Errorf("Alias = %q, %v", path, ok) + } + if want := []string{"gemma4", "qwen3.6-27b"}; !reflect.DeepEqual(got.AliasNames(), want) { + t.Errorf("AliasNames = %v, want %v (sorted)", got.AliasNames(), want) + } + + info, err := os.Stat(Path()) + if err != nil { + t.Fatal(err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("config file mode = %v, want 0600", perm) + } + dirInfo, err := os.Stat(filepath.Join(dir, "outfit")) + if err != nil { + t.Fatal(err) + } + if perm := dirInfo.Mode().Perm(); perm != 0o700 { + t.Errorf("config dir mode = %v, want 0700", perm) + } +} + +// TestSaveOmitsEmptyKeys checks that a config holding only aliases does not +// record an empty harness, and vice versa. +func TestSaveOmitsEmptyKeys(t *testing.T) { + isolate(t) + + f := &File{} + f.SetAlias("q3", "/models/qwen/Outfit") + if err := f.Save(); err != nil { + t.Fatalf("Save: %v", err) + } + doc := readRaw(t) + if _, ok := doc[keyHarness]; ok { + t.Errorf("harness key written for an alias-only config: %v", doc) + } + + if err := Update(func(f *File) error { + f.RemoveAlias("q3") + f.Harness = "pi" + return nil + }); err != nil { + t.Fatalf("Update: %v", err) + } + doc = readRaw(t) + if _, ok := doc[keyAliases]; ok { + t.Errorf("aliases key kept after the last alias was removed: %v", doc) + } +} + +// TestUpdatePreservesUnknownKeys checks that a read-modify-write keeps +// top-level keys this version does not model, so a newer (or foreign) writer's +// settings are not silently dropped. +func TestUpdatePreservesUnknownKeys(t *testing.T) { + dir := isolate(t) + + if err := os.MkdirAll(filepath.Join(dir, "outfit"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(Path(), []byte(`{"harness":"pi","future":{"x":1}}`), 0o600); err != nil { + t.Fatal(err) + } + + if err := Update(func(f *File) error { + f.SetAlias("q3", "/models/qwen/Outfit") + return nil + }); err != nil { + t.Fatalf("Update: %v", err) + } + + doc := readRaw(t) + future, ok := doc["future"].(map[string]any) + if !ok { + t.Fatalf("unknown key was dropped: %v", doc) + } + if future["x"] != float64(1) { + t.Errorf("future.x = %v, want 1", future["x"]) + } + if doc[keyHarness] != "pi" { + t.Errorf("harness = %v, want pi", doc[keyHarness]) + } +} + +// TestUpdateIsReadModifyWrite is the anti-clobber guarantee: two independent +// settings, written one at a time, must both survive. +func TestUpdateIsReadModifyWrite(t *testing.T) { + isolate(t) + + if err := Update(func(f *File) error { f.Harness = "pi"; return nil }); err != nil { + t.Fatalf("Update: %v", err) + } + if err := Update(func(f *File) error { + f.SetAlias("q3", "/models/qwen/Outfit") + return nil + }); err != nil { + t.Fatalf("Update: %v", err) + } + + f, err := Load() + if err != nil { + t.Fatalf("Load: %v", err) + } + if f.Harness != "pi" { + t.Errorf("Harness = %q, want pi — storing an alias clobbered it", f.Harness) + } + if _, ok := f.Alias("q3"); !ok { + t.Error("alias missing after a second Update") + } +} + +// TestUpdateStopsOnMutateError checks that a mutate that fails writes nothing. +func TestUpdateStopsOnMutateError(t *testing.T) { + isolate(t) + + wantErr := os.ErrInvalid + if err := Update(func(f *File) error { + f.SetAlias("q3", "/models/qwen/Outfit") + return wantErr + }); err != wantErr { + t.Fatalf("Update error = %v, want %v", err, wantErr) + } + if _, err := os.Stat(Path()); !os.IsNotExist(err) { + t.Errorf("config was written despite a mutate error (stat: %v)", err) + } +} + +// TestSetRemoveAlias checks the registry primitives, including SetAlias on the +// nil map a fresh File carries. +func TestSetRemoveAlias(t *testing.T) { + f := &File{} + f.SetAlias("q3", "/a/Outfit") + if path, ok := f.Alias("q3"); !ok || path != "/a/Outfit" { + t.Errorf("Alias = %q, %v", path, ok) + } + if f.RemoveAlias("nope") { + t.Error("RemoveAlias reported a removal for an unregistered name") + } + if !f.RemoveAlias("q3") { + t.Error("RemoveAlias did not report removing a registered name") + } + if _, ok := f.Alias("q3"); ok { + t.Error("alias survived RemoveAlias") + } +} + +// TestValidAliasName checks that a name can never be confused with a path or a +// flag. +func TestValidAliasName(t *testing.T) { + bad := []string{"", "a/b", `a\b`, ".", "..", "-x", "--list", "a b", "a\tb"} + for _, name := range bad { + if err := ValidAliasName(name); err == nil { + t.Errorf("ValidAliasName(%q) = nil, want an error", name) + } + if NameShaped(name) { + t.Errorf("NameShaped(%q) = true, want false", name) + } + } + good := []string{"qwen3.6-27b", "gemma_4", "Outfit", "a.b.c"} + for _, name := range good { + if err := ValidAliasName(name); err != nil { + t.Errorf("ValidAliasName(%q) = %v, want nil", name, err) + } + if !NameShaped(name) { + t.Errorf("NameShaped(%q) = false, want true", name) + } + } +} + +// TestLoadMalformedJSON checks that a corrupt config names itself in the error. +func TestLoadMalformedJSON(t *testing.T) { + dir := isolate(t) + + if err := os.MkdirAll(filepath.Join(dir, "outfit"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(Path(), []byte("{not json"), 0o600); err != nil { + t.Fatal(err) + } + + _, err := Load() + if err == nil { + t.Fatal("expected an error for a malformed config") + } + if !strings.Contains(err.Error(), Path()) { + t.Errorf("error %q does not name the config file", err) + } +} + +// TestLoadWrongAliasType checks that a plausible hand-edit (aliases as a list) +// errors rather than being silently ignored. +func TestLoadWrongAliasType(t *testing.T) { + dir := isolate(t) + + if err := os.MkdirAll(filepath.Join(dir, "outfit"), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(Path(), []byte(`{"aliases":["q3"]}`), 0o600); err != nil { + t.Fatal(err) + } + if _, err := Load(); err == nil { + t.Fatal("expected an error for an aliases list") + } +} diff --git a/internal/harness/harness.go b/internal/harness/harness.go index 6d220917..c3b3aa25 100644 --- a/internal/harness/harness.go +++ b/internal/harness/harness.go @@ -8,18 +8,19 @@ // --harness/-H flag > OUTFIT_HARNESS env > stored preference > opencode // // The stored preference lives in ${XDG_CONFIG_HOME:-~/.config}/outfit/config.json -// and is managed with `outfit harness --set `. +// and is managed with `outfit harness --set `. That file is owned by +// internal/config, which shares it with the Outfit alias registry; this package +// only reads and writes the one field. package harness import ( - "encoding/json" "fmt" "os" - "path/filepath" "sort" "strings" "github.com/lucinate-ai/outfit/internal/catalog" + "github.com/lucinate-ai/outfit/internal/config" "github.com/lucinate-ai/outfit/internal/outfit" ) @@ -116,49 +117,26 @@ func Resolve(flag string) (Harness, string, error) { // PreferencePath returns the path to outfit's own config file, where the // default-harness preference is stored. -func PreferencePath() string { - dir := os.Getenv("XDG_CONFIG_HOME") - if dir == "" { - home, _ := os.UserHomeDir() - dir = filepath.Join(home, ".config") - } - return filepath.Join(dir, "outfit", "config.json") -} - -// preference is the on-disk shape of outfit's config file. -type preference struct { - Harness string `json:"harness"` -} +func PreferencePath() string { return config.Path() } // LoadPreference returns the stored default harness, or "" when none is set. func LoadPreference() (string, error) { - data, err := os.ReadFile(PreferencePath()) + f, err := config.Load() if err != nil { - if os.IsNotExist(err) { - return "", nil - } return "", err } - var p preference - if err := json.Unmarshal(data, &p); err != nil { - return "", fmt.Errorf("parsing %s: %w", PreferencePath(), err) - } - return p.Harness, nil + return f.Harness, nil } -// SavePreference stores name as the default harness, validating it first. +// SavePreference stores name as the default harness, validating it first. Only +// the harness field is touched: everything else in outfit's config (the alias +// registry) survives, because the write is a read-modify-write. func SavePreference(name string) error { if _, ok := registry[name]; !ok { return fmt.Errorf("unknown harness %q (available: %s)", name, strings.Join(Names(), ", ")) } - path := PreferencePath() - if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { - return err - } - data, err := json.MarshalIndent(preference{Harness: name}, "", " ") - if err != nil { - return err - } - data = append(data, '\n') - return os.WriteFile(path, data, 0o600) + return config.Update(func(f *config.File) error { + f.Harness = name + return nil + }) } diff --git a/internal/harness/harness_test.go b/internal/harness/harness_test.go index 0d90e92c..9f53b7ac 100644 --- a/internal/harness/harness_test.go +++ b/internal/harness/harness_test.go @@ -4,6 +4,8 @@ import ( "os" "path/filepath" "testing" + + "github.com/lucinate-ai/outfit/internal/config" ) func TestCommand(t *testing.T) { @@ -92,6 +94,36 @@ func TestPreferenceRoundTrip(t *testing.T) { } } +// TestSavePreferenceKeepsAliases guards the reason the config file moved into +// internal/config: storing the harness rewrites the document, so it must not +// take the alias registry with it. +func TestSavePreferenceKeepsAliases(t *testing.T) { + dir := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", dir) + + if err := config.Update(func(f *config.File) error { + f.SetAlias("qwen3.6-27b", "/models/qwen/Outfit") + return nil + }); err != nil { + t.Fatal(err) + } + + if err := SavePreference("pi"); err != nil { + t.Fatal(err) + } + + f, err := config.Load() + if err != nil { + t.Fatal(err) + } + if _, ok := f.Alias("qwen3.6-27b"); !ok { + t.Error("SavePreference dropped the alias registry") + } + if f.Harness != "pi" { + t.Errorf("Harness = %q, want pi", f.Harness) + } +} + func TestNamesAndLookup(t *testing.T) { names := Names() if len(names) != 2 || names[0] != "opencode" || names[1] != "pi" { From 62e09e7ec225c174239c65e3caf49b2c3f86cd49 Mon Sep 17 00:00:00 2001 From: Pete Cornish Date: Tue, 28 Jul 2026 00:31:48 +0100 Subject: [PATCH 2/3] feat(cli): add zsh and powershell completion alongside bash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `outfit completion` now prints a script for zsh and powershell as well as bash. The `__complete` helper was already shell-agnostic — candidates plus a :nofile/:file/:dir directive — so each shell needs only a thin script that hands over the words and inserts a result. The one shared change is in completions(): bash tokenizes `--outfit=foo` into three words on the `=`, but zsh and powershell pass it as one, so the helper now also recognises the attached `--flag=value` form. That makes `--outfit`/-O, whose value must be attached, completable in every shell. The zsh script doubles as a sourced script and an autoloaded `_outfit` in $fpath; goreleaser now installs bash and zsh completions on `brew install`. PowerShell is best-effort: it offers the same candidates and falls back to its own path completion when none match. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0133vx4Ro9QiKySZTfgdF1rT --- .goreleaser.yaml | 6 +- README.md | 7 ++- cmd/outfit/complete.go | 58 +++++++++++++----- cmd/outfit/complete_test.go | 113 ++++++++++++++++++++++++++++++------ cmd/outfit/completion.ps1 | 51 ++++++++++++++++ cmd/outfit/completion.zsh | 50 ++++++++++++++++ cmd/outfit/main.go | 11 ++-- 7 files changed, 250 insertions(+), 46 deletions(-) create mode 100644 cmd/outfit/completion.ps1 create mode 100644 cmd/outfit/completion.zsh diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 5f8bf826..0b0b3546 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -47,10 +47,10 @@ brews: commit_author: name: goreleaser-bot email: bot@goreleaser.com - # Install the bash completion the binary prints, so `brew install` wires - # TAB up with no extra step. Only bash is supported, so ask for just that. + # Install the completions the binary prints, so `brew install` wires TAB up + # with no extra step. (PowerShell is supported too, but not by Homebrew.) extra_install: | - generate_completions_from_executable(bin/"outfit", "completion", shells: [:bash]) + generate_completions_from_executable(bin/"outfit", "completion", shells: [:bash, :zsh]) test: | system "#{bin}/outfit", "--help" diff --git a/README.md b/README.md index d1df6c64..537b7fbe 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ outfit init-providers [path] # write the built-in catalogue out to e outfit harness [] [-H ] [--outfit[=]] [args...] # launch the harness (a leading Outfit or alias is # applied first; --get shows it; --set stores it) -outfit completion bash # shell tab completion +outfit completion # tab completion (bash, zsh, powershell) ``` Short flags: `-p` (provider), `-f` (model-family), `-m` (model), `-a` (alias), `-c` (context), `-o` (output), `-u` (base-url), `-H` (harness), `-O` (outfit), and under `alias`: `-n` (name), `-l` (list), `-F` (force). @@ -279,12 +279,13 @@ your `Outfit` files stay as portable and committable as they were. ```sh source <(outfit completion bash) # add to ~/.bashrc +source <(outfit completion zsh) # or ~/.zshrc (needs compinit) +outfit completion powershell | Out-String | Invoke-Expression # or $PROFILE ``` TAB then completes commands, flags, providers, harnesses, and your registered aliases — `outfit unalias ` offers exactly the names you have. Homebrew -installs it for you. (zsh can use it via `autoload -U +X bashcompinit && -bashcompinit` first.) +installs the bash and zsh completions for you. ## Serving a local model diff --git a/cmd/outfit/complete.go b/cmd/outfit/complete.go index 62c9053a..180143ab 100644 --- a/cmd/outfit/complete.go +++ b/cmd/outfit/complete.go @@ -1,8 +1,11 @@ package main -// Tab completion. `outfit completion bash` prints the script below; the script -// calls the hidden `outfit __complete` with every word up to the cursor, and -// this file works out what could come next. +// Tab completion. `outfit completion ` prints one of the embedded +// scripts below; each calls the hidden `outfit __complete` with every word up +// to the cursor, and this file works out what could come next. The scripts are +// deliberately thin — bash, zsh, and PowerShell differ only in how they hand +// over the words and insert a result, never in the candidates — so this file +// stays the one source of truth for what completes to what. // // The command and flag tables here mirror run()'s switch and each cmdX's // FlagSet. That duplication is deliberate — the flag package offers no way to @@ -23,6 +26,23 @@ import ( //go:embed completion.bash var bashCompletion string +//go:embed completion.zsh +var zshCompletion string + +//go:embed completion.ps1 +var powershellCompletion string + +// completionScripts maps a shell name to its embedded completion script. +var completionScripts = map[string]string{ + "bash": bashCompletion, + "zsh": zshCompletion, + "powershell": powershellCompletion, +} + +// completionShells lists the supported shells in a stable order, for the +// `completion` argument's own completion and for error messages. +var completionShells = []string{"bash", "powershell", "zsh"} + // Directives tell the shell script whether to offer paths alongside whatever // candidates were printed. const ( @@ -147,18 +167,17 @@ var commands = map[string]command{ "help": {}, } -// cmdCompletion prints the shell completion script. +// cmdCompletion prints the shell completion script for the named shell. func cmdCompletion(args []string) error { if len(args) == 0 { - return fmt.Errorf("completion needs a shell (supported: bash)") + return fmt.Errorf("completion needs a shell (supported: %s)", strings.Join(completionShells, ", ")) } - switch args[0] { - case "bash": - fmt.Print(bashCompletion) - return nil - default: - return fmt.Errorf("unsupported shell %q (supported: bash)", args[0]) + script, ok := completionScripts[args[0]] + if !ok { + return fmt.Errorf("unsupported shell %q (supported: %s)", args[0], strings.Join(completionShells, ", ")) } + fmt.Print(script) + return nil } // cmdComplete prints the completion candidates for a partially-typed command @@ -192,10 +211,17 @@ func completions(args []string) ([]string, string) { return nil, directiveNoFile } - // bash breaks words on "=", so `--outfit=` arrives as - // [..., "--outfit", "=", ""]. Look back past the separator for the flag - // being given a value — this is the only way to complete --outfit/-O, whose - // value has to be attached. + // A flag and its value can arrive attached as one word (`--outfit=`, + // how zsh and PowerShell pass it) or split into three (`--outfit`, `=`, ``, + // how bash tokenizes because "=" is a word-break). Handle the attached form + // here; the split form falls through to the prev-word check below. Either + // way this is the only route to completing --outfit/-O, whose value has to + // be attached. + if flag, _, ok := strings.Cut(cur, "="); ok && strings.HasPrefix(flag, "-") { + if kind, found := cmd.values[flag]; found { + return candidatesFor(kind, words) + } + } prev := words[len(words)-1] if prev == "=" && len(words) > 1 { prev = words[len(words)-2] @@ -279,7 +305,7 @@ func candidatesFor(kind candidateKind, words []string) ([]string, string) { } return modelNames(p, flagValue(words, "--model-family", "-f")), directiveNoFile case kindShell: - return []string{"bash"}, directiveNoFile + return completionShells, directiveNoFile default: return nil, directiveNoFile } diff --git a/cmd/outfit/complete_test.go b/cmd/outfit/complete_test.go index cb072589..f83cd344 100644 --- a/cmd/outfit/complete_test.go +++ b/cmd/outfit/complete_test.go @@ -205,7 +205,7 @@ func TestComplete_FlagValues(t *testing.T) { if _, directive := complete(t, "apply", "--providers", ""); directive != directiveFile { t.Errorf("--providers should complete paths, got %q", directive) } - if got, _ := complete(t, "completion", ""); !hasAll(got, "bash") { + if got, _ := complete(t, "completion", ""); !hasAll(got, "bash", "zsh", "powershell") { t.Errorf("shells missing from %v", got) } } @@ -234,6 +234,31 @@ func TestComplete_EqualsForm(t *testing.T) { } } +// TestComplete_AttachedEqualsForm checks the same flag=value case as it arrives +// from zsh and PowerShell, which pass `--outfit=qw` as a single word rather than +// splitting it on "=" the way bash does. +func TestComplete_AttachedEqualsForm(t *testing.T) { + isolateConfig(t) + registerOutfit(t, "PROVIDER llamacpp\nALIAS qwen\n") + + got, directive := complete(t, "harness", "--outfit=") + if !hasAll(got, "qwen") { + t.Errorf("aliases missing from %v", got) + } + if directive != directiveFile { + t.Errorf("directive = %q, want %q", directive, directiveFile) + } + + // A partially-typed value is still the flag's value, not a new flag. + if got, _ := complete(t, "harness", "--outfit=qw"); !hasAll(got, "qwen") { + t.Errorf("aliases missing for a partial attached value: %v", got) + } + // The short flag attaches its value the same way. + if got, _ := complete(t, "add", "-p=llamacpp", "-f="); len(got) == 0 { + t.Error("no families offered for -p= -f=") + } +} + // TestComplete_NeverErrors checks the one hard rule: whatever the state of the // machine, completion prints candidates or nothing — never a failure. func TestComplete_NeverErrors(t *testing.T) { @@ -268,34 +293,84 @@ func TestComplete_NeverErrors(t *testing.T) { } } -// TestCompletionCommand checks the script the shell actually sources. -func TestCompletionCommand(t *testing.T) { - out := captureStdout(t, func() { - if err := cmdCompletion([]string{"bash"}); err != nil { - t.Fatalf("cmdCompletion bash: %v", err) +// completionOf returns the script printed for a shell. +func completionOf(t *testing.T, shell string) string { + t.Helper() + return captureStdout(t, func() { + if err := cmdCompletion([]string{shell}); err != nil { + t.Fatalf("cmdCompletion %s: %v", shell, err) } }) - for _, want := range []string{"_outfit()", "complete -F _outfit outfit", "__complete"} { - if !strings.Contains(out, want) { - t.Errorf("script is missing %q:\n%s", want, out) +} + +// TestCompletionCommand checks that every supported shell prints a script that +// wires itself up and calls the __complete helper. +func TestCompletionCommand(t *testing.T) { + markers := map[string][]string{ + "bash": {"_outfit()", "complete -F _outfit outfit", "__complete"}, + "zsh": {"#compdef outfit", "compdef _outfit outfit", "__complete"}, + "powershell": {"Register-ArgumentCompleter", "-CommandName outfit", "__complete"}, + } + for shell, wants := range markers { + out := completionOf(t, shell) + for _, want := range wants { + if !strings.Contains(out, want) { + t.Errorf("%s script is missing %q:\n%s", shell, want, out) + } + } + } + + // Every advertised shell has a script. + for _, shell := range completionShells { + if completionScripts[shell] == "" { + t.Errorf("shell %q is advertised but has no embedded script", shell) } } if err := cmdCompletion(nil); err == nil { t.Error("expected an error with no shell named") } - if err := cmdCompletion([]string{"zsh"}); err == nil { + if err := cmdCompletion([]string{"fish"}); err == nil { t.Error("expected an error for an unsupported shell") } +} - // The script has to be valid bash, not just plausible text. - bash, err := exec.LookPath("bash") - if err != nil { - t.Skip("bash not installed") - } - script := filepath.Join(t.TempDir(), "outfit.bash") - mustWrite(t, script, out) - if got, err := exec.Command(bash, "-n", script).CombinedOutput(); err != nil { - t.Errorf("bash -n rejected the completion script: %v\n%s", err, got) +// TestCompletionScriptsAreValid runs each script through its own shell's syntax +// checker, when that shell is installed. Skipped shells are just not exercised +// on this machine — CI has bash and zsh. +func TestCompletionScriptsAreValid(t *testing.T) { + cases := []struct { + shell string + bin string + ext string + checker func(bin, path string) *exec.Cmd + }{ + {"bash", "bash", "bash", func(bin, path string) *exec.Cmd { + return exec.Command(bin, "-n", path) + }}, + {"zsh", "zsh", "zsh", func(bin, path string) *exec.Cmd { + return exec.Command(bin, "-n", path) + }}, + {"powershell", "pwsh", "ps1", func(bin, path string) *exec.Cmd { + // Parse the file without running it; any parse error fails the test. + script := "$errs=$null;" + + "[System.Management.Automation.Language.Parser]::ParseFile('" + + path + "',[ref]$null,[ref]$errs)|Out-Null;" + + "if($errs.Count){exit 1}" + return exec.Command(bin, "-NoProfile", "-Command", script) + }}, + } + for _, c := range cases { + t.Run(c.shell, func(t *testing.T) { + bin, err := exec.LookPath(c.bin) + if err != nil { + t.Skipf("%s not installed", c.bin) + } + path := filepath.Join(t.TempDir(), "outfit."+c.ext) + mustWrite(t, path, completionOf(t, c.shell)) + if out, err := c.checker(bin, path).CombinedOutput(); err != nil { + t.Errorf("%s rejected the completion script: %v\n%s", c.bin, err, out) + } + }) } } diff --git a/cmd/outfit/completion.ps1 b/cmd/outfit/completion.ps1 new file mode 100644 index 00000000..03af4a02 --- /dev/null +++ b/cmd/outfit/completion.ps1 @@ -0,0 +1,51 @@ +# PowerShell completion for outfit. Install with: +# +# outfit completion powershell | Out-String | Invoke-Expression +# +# Add that line to your profile ($PROFILE) to load it in every session. +# +# The candidates come from the binary itself, via the hidden `outfit __complete` +# command: it is handed every word up to the cursor and prints one candidate per +# line, then a directive line saying whether paths belong in the list too. + +Register-ArgumentCompleter -Native -CommandName outfit -ScriptBlock { + param($wordToComplete, $commandAst, $cursorPosition) + + # Everything after the binary, up to and including the word under the cursor. + # When completing a fresh word there is a trailing space, so $wordToComplete + # is empty and the parsed elements stop one short — add the empty word back. + $elements = @($commandAst.CommandElements | Select-Object -Skip 1 | ForEach-Object { $_.ToString() }) + if ([string]::IsNullOrEmpty($wordToComplete)) { + $elements += '' + } + + $exe = $commandAst.CommandElements[0].Value + $out = & $exe __complete @elements 2>$null + + $candidates = @() + foreach ($line in $out) { + if ($line -like ':*' -or $line -eq '') { continue } + $candidates += $line + } + + # For a --flag=value word, candidates fill in only the part after "="; the + # inserted text carries the "--flag=" back so the whole word stays intact. + $prefix = '' + $value = $wordToComplete + if ($wordToComplete -match '^(--?[^=]*=)(.*)$') { + $prefix = $Matches[1] + $value = $Matches[2] + } + + $results = @() + foreach ($c in $candidates) { + if ($c -like "$value*") { + $results += [System.Management.Automation.CompletionResult]::new( + $prefix + $c, $c, 'ParameterValue', $c) + } + } + + # With no matching candidate, return nothing so PowerShell falls back to its + # own path completion — the file directives (:file/:dir) resolve to that. + $results +} diff --git a/cmd/outfit/completion.zsh b/cmd/outfit/completion.zsh new file mode 100644 index 00000000..251c2577 --- /dev/null +++ b/cmd/outfit/completion.zsh @@ -0,0 +1,50 @@ +#compdef outfit +# zsh completion for outfit. Install with: +# +# source <(outfit completion zsh) +# +# or place it on your $fpath as a file named _outfit — Homebrew does this for +# you — and make sure `autoload -U compinit && compinit` runs from your ~/.zshrc. +# +# The candidates come from the binary itself, via the hidden `outfit __complete` +# command: it is handed every word up to the cursor and prints one candidate per +# line, then a directive line saying whether paths belong in the list too. + +_outfit() { + local line directive out + local -a candidates + + # $words is the command line (1-indexed; $words[1] is the binary) and $CURRENT + # is the index of the word under the cursor. Hand __complete everything after + # the binary up to and including that word, exactly as the bash script does. + # (@) keeps an empty word-under-cursor as its own argument. + out="$(${words[1]} __complete "${(@)words[2,$CURRENT]}" 2>/dev/null)" + + directive=":nofile" + while IFS= read -r line; do + case "${line}" in + :*) directive="${line}" ;; + "") ;; + *) candidates+=("${line}") ;; + esac + done <<< "${out}" + + # For a --flag=value word, only the part after "=" is matched and inserted; + # compset moves the "--flag=" into the ignored prefix so compadd fills the tail. + compset -P '*=' + + case "${directive}" in + :file) _files ;; + :dir) _files -/ ;; + esac + (( ${#candidates} )) && compadd -a candidates +} + +# Support both `source <(outfit completion zsh)` and being autoloaded from +# $fpath. Autoloaded, the #compdef tag registers us and zsh calls the file as +# the completion function; sourced, we register ourselves. +if [ "${funcstack[1]}" = "_outfit" ]; then + _outfit "$@" +else + compdef _outfit outfit +fi diff --git a/cmd/outfit/main.go b/cmd/outfit/main.go index 24a2e924..625735ea 100644 --- a/cmd/outfit/main.go +++ b/cmd/outfit/main.go @@ -24,7 +24,7 @@ // outfit init-providers [path] # write the embedded providers.yaml out // outfit harness [-H name] [-O[=path]] # launch the harness, optionally applying an // # Outfit first (--get shows it; --set stores the default) -// outfit completion bash # print the tab-completion script +// outfit completion # print the tab-completion script // // Short flags: -p (provider), -f (model-family), -m (model), -a (alias), // -c (context), -o (output), -u (base-url), -H (harness), -O (outfit). @@ -141,7 +141,7 @@ Usage: outfit init-providers [path] (defaults to ./providers.yaml) outfit harness [] [-H ] [--outfit[=]] [args...] (launch the harness; available: %s) - outfit completion bash (shell tab completion; see below) + outfit completion (tab completion: bash, zsh, powershell) outfit version (or -v/--version) Flags: @@ -200,9 +200,10 @@ harness: launches the active harness, forwarding any trailing args to it. A show: lists the providers and models actually configured in the active harness's config (where list shows the catalogue of what you could configure), and the aliases you have registered. -completion: prints a tab-completion script. Add - source <(outfit completion bash) to your ~/.bashrc and TAB then completes - commands, flags, providers, harnesses, and your registered aliases. +completion: prints a tab-completion script for bash, zsh, or powershell. Add + source <(outfit completion bash) to your ~/.bashrc (zsh: swap in zsh) and + TAB then completes commands, flags, providers, harnesses, and your + registered aliases. `, strings.Join(harness.Names(), ", ")) } From bba8157124e526d1242748f53439df3ea3634a70 Mon Sep 17 00:00:00 2001 From: Pete Cornish Date: Tue, 28 Jul 2026 00:34:37 +0100 Subject: [PATCH 3/3] test(cli): cover model-value completion and launch with a corrupt config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two behaviour gaps on this branch went untested: completing --model/-m from the catalogue (modelNames had no coverage at all), and the property that a config outfit cannot parse still lets the harness launch — the leading word is forwarded rather than erroring. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0133vx4Ro9QiKySZTfgdF1rT --- cmd/outfit/complete_test.go | 29 +++++++++++++++++++++++++++++ cmd/outfit/harness_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/cmd/outfit/complete_test.go b/cmd/outfit/complete_test.go index f83cd344..343f8f47 100644 --- a/cmd/outfit/complete_test.go +++ b/cmd/outfit/complete_test.go @@ -234,6 +234,35 @@ func TestComplete_EqualsForm(t *testing.T) { } } +// TestComplete_ModelValues checks that --model/-m offers the catalogue's model +// keys — every family's when no family is fixed, and just one family's when it +// is. +func TestComplete_ModelValues(t *testing.T) { + isolateConfig(t) + + // With a family fixed, only that family's models. + got, directive := complete(t, "add", "-p", "openrouter", "-f", "deepseek-v4", "-m", "") + if !hasAll(got, "deepseek/deepseek-v4-flash", "deepseek/deepseek-v4-pro") { + t.Errorf("family's models missing from %v", got) + } + if directive != directiveNoFile { + t.Errorf("directive = %q, want %q", directive, directiveNoFile) + } + + // Without a family, every family's models are offered. + if got, _ := complete(t, "add", "--provider", "openrouter", "--model", ""); len(got) == 0 { + t.Error("no models offered for a provider with no family fixed") + } + // With no provider named at all, there is nothing to draw models from. + if got, _ := complete(t, "add", "-m", ""); len(got) != 0 { + t.Errorf("models offered with no provider: %v", got) + } + // An unknown provider yields no models rather than an error. + if got, _ := complete(t, "add", "-p", "nope", "-m", ""); len(got) != 0 { + t.Errorf("models offered for an unknown provider: %v", got) + } +} + // TestComplete_AttachedEqualsForm checks the same flag=value case as it arrives // from zsh and PowerShell, which pass `--outfit=qw` as a single word rather than // splitting it on "=" the way bash does. diff --git a/cmd/outfit/harness_test.go b/cmd/outfit/harness_test.go index 6b9563ee..bb68af20 100644 --- a/cmd/outfit/harness_test.go +++ b/cmd/outfit/harness_test.go @@ -305,6 +305,30 @@ func TestHarness_TerminatorAfterDetachedFlagValue(t *testing.T) { } } +// TestHarness_MalformedConfigStillLaunches checks the documented safety +// property: a config outfit cannot parse must not stop the harness launching. +// The leading word is simply forwarded, and a real alias error surfaces later +// under `outfit apply`, where it is actionable. +func TestHarness_MalformedConfigStillLaunches(t *testing.T) { + home := isolateConfig(t) + + configPath := filepath.Join(home, ".config", "outfit", "config.json") + if err := os.MkdirAll(filepath.Dir(configPath), 0o700); err != nil { + t.Fatal(err) + } + mustWrite(t, configPath, "{not json") + + // "run" is name-shaped and not on disk, so resolving it consults the config + // — which is corrupt. That must degrade to "not an Outfit", forwarding it. + forwarded, _ := launchedArgs(t, []string{"run", "hello"}) + if forwarded != "run\nhello" { + t.Errorf("forwarded args = %q, want them forwarded verbatim", forwarded) + } + if _, err := os.Stat(filepath.Join(home, ".config", "opencode", "opencode.json")); !os.IsNotExist(err) { + t.Error("a corrupt config should have applied nothing") + } +} + // TestHarness_LeadingOutfitPathApplies checks that a path works positionally // too, so `harness` reads like apply and serve. func TestHarness_LeadingOutfitPathApplies(t *testing.T) {