From 2f797069e4fbaa172d66b4899b156c355fc858e1 Mon Sep 17 00:00:00 2001 From: Larah Vasquez Date: Wed, 12 Aug 2026 08:45:31 -0500 Subject: [PATCH 1/8] docs: add guide for adding a new client Adds docs/adding-a-client.md, a step-by-step contributor guide covering the clients.Client interface, both routing variants (environment variables and a generated config file), the menu flow, replay, and the CI gates. Also ignores .artifacts/, which holds local session scratch output. --- .gitignore | 1 + docs/adding-a-client.md | 965 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 966 insertions(+) create mode 100644 docs/adding-a-client.md diff --git a/.gitignore b/.gitignore index b70978c..19868a1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ .build/ *.test *.out +.artifacts/ diff --git a/docs/adding-a-client.md b/docs/adding-a-client.md new file mode 100644 index 0000000..b5cdec2 --- /dev/null +++ b/docs/adding-a-client.md @@ -0,0 +1,965 @@ +# Add support for a new client + +This guide is for contributors who want to add a new coding agent to `aperture-cli`. The repository ships several command-line agents, each in its own self-contained package under `internal/clients`, plus Claude Cowork as a desktop app in `internal/profiles`. If you have used a harness like OpenCode or Pi and want `aperture` to launch it preconfigured against an Aperture endpoint, this walks you through the whole path. By the end you will have a new package that appears in the launcher menu, installs and uninstalls itself, routes the harness through an Aperture endpoint, replays the last session on the quick-select row, and passes CI. + +One naming note before you start. This guide says "harness" because that is the word you probably arrived with, but the codebase does not use it. In the code, a harness is a **client**: the interface is `clients.Client`, the registry is `internal/clients`, and each harness is a sub-package such as `internal/clients/opencode`. When you read code or write comments, use "client" so your work matches everything around it. + +## Requirements and assumptions + +Before you start, make sure the following are true. The guide does not teach these things and will not work well without them. + +- You have Go 1.26.2 or newer installed, which is the version pinned in `go.mod` and used by CI. +- You have cloned the `tailscale/aperture-cli` repository and can run `make build` and `make test` successfully on an unmodified checkout. +- You are comfortable reading and writing Go, including interfaces, methods on pointer receivers, closures, and table-driven tests. +- You have the harness you want to add already installed on your machine, so you can test a real launch. +- You have a reachable Aperture endpoint to test against, and you know its URL. The default is `http://ai`, which resolves over Tailscale. +- You know, or can find out, how your harness accepts a custom API base URL and a custom API key. This is the single most important prerequisite, and Step 1 covers how to find it. +- You are working on macOS or Linux. The guide's commands assume a POSIX shell. The code you write will be cross-platform, but the shell snippets are not. +- You have read the top-level comment in `internal/clients/registry.go`, which is the closest thing the repo has to a contract for this work. + +The guide does not cover adding a graphical desktop application. Claude Cowork is a desktop app and it lives in `internal/profiles` behind an adapter, which is a different and more awkward path. Everything here assumes your harness is a command-line binary. + +## Variables + +You will substitute your own values into the code and commands throughout this guide. Decide each of these before you start writing code, and use the same value everywhere the placeholder appears. + +| Variable | Description | +|---|---| +| `` | The Go package name and directory name for your client, lowercase with no separators. Existing examples are `opencode`, `codex`, and `claudecode`. | +| `` | The name the user sees in the launcher menu, written the way the vendor writes it. Existing examples are `OpenCode`, `Gemini CLI`, and `GitHub Copilot`. | +| `` | The name of the executable as it appears on `$PATH`, for example `opencode` or `claude`. | +| `` | The shell command that installs the harness, for example `npm install -g @openai/codex`. Get this from the harness's own documentation. | +| `` | The shell command that uninstalls the harness, for example `npm uninstall -g @openai/codex`. Step 5 uses this twice: once verbatim as a display hint, and once split into separate arguments (`"npm", "uninstall", "-g", "@openai/codex"`) because there is no shell to split it. If the harness has no uninstall command, Step 5 explains what to do instead. | +| `` | The compatibility key your harness needs a provider to support, for example `openai_responses`. Step 2 explains how to find the valid keys and pick yours. | +| `` | The URL of the Aperture endpoint you will test against, for example `http://ai` or `https://ai.example.com`. | +| `` | The environment variable your harness reads for its API base URL, for example `OPENAI_BASE_URL`. Step 1 explains how to find it. | +| `` | The environment variable your harness reads for its API key, for example `OPENAI_API_KEY`. Found the same way. | +| `` | The environment variable your harness reads for its default model, for example `OPENAI_MODEL`. If your harness has no such variable, delete the whole `if model != ""` block that sets it in Step 9 rather than leaving the placeholder in place. | +| `` | The command-line flag that makes your harness skip permission prompts, for example `--yolo`. If your harness has no such flag, delete the whole `args` block in Step 9 rather than leaving the placeholder in place. | +| `` | The environment variable that points your harness at a config file or config directory, for example `OPENCODE_CONFIG`. Only needed if Step 1 told you your harness requires a config file. | + +## How it works + +The launcher is a registry of clients plus a generic menu engine, and the two know almost nothing about each other. Each client sub-package declares itself at startup by calling `clients.Register` from an `init()` function, as in `internal/clients/opencode/opencode.go:20`. That `init()` only runs if the package is linked into the binary, which is why `cmd/aperture/main.go:21` holds a block of underscore imports whose only purpose is that side effect. Forgetting to add your package to that block is the most common way for a new client to silently not exist. + +That `init()` call is also why this guide leaves it until the very end. `clients.Register` takes a `clients.Client`, so the moment you write it, your package stops compiling until every one of the interface's nine methods exists. Adding it last keeps the package buildable and testable at each step along the way. + +Everything the launcher can do with a client goes through the `clients.Client` interface in `internal/clients/registry.go:16`. The interface is deliberately wide, because each client owns its own flow end to end. The TUI never asks "what providers does this client support" or "what environment variables does it need". It asks for a `menu.MenuItem` and renders it, and the client's own closures take over from there. The TUI reads the registry through one indirection, `registeredClients` in `internal/tui/tui.go:880`, which exists so tests can swap in fakes. + +The user's path through a client looks like this. Nothing in the diagram is mandatory except the first and last box, and Step 7 explains how to collapse the middle steps when there is only one option. + +```mermaid +flowchart TD + A["Root menu
(installed clients)"] --> B["Menu()
returns your MenuItem"] + B --> C["providerStep
filter by compatibility key"] + C --> D["backendStep
pick a routing flavor"] + D --> E["modelStep
pick a default model"] + E --> F["launch()
build env, write config"] + F --> G["clients.Launch
exec the binary in the foreground"] + G --> H["ExecDoneMsg
TUI regains control, re-runs preflight"] +``` + +Two things decide most of the work. The first is how your harness accepts a custom base URL. Some harnesses read environment variables only, which makes the client short: GitHub Copilot is entirely `buildEnv` at `internal/clients/copilot/copilot.go:173`. Others need a config file on disk, so the client writes one per launch and points the harness at it with a single environment variable, which is what `writeProviderConfig` does at `internal/clients/opencode/sdk.go:79`. The second is which API protocols your harness speaks, because the launcher only offers a client the providers that can serve it. + +That second part works through the compatibility map. On startup the TUI fetches `GET /api/providers` from the active Aperture endpoint and unmarshals it into `[]config.ProviderInfo` (see `internal/tui/tui.go:123` and `internal/config/providers.go:4`). Each provider carries a `Compatibility map[string]bool` describing which wire protocols it can serve, such as `openai_responses` or `anthropic_messages`. Your client filters that list down to providers it can actually talk to, and if the list comes back empty it shows an error instead of a menu. + +## Step 1: Work out how your harness accepts a custom base URL + +Everything downstream depends on this answer, so get it before you write any Go. You are looking for two things: how to point the harness at an arbitrary HTTP endpoint instead of the vendor's own API, and how to satisfy its API key check without a real key. + +Start with the harness's own documentation, searching for "base URL", "custom endpoint", "proxy", "self-hosted", or "OpenAI-compatible". Then check the harness's help output and its environment, which often reveals more than the docs do. + +```bash + --help +env | grep -i +``` + +If the harness is open source, searching its source for `baseURL`, `base_url`, or `BASE_URL` is usually faster than reading its documentation. + +Sort what you find into one of three shapes. In the environment-variable shape, the harness reads a base URL and an API key from the process environment and needs nothing on disk. GitHub Copilot works this way through `COPILOT_PROVIDER_BASE_URL` and friends. In the config-file shape, the harness insists on reading a config file, so your client writes that file at launch time and passes its path or its parent directory in one environment variable. OpenCode works this way through `OPENCODE_CONFIG`, Codex through `CODEX_HOME`, and Gemini CLI through `GEMINI_CLI_HOME`. + +The third shape is a plugin: the harness has no base-URL variable at all and no config file you can point at in isolation, but it can load a file of code that registers a provider at startup. Pi works this way — it accepts `-e ` and calls the file's exported function with its own extension API. Treat this like the config-file shape, writing the file per launch and cleaning it up after, but note that it is code rather than data, so generate it by marshaling values to JSON and interpolating them rather than by hand-writing strings. + +Do not assume a variable exists just because every other harness has one. Search the harness's own documentation and source for the exact name before writing it down. If you cannot find one, that is a finding, not a gap in your search — invent nothing. A harness with a permissive plugin API often has no URL variable at all, and a variable that looks right may not be an input: Pi *sets* `PI_MODEL` and `PI_PROVIDER` for the tools it spawns to read, so setting them yourself does nothing. + +You will also need a value to satisfy the harness's API key check. Aperture handles authentication itself, so no real key is involved. The convention in this repo is a placeholder string, and existing clients use `not-needed`, `not-required`, or a bare `-` depending on what the harness accepts. Check whether the key is genuinely optional: Pi loads a provider without one, then silently hides its models from every picker, which looks like a compatibility bug rather than a missing placeholder. + +Two more questions are worth settling now, because both are cheap to answer and expensive to discover later. First, if the harness needs an on-disk home directory, find out what else lives there — if the same directory also holds the user's saved logins, settings, or session history, redirecting it to an Aperture-owned path will hide all of that, and a per-launch plugin or config file is the better route. Second, if the harness has a "skip permission prompts" flag, confirm it actually governs tool approval. Pi's `--approve` looks like one but controls whether project-local config files are trusted, and wiring `YoloMode` to it would grant something the user did not ask for while still prompting for everything they did. + +Write down the exact variable names and the exact config file schema before you continue. To verify you have enough, launch the harness by hand with those values set and confirm it reaches your Aperture endpoint. This one-off check saves you from debugging your Go code when the problem was the harness contract all along. + +Then confirm your endpoint is reachable and answering, because nothing later in this guide works without it. + +```bash +curl -s /api/providers +``` + +You should get back a JSON array of provider objects, each with an `id`, a `models` list, and a `compatibility` map. Keep that output open, because the next step reads it. If the request fails or returns nothing, fix your Aperture connectivity before continuing. + +## Step 2: Choose the compatibility keys your harness can speak + +The compatibility key is how your client declares which providers it can use. Pick the wrong key and your client will either never appear in a provider list or will appear and then fail at runtime. + +The keys are defined by the Aperture server, not by this repository, so the authoritative list for your endpoint is the response you just fetched from `/api/providers`. The longest list in the codebase is `compatKeys` in `internal/clients/opencode/opencode.go:34`, reproduced below. + +```go +var compatKeys = []string{ + "openai_responses", + "anthropic_messages", + "openai_chat", + "google_generate_content", + "google_raw_predict", + "bedrock_model_invoke", + "bedrock_converse", + "gemini_generate_content", +} +``` + +That list is not the complete set. Each client declares its own keys independently, so keys used by one client can be absent from another's list — `internal/clients/gemini/gemini.go:44` uses `experimental_gemini_cli_vertex_compat`, which does not appear above. To see every key the repository knows about, grep for the declarations rather than trusting any single list. + +```bash +grep -rn 'compatKey\|compatKeys' internal/clients/ +``` + +Map the protocol you found in Step 1 onto one or more of these keys. A harness that speaks OpenAI Chat Completions wants `openai_chat`. One that speaks the newer OpenAI Responses API wants `openai_responses`. One that speaks Anthropic's Messages API wants `anthropic_messages`. + +How many keys you need decides how much menu you write. If your harness speaks exactly one protocol, you need one key and no backend step, which is what Codex does with a single `compatKey` constant at `internal/clients/codex/codex.go:29`. If it speaks several and the user should choose between them, you need a `backend` struct with one entry per protocol, which is what Copilot does at `internal/clients/copilot/copilot.go:37`. If it speaks several but the choice can be made for the user automatically, you need a list of keys and a resolver, which is what OpenCode does in `pickSDK` at `internal/clients/opencode/sdk.go:35`. + +To verify your choice, inspect the JSON from Step 1 and confirm at least one provider on your endpoint has your key set to `true`. If no provider does, your client will correctly refuse to launch, and that is a configuration problem on the Aperture side rather than something to work around in code. + +## Step 3: Create the package directory and files + +Now create the package. Every client sub-package follows the same file layout, and matching it makes your code reviewable by anyone who has read the others. + +Run this from the repository root. It creates the directory and the three files you will fill in, each with its package clause already in place. + +```bash +mkdir -p internal/clients/ +cd internal/clients/ +printf 'package \n' > .go +printf 'package \n' > install.go +printf 'package \n' > _test.go +cd - +``` + +Write the package clause now rather than creating the files empty. A zero-byte `.go` file is a parse error, not an empty package, so `go build ./...` fails with `expected 'package', found 'EOF'` for every empty file in the directory — which would break the verification at the end of this step and every step after it until all three files have content. + +The main file holds the `Client` type and every interface method. The `install.go` file holds only `commonBinaryPaths`, kept separate because it is the one function that tends to differ per operating system. The test file holds your table-driven tests. If your harness needs a config file, Step 8 adds a fourth file for it: `sdk.go` in OpenCode, `config.go` in Codex and Gemini. + +Open the main file and replace its bare package clause with the doc comment, the package declaration, the type, and your constants. The doc comment matters more here than in most Go code, because the existing client packages each explain their routing model up front and reviewers will look for that. + +```go +// Package is the client. Describe +// here which protocols it speaks, how routing is configured (environment +// variables, a config file, or both), and what the menu flow looks like. +package + +// Client is the client. +type Client struct{} + +const ( + name = "" + binaryName = "" + compatKey = "" +) +``` + +`Client` is an empty struct because clients hold no state of their own. All state lives in the `*config.Global` that gets passed into each method. + +There are deliberately no imports and no `init()` yet. Go treats an unused import as a compile error, so adding the import block before the code that uses it would break the build, and Step 11 adds the `init()` once every interface method exists. Add each import as the step that needs it arrives, or let your editor do it. + +To verify, build the whole module. + +```bash +go build ./... +``` + +That should produce no output at all. Your package now compiles, which means you can keep it compiling after every step that follows. If you see an error about an unused import, delete the import rather than the code. If you see `expected 'package', found 'EOF'`, one of your three files is still empty; give it the package clause shown above. + +## Step 4: Implement identity and binary discovery + +These four methods tell the launcher what your client is called and whether it is installed. They are the shortest methods in the interface and none of them make decisions. + +Add them to your main file, below the constants, along with the `clients` import they need. + +```go +import "github.com/tailscale/aperture-cli/internal/clients" +``` + +```go +// Name implements clients.Client. +func (c *Client) Name() string { return name } + +// BinaryName implements clients.Client. +func (c *Client) BinaryName() string { return binaryName } + +// CommonPaths implements clients.Client. +func (c *Client) CommonPaths() []string { return commonBinaryPaths() } + +// IsInstalled implements clients.Client. +func (c *Client) IsInstalled() bool { + return clients.IsInstalled(binaryName, c.CommonPaths()) +} +``` + +`Name` is what the user reads in the menu. `BinaryName` is what gets looked up on `$PATH`. `CommonPaths` covers the case where the binary exists but `$PATH` does not know about it yet, which happens constantly right after an install has updated a shell profile that the running shell has not reloaded. Delegate `IsInstalled` to the shared helper rather than writing your own check, so binary discovery stays consistent across clients. + +Now fill in `install.go` with the paths where your harness's installer actually puts the binary. + +```go +package + +import ( + "os" + "path/filepath" +) + +// commonBinaryPaths returns the non-PATH locations where +// is commonly installed. +func commonBinaryPaths() []string { + home, err := os.UserHomeDir() + if err != nil { + return nil + } + return []string{ + filepath.Join(home, ".local", "bin", ""), + } +} +``` + +Return full paths to the binary, not directories. `FindBinary` at `internal/clients/binary.go:19` treats these entries as complete paths and stats each one directly. You do not need to list `~/.local/bin`, `~/bin`, or `~/.npm-global/bin`, because `commonBinDirs` in the same file already checks those for every client. Add an entry only for a location specific to your harness, the way OpenCode adds `~/.opencode/bin/opencode`. + +To verify, confirm the package still builds and that your harness is where you think it is. + +```bash +go build ./... && which +``` + +The build should print nothing, and `which` should print a path. If `which` prints a path, `IsInstalled` will return `true` through the `$PATH` lookup alone. To confirm your `commonBinaryPaths` entries also work, temporarily remove the binary's directory from `$PATH` in a throwaway shell and check that `which` fails while the path you listed still exists on disk. + +## Step 5: Describe how to install and uninstall the harness + +The launcher can install a harness for the user from the `[i] Install agents` menu, and remove it from `Settings` then `Uninstall`. Both are described declaratively: your client returns a plan, and the TUI shows the hint, asks for confirmation, and runs the command. + +Add both methods to your main file, and add `"os/exec"` and `"github.com/tailscale/aperture-cli/internal/config"` to its imports. + +```go +// Install implements clients.Client. +func (c *Client) Install(_ *config.Global) clients.InstallPlan { + return clients.InstallPlan{ + Hint: "", + Run: func() (*exec.Cmd, error) { + return exec.Command("/bin/sh", "-c", ""), nil + }, + } +} + +// Uninstall implements clients.Client. +func (c *Client) Uninstall() clients.UninstallPlan { + return clients.UninstallPlan{ + Hint: "", + Run: func() error { + // Split into separate arguments: there is no shell here. + return exec.Command("npm", "uninstall", "-g", "@openai/codex").Run() + }, + } +} +``` + +Note the difference between the two `Run` fields. `Install.Run` passes the command as one string to `/bin/sh -c`, which splits it. `Uninstall.Run` has no shell, so you must split `` into its arguments yourself, exactly as Codex does at `internal/clients/codex/codex.go:63`. The example above is Codex's literal argument list — substitute your own. Passing the whole command as a single argument compiles fine and then fails at runtime with `fork/exec npm uninstall -g ...: no such file or directory`, because it looks for one executable whose filename contains spaces. No build or test step catches this, so get it right here. + +The `Hint` is shown to the user verbatim before they confirm, so write the actual command rather than a description of it. `Install.Run` returns an `*exec.Cmd` that the TUI executes, and it returns rather than runs the command so the TUI controls the terminal handoff. Wrapping the install in `/bin/sh -c` is what the existing clients do, and it is what makes a piped command such as `curl ... | bash` work. + +Two cases need different handling. If your harness has no scripted install, set `Run` to `nil` and the TUI will show the hint and do nothing, leaving the user to install it by hand. If uninstalling means deleting files rather than running a command, do the deletion in Go, the way Claude Code does at `internal/clients/claudecode/claudecode.go:80`. + +Verify with a test rather than by actually installing anything, following the pattern at `internal/clients/codex/codex_test.go:87`. Put this in your test file, which needs the package clause and two imports. + +```go +package + +import ( + "testing" + + "github.com/tailscale/aperture-cli/internal/config" +) + +func TestInstallUninstall(t *testing.T) { + c := &Client{} + install := c.Install(&config.Global{}) + if install.Hint != "" { + t.Errorf("Install.Hint = %q", install.Hint) + } + if install.Run == nil { + t.Error("Install.Run is nil") + } + + uninstall := c.Uninstall() + if uninstall.Hint != "" { + t.Errorf("Uninstall.Hint = %q", uninstall.Hint) + } + if uninstall.Run == nil { + t.Error("Uninstall.Run is nil") + } +} +``` + +Replace the bare package clause in your test file with the block above. The test lives in your own package rather than a `_test` package, so it can reach unexported identifiers such as `name` and `compatKey`. Note that it asserts only on the hints and on `Run` being non-nil — it never invokes `Run`, because doing so would really uninstall your harness. That is why the argument-splitting mistake described above survives a green test suite. + +Run it to confirm. + +```bash +go test ./internal/clients// +``` + +You should see `ok` and the package path. Tests run at this point precisely because you have not added `init()` yet. + +## Step 6: Filter providers by compatibility + +Your client must decide which of the endpoint's providers it can use. This is a small piece of code with an outsized effect, because it gates whether the client shows a menu at all, and Step 10 reuses it to decide whether a replay is still valid. + +Add these helpers near the bottom of your main file, next to the other unexported functions. + +```go +// compatibleProviders returns the subset of providers this client can use. +func compatibleProviders(all []config.ProviderInfo) []config.ProviderInfo { + var out []config.ProviderInfo + for _, p := range all { + if providerMatches(p) { + out = append(out, p) + } + } + return out +} + +func providerMatches(p config.ProviderInfo) bool { + return p.Compatibility[compatKey] +} +``` + +That is the single-protocol version. Reading a missing key from a `map[string]bool` yields `false`, so there is no need to check whether the key exists. + +If your harness speaks several protocols, `providerMatches` should return true when any of them match, as OpenCode does at `internal/clients/opencode/opencode.go:186`. If the user chooses between protocols, replace `providerMatches` with a `backendsFor` function that returns every matching backend and treat a non-empty result as a match, as Copilot does at `internal/clients/copilot/copilot.go:248`. + +Most clients also need the model list in fully-qualified form, because the launcher displays models as `provider_id/model_id` while harnesses usually want the bare model ID. If your client offers a model choice, add both helpers, and add `"strings"` to your imports for the second one. + +```go +// fqnModels returns the provider's models in "provider_id/model_id" form. +func fqnModels(p config.ProviderInfo) []string { + out := make([]string, len(p.Models)) + for i, m := range p.Models { + out[i] = p.ID + "/" + m + } + return out +} + +func stripProviderPrefix(fqn string) string { + if _, after, ok := strings.Cut(fqn, "/"); ok { + return after + } + return fqn +} +``` + +Using `stripProviderPrefix` before you put a model name into the environment is not optional. Leaving the prefix on breaks path-based routing, and there is a comment explaining a concrete instance of that breakage at `internal/clients/claudecode/claudecode.go:307`. + +Verify with a test in the style of `internal/clients/opencode/opencode_test.go:14`. Add it to the test file you started in Step 5. + +```go +func TestCompatibleProviders(t *testing.T) { + provs := []config.ProviderInfo{ + {ID: "match", Compatibility: map[string]bool{compatKey: true}}, + {ID: "nomatch", Compatibility: map[string]bool{"something_else": true}}, + } + got := compatibleProviders(provs) + if len(got) != 1 || got[0].ID != "match" { + t.Errorf("compatibleProviders = %+v, want just the matching provider", got) + } +} +``` + +Run the tests and confirm both pass. + +```bash +go test ./internal/clients// +``` + +The filter is now the only thing standing between the provider list and your menu. + +## Step 7: Build the menu flow + +This step turns your client into something the user can actually select. `Menu` is the entry point the root menu renders, and each subsequent step either descends automatically or shows a submenu. + +Add `Menu` and the provider step to your main file. Both need the `menu` package, and the error helper at the end of this step needs Bubble Tea, so add these two imports now. + +```go + tea "github.com/charmbracelet/bubbletea" + "github.com/tailscale/aperture-cli/internal/menu" +``` + +```go +// Menu implements clients.Client. +func (c *Client) Menu(g *config.Global) menu.MenuItem { + return menu.MenuItem{ + Label: name, + Action: func() menu.Result { return c.providerStep(g) }, + } +} + +func (c *Client) providerStep(g *config.Global) menu.Result { + provs := compatibleProviders(g.Providers) + if len(provs) == 0 { + return errorResult("No providers support .") + } + if len(provs) == 1 { + return c.modelStep(g, provs[0]) + } + items := make([]menu.MenuItem, 0, len(provs)) + for _, p := range provs { + items = append(items, menu.MenuItem{ + Label: p.DisplayName(), + Description: p.Description, + Action: func() menu.Result { return c.modelStep(g, p) }, + }) + } + return menu.Result{Next: &menu.Menu{ + Title: "Choose a provider for " + name + ":", + Items: items, + }} +} +``` + +Three conventions are at work there, and every existing client follows all three. An empty list produces an error rather than an empty menu. A single option descends straight to the next step instead of making the user press Enter on a menu of one. Anything more shows a submenu returned as `Result.Next`, which pushes onto the TUI's menu stack so Esc pops back. + +The loop variable capture is safe here because each iteration gets a fresh `p`. That has been true since Go 1.22, and this repository targets Go 1.26.2, so you do not need the old workaround. You may still see `c := c` or `p := p` lines in code such as `internal/tui/menus.go:377`; they are no longer necessary and you do not need to copy them. + +Add the model step next. This one shows the model picker only when there is a real choice to make. + +```go +func (c *Client) modelStep(g *config.Global, p config.ProviderInfo) menu.Result { + models := fqnModels(p) + if len(models) <= 1 { + var m string + if len(models) == 1 { + m = models[0] + } + return c.launch(g, p, m) + } + items := make([]menu.MenuItem, 0, len(models)) + for _, m := range models { + items = append(items, menu.MenuItem{ + Label: m, + Action: func() menu.Result { return c.launch(g, p, m) }, + }) + } + return menu.Result{Next: &menu.Menu{ + Title: "Choose a default model for " + name + " via " + p.DisplayName() + ":", + Items: items, + }} +} +``` + +Note that zero models is not an error. It passes an empty model string through to `launch`, which then omits the model environment variable entirely and lets the harness pick its own default. Some harnesses, OpenCode among them, prefer this because they have their own model picker inside the application. + +If Step 2 told you the user needs to choose a protocol, insert a backend step between the provider step and the model step. Define a `backend` struct with the fields your routing needs and a package-level slice of them, then write a `backendStep` with the same empty-check, single-option, submenu shape. `internal/clients/copilot/copilot.go:107` is the clearest example, and `internal/clients/gemini/gemini.go:119` shows a two-backend version. + +Every client also needs a way to surface an error, so add this helper at the bottom of the file. + +```go +func errorResult(msg string) menu.Result { + return menu.Result{Cmd: func() tea.Msg { + return menu.SimpleDoneMsg{Err: errString(msg)} + }} +} + +type errString string + +func (e errString) Error() string { return string(e) } +``` + +A `SimpleDoneMsg` carrying an error puts the TUI into its error state and prints your message, which you can see handled at `internal/tui/tui.go:319`. The tiny `errString` type exists so you can build an error from a string without importing `errors` or `fmt`, and every client package declares its own copy. + +The build will fail at this point, because your menu closures call a `launch` method that does not exist yet. + +```bash +go build ./... +``` + +Expect two errors, one per closure that calls `launch`, both reading `c.launch undefined (type *Client has no field or method launch)`: + +``` +internal/clients//.go:76:12: c.launch undefined (type *Client has no field or method launch) +internal/clients//.go:82:42: c.launch undefined (type *Client has no field or method launch) +``` + +Your line numbers will differ. Step 9 resolves both. If you see errors naming anything other than `c.launch`, fix those before moving on. + +This is the last verification until Step 9 if your harness needs no config file, or until the end of Step 8 if it does. Both of those steps end by building, so you will find out then whether anything you wrote here was wrong. + +## Step 8: Write the routing config, if your harness needs one + +Skip this step if Step 1 told you your harness is configured entirely through environment variables. Copilot has no config file at all, and its client is simpler for it. + +If your harness does need a file, you have a choice about lifetime. A per-launch temporary file is right when the file's contents depend on the provider and model the user just picked, and it should be deleted when the harness exits. OpenCode works this way. A persistent directory is right when the harness stores its own state alongside your config, such as credentials you do not want to destroy on every run. Codex and Gemini CLI work this way. + +For the per-launch shape, create a new file `config.go` in your package and write a function that returns the path plus a cleanup closure. This is a condensed version of `writeProviderConfig` at `internal/clients/opencode/sdk.go:79`. + +The whole file follows, including its imports and the `harnessConfig` struct. You must define that struct yourself: its fields and JSON tags have to match the schema your harness expects, which you wrote down in Step 1. The version below is a plausible shape, not a real harness's schema, so treat it as a template to replace rather than code to keep. + +```go +package + +import ( + "encoding/json" + "os" + "path/filepath" + + "github.com/tailscale/aperture-cli/internal/config" +) + +// harnessConfig is the on-disk schema expects. Replace +// these fields and JSON tags with your harness's real schema. +type harnessConfig struct { + BaseURL string `json:"baseUrl"` + APIKey string `json:"apiKey"` + Provider string `json:"provider"` + Models []string `json:"models"` +} + +// writeProviderConfig writes the per-launch config and returns its path plus +// a cleanup function that removes the file. +func writeProviderConfig(apertureHost string, p config.ProviderInfo) (string, func(), error) { + cfg := harnessConfig{ + BaseURL: apertureHost + "/v1", + APIKey: "not-needed", + Provider: p.ID, + Models: p.Models, + } + data, err := json.Marshal(cfg) + if err != nil { + return "", nil, err + } + dir, err := config.ClientConfigDir("") + if err != nil { + return "", nil, err + } + path := filepath.Join(dir, "tmp_aperture_config.json") + if err := os.WriteFile(path, data, 0o600); err != nil { + return "", nil, err + } + return path, func() { os.Remove(path) }, nil +} +``` + +The cleanup closure is the important part. You hand it to `clients.Launch` as `LaunchSpec.Cleanup`, and the TUI calls it after the harness process exits, as you can see at `internal/clients/launch.go:61`. Without it you leave a file containing your endpoint URL behind after every session. + +Use `config.ClientConfigDir` from `internal/config/client_config.go:13` rather than building a path by hand. It returns `/aperture/clients/`, creates the directory with mode `0o700`, and keeps every client's files in one predictable place. Write files themselves with mode `0o600`. If your harness insists on a fixed location in the user's home directory, follow OpenCode's example and write there instead, but keep the permissions. + +For the persistent shape, drop the cleanup function and return just the directory path, as `writeConfig` does at `internal/clients/codex/config.go:21`. Note the comments in both `codex/config.go` and `gemini/config.go` explaining that their paths are deliberately the pre-refactor legacy ones, kept so existing user credentials keep resolving. If you ever need to move a path like that, expect to migrate the contents. + +First confirm the new file compiles. Your package as a whole still will not build, because Step 7's menu closures are still waiting on `launch`, so build just this package and expect the same two `c.launch undefined` errors and nothing else. + +```bash +go build ./internal/clients// +``` + +If you see `undefined: json`, `undefined: os`, `undefined: filepath`, `undefined: config`, or `undefined: harnessConfig`, you are missing part of the file above — the import block or the struct definition. + +Then verify the behavior with a test. Add this to your test file, which now needs four more imports: `encoding/json`, `os`, `path/filepath`, and `testing`. + +```go +func TestWriteProviderConfig(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(tmp, ".config")) + + p := config.ProviderInfo{ID: "openai", Models: []string{"gpt-5"}} + path, cleanup, err := writeProviderConfig("http://ai.example.com", p) + if err != nil { + t.Fatalf("writeProviderConfig: %v", err) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("config unreadable: %v", err) + } + var got harnessConfig + if err := json.Unmarshal(data, &got); err != nil { + t.Fatal(err) + } + if got.BaseURL != "http://ai.example.com/v1" { + t.Errorf("BaseURL = %q, want http://ai.example.com/v1", got.BaseURL) + } + + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("perm = %o, want 600", perm) + } + + cleanup() + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Error("config file still exists after cleanup") + } +} +``` + +```bash +go test -run TestWriteProviderConfig ./internal/clients// +``` + +That should report `ok`. The two `t.Setenv` calls are the part to copy without thinking about it: `config.ClientConfigDir` resolves through `os.UserConfigDir()`, so without them the test writes into your own `~/.config`. `internal/clients/opencode/opencode_test.go:61` uses the same isolation for the same reason. + +## Step 9: Implement the launch + +This is where the client stops describing itself and does something. `launch` resolves the binary, assembles the environment, records what the user chose, and hands off to the shared launcher. + +There are two versions below and you want exactly one of them. Use variant A if your harness is configured entirely through environment variables and you skipped Step 8. Use variant B if you wrote a config file in Step 8. They are complete alternatives, not a base plus a patch — do not paste both. + +### Variant A: environment variables only + +Add this to your main file, above `Replay`. + +```go +func (c *Client) launch(g *config.Global, p config.ProviderInfo, model string) menu.Result { + bin := clients.FindBinary(binaryName, c.CommonPaths()) + if bin == "" { + bin = binaryName + } + + env := map[string]string{ + "": strings.TrimRight(g.ApertureHost, "/") + "/v1", + "": "not-needed", + } + if model != "" { + env[""] = stripProviderPrefix(model) + } + + var args []string + if g.Settings.YoloMode { + args = append(args, "") + } + + _ = g.RecordLaunch(config.LaunchState{ + LastClientName: name, + LastBackendType: "", + LastProviderID: p.ID, + LastModel: model, + }) + + cmd := clients.Launch(clients.LaunchSpec{ + Binary: bin, + Args: args, + Env: env, + Debug: g.Debug, + }) + return menu.Result{Cmd: cmd, PopOnDone: true} +} +``` + +Substitute the four harness-specific names from the values you gathered in Step 1. If your harness has no model environment variable, delete the whole `if model != ""` block rather than leaving the placeholder in the map, or you will set a variable literally named ``. If it has no permission-skipping flag, delete the whole `args` block the same way and drop `Args` from the `LaunchSpec`, as OpenCode does with an explanatory comment at `internal/clients/opencode/opencode.go:136`. + +Several details there are easy to get wrong. Falling back to the bare `binaryName` when `FindBinary` returns empty is deliberate: it lets the operating system try one more time and produces a clearer error than an empty path would. Trimming the trailing slash off `g.ApertureHost` matters because the user may have typed one and string concatenation will happily produce `//v1`. The `/v1` suffix is a guess based on the most common case, so use whatever path your harness and provider protocol actually need, and compare against `internal/clients/copilot/copilot.go:180`, where the suffix is added for OpenAI-style routing but not for Anthropic. The `Env` map is overlaid on the user's real environment rather than replacing it, as `internal/clients/launch.go:39` shows, so you only need to set what you are changing. + +The `RecordLaunch` error is deliberately discarded, matching every other client. A failure to persist the quick-select record is not worth interrupting a launch the user has already confirmed. `LastBackendType` must be a stable string you can match again in Step 10; if you built a `backend` struct in Step 7, use `b.id` here instead of the compatibility key. + +Setting `PopOnDone: true` is what returns the user to the root menu after the harness exits. `clients.Launch` runs the binary in the foreground through `tea.ExecProcess`, so the TUI gives up the terminal entirely and takes it back when the child exits, at which point `ExecDoneMsg` triggers a fresh preflight (`internal/tui/tui.go:297`). + +### Variant B: config file + +If you wrote a config file in Step 8, use this instead of variant A. It is the same function with the `env` map built from the config path and the cleanup closure threaded into the `LaunchSpec`. Do not paste it below variant A — a second `env := ...` in the same function is a compile error (`no new variables on left side of :=`), and an unused `cleanup` is another (`declared and not used: cleanup`). + +```go +func (c *Client) launch(g *config.Global, p config.ProviderInfo, model string) menu.Result { + bin := clients.FindBinary(binaryName, c.CommonPaths()) + if bin == "" { + bin = binaryName + } + + configPath, cleanup, err := writeProviderConfig(g.ApertureHost, p) + if err != nil { + return errorResult("Failed to write config: " + err.Error()) + } + + env := map[string]string{ + "": configPath, + } + if model != "" { + env[""] = stripProviderPrefix(model) + } + + var args []string + if g.Settings.YoloMode { + args = append(args, "") + } + + _ = g.RecordLaunch(config.LaunchState{ + LastClientName: name, + LastBackendType: "", + LastProviderID: p.ID, + LastModel: model, + }) + + cmd := clients.Launch(clients.LaunchSpec{ + Binary: bin, + Args: args, + Env: env, + Cleanup: cleanup, + Debug: g.Debug, + }) + return menu.Result{Cmd: cmd, PopOnDone: true} +} +``` + +`Cleanup: cleanup` is the field to not forget; without it every launch leaves a config file behind. Whether your harness also needs a base URL and API key in the environment depends on its contract: some read everything from the config file, others still want the URL in both places. Set whichever Step 1 told you it reads, and note that if you end up needing none of `strings.TrimRight`, `stripProviderPrefix`, or anything else from `strings`, Go will reject the now-unused import. + +### Verify either variant + +The build should now be clean again, because `launch` exists and every menu closure can reach it. + +```bash +go build ./... && go test ./internal/clients// +``` + +The build should print nothing and the tests should report `ok`. Now test your environment construction the way Copilot's tests do at `internal/clients/copilot/copilot_test.go:11`, by pulling the environment building out into its own `buildEnv` function and asserting on the map it returns. That refactor is worth doing precisely because it makes the routing testable without launching anything. + +## Step 10: Implement replay and quick select + +The root menu offers a `[0]` row that re-runs the user's last session in one keystroke. `Replay` decides whether your client can honor that, and `QuickSelectLabel` describes it. + +Add both methods, plus the `"slices"` import that the model check needs. + +```go +// Replay implements clients.Client. +func (c *Client) Replay(g *config.Global) tea.Cmd { + if g.LastLaunch.LastClientName != name || !c.IsInstalled() { + return nil + } + prov, ok := g.Provider(g.LastLaunch.LastProviderID) + if !ok { + return nil + } + if !providerMatches(prov) { + return nil + } + model := g.LastLaunch.LastModel + if model != "" && !slices.Contains(fqnModels(prov), model) { + return nil + } + res := c.launch(g, prov, model) + return res.Cmd +} + +// QuickSelectLabel implements clients.Client. +func (c *Client) QuickSelectLabel(g *config.Global) string { + prov, _ := g.Provider(g.LastLaunch.LastProviderID) + label := name + " via " + prov.DisplayName() + if g.LastLaunch.LastModel != "" { + label += " - " + g.LastLaunch.LastModel + } + return label +} +``` + +`Replay` is a chain of staleness checks, and returning `nil` from any of them means "I cannot replay this", which is normal rather than an error. Check all four things. The launch record must name your client, or another client owns it. The binary must still be installed, since the user may have removed it. The provider must still exist in the freshly fetched list, since the endpoint's configuration may have changed. The recorded model must still be offered by that provider, since model lists change often. If you added a backend step, also confirm the recorded backend ID still exists and that the provider still supports it, as at `internal/clients/copilot/copilot.go:203`. + +Two subtleties are worth knowing. `Replay` returns the `tea.Cmd` from inside the `menu.Result` rather than the `Result` itself, because the root menu wraps it in a fresh `Result` at `internal/tui/menus.go:43`. And `QuickSelectLabel` is only ever called after `Replay` returned non-nil, which is why ignoring the `ok` from `g.Provider` is safe there. On a zero `ProviderInfo`, `DisplayName()` returns an empty string rather than panicking. + +Verify with a test in the style of `internal/clients/codex/codex_test.go:105`, added to your existing test file. + +```go +func TestReplay_NotReplayable(t *testing.T) { + c := &Client{} + g := &config.Global{ + LastLaunch: config.LaunchState{ + LastClientName: name, + LastProviderID: "missing", + }, + } + // Returns nil on the first failing check. On a machine without the + // harness installed that is !IsInstalled(), so this asserts "does not + // replay" rather than "rejects a stale provider" specifically. + if cmd := c.Replay(g); cmd != nil { + t.Error("Replay should return nil when the launch cannot be replayed") + } +} +``` + +Be clear about what that test does and does not prove. `Replay` returns `nil` at the first check that fails, and `!c.IsInstalled()` comes before the provider lookup. On any machine where your harness is not installed — including CI, which installs no agents — this test passes without ever reaching the provider check, and it would keep passing if you deleted that check entirely. The codex test it is modeled on says as much in a comment at `internal/clients/codex/codex_test.go:113`. + +If you want real coverage of the staleness logic, test the parts that do not depend on the binary being present. `providerMatches` and `fqnModels` are both unexported and directly callable, and between them they decide three of the four checks: + +```go +func TestReplayStalenessChecks(t *testing.T) { + prov := config.ProviderInfo{ + ID: "openai", + Models: []string{"gpt-5"}, + Compatibility: map[string]bool{compatKey: true}, + } + if !providerMatches(prov) { + t.Error("provider with our compat key should match") + } + if providerMatches(config.ProviderInfo{ + Compatibility: map[string]bool{"something_else": true}, + }) { + t.Error("provider without our compat key should not match") + } + if got := fqnModels(prov); len(got) != 1 || got[0] != "openai/gpt-5" { + t.Errorf("fqnModels = %v, want [openai/gpt-5]", got) + } + // A recorded model that the provider no longer lists is what makes + // Replay bail on the model check. + if slices.Contains(fqnModels(prov), "openai/gpt-4") { + t.Error("stale model should not be found in the current model list") + } +} +``` + +Your type now has every method the interface requires, so check the build, the vet pass, and the tests together. + +```bash +go build ./... && go vet ./... && go test ./internal/clients// +``` + +The first two should produce no output and the tests should report `ok`. To confirm you really did satisfy the interface, which nothing has actually asserted yet, add this line to your main file. + +```go +var _ clients.Client = (*Client)(nil) +``` + +That is a compile-time assertion: if any method is missing or has the wrong signature, `go build` names it. Step 11 replaces the need for it with the real `init()`, but it is a faster way to find a typo in a method signature right now. If the build reports a missing method, compare your method set against the interface at `internal/clients/registry.go:16` and check that every receiver is `*Client` rather than `Client`. + +## Step 11: Register the client, test it, and update the README + +Your package compiles and satisfies the interface, but the launcher still does not know it exists, because nothing registers it and nothing links it into the binary. + +First add the registration hook to your main file, just below the `Client` type. If you added the `var _ clients.Client` assertion in Step 10, delete it now, since `Register` does the same job. + +```go +func init() { + clients.Register(&Client{}) +} +``` + +Then open `cmd/aperture/main.go` and add your package to the side-effect import block that starts at line 20. Insert the line in its correct alphabetical position, not at the end of the block. The example below shows where a package named `nimbus` would go. + +```go + // Side-effect imports register each client with internal/clients. + _ "github.com/tailscale/aperture-cli/internal/clients/claudecode" + _ "github.com/tailscale/aperture-cli/internal/clients/codex" + _ "github.com/tailscale/aperture-cli/internal/clients/copilot" + _ "github.com/tailscale/aperture-cli/internal/clients/gemini" + _ "github.com/tailscale/aperture-cli/internal/clients/nimbus" + _ "github.com/tailscale/aperture-cli/internal/clients/opencode" +``` + +Alphabetical order is not a style preference here, it is what gofmt enforces. gofmt sorts the paths within an import block, so appending your line at the end leaves the file unformatted and fails the CI formatting gate described below. If you are unsure where the line goes, put it anywhere and run `gofmt -w cmd/aperture/main.go` to have it moved for you. + +The blank identifier import exists purely to run your `init()`, which calls `clients.Register`. Registration order is display order in the menu, per the comment at `internal/clients/registry.go:78`, and registration order follows the order of the imports in this block. That means your client's position in the menu is decided by where your package name sorts alphabetically, and you cannot change it by moving the import line: gofmt will sort it straight back, and leaving it out of order fails CI. Your client will appear between the packages that alphabetically surround it. If a client ever genuinely needs a different position, that calls for an explicit ordering mechanism in `internal/clients`, not a hand-ordered import block. + +From here on, a missing or misnamed interface method breaks the build rather than showing up as a missing menu row, which is exactly what you want. + +Next, finish your tests. Aim to cover the environment or config your client produces for each protocol it supports, the provider filter, the backend filter if you have one, the install and uninstall hints, and at least one `Replay` staleness path. Every existing client package covers roughly that set, and `internal/clients/claudecode/claudecode_test.go` is the most thorough example. Test the unexported helpers directly, in the same package, rather than trying to drive the TUI. + +Run the checks CI runs. The formatting check is not advisory: the Linux CI job fails the build if `gofmt -l .` prints anything. + +```bash +gofmt -l . && make test && make build +``` + +`gofmt -l .` should print nothing at all. `make test` should report `ok` for every package including yours. If `gofmt` lists files, run `gofmt -w .` and re-check. + +Now test the real thing by launching the built binary. + +```bash +./.build/aperture +``` + +Your client should appear in the root menu if the harness is installed, or under `[i] Install agents` if it is not. Select it, pick a provider and model, and confirm the harness starts and can complete a request through Aperture. Then quit the harness, confirm you land back on the root menu, and check that `[0] Quick select` now names your client. Re-run with the debug flag to see exactly what you are setting. + +```bash +./.build/aperture -debug +``` + +That prints the resolved environment and arguments to stderr before exec, which is the fastest way to spot a wrong variable name or a doubled slash in the URL. + +Finally, add your harness to the `Supported agents` list in `README.md`, with a link to its documentation, so the list stays accurate. Then commit. The repository's commit messages lead with the touched paths, as in `internal/profiles: add z.ai backend for Claude Code with fixed models (#14)`, so a message like `internal/clients: add client` fits the house style. + +## Troubleshooting + +These are the failure modes you are most likely to hit, roughly in order of how often they come up. + +### The client does not appear in the root menu + +First check whether the launcher thinks the harness is uninstalled. Press `i` for `Install agents` and look for your client's name there. If it is in that list, your discovery logic is the problem rather than your registration, so re-read Step 4 and confirm `binaryName` exactly matches the executable name and that your `commonBinaryPaths` entries are full paths to the binary rather than directories. + +If it appears in neither list, your package is not linked into the binary. Confirm your import line is present in `cmd/aperture/main.go` and that it uses the blank identifier. A normal import of a package you never reference will not compile, and a missing import produces no error at all, which is why this failure is silent. + +Then confirm your `init()` function actually calls `clients.Register(&Client{})`, and that you are running a freshly built binary. Run `make build` again and use `./.build/aperture` rather than an `aperture` on your `$PATH` from an earlier `make install`. + +One more silent case: the root menu skips any installed client whose `Menu()` returns a `MenuItem` with a nil `Action`, at `internal/tui/menus.go:49`. If your client registers and is installed but still never appears, confirm `Menu` sets `Action`. + +### The launcher says no providers support your client + +Your compatibility key does not match anything the endpoint offers. Fetch the provider list directly and look at the actual keys. + +```bash +curl -s /api/providers +``` + +Compare the `compatibility` object in that response against the key in your code, watching for typos and for singular versus plural forms. If no provider sets your key, the client is behaving correctly and the gap is on the Aperture side, so choose a different protocol your harness also speaks or configure the provider in Aperture. + +If the response is empty or the request fails, the problem is connectivity rather than compatibility, and the launcher's own preflight would have shown you its setup guide before you got this far. + +### The harness starts but every request fails + +Run the launcher with `-debug` and read the environment it printed. Check the base URL first, looking for a missing or doubled `/v1`, a doubled slash from an untrimmed host, or a trailing slash the harness does not tolerate. + +Next check whether the model name still carries its provider prefix. If your debug output shows something like `openai/gpt-5` where the harness expects `gpt-5`, you are missing a `stripProviderPrefix` call, and path-based routing will produce a confusing 404 rather than a clear error. + +If the failure looks like an authentication error, your placeholder API key value may not satisfy the harness's validation. Try the other conventions used in this repo, which are `not-needed`, `not-required`, and a bare `-`. + +If the harness reads a config file, confirm the file exists and contains what you expect while the harness is running. Add a temporary `fmt.Fprintln(os.Stderr, configPath)` in `launch`, or comment out the cleanup closure so the file survives the exit, then inspect it. + +One case is specific to Gemini CLI and may apply to your harness too. Gemini CLI rejects base URLs that are not HTTPS with a fully-qualified domain name, so the default `http://ai` endpoint cannot work with it. The client blocks the launch with an explanation rather than letting the harness fail confusingly, in `validateHost` at `internal/clients/gemini/gemini.go:256`. If your harness validates URLs similarly, copy that approach. + +### The quick select row never appears + +`Replay` is returning `nil`. Work through its four checks in order. Confirm the `name` constant you compare against `LastClientName` is byte-identical to the one you pass to `RecordLaunch`, since a display name that changed between the two will never match. Confirm the binary is still installed. Confirm the recorded provider ID is still in the fetched list. Confirm the recorded model is still in `fqnModels(prov)`. + +Read the persisted record directly to see what was actually stored. On macOS the file is here. + +```bash +cat ~/Library/Application\ Support/aperture/launcher.json +``` + +On Linux it is at `~/.config/aperture/launcher.json` instead, or under `$XDG_CONFIG_HOME` if you have set that. Both paths come from `os.UserConfigDir()` in `statePath` at `internal/config/state.go:19`. + +```bash +cat ~/.config/aperture/launcher.json +``` + +Compare the `lastClientName`, `lastProviderId`, and `lastModel` values in that JSON against what your checks expect. If the file is missing entirely, `RecordLaunch` never succeeded, so confirm you are calling it inside `launch`. + +### CI fails on formatting + +The Linux CI job runs `gofmt -l .` and fails if it prints any filename. Run `gofmt -w .` from the repository root and commit the result. This catches people who write Go without a formatting editor hook, and it is the single most common CI failure in this repository. + +### A temporary config file is left behind after the harness exits + +You built a cleanup closure but did not pass it through. Confirm you set `Cleanup: cleanup` on the `clients.LaunchSpec` in `launch`. The TUI calls it after the child process exits, at `internal/clients/launch.go:61`, and it is never called if the field is nil. + +Note that cleanup does not run if the launcher itself is killed mid-session, so treat the file as best-effort and never put a real secret in it. + +## Security notes + +The launcher deliberately writes placeholder credentials rather than real ones. Aperture authenticates the caller itself over Tailscale, so the harness's own API key check has nothing to validate. That is why you see literal strings such as `not-needed`, `not-required`, and `-` throughout the client packages. Keep using placeholders, and never add a code path that reads a real API key from the user's environment and forwards it, because that would move a live credential into a config file the launcher writes. + +Set file permissions the way the existing code does. Config files are written with mode `0o600` and directories created with `0o700`, so nothing you write is readable by other users on a shared machine. `config.ClientConfigDir` already creates its directory with `0o700`, so use it rather than calling `os.MkdirAll` yourself, and pass `0o600` to every `os.WriteFile`. + +Remember that the config files you write contain the user's Aperture endpoint URL, which reveals a tailnet hostname. That is not a credential, but it is not something to leave lying around either, which is the practical reason the per-launch cleanup closure exists. Cleanup is best-effort and will not run if the launcher is killed, so do not rely on it to protect anything that actually matters. + +Be careful with the `-debug` flag. It dumps the full resolved environment to stderr before exec, at `internal/clients/launch.go:44`, so anything you put in the `Env` map ends up in the user's terminal scrollback and in any log they paste into a bug report. Since the values are placeholders this is safe today, and it stays safe only as long as you keep real secrets out of that map. + +Finally, keep real endpoints out of your tests. The existing test files use a `testHost` constant set to `http://ai.example.com`, and they redirect `HOME` and `XDG_CONFIG_HOME` to a `t.TempDir()` so nothing touches the real config directory. Copy both habits. A test that writes to your actual `~/.config` will eventually corrupt someone's working setup, and a committed internal hostname is a small but needless disclosure. From da5e4c5a2a3da970139b7c1be76315da27132a66 Mon Sep 17 00:00:00 2001 From: Larah Vasquez Date: Wed, 12 Aug 2026 08:45:37 -0500 Subject: [PATCH 2/8] internal/clients: add Pi client Pi has no environment variable for a custom API base URL, so routing is expressed as a provider definition. This client generates a per-launch extension and loads it with 'pi -e', leaving the user's own ~/.pi/agent directory (settings, logins, session history) untouched. Supports the four wire protocols Aperture serves: OpenAI Responses, Anthropic Messages, OpenAI Chat Completions, and Google Vertex. Bedrock is omitted because Pi's bedrock API type fails at request time. Registers the client in cmd/aperture/main.go and lists it in the README. --- README.md | 1 + cmd/aperture/main.go | 1 + internal/clients/pi/extension.go | 160 +++++++ internal/clients/pi/install.go | 25 ++ internal/clients/pi/pi.go | 337 +++++++++++++++ internal/clients/pi/pi_test.go | 710 +++++++++++++++++++++++++++++++ 6 files changed, 1234 insertions(+) create mode 100644 internal/clients/pi/extension.go create mode 100644 internal/clients/pi/install.go create mode 100644 internal/clients/pi/pi.go create mode 100644 internal/clients/pi/pi_test.go diff --git a/README.md b/README.md index 4f57bab..40d2e89 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ A CLI launcher for coding agents preconfigured to work with [Aperture](https://a - [OpenCode](https://github.com/sst/opencode) - [Codex](https://github.com/openai/codex) - [GitHub Copilot CLI](https://docs.github.com/en/copilot/how-tos/copilot-cli/cli-getting-started) +- [Pi](https://pi.dev) - [Claude Cowork](https://support.claude.com/en/articles/13345190-get-started-with-claude-cowork) ## Installation diff --git a/cmd/aperture/main.go b/cmd/aperture/main.go index cdc5327..ab36757 100644 --- a/cmd/aperture/main.go +++ b/cmd/aperture/main.go @@ -23,6 +23,7 @@ import ( _ "github.com/tailscale/aperture-cli/internal/clients/copilot" _ "github.com/tailscale/aperture-cli/internal/clients/gemini" _ "github.com/tailscale/aperture-cli/internal/clients/opencode" + _ "github.com/tailscale/aperture-cli/internal/clients/pi" ) var ( diff --git a/internal/clients/pi/extension.go b/internal/clients/pi/extension.go new file mode 100644 index 0000000..0051bba --- /dev/null +++ b/internal/clients/pi/extension.go @@ -0,0 +1,160 @@ +package pi + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + + "github.com/tailscale/aperture-cli/internal/config" +) + +// piProvider is the provider-config form accepted by pi's +// pi.registerProvider(id, config) extension API. Field names match pi's +// documented models.json / registerProvider schema. +type piProvider struct { + Name string `json:"name"` + BaseURL string `json:"baseUrl"` + APIKey string `json:"apiKey"` + API string `json:"api"` + Models []piModel `json:"models"` +} + +// piModel is one entry in a provider's model list. +// +// Every field must be populated. Unlike the models.json path, which fills in +// defaults for a partial model definition, a provider registered from an +// extension is used as given: an omitted maxTokens reaches the endpoint as a +// literal null and Anthropic rejects the request with "max_tokens: expected +// number, received null". An omitted input list crashes pi outright, because +// its --list-models formatter dereferences it without a nil check. +// +// reasoning stays false because GET /api/providers reports no thinking +// capability, and claiming it makes pi send parameters the model may reject. +type piModel struct { + ID string `json:"id"` + Name string `json:"name"` + Reasoning bool `json:"reasoning"` + Input []string `json:"input"` + ContextWindow int `json:"contextWindow"` + MaxTokens int `json:"maxTokens"` + Cost piCost `json:"cost"` +} + +// Aperture's provider list carries no token metadata, so every model gets +// pi's own documented defaults rather than invented per-model numbers. Users +// who need different limits can override them per model in models.json, +// which pi composes above an extension-registered provider. +const ( + defaultContextWindow = 128000 + defaultMaxTokens = 16384 +) + +// piCost is pi's per-million-token rate block. Aperture reports no pricing, +// so every rate is zero. +type piCost struct { + Input float64 `json:"input"` + Output float64 `json:"output"` + CacheRead float64 `json:"cacheRead"` + CacheWrite float64 `json:"cacheWrite"` +} + +// piProviderID namespaces the Aperture provider ID so registering it can +// never overwrite one of pi's built-in providers (pi merges a registration +// into a same-named built-in, which would silently retarget the user's own +// "anthropic" or "openai" models at Aperture). +func piProviderID(providerID string) string { + return "aperture-" + providerID +} + +// piModelRef is the "provider/model" reference pi's --model flag expects, +// built from the namespaced provider ID and a bare model ID. +func piModelRef(providerID, model string) string { + return piProviderID(providerID) + "/" + stripProviderPrefix(model) +} + +// baseURL returns the endpoint pi should call for this backend. The suffix +// differs per wire protocol: OpenAI-style APIs are rooted at /v1, Anthropic +// takes the bare host because pi appends /v1/messages itself, and Vertex +// needs the project-scoped publisher path that Aperture's router matches. +func (b backend) baseURL(apertureHost string) string { + host := strings.TrimRight(apertureHost, "/") + switch b.id { + case "anthropic": + return host + case "vertex": + // The magic _aperture_auto_*_ placeholders are rewritten upstream, + // as in internal/clients/opencode/sdk.go. + return host + "/v1/projects/_aperture_auto_vertex_project_id_/locations/_aperture_auto_vertex_region_/publishers/google" + default: + return host + "/v1" + } +} + +// buildProvider assembles the pi provider config for one Aperture provider +// routed over the given backend. +func buildProvider(apertureHost string, p config.ProviderInfo, b backend) piProvider { + models := make([]piModel, len(p.Models)) + for i, m := range p.Models { + models[i] = piModel{ + ID: m, + Name: m, + Input: []string{"text"}, + Reasoning: false, + ContextWindow: defaultContextWindow, + MaxTokens: defaultMaxTokens, + } + } + return piProvider{ + Name: "Aperture (" + p.ID + ")", + BaseURL: b.baseURL(apertureHost), + APIKey: "not-needed", + API: b.api, + Models: models, + } +} + +// extensionSource renders the JavaScript extension pi loads with -e. The +// provider config is emitted as marshaled JSON so no value needs hand +// escaping. +func extensionSource(providerID string, prov piProvider) (string, error) { + id, err := json.Marshal(piProviderID(providerID)) + if err != nil { + return "", err + } + cfg, err := json.MarshalIndent(prov, " ", " ") + if err != nil { + return "", err + } + return "// Generated by aperture-cli. Rewritten on every launch and removed\n" + + "// when the agent exits; safe to delete.\n" + + "export default function (pi) {\n" + + " pi.registerProvider(" + string(id) + ", " + string(cfg) + ");\n" + + "}\n", nil +} + +// writeProviderExtension writes the per-launch pi extension and returns its +// path plus a cleanup function that removes the file. +// +// Routing pi through an extension rather than its own models.json is +// deliberate. pi reads models.json from the directory named by +// PI_CODING_AGENT_DIR, but that same directory also roots settings.json, +// auth.json, sessions, themes, and extensions. Redirecting it would hide the +// user's saved logins, settings, and session history — breaking --continue +// and --resume — so instead we inject the provider for one run and leave +// ~/.pi/agent untouched. +func writeProviderExtension(apertureHost string, p config.ProviderInfo, b backend) (string, func(), error) { + src, err := extensionSource(p.ID, buildProvider(apertureHost, p, b)) + if err != nil { + return "", nil, err + } + dir, err := config.ClientConfigDir("pi") + if err != nil { + return "", nil, err + } + path := filepath.Join(dir, "tmp_aperture_provider.js") + if err := os.WriteFile(path, []byte(src), 0o600); err != nil { + return "", nil, err + } + return path, func() { os.Remove(path) }, nil +} diff --git a/internal/clients/pi/install.go b/internal/clients/pi/install.go new file mode 100644 index 0000000..4da75fd --- /dev/null +++ b/internal/clients/pi/install.go @@ -0,0 +1,25 @@ +package pi + +import ( + "os" + "path/filepath" +) + +// commonBinaryPaths returns the non-PATH locations where pi is commonly +// installed. Homebrew's npm prefix is listed because the pi.dev installer +// and `npm install -g` both land there on a Homebrew-managed Node, which is +// not always on PATH in a fresh shell. +func commonBinaryPaths() []string { + paths := []string{ + filepath.Join("/opt", "homebrew", "bin", "pi"), + filepath.Join("/usr", "local", "bin", "pi"), + } + home, err := os.UserHomeDir() + if err != nil { + return paths + } + return append(paths, + filepath.Join(home, ".pi", "bin", "pi"), + filepath.Join(home, ".npm-global", "bin", "pi"), + ) +} diff --git a/internal/clients/pi/pi.go b/internal/clients/pi/pi.go new file mode 100644 index 0000000..bd733ef --- /dev/null +++ b/internal/clients/pi/pi.go @@ -0,0 +1,337 @@ +// Package pi is the Pi coding agent client. Pi has no environment variable +// for a custom API base URL — routing is expressed as a provider definition, +// either in ~/.pi/agent/models.json or through an extension that calls +// pi.registerProvider(). This client writes a per-launch extension and loads +// it with `pi -e`, which leaves the user's own pi config directory (settings, +// logins, session history) untouched. See extension.go for why. +// +// Pi speaks four wire protocols that Aperture serves: OpenAI Chat +// Completions, OpenAI Responses, Anthropic Messages, and Google Generative +// AI (Vertex), so the menu flow is provider, then backend, then model. +package pi + +import ( + "os/exec" + "slices" + "strings" + + tea "github.com/charmbracelet/bubbletea" + "github.com/tailscale/aperture-cli/internal/clients" + "github.com/tailscale/aperture-cli/internal/config" + "github.com/tailscale/aperture-cli/internal/menu" +) + +func init() { + clients.Register(&Client{}) +} + +// Client is the Pi client. +type Client struct{} + +const ( + name = "Pi" + binaryName = "pi" + + installCmd = "npm install -g --ignore-scripts @earendil-works/pi-coding-agent" + uninstallCmd = "npm uninstall -g @earendil-works/pi-coding-agent" +) + +// backend is one Pi wire protocol paired with the Aperture compatibility key +// a provider must set to serve it. +type backend struct { + id string + displayName string + // api is the value Pi expects in a provider definition's "api" field. + api string + // compatKeys are the Aperture keys that satisfy this backend; a provider + // matches if any one is set. + compatKeys []string +} + +// backends is ordered most-preferred first, which is also the order the +// backend menu shows. Bedrock is absent on purpose: Pi's +// bedrock-converse-stream API type loads from a provider definition but +// fails at request time against Aperture, so offering it would only produce +// a confusing runtime error. +var backends = []backend{ + {id: "openai_responses", displayName: "OpenAI Responses", api: "openai-responses", compatKeys: []string{"openai_responses"}}, + {id: "anthropic", displayName: "Anthropic Messages", api: "anthropic-messages", compatKeys: []string{"anthropic_messages"}}, + {id: "openai_chat", displayName: "OpenAI Chat Completions", api: "openai-completions", compatKeys: []string{"openai_chat"}}, + {id: "vertex", displayName: "Google Vertex", api: "google-generative-ai", compatKeys: []string{"google_generate_content", "google_raw_predict"}}, +} + +// Name implements clients.Client. +func (c *Client) Name() string { return name } + +// BinaryName implements clients.Client. +func (c *Client) BinaryName() string { return binaryName } + +// CommonPaths implements clients.Client. +func (c *Client) CommonPaths() []string { return commonBinaryPaths() } + +// IsInstalled implements clients.Client. +func (c *Client) IsInstalled() bool { + return clients.IsInstalled(binaryName, c.CommonPaths()) +} + +// Install implements clients.Client. +func (c *Client) Install(_ *config.Global) clients.InstallPlan { + return clients.InstallPlan{ + Hint: installCmd, + Run: func() (*exec.Cmd, error) { + return exec.Command("/bin/sh", "-c", installCmd), nil + }, + } +} + +// Uninstall implements clients.Client. +func (c *Client) Uninstall() clients.UninstallPlan { + return clients.UninstallPlan{ + Hint: uninstallCmd, + Run: func() error { + // Split into separate arguments: there is no shell here. + return exec.Command("npm", "uninstall", "-g", "@earendil-works/pi-coding-agent").Run() + }, + } +} + +// Menu implements clients.Client. +func (c *Client) Menu(g *config.Global) menu.MenuItem { + return menu.MenuItem{ + Label: name, + Action: func() menu.Result { return c.providerStep(g) }, + } +} + +func (c *Client) providerStep(g *config.Global) menu.Result { + provs := compatibleProviders(g.Providers) + if len(provs) == 0 { + return errorResult("No providers support " + name + ".") + } + if len(provs) == 1 { + return c.backendStep(g, provs[0]) + } + items := make([]menu.MenuItem, 0, len(provs)) + for _, p := range provs { + items = append(items, menu.MenuItem{ + Label: p.DisplayName(), + Description: p.Description, + Action: func() menu.Result { return c.backendStep(g, p) }, + }) + } + return menu.Result{Next: &menu.Menu{ + Title: "Choose a provider for " + name + ":", + Items: items, + }} +} + +func (c *Client) backendStep(g *config.Global, p config.ProviderInfo) menu.Result { + bs := backendsFor(p) + if len(bs) == 0 { + return errorResult("No compatible backends for " + p.DisplayName() + ".") + } + if len(bs) == 1 { + return c.modelStep(g, p, bs[0]) + } + items := make([]menu.MenuItem, 0, len(bs)) + for _, b := range bs { + items = append(items, menu.MenuItem{ + Label: b.displayName, + Action: func() menu.Result { return c.modelStep(g, p, b) }, + }) + } + return menu.Result{Next: &menu.Menu{ + Title: "Choose a backend for " + name + " via " + p.DisplayName() + ":", + Items: items, + }} +} + +func (c *Client) modelStep(g *config.Global, p config.ProviderInfo, b backend) menu.Result { + models := fqnModels(p) + if len(models) <= 1 { + var m string + if len(models) == 1 { + m = models[0] + } + return c.launch(g, p, b, m) + } + items := make([]menu.MenuItem, 0, len(models)) + for _, m := range models { + items = append(items, menu.MenuItem{ + Label: m, + Action: func() menu.Result { return c.launch(g, p, b, m) }, + }) + } + return menu.Result{Next: &menu.Menu{ + Title: "Choose a default model for " + name + " via " + p.DisplayName() + ":", + Items: items, + }} +} + +func (c *Client) launch(g *config.Global, p config.ProviderInfo, b backend, model string) menu.Result { + bin := clients.FindBinary(binaryName, c.CommonPaths()) + if bin == "" { + bin = binaryName + } + + extPath, cleanup, err := writeProviderExtension(g.ApertureHost, p, b) + if err != nil { + return errorResult("Failed to write " + name + " provider extension: " + err.Error()) + } + + args := buildArgs(extPath, p.ID, model) + + _ = g.RecordLaunch(config.LaunchState{ + LastClientName: name, + LastBackendType: b.id, + LastProviderID: p.ID, + LastModel: model, + }) + + cmd := clients.Launch(clients.LaunchSpec{ + Binary: bin, + Args: args, + Cleanup: cleanup, + Debug: g.Debug, + }) + return menu.Result{Cmd: cmd, PopOnDone: true} +} + +// buildArgs assembles Pi's command line: load the generated extension, and +// preselect the model when the user chose one. Routing lives entirely in the +// extension, so no environment variables are set. +// +// Nothing here honors g.Settings.YoloMode, and nothing should. Pi ships no +// sandbox and never prompts before running a tool, so it has no +// skip-permissions flag to pass. Its --approve/-a flag looks like one but +// governs whether project-local .pi files are trusted, which is unrelated to +// tool approval and not the user's intent when they enable yolo mode. +func buildArgs(extPath, providerID, model string) []string { + args := []string{"-e", extPath} + if model != "" { + args = append(args, "--model", piModelRef(providerID, model)) + } + return args +} + +// resolveReplay reports whether g.LastLaunch still describes a launch this +// client can repeat, returning the provider, backend, and model to use. +// +// Every staleness check except the binary-installed one lives here so it can +// be tested directly. Driving Replay instead would prove very little: Replay +// returns nil at !IsInstalled() before reaching any of this, so on a machine +// without pi — including CI, which installs no agents — such a test passes +// even if the checks below are deleted. +func resolveReplay(g *config.Global) (config.ProviderInfo, backend, string, bool) { + if g.LastLaunch.LastClientName != name { + return config.ProviderInfo{}, backend{}, "", false + } + // The provider must still exist in the freshly fetched list. + prov, ok := g.Provider(g.LastLaunch.LastProviderID) + if !ok { + return config.ProviderInfo{}, backend{}, "", false + } + // The recorded backend must still be one we offer. + b, ok := backendByID(g.LastLaunch.LastBackendType) + if !ok { + return config.ProviderInfo{}, backend{}, "", false + } + // The provider must still serve that backend's protocol. + if !providerSupports(prov, b) { + return config.ProviderInfo{}, backend{}, "", false + } + // The recorded model must still be offered by that provider. + model := g.LastLaunch.LastModel + if model != "" && !slices.Contains(fqnModels(prov), model) { + return config.ProviderInfo{}, backend{}, "", false + } + return prov, b, model, true +} + +// Replay implements clients.Client. +func (c *Client) Replay(g *config.Global) tea.Cmd { + if !c.IsInstalled() { + return nil + } + prov, b, model, ok := resolveReplay(g) + if !ok { + return nil + } + res := c.launch(g, prov, b, model) + return res.Cmd +} + +// QuickSelectLabel implements clients.Client. +func (c *Client) QuickSelectLabel(g *config.Global) string { + prov, _ := g.Provider(g.LastLaunch.LastProviderID) + label := name + " via " + prov.DisplayName() + if b, ok := backendByID(g.LastLaunch.LastBackendType); ok { + label += " - " + b.displayName + } + if g.LastLaunch.LastModel != "" { + label += " - " + g.LastLaunch.LastModel + } + return label +} + +func compatibleProviders(all []config.ProviderInfo) []config.ProviderInfo { + var out []config.ProviderInfo + for _, p := range all { + if len(backendsFor(p)) > 0 { + out = append(out, p) + } + } + return out +} + +func backendsFor(p config.ProviderInfo) []backend { + var out []backend + for _, b := range backends { + if providerSupports(p, b) { + out = append(out, b) + } + } + return out +} + +func providerSupports(p config.ProviderInfo, b backend) bool { + for _, k := range b.compatKeys { + if p.Compatibility[k] { + return true + } + } + return false +} + +func backendByID(id string) (backend, bool) { + idx := slices.IndexFunc(backends, func(b backend) bool { return b.id == id }) + if idx < 0 { + return backend{}, false + } + return backends[idx], true +} + +func fqnModels(p config.ProviderInfo) []string { + out := make([]string, len(p.Models)) + for i, m := range p.Models { + out[i] = p.ID + "/" + m + } + return out +} + +func stripProviderPrefix(fqn string) string { + if _, after, ok := strings.Cut(fqn, "/"); ok { + return after + } + return fqn +} + +func errorResult(msg string) menu.Result { + return menu.Result{Cmd: func() tea.Msg { + return menu.SimpleDoneMsg{Err: errString(msg)} + }} +} + +type errString string + +func (e errString) Error() string { return string(e) } diff --git a/internal/clients/pi/pi_test.go b/internal/clients/pi/pi_test.go new file mode 100644 index 0000000..cd96c8e --- /dev/null +++ b/internal/clients/pi/pi_test.go @@ -0,0 +1,710 @@ +package pi + +import ( + "encoding/json" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/tailscale/aperture-cli/internal/config" +) + +const testHost = "http://ai.example.com" + +// isolateConfigDir points config.ClientConfigDir at a temp directory so tests +// never write into the developer's real ~/.config. +func isolateConfigDir(t *testing.T) { + t.Helper() + tmp := t.TempDir() + t.Setenv("HOME", tmp) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(tmp, ".config")) +} + +func backendByIDOrFatal(t *testing.T, id string) backend { + t.Helper() + b, ok := backendByID(id) + if !ok { + t.Fatalf("no backend with id %q", id) + } + return b +} + +func TestBackendBaseURL(t *testing.T) { + vertexPath := "/v1/projects/_aperture_auto_vertex_project_id_/locations/_aperture_auto_vertex_region_/publishers/google" + cases := []struct { + backendID string + want string + }{ + // Pi appends /v1/messages itself, so Anthropic takes the bare host. + {"anthropic", testHost}, + {"openai_chat", testHost + "/v1"}, + {"openai_responses", testHost + "/v1"}, + {"vertex", testHost + vertexPath}, + } + for _, tc := range cases { + t.Run(tc.backendID, func(t *testing.T) { + b := backendByIDOrFatal(t, tc.backendID) + if got := b.baseURL(testHost); got != tc.want { + t.Errorf("baseURL = %q, want %q", got, tc.want) + } + }) + } +} + +// TestBackendBaseURL_TrimsTrailingSlash guards the routing bug this would +// otherwise cause: pi hangs on a doubled slash rather than following the +// endpoint's redirect. +func TestBackendBaseURL_TrimsTrailingSlash(t *testing.T) { + for _, b := range backends { + t.Run(b.id, func(t *testing.T) { + got := b.baseURL(testHost + "/") + if strings.Contains(got, "//v1") || strings.HasSuffix(got, "//") { + t.Errorf("baseURL = %q, contains a doubled slash", got) + } + if want := b.baseURL(testHost); got != want { + t.Errorf("baseURL with trailing slash = %q, want %q", got, want) + } + }) + } +} + +func TestBuildProvider(t *testing.T) { + p := config.ProviderInfo{ + ID: "openai-api", + Name: "OpenAI", + Models: []string{"gpt-5", "gpt-5-mini"}, + Compatibility: map[string]bool{"openai_responses": true}, + } + b := backendByIDOrFatal(t, "openai_responses") + prov := buildProvider(testHost, p, b) + + if prov.API != "openai-responses" { + t.Errorf("api = %q, want openai-responses", prov.API) + } + if prov.BaseURL != testHost+"/v1" { + t.Errorf("baseUrl = %q, want %q", prov.BaseURL, testHost+"/v1") + } + // Pi drops a provider whose models carry no apiKey: it loads the file but + // never offers the models in /model or --list-models. + if prov.APIKey != "not-needed" { + t.Errorf("apiKey = %q, want not-needed", prov.APIKey) + } + if prov.Name != "Aperture (openai-api)" { + t.Errorf("name = %q, want Aperture (openai-api)", prov.Name) + } + if len(prov.Models) != 2 { + t.Fatalf("models len = %d, want 2", len(prov.Models)) + } + // Model IDs must be bare: pi sends id verbatim as the wire model name. + for i, want := range []string{"gpt-5", "gpt-5-mini"} { + if prov.Models[i].ID != want { + t.Errorf("models[%d].id = %q, want %q", i, prov.Models[i].ID, want) + } + } + // input must be non-empty or pi's --list-models dereferences nil. + for i, m := range prov.Models { + if len(m.Input) == 0 { + t.Errorf("models[%d].input is empty; pi crashes on a nil input list", i) + } + // A model registered from an extension gets no default token limits, + // unlike one declared in models.json. Leaving these zero sends + // max_tokens: null and the request fails at the provider. + if m.MaxTokens <= 0 { + t.Errorf("models[%d].maxTokens = %d; must be positive or the request is rejected", i, m.MaxTokens) + } + if m.ContextWindow <= 0 { + t.Errorf("models[%d].contextWindow = %d; must be positive", i, m.ContextWindow) + } + } +} + +// TestExtensionSource_NoNullFields guards the failure that end-to-end testing +// found: a model field omitted from the generated JSON reaches the provider as +// a literal null rather than falling back to a pi default. +func TestExtensionSource_NoNullFields(t *testing.T) { + p := config.ProviderInfo{ID: "anthropic", Models: []string{"claude-sonnet-4-5"}} + src, err := extensionSource(p.ID, buildProvider(testHost, p, backendByIDOrFatal(t, "anthropic"))) + if err != nil { + t.Fatalf("extensionSource: %v", err) + } + if strings.Contains(src, "null") { + t.Errorf("generated extension contains a null value:\n%s", src) + } + for _, field := range []string{"maxTokens", "contextWindow", "input", "apiKey", "baseUrl", "api"} { + if !strings.Contains(src, `"`+field+`"`) { + t.Errorf("generated extension omits %q", field) + } + } +} + +func TestBuildProvider_NoModels(t *testing.T) { + p := config.ProviderInfo{ID: "empty", Compatibility: map[string]bool{"openai_chat": true}} + prov := buildProvider(testHost, p, backendByIDOrFatal(t, "openai_chat")) + if len(prov.Models) != 0 { + t.Errorf("models len = %d, want 0", len(prov.Models)) + } +} + +func TestExtensionSource(t *testing.T) { + p := config.ProviderInfo{ + ID: "anthropic", + Models: []string{"claude-sonnet-4-5"}, + } + b := backendByIDOrFatal(t, "anthropic") + src, err := extensionSource(p.ID, buildProvider(testHost, p, b)) + if err != nil { + t.Fatalf("extensionSource: %v", err) + } + + // The extension must default-export a function that registers the + // provider; anything else and pi loads the file and does nothing. + if !strings.Contains(src, "export default function") { + t.Error("source has no default-exported function") + } + if !strings.Contains(src, "pi.registerProvider(") { + t.Error("source never calls pi.registerProvider") + } + // The registered ID must be namespaced so it cannot merge into pi's own + // built-in "anthropic" provider and retarget the user's models. + if !strings.Contains(src, `"aperture-anthropic"`) { + t.Errorf("source does not register the namespaced provider id:\n%s", src) + } + if strings.Contains(src, `registerProvider("anthropic"`) { + t.Error("source registers the bare provider id, which would override pi's built-in provider") + } +} + +func TestPiProviderIDAndModelRef(t *testing.T) { + if got := piProviderID("openai-api"); got != "aperture-openai-api" { + t.Errorf("piProviderID = %q, want aperture-openai-api", got) + } + // The model arrives fully-qualified from the menu; the reference pi wants + // is the namespaced provider plus the bare model ID. + if got := piModelRef("openai-api", "openai-api/gpt-5"); got != "aperture-openai-api/gpt-5" { + t.Errorf("piModelRef = %q, want aperture-openai-api/gpt-5", got) + } + if got := piModelRef("vertex", "gemini-2.5-pro"); got != "aperture-vertex/gemini-2.5-pro" { + t.Errorf("piModelRef = %q, want aperture-vertex/gemini-2.5-pro", got) + } +} + +func TestWriteProviderExtension(t *testing.T) { + isolateConfigDir(t) + + p := config.ProviderInfo{ + ID: "anthropic", + Name: "Anthropic", + Models: []string{"claude-sonnet-4-5"}, + Compatibility: map[string]bool{"anthropic_messages": true}, + } + b := backendByIDOrFatal(t, "anthropic") + + path, cleanup, err := writeProviderExtension(testHost, p, b) + if err != nil { + t.Fatalf("writeProviderExtension: %v", err) + } + + // Pi resolves -e by extension, so a non-.js path is never loaded. + if filepath.Ext(path) != ".js" { + t.Errorf("path = %q, want a .js file", path) + } + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("extension unreadable: %v", err) + } + if !strings.Contains(string(data), testHost) { + t.Errorf("extension does not contain the aperture host:\n%s", data) + } + + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("perm = %o, want 600", perm) + } + + cleanup() + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Error("extension file still exists after cleanup") + } +} + +// TestWriteProviderExtension_EmbeddedJSONIsValid checks the generated file's +// provider config parses as JSON, which is what catches an unescaped value +// silently producing a broken extension. +func TestWriteProviderExtension_EmbeddedJSONIsValid(t *testing.T) { + isolateConfigDir(t) + + p := config.ProviderInfo{ + ID: "openai-api", + Models: []string{"gpt-5"}, + Compatibility: map[string]bool{"openai_responses": true}, + } + b := backendByIDOrFatal(t, "openai_responses") + path, cleanup, err := writeProviderExtension(testHost, p, b) + if err != nil { + t.Fatalf("writeProviderExtension: %v", err) + } + defer cleanup() + + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + src := string(data) + + // The config object is the second argument to registerProvider: it starts + // at the ", {" that follows the provider ID and ends at the closing ");". + start := strings.Index(src, ", {") + end := strings.LastIndex(src, ");") + if start < 0 || end <= start { + t.Fatalf("no provider config object found in:\n%s", src) + } + blob := src[start+2 : end] + + var got piProvider + if err := json.Unmarshal([]byte(blob), &got); err != nil { + t.Fatalf("embedded provider config is not valid JSON: %v\n%s", err, blob) + } + if got.API != "openai-responses" { + t.Errorf("api = %q, want openai-responses", got.API) + } + if got.BaseURL != testHost+"/v1" { + t.Errorf("baseUrl = %q, want %q", got.BaseURL, testHost+"/v1") + } + if len(got.Models) != 1 || got.Models[0].ID != "gpt-5" { + t.Errorf("models = %+v, want one gpt-5 entry", got.Models) + } +} + +// TestEveryBackendEmitsALoadableExtension is the offline form of the live +// end-to-end run: every backend in the table was driven against a real +// Aperture endpoint with both a text request and a tool-calling request, and +// all four returned successfully. What that run actually exercised is the +// artifact each backend produces, so this asserts the same properties on every +// backend without needing an endpoint at test time. +// +// It is a loop over backends rather than fixed cases on purpose: a new +// protocol added to the table is covered the moment it is added, instead of +// shipping untested because nobody remembered to add a case. +func TestEveryBackendEmitsALoadableExtension(t *testing.T) { + for _, b := range backends { + t.Run(b.id, func(t *testing.T) { + p := config.ProviderInfo{ + ID: "prov", + Name: "Prov", + Models: []string{"model-a"}, + } + src, err := extensionSource(p.ID, buildProvider(testHost, p, b)) + if err != nil { + t.Fatalf("extensionSource: %v", err) + } + + // A null anywhere is the failure mode the live run was built to + // catch: pi passes the value through and the provider rejects it. + if strings.Contains(src, "null") { + t.Errorf("extension contains a null value:\n%s", src) + } + // The base URL must never carry a doubled slash; pi hangs rather + // than following the endpoint's redirect. + if strings.Contains(b.baseURL(testHost), "//v1") { + t.Errorf("baseURL = %q, contains a doubled slash", b.baseURL(testHost)) + } + + start := strings.Index(src, ", {") + end := strings.LastIndex(src, ");") + if start < 0 || end <= start { + t.Fatalf("no provider config object found in:\n%s", src) + } + var got piProvider + if err := json.Unmarshal([]byte(src[start+2:end]), &got); err != nil { + t.Fatalf("embedded provider config is not valid JSON: %v", err) + } + + // The api value is what selects pi's wire protocol; a wrong or + // empty one routes the request at the wrong endpoint shape. + if got.API != b.api { + t.Errorf("api = %q, want %q", got.API, b.api) + } + if got.BaseURL != b.baseURL(testHost) { + t.Errorf("baseUrl = %q, want %q", got.BaseURL, b.baseURL(testHost)) + } + if got.APIKey == "" { + t.Error("apiKey is empty; pi drops a provider with no key") + } + if len(got.Models) != 1 { + t.Fatalf("models len = %d, want 1", len(got.Models)) + } + // Every field the tool-calling path needs must be populated. + m := got.Models[0] + if m.ID != "model-a" { + t.Errorf("models[0].id = %q, want model-a", m.ID) + } + if len(m.Input) == 0 { + t.Error("models[0].input is empty; pi crashes on a nil input list") + } + if m.MaxTokens <= 0 { + t.Errorf("models[0].maxTokens = %d; a zero value is sent as null and rejected", m.MaxTokens) + } + if m.ContextWindow <= 0 { + t.Errorf("models[0].contextWindow = %d; must be positive", m.ContextWindow) + } + + // The argv pi is launched with must name the extension and a + // model reference carrying no provider/ prefix on the model half. + args := buildArgs("/tmp/ext.js", p.ID, "prov/model-a") + want := []string{"-e", "/tmp/ext.js", "--model", "aperture-prov/model-a"} + if !slices.Equal(args, want) { + t.Errorf("buildArgs = %v, want %v", args, want) + } + }) + } +} + +func TestBuildArgs(t *testing.T) { + t.Run("with_model", func(t *testing.T) { + got := buildArgs("/tmp/ext.js", "openai-api", "openai-api/gpt-5") + want := []string{"-e", "/tmp/ext.js", "--model", "aperture-openai-api/gpt-5"} + if !slices.Equal(got, want) { + t.Errorf("buildArgs = %v, want %v", got, want) + } + }) + t.Run("no_model_omits_flag", func(t *testing.T) { + got := buildArgs("/tmp/ext.js", "openai-api", "") + want := []string{"-e", "/tmp/ext.js"} + if !slices.Equal(got, want) { + t.Errorf("buildArgs = %v, want %v", got, want) + } + }) + t.Run("never_sets_a_yolo_flag", func(t *testing.T) { + // Pi has no permission prompts, so nothing here should look like an + // approval bypass. --approve is project-file trust, not tool approval. + for _, a := range buildArgs("/tmp/ext.js", "p", "p/m") { + if a == "--approve" || a == "-a" || a == "--yolo" { + t.Errorf("buildArgs included %q", a) + } + } + }) +} + +func TestBackendsFor(t *testing.T) { + cases := []struct { + name string + compat map[string]bool + want []string + }{ + { + name: "openai_both_ordered_responses_first", + compat: map[string]bool{"openai_chat": true, "openai_responses": true}, + want: []string{"openai_responses", "openai_chat"}, + }, + { + name: "anthropic_only", + compat: map[string]bool{"anthropic_messages": true}, + want: []string{"anthropic"}, + }, + { + name: "vertex_via_generate_content", + compat: map[string]bool{"google_generate_content": true}, + want: []string{"vertex"}, + }, + { + name: "vertex_via_raw_predict", + compat: map[string]bool{"google_raw_predict": true}, + want: []string{"vertex"}, + }, + { + name: "vertex_not_duplicated_when_both_keys_set", + compat: map[string]bool{"google_generate_content": true, "google_raw_predict": true}, + want: []string{"vertex"}, + }, + { + // Pi's bedrock API type fails at request time, so it is not offered. + name: "bedrock_unsupported", + compat: map[string]bool{"bedrock_converse": true, "bedrock_model_invoke": true}, + want: nil, + }, + { + name: "unknown_key", + compat: map[string]bool{"something_else": true}, + want: nil, + }, + { + name: "all_four", + compat: map[string]bool{"openai_chat": true, "openai_responses": true, "anthropic_messages": true, "google_raw_predict": true}, + want: []string{"openai_responses", "anthropic", "openai_chat", "vertex"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + bs := backendsFor(config.ProviderInfo{Compatibility: tc.compat}) + got := make([]string, len(bs)) + for i, b := range bs { + got[i] = b.id + } + if !slices.Equal(got, tc.want) { + t.Errorf("backendsFor = %v, want %v", got, tc.want) + } + }) + } +} + +func TestCompatibleProviders(t *testing.T) { + provs := []config.ProviderInfo{ + {ID: "openai-api", Compatibility: map[string]bool{"openai_responses": true}}, + {ID: "bedrock", Compatibility: map[string]bool{"bedrock_converse": true}}, + {ID: "anthropic", Compatibility: map[string]bool{"anthropic_messages": true}}, + {ID: "none", Compatibility: map[string]bool{"something_else": true}}, + } + got := compatibleProviders(provs) + ids := make([]string, len(got)) + for i, p := range got { + ids[i] = p.ID + } + // bedrock and none are both unusable from Pi. + if want := []string{"openai-api", "anthropic"}; !slices.Equal(ids, want) { + t.Errorf("compatibleProviders = %v, want %v", ids, want) + } +} + +func TestBackendByID(t *testing.T) { + if b, ok := backendByID("vertex"); !ok || b.api != "google-generative-ai" { + t.Errorf("backendByID(vertex) = %+v, %v", b, ok) + } + if _, ok := backendByID("bedrock"); ok { + t.Error("backendByID(bedrock) should not resolve") + } + if _, ok := backendByID(""); ok { + t.Error("backendByID(\"\") should not resolve") + } +} + +// TestResolveReplay covers every staleness path Replay depends on. It calls +// resolveReplay rather than Replay on purpose: Replay returns nil at +// !IsInstalled() before reaching any of these checks, so a test driving +// Replay would pass on CI even with the checks deleted. +func TestResolveReplay(t *testing.T) { + liveProvider := config.ProviderInfo{ + ID: "openai-api", + Name: "OpenAI", + Models: []string{"gpt-5"}, + Compatibility: map[string]bool{"openai_responses": true, "openai_chat": true}, + } + base := config.LaunchState{ + LastClientName: name, + LastBackendType: "openai_responses", + LastProviderID: "openai-api", + LastModel: "openai-api/gpt-5", + } + + t.Run("replayable", func(t *testing.T) { + g := &config.Global{Providers: []config.ProviderInfo{liveProvider}, LastLaunch: base} + prov, b, model, ok := resolveReplay(g) + if !ok { + t.Fatal("resolveReplay = not ok, want a replayable launch") + } + if prov.ID != "openai-api" { + t.Errorf("provider = %q, want openai-api", prov.ID) + } + if b.id != "openai_responses" { + t.Errorf("backend = %q, want openai_responses", b.id) + } + if model != "openai-api/gpt-5" { + t.Errorf("model = %q, want openai-api/gpt-5", model) + } + }) + + t.Run("empty_model_is_replayable", func(t *testing.T) { + ls := base + ls.LastModel = "" + g := &config.Global{Providers: []config.ProviderInfo{liveProvider}, LastLaunch: ls} + if _, _, model, ok := resolveReplay(g); !ok || model != "" { + t.Errorf("resolveReplay = %q, %v; want \"\", true", model, ok) + } + }) + + stale := []struct { + name string + providers []config.ProviderInfo + mutate func(*config.LaunchState) + }{ + { + name: "another_client_owns_the_record", + providers: []config.ProviderInfo{liveProvider}, + mutate: func(ls *config.LaunchState) { ls.LastClientName = "Codex" }, + }, + { + name: "provider_gone_from_endpoint", + providers: []config.ProviderInfo{liveProvider}, + mutate: func(ls *config.LaunchState) { ls.LastProviderID = "removed" }, + }, + { + name: "backend_id_no_longer_offered", + providers: []config.ProviderInfo{liveProvider}, + mutate: func(ls *config.LaunchState) { ls.LastBackendType = "bedrock" }, + }, + { + name: "backend_id_empty", + providers: []config.ProviderInfo{liveProvider}, + mutate: func(ls *config.LaunchState) { ls.LastBackendType = "" }, + }, + { + // The provider still exists but dropped the protocol we recorded. + name: "provider_dropped_the_protocol", + providers: []config.ProviderInfo{{ + ID: "openai-api", + Models: []string{"gpt-5"}, + Compatibility: map[string]bool{"openai_chat": true}, + }}, + mutate: func(ls *config.LaunchState) {}, + }, + { + name: "model_no_longer_listed", + providers: []config.ProviderInfo{liveProvider}, + mutate: func(ls *config.LaunchState) { ls.LastModel = "openai-api/gpt-4" }, + }, + { + name: "provider_has_no_models_anymore", + providers: []config.ProviderInfo{{ + ID: "openai-api", + Compatibility: map[string]bool{"openai_responses": true}, + }}, + mutate: func(ls *config.LaunchState) {}, + }, + } + for _, tc := range stale { + t.Run(tc.name, func(t *testing.T) { + ls := base + tc.mutate(&ls) + g := &config.Global{Providers: tc.providers, LastLaunch: ls} + if _, _, _, ok := resolveReplay(g); ok { + t.Error("resolveReplay = ok, want not replayable") + } + }) + } +} + +// TestReplayStalenessChecks exercises the decisions Replay makes, directly +// against the unexported helpers. Calling Replay itself would return nil at +// the !IsInstalled() check on any machine without pi, so it would pass even +// if the staleness logic were deleted. +func TestReplayStalenessChecks(t *testing.T) { + prov := config.ProviderInfo{ + ID: "openai-api", + Models: []string{"gpt-5"}, + Compatibility: map[string]bool{"openai_responses": true}, + } + + b := backendByIDOrFatal(t, "openai_responses") + if !providerSupports(prov, b) { + t.Error("provider should support the recorded backend") + } + + // A backend the provider no longer serves must not replay. + if providerSupports(prov, backendByIDOrFatal(t, "anthropic")) { + t.Error("provider without anthropic_messages should not support the anthropic backend") + } + + // A backend ID that no longer exists in the table must not replay. + if _, ok := backendByID("openai_completions_legacy"); ok { + t.Error("an unknown recorded backend id should not resolve") + } + + if got := fqnModels(prov); !slices.Equal(got, []string{"openai-api/gpt-5"}) { + t.Errorf("fqnModels = %v, want [openai-api/gpt-5]", got) + } + // A recorded model the provider no longer lists is what makes Replay bail. + if slices.Contains(fqnModels(prov), "openai-api/gpt-4") { + t.Error("stale model should not be found in the current model list") + } +} + +func TestQuickSelectLabel(t *testing.T) { + g := &config.Global{ + Providers: []config.ProviderInfo{ + {ID: "openai-api", Name: "OpenAI", Models: []string{"gpt-5"}}, + }, + LastLaunch: config.LaunchState{ + LastClientName: name, + LastBackendType: "openai_responses", + LastProviderID: "openai-api", + LastModel: "openai-api/gpt-5", + }, + } + c := &Client{} + want := "Pi via OpenAI - OpenAI Responses - openai-api/gpt-5" + if got := c.QuickSelectLabel(g); got != want { + t.Errorf("QuickSelectLabel = %q, want %q", got, want) + } + + // No recorded model: the label stops after the backend. + g.LastLaunch.LastModel = "" + if got, want := c.QuickSelectLabel(g), "Pi via OpenAI - OpenAI Responses"; got != want { + t.Errorf("QuickSelectLabel = %q, want %q", got, want) + } +} + +func TestFqnModels(t *testing.T) { + p := config.ProviderInfo{ID: "openai-api", Models: []string{"gpt-5", "gpt-5-mini"}} + want := []string{"openai-api/gpt-5", "openai-api/gpt-5-mini"} + if got := fqnModels(p); !slices.Equal(got, want) { + t.Errorf("fqnModels = %v, want %v", got, want) + } +} + +func TestStripProviderPrefix(t *testing.T) { + cases := map[string]string{ + "openai-api/gpt-5": "gpt-5", + "anthropic/claude-sonnet-4": "claude-sonnet-4", + "bare-model": "bare-model", + "provider/nested/model": "nested/model", + } + for in, want := range cases { + if got := stripProviderPrefix(in); got != want { + t.Errorf("stripProviderPrefix(%q) = %q, want %q", in, got, want) + } + } +} + +func TestInstallUninstall(t *testing.T) { + c := &Client{} + install := c.Install(&config.Global{}) + if install.Hint != installCmd { + t.Errorf("Install.Hint = %q, want %q", install.Hint, installCmd) + } + if install.Run == nil { + t.Error("Install.Run is nil") + } + + uninstall := c.Uninstall() + if uninstall.Hint != uninstallCmd { + t.Errorf("Uninstall.Hint = %q, want %q", uninstall.Hint, uninstallCmd) + } + if uninstall.Run == nil { + t.Error("Uninstall.Run is nil") + } +} + +func TestIdentity(t *testing.T) { + c := &Client{} + if c.Name() != "Pi" { + t.Errorf("Name = %q, want Pi", c.Name()) + } + if c.BinaryName() != "pi" { + t.Errorf("BinaryName = %q, want pi", c.BinaryName()) + } + // CommonPaths must be full paths to the binary, not directories: + // FindBinary stats each entry directly. + for _, p := range c.CommonPaths() { + if filepath.Base(p) != "pi" { + t.Errorf("CommonPaths entry %q does not end in the binary name", p) + } + if !filepath.IsAbs(p) { + t.Errorf("CommonPaths entry %q is not absolute", p) + } + } +} From ecab65ffe970070885f8e4395f64600a41441eba Mon Sep 17 00:00:00 2001 From: Larah Vasquez Date: Wed, 12 Aug 2026 08:45:43 -0500 Subject: [PATCH 3/8] .agents: add skills for adding and testing a client Two agent skills, kept in the repo so they stay in step with the code they describe: - adding-aperture-cli-client points at docs/adding-a-client.md and cites internal/clients/pi as the fullest worked example. - testing-an-aperture-cli-client covers the four verification layers: the CI gates, a headless live request per protocol, tool calling, and the interactive TUI checks that need a human. --- .../adding-aperture-cli-client/SKILL.md | 88 ++++++++++ .../testing-an-aperture-cli-client/SKILL.md | 151 ++++++++++++++++++ 2 files changed, 239 insertions(+) create mode 100644 .agents/skills/adding-aperture-cli-client/SKILL.md create mode 100644 .agents/skills/testing-an-aperture-cli-client/SKILL.md diff --git a/.agents/skills/adding-aperture-cli-client/SKILL.md b/.agents/skills/adding-aperture-cli-client/SKILL.md new file mode 100644 index 0000000..e4be6d8 --- /dev/null +++ b/.agents/skills/adding-aperture-cli-client/SKILL.md @@ -0,0 +1,88 @@ +--- +name: adding-aperture-cli-client +description: "Add a new coding agent (a \"harness\", called a \"client\" in the code) to the aperture-cli launcher, so it appears in the menu, installs and uninstalls itself, routes through an Aperture endpoint, and replays on quick-select. Use when adding or modifying a client in the aperture-cli repo, or when debugging why a client does not appear in the menu or never offers quick select. To verify a client use testing-an-aperture-cli-client. For the Aperture server repo use aperture-dev." +--- + +# Adding a client to aperture-cli + +`aperture-cli` is the launcher that starts coding agents preconfigured against an Aperture endpoint. Each agent is a **client**: a sub-package of `internal/clients` implementing the `clients.Client` interface. Users and vendors say "harness" or "agent"; the code says "client". Use "client" in code, comments, and commit messages. + +This skill covers adding one command-line client. Desktop apps (Claude Cowork) live in `internal/profiles` behind an adapter and are out of scope. + +## Before writing any Go + +**Establish the harness contract first, and do not invent it.** This is the one input the codebase cannot give you, and getting it wrong produces a client that compiles, passes tests, and silently fails to route. Research the harness's own docs and source for: + +- the env var it reads for the API base URL, and for the API key +- the env var for a default model, if any +- whether it requires a config file on disk, and that file's real schema +- the flag that skips permission prompts, if any +- which wire protocol(s) it speaks, mapped to compatibility keys +- whether it validates the base URL in a way that rejects `http://ai` — Gemini CLI does, see `validateHost` in `internal/clients/gemini/gemini.go` + +Confirm the contract by launching the harness by hand with those values set before writing Go. If the harness has no way to accept a custom base URL, stop and say so: the task is infeasible, not a thing to work around. + +Two answers shape everything downstream. **Routing style:** env-vars-only gives a short client (see `copilot`), a config file means writing one per launch and pointing the harness at it with one env var (see `opencode`, `codex`, `gemini`). **Protocol count:** one protocol needs a single `compatKey` const (`codex`); several with a user choice needs a `backend` struct and a `backendStep` (`copilot`, `gemini`); several resolved automatically needs a resolver (`opencode`'s `pickSDK`). + +## The full procedure + +The repo ships a step-by-step guide covering every method, snippet, and checkpoint: **`docs/adding-a-client.md`**, at the root of the aperture-cli checkout. Read it and follow it. It was independently verified against the code — both routing variants were built from scratch and confirmed to pass every CI gate — so trust it over your own recollection of Go idiom here. + +`internal/clients/pi` is the most complete worked example: a four-protocol client that routes through a generated per-launch extension rather than env vars, with an in-package test file covering every unexported helper. + +This skill is the orientation layer: the traps below are the ones that cost real time, and several are invisible to the compiler. + +## Non-obvious facts about this codebase + +**The interface has nine methods, all on pointer receivers.** `Name`, `BinaryName`, `CommonPaths`, `IsInstalled`, `Install`, `Uninstall`, `Menu`, `Replay`, `QuickSelectLabel` — defined in `internal/clients/registry.go`. A `Client` receiver instead of `*Client` fails the interface check with a confusing message. + +**Registration is a side effect, and forgetting it fails silently.** Your `init()` calls `clients.Register(&Client{})`, but `init()` only runs if the package is linked, which is what the blank-identifier import block in `cmd/aperture/main.go` is for. A missing import produces no error at all — the client just does not exist. + +**You cannot choose your menu position.** Registration order is display order, and it follows that import block's order — which gofmt sorts alphabetically. Insert your import in correct alphabetical position; appending it leaves the file unformatted and fails CI. Position is decided by package name, full stop. Wanting a different order means adding a real ordering mechanism to `internal/clients`, not hand-editing imports. + +**`Install.Run` and `Uninstall.Run` take different shapes.** `Install.Run` passes one string to `/bin/sh -c`, so pipes work. `Uninstall.Run` has no shell, so you must split the command into separate arguments yourself. Passing the whole command as one argument compiles and then fails at runtime with `fork/exec ...: no such file or directory`. No build or test step catches this. + +**Trim the trailing slash before appending a path.** Use `strings.TrimRight(g.ApertureHost, "/")`. Users do configure `http://ai/`, and preflight tolerates it, so untrimmed concatenation yields `http://ai//v1`. `copilot` and `gemini` trim; `codex` and `opencode` do not and are inconsistent — follow the ones that trim. + +**Strip the provider prefix out of model names.** The launcher displays models as `provider_id/model_id`; harnesses want the bare ID. Leaving the prefix on breaks path-based routing and produces a puzzling 404 rather than a clear error. + +**Empty provider list is an error, one option auto-descends.** Every client follows this: zero compatible providers returns an error result rather than an empty menu, exactly one descends straight to the next step without making the user press Enter, more than one shows a submenu as `Result.Next`. Zero *models* is not an error — pass an empty model string and let the harness pick. + +**Compat keys are not centrally defined.** Each client declares its own. `opencode`'s `compatKeys` is the longest list but is not complete: `gemini` uses `experimental_gemini_cli_vertex_compat`, absent from it. Grep `compatKey\|compatKeys` across `internal/clients/` for the real set, and confirm against a live `GET /api/providers`. + +**Use `config.ClientConfigDir`, not a hand-built path.** It returns `/aperture/clients/` and creates it `0o700`. Write files `0o600`. Config files hold the endpoint URL, which reveals a tailnet hostname, and per-launch files need the `Cleanup` closure passed through to `LaunchSpec` or they accumulate. + +**Credentials are always placeholders.** Aperture authenticates over Tailscale, so the harness's key check has nothing to validate. Existing clients use `not-needed`, `not-required`, or a bare `-`. Never add a path that reads a real key from the environment and forwards it into a file the launcher writes. Note `-debug` dumps the whole env map to stderr. + +## Testing + +Tests live **in-package** (not `_test`) so they reach unexported helpers. Set `testHost = "http://ai.example.com"` — never a real endpoint. Any test touching config paths must `t.Setenv("HOME", tmp)` and `t.Setenv("XDG_CONFIG_HOME", ...)` to a `t.TempDir()`, because `os.UserConfigDir()` otherwise resolves to the developer's real user config directory. + +**Do not copy the `TestReplay_StaleProvider` shape from `codex_test.go`.** It passes vacuously: `Replay` returns `nil` at the `!IsInstalled()` check before ever reaching the provider lookup, so on any machine without the harness installed — including CI — it would still pass if the provider check were deleted. Test the unexported helpers directly instead: pull env construction into a `buildEnv` function and assert on the returned map (as `copilot_test.go` does), and exercise `providerMatches` and `fqnModels` on their own. + +Cover: the env or config produced for each protocol, the provider filter, the backend filter if you have one, install and uninstall hints, and at least one replay path. + +## Verification gates + +CI runs exactly two things: `gofmt -l .` (fails on **any** output) and `make test`. Before declaring done, run all four and report real output: + +```bash +gofmt -l . # must print nothing +go vet ./... +make test +make build +``` + +**Green gates do not mean the client routes.** Unit tests check the strings the client builds, not whether the harness accepts them, so a client can pass everything above and fail on its first real request. Follow **testing-an-aperture-cli-client** for the rest: a headless live request per protocol, a tool-calling check, and the interactive TUI steps only a human can do. `docs/adding-a-client.md` covers the same ground in its "Test it end to end" section. + +Finally, add the harness to the `Supported agents` list in `README.md`, and lead the commit message with the touched path: `internal/clients: add client`. + +## Debugging a client that misbehaves + +**Not in the root menu at all.** Check `[i] Install agents` first — if it is there, discovery is the problem, so verify `binaryName` matches the executable and that `commonBinaryPaths` returns full paths to the binary, not directories. If it is in neither list, the package is not linked: confirm the blank import. If it registers and is installed but still absent, check that `Menu()` sets `Action` — the root menu skips items with a nil `Action`. + +**"No providers support ..."** The compat key does not match the endpoint. Fetch `GET /api/providers` and compare keys literally. If nothing sets your key, the client is behaving correctly and the gap is Aperture-side. + +**Starts but every request fails.** Run `-debug`. Check the base URL for a missing or doubled `/v1`, then check whether the model still carries its `provider/` prefix. + +**Quick select never appears.** `Replay` returned `nil`. Walk its checks in order, and read what the launcher actually persisted — `statePath` in `internal/config/state.go` resolves it through `os.UserConfigDir()`, giving `$HOME/Library/Application Support/aperture/launcher.json` on macOS and `$HOME/.config/aperture/launcher.json` on Linux. A `name` constant that changed since the launch was recorded will never match. diff --git a/.agents/skills/testing-an-aperture-cli-client/SKILL.md b/.agents/skills/testing-an-aperture-cli-client/SKILL.md new file mode 100644 index 0000000..17732c7 --- /dev/null +++ b/.agents/skills/testing-an-aperture-cli-client/SKILL.md @@ -0,0 +1,151 @@ +--- +name: testing-an-aperture-cli-client +description: "Test a client (coding agent harness) in the aperture-cli launcher end to end: the CI gates, a headless live request through every wire protocol, tool calling, and the interactive TUI checks a human must do. Use when verifying a new or modified client in internal/clients, when asked whether a harness routes correctly, when a client passes tests but fails at runtime, or before opening a PR touching a client package. To write the client use adding-aperture-cli-client." +--- + +# Testing a client in aperture-cli + +Four layers, cheapest first. Each catches a class of failure the one before it cannot see. + +| Layer | Catches | Automatable | +| --- | --- | --- | +| 1. CI gates | Compile errors, formatting, unit-level logic | Yes | +| 2. Headless live request | Wrong base URL, wrong protocol, bad config schema | Yes | +| 3. Tool calling | Missing/null fields the harness only needs mid-loop | Yes | +| 4. Interactive TUI | Menu wiring, replay, cleanup on exit | No — human at a terminal | + +**Green unit tests prove almost nothing about routing.** They exercise the strings a client builds, not whether a harness accepts them. Layers 2 and 3 are where clients actually fail, and they are scriptable, so run them before declaring anything done. + +## Layer 1: the CI gates + +CI runs exactly two things — `gofmt -l .` (fails on **any** output) and `make test`. Run all four and report real output, never a prediction: + +```bash +gofmt -l . # must print nothing +go vet ./... +make test +make build +``` + +`go vet` is not in CI but currently passes clean repo-wide, so any vet failure you see is yours. + +## Layer 2: a headless live request per protocol + +Generate the exact artifact the client produces, then feed it to the real harness against a live Aperture. Do not hand-write the config or the arguments — call the client's own unexported builders, or a bug in them is exactly what you fail to catch. + +Confirm the endpoint first and pick a real provider and model for each protocol the client supports: + +```bash +curl -s http://ai/api/providers | python3 -c " +import json,sys +for p in json.load(sys.stdin): + ck=[k for k,v in (p['compatibility'] or {}).items() if v] + m=p.get('models') or [] + print(f\"{p['id']:28} | {','.join(ck):45} | {len(m):3} models | {m[:2]}\") +" +``` + +Note some providers report `models: null`, not `[]` — hence the `or []`. Pick the cheapest capable model per protocol; a smoke test does not need a frontier model. + +**For a config-file or extension client**, add a temporary in-package test that writes the real artifact for each backend and logs the real argv. Gate it on an env var so it skips in CI, name it so it sorts last, and delete it when you are done: + +```go +// zz_manual_e2e_test.go — throwaway, delete after use. +func TestManualE2EEmitConfigs(t *testing.T) { + out := os.Getenv("E2E_OUT") + if out == "" { + t.Skip("set E2E_OUT to emit") + } + host := os.Getenv("E2E_HOST") + // one case per (provider, backend) pair the client supports + for _, c := range cases { + b, ok := backendByID(c.bID) + if !ok { + t.Fatalf("no backend %q", c.bID) + } + src, err := extensionSource(c.prov.ID, buildProvider(host, c.prov, b)) + if err != nil { + t.Fatal(err) + } + p := filepath.Join(out, c.file) + if err := os.WriteFile(p, []byte(src), 0o600); err != nil { + t.Fatal(err) + } + t.Logf("%s -> %s args=%v", c.bID, p, buildArgs(p, c.prov.ID, c.model)) + } +} +``` + +```bash +E2E_DIR=$(mktemp -d "${TMPDIR:-/tmp}/-e2e-XXXXXX") +E2E_OUT="$E2E_DIR" E2E_HOST=http://ai go test ./internal/clients// \ + -run TestManualE2EEmitConfigs -v +``` + +Read the emitted file before running anything. Check the base URL for a doubled `//`, a missing or doubled `/v1`, and that model IDs carry no `provider/` prefix. Then grep for `null` — a Go zero value marshals to `0`/`""`/`null` and reaches the harness literally. + +Now drive the harness with each artifact, using its non-interactive flag: + +```bash +for b in ; do + echo "===== $b" + "$E2E_DIR/$b." --model "" \ + -p --no-session "Reply with exactly: PONG" 2>&1 | tail -6 + echo "--- pipestatus=${pipestatus[1]}" # zsh; bash uses ${PIPESTATUS[1]} +done +``` + +Non-interactive flags differ per harness: pi and Claude Code use `-p`/`--print`, OpenCode uses `opencode run`. Check `--help`. Do not prefix these with `timeout` in zsh — it is not a builtin and the whole command becomes an unfound command name whose exit status still reads 0, which looks like a pass. + +**For an env-vars-only client**, skip the artifact and set the same variables the client sets, read off `./.build/aperture -debug`. + +## Layer 3: tool calling + +Text generation and a tool loop exercise different fields. Run this for every protocol the client supports: + +```bash + "$E2E_DIR/$b." --model "" \ + -p --no-session -t bash "Run the shell command 'echo tool-ok' and tell me its output" +``` + +Expect `tool-ok` in the output. This is the check to re-run after **any** edit to config or extension generation. Pi's own extension code documents why: an omitted `maxTokens` reaches Anthropic as a literal `null` and is rejected, and an omitted `input` list crashes pi's model formatter outright — see the `piModel` comment in `internal/clients/pi/extension.go`. Neither shows up in a text-only smoke test. + +## Layer 4: the interactive TUI + +Layers 1–3 all bypass the menu. These need a human: + +```bash +./.build/aperture -debug +``` + +1. The client appears in the root menu when installed, or under `[i] Install agents` when not. +2. Walking provider → backend → model reaches a launch. One option should auto-descend without an Enter press; zero compatible providers should show an error, not an empty menu. +3. `-debug` prints the resolved env and args to stderr before exec — the fastest way to spot a wrong variable name or a doubled slash. +4. The harness starts and completes a real request. +5. Quitting the harness lands back on the root menu. +6. `[0] Quick select` now names the client, with the provider, backend, and model from `QuickSelectLabel`. +7. The per-launch config file is **gone**. Check `$HOME/Library/Application Support/aperture/clients//` (macOS) or `${XDG_CONFIG_HOME:-$HOME/.config}/aperture/clients//` (Linux). A leftover file means `Cleanup` was not passed through to `LaunchSpec`. + +Replay is only testable this way, and only against a launcher state that names your client. Read what was actually persisted: + +```bash +cat "$HOME/Library/Application Support/aperture/launcher.json" +``` + +If `lastClientName` names a different client, quick-select will not exercise yours — launch yours once first. Note `resolveReplay`-style helpers exist precisely so staleness logic is unit-testable; `Replay` itself returns `nil` at `!IsInstalled()` before any other check, so a test driving `Replay` on a machine without the harness passes vacuously. + +## Cleaning up + +Delete the throwaway test and the emitted configs, then re-run the gates so the tree you hand off is the tree you tested: + +```bash +rm -f internal/clients//zz_manual_e2e_test.go +rm -rf "$E2E_DIR" +gofmt -l . && go vet ./... && make test +``` + +Never leave the e2e test committed. It needs a live endpoint and a real installed harness, so it would fail or vacuously skip in CI. + +## Reporting + +State which layers ran and which did not. Layer 4 is frequently impossible in an agent session — say so plainly rather than implying full coverage, and give the exact commands for a human to finish. A protocol you did not exercise is untested, not "should work": report it per protocol, with the provider and model used, since a client that works on Anthropic Messages can fail on Vertex over the same code path. From 51de975e08aacd51ee8d86855f198de863274132 Mon Sep 17 00:00:00 2001 From: Larah Vasquez Date: Wed, 12 Aug 2026 09:01:32 -0500 Subject: [PATCH 4/8] .agents: add skill for auditing docs against the code Audits README.md, docs/adding-a-client.md, and every .agents/skills/*/SKILL.md against the code, including the skills themselves. Findings carry a tier label proving each claim: static citation checks, executed CI gates and /api/providers output, and live per-protocol requests through a client's own builders. Read-only by default; only mechanical drift is offered for auto-fix, with a diff. Records two findings verified present today: a dangling "Test it end to end" cross-reference, and a README with no pointer to docs/ or .agents/skills/. --- .../auditing-aperture-cli-docs/SKILL.md | 150 ++++++++++++++++++ 1 file changed, 150 insertions(+) create mode 100644 .agents/skills/auditing-aperture-cli-docs/SKILL.md diff --git a/.agents/skills/auditing-aperture-cli-docs/SKILL.md b/.agents/skills/auditing-aperture-cli-docs/SKILL.md new file mode 100644 index 0000000..8aac304 --- /dev/null +++ b/.agents/skills/auditing-aperture-cli-docs/SKILL.md @@ -0,0 +1,150 @@ +--- +name: auditing-aperture-cli-docs +description: "Audit the aperture-cli repo's docs and its own in-repo skills against the code, reporting drift ranked by severity with a tier label proving each claim: static citations, executed commands, live routing. Use when asked to audit or fact-check the docs, to check whether README.md, docs/adding-a-client.md, or .agents/skills/*/SKILL.md still match the code, or after a change touching a client package or the client interface. Read-only by default. To write a client use adding-aperture-cli-client; to verify one use testing-an-aperture-cli-client." +--- + +# Auditing aperture-cli docs against the code + +The repo documents itself three times over: `README.md` for users, `docs/adding-a-client.md` as the step-by-step guide, and `.agents/skills/*/SKILL.md` as the agent-facing orientation layer. All three make checkable claims about code that moves underneath them. This skill finds the ones that have stopped being true. + +**Audit the skills, not just the prose.** The skills are the most drift-prone documents in the repo and the least likely to be read by a human who would notice. They cite line numbers, name unexported helpers, describe which clients follow which convention, and cross-reference sections of the guide by title. Every one of those is a claim that can rot. A skill that confidently misdescribes the codebase is worse than no skill, because an agent will act on it. + +## The corpus + +Audit exactly these, and enumerate rather than assume — a new skill directory is easy to miss: + +```bash +ls README.md docs/*.md .agents/skills/*/SKILL.md +``` + +## Three tiers, and every finding must name the one that proved it + +The tier is not decoration. It tells the reader how much to trust the finding and how much work reproducing it costs. A claim "verified" at the wrong tier is the failure mode this skill exists to prevent: green unit tests and resolving line anchors both look like proof and neither touches routing. + +| Tier | Question it answers | Cost | Can it fail spuriously? | +| --- | --- | --- | --- | +| 1. Static | Does the cited thing exist and still say this? | Free | No | +| 2. Executed | Does the promised command produce the promised output? | Seconds | Only on a broken tree | +| 3. Live | Does the documented routing actually route? | Minutes, needs endpoint + harness | Yes — missing prerequisites are **not** a pass | + +### Tier 1: static + +The guide carries 40+ `path/file.go:NN` anchors. Check that each file exists, the line is in range, **and the line still says what the surrounding prose claims** — an anchor that drifted onto a neighbouring function resolves fine and is still wrong. Read the citing sentence, then the cited line, and compare meaning. + +```bash +python3 - <<'PY' +import re, os +corpus = ["README.md"] + [os.path.join("docs", f) for f in os.listdir("docs") if f.endswith(".md")] +corpus += [os.path.join(r, "SKILL.md") for r, _, fs in os.walk(".agents/skills") if "SKILL.md" in fs] +pat = re.compile(r'`([A-Za-z0-9_./-]+\.go):(\d+)`') +for doc in corpus: + for ln, line in enumerate(open(doc), 1): + for m in pat.finditer(line): + path, n = m.group(1), int(m.group(2)) + if not os.path.exists(path): + print(f"MISSING-FILE {doc}:{ln} -> {path}:{n}"); continue + src = open(path).read().splitlines() + if n > len(src): + print(f"OUT-OF-RANGE {doc}:{ln} -> {path}:{n} (file has {len(src)})"); continue + print(f"OK {doc}:{ln} -> {path}:{n} | {src[n-1].strip()[:90]}") +PY +``` + +Then check the claims that carry no line number, which are the ones that actually break: + +- **Cross-document section references.** A doc naming a section of another doc (`its "Test it end to end" section`) must match a real heading. Extract quoted section names and grep the target's headings — `grep -n '^#\{1,3\} ' docs/adding-a-client.md`. This class has a live finding today; see Known drift. +- **Counts and enumerations.** "The interface has nine methods" against `grep -cE '^\t[A-Z][A-Za-z]*\(' internal/clients/registry.go`. The README's `Supported agents` list against `ls -d internal/clients/*/` plus the desktop adapters in `internal/profiles` — the list is legitimately longer than the client count, because Claude Cowork is a profile, not a client. +- **"Client X does this, client Y does not" claims.** These are the fastest-rotting sentences in the corpus, because a new client silently joins or breaks the pattern. Verify each side: `grep -rn "TrimRight" internal/clients/`. +- **Named symbols, flags, paths, make targets.** Every backticked identifier should resolve: `grep -rn "func validateHost" internal/clients/`, flags against `grep -n 'flag\.' cmd/aperture/main.go`, `make ` against the `Makefile`, and version claims against `go.mod`. +- **CI claims.** "CI runs exactly two things" is a claim about `.github/workflows/`. Check the trigger blocks, not just the file count: there are three workflows, and `govulncheck.yaml` fires only on a schedule and on changes to itself, so it is not a PR gate. Read `on:` before calling this drift. + +### Tier 2: executed + +Run the read-only commands the docs promise and diff real output against documented output. Never predict. + +```bash +gofmt -l . # documented as printing nothing +go vet ./... # documented as passing clean repo-wide +make test +make build +curl -s http://ai/api/providers +``` + +For `/api/providers`, verify the shape the docs describe, and note that **some providers report `models: null`, not `[]`** — any snippet indexing models without an `or []` guard is a real bug in the doc, not a nitpick. + +```bash +curl -s http://ai/api/providers | python3 -c " +import json,sys +for p in json.load(sys.stdin): + ck=[k for k,v in (p.get('compatibility') or {}).items() if v] + m=p.get('models') + print(f\"{p['id']:26} | {','.join(ck):58} | models={'null' if m is None else len(m)}\") +" +``` + +Cross-check documented compat keys against that live list plus the in-repo declarations (`grep -rn 'compatKey\|compatKeys' internal/clients/`). A key in the docs that no provider sets is worth reporting; a key in the code that no provider sets is an Aperture-side gap, not doc drift. + +### Tier 3: live + +For routing claims — base URL shapes, `/v1` suffixes, model reference formats — nothing below this tier is evidence. Emit configs through **the client's own builders**, never by hand, then send a real request per protocol and a tool-calling check. + +Add a throwaway in-package test gated on an env var, named to sort last, and delete it when done: + +```bash +E2E_DIR=$(mktemp -d "${TMPDIR:-/tmp}/audit-e2e-XXXXXX") +E2E_OUT="$E2E_DIR" E2E_HOST=http://ai go test ./internal/clients/pi/ \ + -run TestAuditE2EEmitConfigs -v +``` + +**Populate `ProviderInfo.Models` from the live endpoint.** A synthetic `config.ProviderInfo{ID: "anthropic"}` emits `"models": []`, and pi then rejects the model reference with `Model "..." not found` — which looks exactly like a routing failure and is not one. Fetch the real model list and pass it in, or you will chase a bug you created. + +Read the emitted artifact before running it: check for a doubled `//`, a missing or doubled `/v1`, a literal `null`, and whether model IDs still carry a `provider/` prefix. Then drive the harness: + +```bash +pi -e "$E2E_DIR/anthropic.ts" --model "" -p --no-session "Reply with exactly: PONG" +echo "pipestatus=${pipestatus[1]}" # zsh; bash uses ${PIPESTATUS[1]} + +pi -e "$E2E_DIR/anthropic.ts" --model "" -p --no-session -t bash \ + "Run the shell command 'echo tool-ok' and tell me its output" +``` + +Note the model reference is `aperture-/` — `piModelRef` namespaces the provider ID so a registration cannot overwrite one of pi's built-ins. A bare `provider/model` fails. + +**Do not prefix these with `timeout`.** It is not a zsh builtin: the whole command becomes an unfound command name whose exit status still reads 0, which looks like a pass. + +**If the endpoint is unreachable or the harness is missing, report the claim as UNVERIFIED.** Not "should work", not silently omitted. An untested protocol is untested, and saying so is the whole value of the tier label. + +Clean up, then re-run the gates so the tree you hand off is the tree you tested: + +```bash +rm -f internal/clients/*/zz_audit_e2e_test.go && rm -rf "$E2E_DIR" +gofmt -l . && go vet ./... && make test +``` + +## Known drift, as of this skill's last verification + +Confirm rather than inherit these — a fix may have landed. Both were verified present at Tier 1: + +1. `.agents/skills/adding-aperture-cli-client/SKILL.md:76` sends the reader to a **"Test it end to end" section of `docs/adding-a-client.md` that does not exist**. The guide's closest heading is Step 11, "Register the client, test it, and update the README". The section was written and then reverted, and the reverted guide is what got committed. An audit that misses this is not working — use it as the smoke test for this skill. +2. `README.md` has **no pointer to `docs/` or `.agents/skills/`**. Its nav lists only Supported agents, Installation, Usage, Development, so a 62KB contributor guide and two skills are undiscoverable from the front door. + +## Fix policy: read-only by default + +Report first, always. After the report, offer to apply **only** mechanical, unambiguous drift, and show a diff before touching anything: + +- stale line anchors where the symbol clearly moved within the same file +- renamed or moved paths with one obvious successor +- wrong counts and enumerations +- broken cross-references where exactly one real heading matches + +Everything judgment-based stays out of the auto-fix set and goes into the handoff prompt: whether a missing section should be written or the pointer to it removed, how to restructure navigation, whether a stale convention claim means fixing the doc or fixing the code. Those are decisions, and a doc audit does not get to make them silently. + +If the tree is dirty or another session is editing the corpus, check before writing — `git status --porcelain` and file mtimes. Concurrent edits to `.agents/skills/` do happen; do not clobber them, and do not attribute them to yourself. + +## Output contract + +Three parts, in order: + +1. **Findings, ranked by severity.** Each one: `file:line`, the claim as written, the contradicting evidence, the tier that proved it, and a proposed edit. Rank by what would mislead a reader into broken work — a wrong routing fact outranks a stale line anchor, and a dangling cross-reference that costs a contributor twenty minutes outranks a cosmetic count. State the corpus coverage and which tiers ran. +2. **The mechanical fixes, offered.** With a diff, as one batch the user can accept or decline. +3. **A copy-pasteable prompt for a fresh session** covering everything judgment-based, with enough context to act without re-auditing: the finding, what was verified, and what the open decision is. From 780149ae9122e4e920c33cd58f4e04ed393f0543 Mon Sep 17 00:00:00 2001 From: Larah Vasquez Date: Wed, 12 Aug 2026 09:11:29 -0500 Subject: [PATCH 5/8] README: add pointer to contributor docs --- README.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 40d2e89..643b18c 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,8 @@ Supported agents | Installation | Usage | - Development + Development | + Contributing

@@ -76,3 +77,9 @@ make test # run tests make install # install to $GOPATH/bin make clean # remove built binary ``` + +## Contributing + +To add a new coding agent, see [docs/adding-a-client.md](./docs/adding-a-client.md). + +Agent-facing skills for this repo live in [.agents/skills/](./.agents/skills/). From fae301decb3fb1581ea32731f6aba091d8e3e4fc Mon Sep 17 00:00:00 2001 From: Larah Vasquez Date: Wed, 12 Aug 2026 09:11:29 -0500 Subject: [PATCH 6/8] .agents: correct trim claim and refresh known drift --- .agents/skills/adding-aperture-cli-client/SKILL.md | 4 ++-- .agents/skills/auditing-aperture-cli-docs/SKILL.md | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.agents/skills/adding-aperture-cli-client/SKILL.md b/.agents/skills/adding-aperture-cli-client/SKILL.md index e4be6d8..391ca64 100644 --- a/.agents/skills/adding-aperture-cli-client/SKILL.md +++ b/.agents/skills/adding-aperture-cli-client/SKILL.md @@ -42,7 +42,7 @@ This skill is the orientation layer: the traps below are the ones that cost real **`Install.Run` and `Uninstall.Run` take different shapes.** `Install.Run` passes one string to `/bin/sh -c`, so pipes work. `Uninstall.Run` has no shell, so you must split the command into separate arguments yourself. Passing the whole command as one argument compiles and then fails at runtime with `fork/exec ...: no such file or directory`. No build or test step catches this. -**Trim the trailing slash before appending a path.** Use `strings.TrimRight(g.ApertureHost, "/")`. Users do configure `http://ai/`, and preflight tolerates it, so untrimmed concatenation yields `http://ai//v1`. `copilot` and `gemini` trim; `codex` and `opencode` do not and are inconsistent — follow the ones that trim. +**Trim the trailing slash before appending a path.** Use `strings.TrimRight(g.ApertureHost, "/")`. Users do configure `http://ai/`, and preflight tolerates it, so untrimmed concatenation yields `http://ai//v1`. `copilot`, `gemini`, and `pi` trim; `codex` and `opencode` do not and are inconsistent — follow the ones that trim. **Strip the provider prefix out of model names.** The launcher displays models as `provider_id/model_id`; harnesses want the bare ID. Leaving the prefix on breaks path-based routing and produces a puzzling 404 rather than a clear error. @@ -73,7 +73,7 @@ make test make build ``` -**Green gates do not mean the client routes.** Unit tests check the strings the client builds, not whether the harness accepts them, so a client can pass everything above and fail on its first real request. Follow **testing-an-aperture-cli-client** for the rest: a headless live request per protocol, a tool-calling check, and the interactive TUI steps only a human can do. `docs/adding-a-client.md` covers the same ground in its "Test it end to end" section. +**Green gates do not mean the client routes.** Unit tests check the strings the client builds, not whether the harness accepts them, so a client can pass everything above and fail on its first real request. Follow **testing-an-aperture-cli-client** for the rest: a headless live request per protocol, a tool-calling check, and the interactive TUI steps only a human can do. `docs/adding-a-client.md` Step 11 covers the CI gates and the interactive walk-through. Finally, add the harness to the `Supported agents` list in `README.md`, and lead the commit message with the touched path: `internal/clients: add client`. diff --git a/.agents/skills/auditing-aperture-cli-docs/SKILL.md b/.agents/skills/auditing-aperture-cli-docs/SKILL.md index 8aac304..13d324c 100644 --- a/.agents/skills/auditing-aperture-cli-docs/SKILL.md +++ b/.agents/skills/auditing-aperture-cli-docs/SKILL.md @@ -52,7 +52,7 @@ PY Then check the claims that carry no line number, which are the ones that actually break: -- **Cross-document section references.** A doc naming a section of another doc (`its "Test it end to end" section`) must match a real heading. Extract quoted section names and grep the target's headings — `grep -n '^#\{1,3\} ' docs/adding-a-client.md`. This class has a live finding today; see Known drift. +- **Cross-document section references.** A doc naming a section of another doc (say, a skill pointing at `its "Test it end to end" section` of the guide) must match a real heading. Extract quoted section names and grep the target's headings — `grep -n '^#\{1,3\} ' docs/adding-a-client.md`. That exact reference was a real finding, fixed at 51de975; see Known drift for how it read. Beware the self-reference: grepping the corpus for a dangling title will match this skill's own prose discussing it. Exclude the auditing skill from that grep, or expect the hit — anything auditing a corpus it belongs to has to account for itself. - **Counts and enumerations.** "The interface has nine methods" against `grep -cE '^\t[A-Z][A-Za-z]*\(' internal/clients/registry.go`. The README's `Supported agents` list against `ls -d internal/clients/*/` plus the desktop adapters in `internal/profiles` — the list is legitimately longer than the client count, because Claude Cowork is a profile, not a client. - **"Client X does this, client Y does not" claims.** These are the fastest-rotting sentences in the corpus, because a new client silently joins or breaks the pattern. Verify each side: `grep -rn "TrimRight" internal/clients/`. - **Named symbols, flags, paths, make targets.** Every backticked identifier should resolve: `grep -rn "func validateHost" internal/clients/`, flags against `grep -n 'flag\.' cmd/aperture/main.go`, `make ` against the `Makefile`, and version claims against `go.mod`. @@ -121,12 +121,12 @@ rm -f internal/clients/*/zz_audit_e2e_test.go && rm -rf "$E2E_DIR" gofmt -l . && go vet ./... && make test ``` -## Known drift, as of this skill's last verification +## Known drift: found at 51de975, both since fixed -Confirm rather than inherit these — a fix may have landed. Both were verified present at Tier 1: +Both were verified present at Tier 1 when this skill was written, and both were fixed in the commits immediately after. They are recorded here as the worked examples of what this skill is looking for, not as open findings — confirm rather than inherit either direction, since the corpus keeps moving. -1. `.agents/skills/adding-aperture-cli-client/SKILL.md:76` sends the reader to a **"Test it end to end" section of `docs/adding-a-client.md` that does not exist**. The guide's closest heading is Step 11, "Register the client, test it, and update the README". The section was written and then reverted, and the reverted guide is what got committed. An audit that misses this is not working — use it as the smoke test for this skill. -2. `README.md` has **no pointer to `docs/` or `.agents/skills/`**. Its nav lists only Supported agents, Installation, Usage, Development, so a 62KB contributor guide and two skills are undiscoverable from the front door. +1. **The dangling cross-reference.** `.agents/skills/adding-aperture-cli-client/SKILL.md:76` sent the reader to a "Test it end to end" section of `docs/adding-a-client.md` that did not exist. The section was written and then reverted, and the reverted guide is what got committed, so no blob in any commit carried the title — it was unrecoverable, not misplaced. The guide's closest heading is Step 11, "Register the client, test it, and update the README", which covers the CI gates and the interactive walk-through but not the headless per-protocol request, so the pointer was wrong in scope as well as in name. It now names Step 11 and its actual scope. **This remains the smoke test for this skill:** an audit that would not have caught it is not working. Re-derive it from the commit rather than trusting this entry. +2. **The undiscoverable contributor docs.** `README.md` had no pointer to `docs/` or `.agents/skills/` anywhere. Its nav listed only Supported agents, Installation, Usage, and Development, leaving a 62KB contributor guide and three skills unreachable from the front door. It now carries a `Contributing` section naming both, wired into the nav row. ## Fix policy: read-only by default From a3a3541a61067ff3e36868e259bc54a857284e2e Mon Sep 17 00:00:00 2001 From: Larah Vasquez Date: Wed, 12 Aug 2026 11:45:12 -0500 Subject: [PATCH 7/8] fix(pi): isolate extension files and empty providers --- internal/clients/pi/extension.go | 23 ++++++++++++++++---- internal/clients/pi/pi.go | 4 ++-- internal/clients/pi/pi_test.go | 36 +++++++++++++++++++++++++++----- 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/internal/clients/pi/extension.go b/internal/clients/pi/extension.go index 0051bba..cda23da 100644 --- a/internal/clients/pi/extension.go +++ b/internal/clients/pi/extension.go @@ -3,7 +3,6 @@ package pi import ( "encoding/json" "os" - "path/filepath" "strings" "github.com/tailscale/aperture-cli/internal/config" @@ -152,9 +151,25 @@ func writeProviderExtension(apertureHost string, p config.ProviderInfo, b backen if err != nil { return "", nil, err } - path := filepath.Join(dir, "tmp_aperture_provider.js") - if err := os.WriteFile(path, []byte(src), 0o600); err != nil { + f, err := os.CreateTemp(dir, "tmp_aperture_provider_*.js") + if err != nil { + return "", nil, err + } + path := f.Name() + remove := func() { _ = os.Remove(path) } + if err := f.Chmod(0o600); err != nil { + _ = f.Close() + remove() + return "", nil, err + } + if _, err := f.WriteString(src); err != nil { + _ = f.Close() + remove() + return "", nil, err + } + if err := f.Close(); err != nil { + remove() return "", nil, err } - return path, func() { os.Remove(path) }, nil + return path, remove, nil } diff --git a/internal/clients/pi/pi.go b/internal/clients/pi/pi.go index bd733ef..19ae46e 100644 --- a/internal/clients/pi/pi.go +++ b/internal/clients/pi/pi.go @@ -237,7 +237,7 @@ func resolveReplay(g *config.Global) (config.ProviderInfo, backend, string, bool return config.ProviderInfo{}, backend{}, "", false } // The provider must still serve that backend's protocol. - if !providerSupports(prov, b) { + if len(prov.Models) == 0 || !providerSupports(prov, b) { return config.ProviderInfo{}, backend{}, "", false } // The recorded model must still be offered by that provider. @@ -277,7 +277,7 @@ func (c *Client) QuickSelectLabel(g *config.Global) string { func compatibleProviders(all []config.ProviderInfo) []config.ProviderInfo { var out []config.ProviderInfo for _, p := range all { - if len(backendsFor(p)) > 0 { + if len(p.Models) > 0 && len(backendsFor(p)) > 0 { out = append(out, p) } } diff --git a/internal/clients/pi/pi_test.go b/internal/clients/pi/pi_test.go index cd96c8e..050e940 100644 --- a/internal/clients/pi/pi_test.go +++ b/internal/clients/pi/pi_test.go @@ -233,6 +233,31 @@ func TestWriteProviderExtension(t *testing.T) { } } +func TestWriteProviderExtension_UniquePaths(t *testing.T) { + isolateConfigDir(t) + + p := config.ProviderInfo{ID: "openai-api", Models: []string{"gpt-5"}} + b := backendByIDOrFatal(t, "openai_responses") + path1, cleanup1, err := writeProviderExtension(testHost, p, b) + if err != nil { + t.Fatalf("first writeProviderExtension: %v", err) + } + defer cleanup1() + path2, cleanup2, err := writeProviderExtension(testHost, p, b) + if err != nil { + t.Fatalf("second writeProviderExtension: %v", err) + } + defer cleanup2() + + if path1 == path2 { + t.Fatalf("concurrent extensions use the same path %q", path1) + } + cleanup1() + if _, err := os.Stat(path2); err != nil { + t.Errorf("cleaning up the first extension affected the second: %v", err) + } +} + // TestWriteProviderExtension_EmbeddedJSONIsValid checks the generated file's // provider config parses as JSON, which is what catches an unescaped value // silently producing a broken extension. @@ -455,17 +480,18 @@ func TestBackendsFor(t *testing.T) { func TestCompatibleProviders(t *testing.T) { provs := []config.ProviderInfo{ - {ID: "openai-api", Compatibility: map[string]bool{"openai_responses": true}}, - {ID: "bedrock", Compatibility: map[string]bool{"bedrock_converse": true}}, - {ID: "anthropic", Compatibility: map[string]bool{"anthropic_messages": true}}, - {ID: "none", Compatibility: map[string]bool{"something_else": true}}, + {ID: "openai-api", Models: []string{"gpt-5"}, Compatibility: map[string]bool{"openai_responses": true}}, + {ID: "bedrock", Models: []string{"model"}, Compatibility: map[string]bool{"bedrock_converse": true}}, + {ID: "anthropic", Models: []string{"claude"}, Compatibility: map[string]bool{"anthropic_messages": true}}, + {ID: "empty", Compatibility: map[string]bool{"openai_responses": true}}, + {ID: "none", Models: []string{"model"}, Compatibility: map[string]bool{"something_else": true}}, } got := compatibleProviders(provs) ids := make([]string, len(got)) for i, p := range got { ids[i] = p.ID } - // bedrock and none are both unusable from Pi. + // bedrock and none have no supported protocol; empty has no routable model. if want := []string{"openai-api", "anthropic"}; !slices.Equal(ids, want) { t.Errorf("compatibleProviders = %v, want %v", ids, want) } From 74e45713390bcd2c5cbd2922350c3c1c42ccec6f Mon Sep 17 00:00:00 2001 From: Larah Vasquez Date: Wed, 12 Aug 2026 14:20:34 -0500 Subject: [PATCH 8/8] feat(clients): add Oh My Pi support Add Oh My Pi as a registered Aperture CLI client with discovery, Bun-based installation, provider and backend selection, model selection, YOLO mode, and quick-select replay. Generate a temporary per-launch OMP extension instead of replacing user configuration. The extension registers Aperture-backed providers for OpenAI Responses, Anthropic Messages, OpenAI Chat Completions, and Google Vertex, then removes itself when the launched process exits. Document Oh My Pi as a supported agent and cover endpoint construction, provider filtering, generated extension content and permissions, launch arguments, replay validation, and install metadata. This means users can launch OMP through compatible Aperture providers while retaining their existing OMP settings, credentials, and sessions. --- README.md | 1 + cmd/aperture/main.go | 1 + internal/clients/omp/extension.go | 126 ++++++++++++++ internal/clients/omp/install.go | 23 +++ internal/clients/omp/omp.go | 263 ++++++++++++++++++++++++++++++ internal/clients/omp/omp_test.go | 149 +++++++++++++++++ 6 files changed, 563 insertions(+) create mode 100644 internal/clients/omp/extension.go create mode 100644 internal/clients/omp/install.go create mode 100644 internal/clients/omp/omp.go create mode 100644 internal/clients/omp/omp_test.go diff --git a/README.md b/README.md index 643b18c..2d46684 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,7 @@ A CLI launcher for coding agents preconfigured to work with [Aperture](https://a - [OpenCode](https://github.com/sst/opencode) - [Codex](https://github.com/openai/codex) - [GitHub Copilot CLI](https://docs.github.com/en/copilot/how-tos/copilot-cli/cli-getting-started) +- [Oh My Pi](https://omp.sh) - [Pi](https://pi.dev) - [Claude Cowork](https://support.claude.com/en/articles/13345190-get-started-with-claude-cowork) diff --git a/cmd/aperture/main.go b/cmd/aperture/main.go index ab36757..d0296af 100644 --- a/cmd/aperture/main.go +++ b/cmd/aperture/main.go @@ -22,6 +22,7 @@ import ( _ "github.com/tailscale/aperture-cli/internal/clients/codex" _ "github.com/tailscale/aperture-cli/internal/clients/copilot" _ "github.com/tailscale/aperture-cli/internal/clients/gemini" + _ "github.com/tailscale/aperture-cli/internal/clients/omp" _ "github.com/tailscale/aperture-cli/internal/clients/opencode" _ "github.com/tailscale/aperture-cli/internal/clients/pi" ) diff --git a/internal/clients/omp/extension.go b/internal/clients/omp/extension.go new file mode 100644 index 0000000..f9dd28b --- /dev/null +++ b/internal/clients/omp/extension.go @@ -0,0 +1,126 @@ +package omp + +import ( + "encoding/json" + "os" + "strings" + + "github.com/tailscale/aperture-cli/internal/config" +) + +type ompProvider struct { + Name string `json:"name"` + BaseURL string `json:"baseUrl"` + APIKey string `json:"apiKey"` + API string `json:"api"` + Models []ompModel `json:"models"` +} + +type ompModel struct { + ID string `json:"id"` + Name string `json:"name"` + Reasoning bool `json:"reasoning"` + Input []string `json:"input"` + ContextWindow int `json:"contextWindow"` + MaxTokens int `json:"maxTokens"` + Cost ompCost `json:"cost"` +} + +type ompCost struct { + Input float64 `json:"input"` + Output float64 `json:"output"` + CacheRead float64 `json:"cacheRead"` + CacheWrite float64 `json:"cacheWrite"` +} + +const ( + defaultContextWindow = 128000 + defaultMaxTokens = 16384 +) + +func ompProviderID(providerID string) string { + return "aperture-" + providerID +} + +func ompModelRef(providerID, model string) string { + return ompProviderID(providerID) + "/" + stripProviderPrefix(model) +} + +func (b backend) baseURL(apertureHost string) string { + host := strings.TrimRight(apertureHost, "/") + switch b.id { + case "anthropic": + return host + case "vertex": + return host + "/v1/projects/_aperture_auto_vertex_project_id_/locations/_aperture_auto_vertex_region_/publishers/google" + default: + return host + "/v1" + } +} + +func buildProvider(apertureHost string, p config.ProviderInfo, b backend) ompProvider { + models := make([]ompModel, len(p.Models)) + for i, m := range p.Models { + models[i] = ompModel{ + ID: m, + Name: m, + Input: []string{"text"}, + ContextWindow: defaultContextWindow, + MaxTokens: defaultMaxTokens, + } + } + return ompProvider{ + Name: "Aperture (" + p.ID + ")", + BaseURL: b.baseURL(apertureHost), + APIKey: "not-needed", + API: b.api, + Models: models, + } +} + +func extensionSource(providerID string, prov ompProvider) (string, error) { + id, err := json.Marshal(ompProviderID(providerID)) + if err != nil { + return "", err + } + cfg, err := json.MarshalIndent(prov, " ", " ") + if err != nil { + return "", err + } + return "// Generated by aperture-cli. Removed when the client exits.\n" + + "export default function (pi) {\n" + + " pi.registerProvider(" + string(id) + ", " + string(cfg) + ");\n" + + "}\n", nil +} + +func writeProviderExtension(apertureHost string, p config.ProviderInfo, b backend) (string, func(), error) { + src, err := extensionSource(p.ID, buildProvider(apertureHost, p, b)) + if err != nil { + return "", nil, err + } + dir, err := config.ClientConfigDir("omp") + if err != nil { + return "", nil, err + } + f, err := os.CreateTemp(dir, "tmp_aperture_provider_*.js") + if err != nil { + return "", nil, err + } + path := f.Name() + remove := func() { _ = os.Remove(path) } + if err := f.Chmod(0o600); err != nil { + _ = f.Close() + remove() + return "", nil, err + } + if _, err := f.WriteString(src); err != nil { + _ = f.Close() + remove() + return "", nil, err + } + if err := f.Close(); err != nil { + remove() + return "", nil, err + } + return path, remove, nil +} diff --git a/internal/clients/omp/install.go b/internal/clients/omp/install.go new file mode 100644 index 0000000..4469f0c --- /dev/null +++ b/internal/clients/omp/install.go @@ -0,0 +1,23 @@ +package omp + +import ( + "os" + "path/filepath" +) + +// commonBinaryPaths returns the non-PATH locations where omp is commonly installed. +func commonBinaryPaths() []string { + paths := []string{ + filepath.Join("/opt", "homebrew", "bin", "omp"), + filepath.Join("/usr", "local", "bin", "omp"), + } + home, err := os.UserHomeDir() + if err != nil { + return paths + } + return append(paths, + filepath.Join(home, ".local", "bin", "omp"), + filepath.Join(home, ".bun", "bin", "omp"), + filepath.Join(home, ".npm-global", "bin", "omp"), + ) +} diff --git a/internal/clients/omp/omp.go b/internal/clients/omp/omp.go new file mode 100644 index 0000000..ea1ba9c --- /dev/null +++ b/internal/clients/omp/omp.go @@ -0,0 +1,263 @@ +// Package omp is the Oh My Pi client. OMP accepts custom providers through +// its pi.registerProvider extension API, so this client writes a per-launch +// extension without replacing the user's OMP settings, credentials, or sessions. +// The menu flow is provider, wire protocol, then model. +package omp + +import ( + "os/exec" + "slices" + "strings" + + tea "github.com/charmbracelet/bubbletea" + "github.com/tailscale/aperture-cli/internal/clients" + "github.com/tailscale/aperture-cli/internal/config" + "github.com/tailscale/aperture-cli/internal/menu" +) + +func init() { + clients.Register(&Client{}) +} + +// Client is the Oh My Pi client. +type Client struct{} + +const ( + name = "Oh My Pi" + binaryName = "omp" + + installCmd = "bun install -g @oh-my-pi/pi-coding-agent" + uninstallCmd = "bun uninstall -g @oh-my-pi/pi-coding-agent" +) + +type backend struct { + id string + displayName string + api string + compatKeys []string +} + +var backends = []backend{ + {id: "openai_responses", displayName: "OpenAI Responses", api: "openai-responses", compatKeys: []string{"openai_responses"}}, + {id: "anthropic", displayName: "Anthropic Messages", api: "anthropic-messages", compatKeys: []string{"anthropic_messages"}}, + {id: "openai_chat", displayName: "OpenAI Chat Completions", api: "openai-completions", compatKeys: []string{"openai_chat"}}, + {id: "vertex", displayName: "Google Vertex", api: "google-generative-ai", compatKeys: []string{"google_generate_content", "google_raw_predict"}}, +} + +// Name implements clients.Client. +func (c *Client) Name() string { return name } + +// BinaryName implements clients.Client. +func (c *Client) BinaryName() string { return binaryName } + +// CommonPaths implements clients.Client. +func (c *Client) CommonPaths() []string { return commonBinaryPaths() } + +// IsInstalled implements clients.Client. +func (c *Client) IsInstalled() bool { return clients.IsInstalled(binaryName, c.CommonPaths()) } + +// Install implements clients.Client. +func (c *Client) Install(_ *config.Global) clients.InstallPlan { + return clients.InstallPlan{ + Hint: installCmd, + Run: func() (*exec.Cmd, error) { + return exec.Command("/bin/sh", "-c", installCmd), nil + }, + } +} + +// Uninstall implements clients.Client. +func (c *Client) Uninstall() clients.UninstallPlan { + return clients.UninstallPlan{ + Hint: uninstallCmd, + Run: func() error { + return exec.Command("bun", "uninstall", "-g", "@oh-my-pi/pi-coding-agent").Run() + }, + } +} + +// Menu implements clients.Client. +func (c *Client) Menu(g *config.Global) menu.MenuItem { + return menu.MenuItem{Label: name, Action: func() menu.Result { return c.providerStep(g) }} +} + +func (c *Client) providerStep(g *config.Global) menu.Result { + provs := compatibleProviders(g.Providers) + if len(provs) == 0 { + return errorResult("No providers support " + name + ".") + } + if len(provs) == 1 { + return c.backendStep(g, provs[0]) + } + items := make([]menu.MenuItem, 0, len(provs)) + for _, p := range provs { + items = append(items, menu.MenuItem{ + Label: p.DisplayName(), Description: p.Description, + Action: func() menu.Result { return c.backendStep(g, p) }, + }) + } + return menu.Result{Next: &menu.Menu{Title: "Choose a provider for " + name + ":", Items: items}} +} + +func (c *Client) backendStep(g *config.Global, p config.ProviderInfo) menu.Result { + bs := backendsFor(p) + if len(bs) == 0 { + return errorResult("No compatible backends for " + p.DisplayName() + ".") + } + if len(bs) == 1 { + return c.modelStep(g, p, bs[0]) + } + items := make([]menu.MenuItem, 0, len(bs)) + for _, b := range bs { + items = append(items, menu.MenuItem{Label: b.displayName, Action: func() menu.Result { return c.modelStep(g, p, b) }}) + } + return menu.Result{Next: &menu.Menu{Title: "Choose a backend for " + name + " via " + p.DisplayName() + ":", Items: items}} +} + +func (c *Client) modelStep(g *config.Global, p config.ProviderInfo, b backend) menu.Result { + models := fqnModels(p) + if len(models) <= 1 { + var model string + if len(models) == 1 { + model = models[0] + } + return c.launch(g, p, b, model) + } + items := make([]menu.MenuItem, 0, len(models)) + for _, model := range models { + items = append(items, menu.MenuItem{Label: model, Action: func() menu.Result { return c.launch(g, p, b, model) }}) + } + return menu.Result{Next: &menu.Menu{Title: "Choose a default model for " + name + " via " + p.DisplayName() + ":", Items: items}} +} + +func (c *Client) launch(g *config.Global, p config.ProviderInfo, b backend, model string) menu.Result { + bin := clients.FindBinary(binaryName, c.CommonPaths()) + if bin == "" { + bin = binaryName + } + extPath, cleanup, err := writeProviderExtension(g.ApertureHost, p, b) + if err != nil { + return errorResult("Failed to write " + name + " provider extension: " + err.Error()) + } + args := buildArgs(extPath, p.ID, model, g.Settings.YoloMode) + _ = g.RecordLaunch(config.LaunchState{ + LastClientName: name, LastBackendType: b.id, LastProviderID: p.ID, LastModel: model, + }) + cmd := clients.Launch(clients.LaunchSpec{Binary: bin, Args: args, Cleanup: cleanup, Debug: g.Debug}) + return menu.Result{Cmd: cmd, PopOnDone: true} +} + +func buildArgs(extPath, providerID, model string, yolo bool) []string { + args := []string{"-e", extPath} + if model != "" { + args = append(args, "--model", ompModelRef(providerID, model)) + } + if yolo { + args = append(args, "--auto-approve") + } + return args +} + +func resolveReplay(g *config.Global) (config.ProviderInfo, backend, string, bool) { + if g.LastLaunch.LastClientName != name { + return config.ProviderInfo{}, backend{}, "", false + } + prov, ok := g.Provider(g.LastLaunch.LastProviderID) + if !ok { + return config.ProviderInfo{}, backend{}, "", false + } + b, ok := backendByID(g.LastLaunch.LastBackendType) + if !ok || len(prov.Models) == 0 || !providerSupports(prov, b) { + return config.ProviderInfo{}, backend{}, "", false + } + model := g.LastLaunch.LastModel + if model != "" && !slices.Contains(fqnModels(prov), model) { + return config.ProviderInfo{}, backend{}, "", false + } + return prov, b, model, true +} + +// Replay implements clients.Client. +func (c *Client) Replay(g *config.Global) tea.Cmd { + if !c.IsInstalled() { + return nil + } + prov, b, model, ok := resolveReplay(g) + if !ok { + return nil + } + return c.launch(g, prov, b, model).Cmd +} + +// QuickSelectLabel implements clients.Client. +func (c *Client) QuickSelectLabel(g *config.Global) string { + prov, _ := g.Provider(g.LastLaunch.LastProviderID) + label := name + " via " + prov.DisplayName() + if b, ok := backendByID(g.LastLaunch.LastBackendType); ok { + label += " - " + b.displayName + } + if g.LastLaunch.LastModel != "" { + label += " - " + g.LastLaunch.LastModel + } + return label +} + +func compatibleProviders(all []config.ProviderInfo) []config.ProviderInfo { + var out []config.ProviderInfo + for _, p := range all { + if len(p.Models) > 0 && len(backendsFor(p)) > 0 { + out = append(out, p) + } + } + return out +} + +func backendsFor(p config.ProviderInfo) []backend { + var out []backend + for _, b := range backends { + if providerSupports(p, b) { + out = append(out, b) + } + } + return out +} + +func providerSupports(p config.ProviderInfo, b backend) bool { + for _, key := range b.compatKeys { + if p.Compatibility[key] { + return true + } + } + return false +} + +func backendByID(id string) (backend, bool) { + idx := slices.IndexFunc(backends, func(b backend) bool { return b.id == id }) + if idx < 0 { + return backend{}, false + } + return backends[idx], true +} + +func fqnModels(p config.ProviderInfo) []string { + out := make([]string, len(p.Models)) + for i, model := range p.Models { + out[i] = p.ID + "/" + model + } + return out +} + +func stripProviderPrefix(fqn string) string { + if _, after, ok := strings.Cut(fqn, "/"); ok { + return after + } + return fqn +} + +func errorResult(msg string) menu.Result { + return menu.Result{Cmd: func() tea.Msg { return menu.SimpleDoneMsg{Err: errString(msg)} }} +} + +type errString string + +func (e errString) Error() string { return string(e) } diff --git a/internal/clients/omp/omp_test.go b/internal/clients/omp/omp_test.go new file mode 100644 index 0000000..62f1f66 --- /dev/null +++ b/internal/clients/omp/omp_test.go @@ -0,0 +1,149 @@ +package omp + +import ( + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/tailscale/aperture-cli/internal/config" +) + +const testHost = "http://ai.example.com" + +func backendByIDOrFatal(t *testing.T, id string) backend { + t.Helper() + b, ok := backendByID(id) + if !ok { + t.Fatalf("no backend with id %q", id) + } + return b +} + +func TestBackendBaseURL(t *testing.T) { + cases := map[string]string{ + "openai_responses": testHost + "/v1", + "anthropic": testHost, + "openai_chat": testHost + "/v1", + "vertex": testHost + "/v1/projects/_aperture_auto_vertex_project_id_/locations/_aperture_auto_vertex_region_/publishers/google", + } + for id, want := range cases { + if got := backendByIDOrFatal(t, id).baseURL(testHost + "/"); got != want { + t.Errorf("%s baseURL = %q, want %q", id, got, want) + } + } +} + +func TestBuildProvider(t *testing.T) { + p := config.ProviderInfo{ID: "openai-api", Models: []string{"gpt-5.6-sol"}} + got := buildProvider(testHost, p, backendByIDOrFatal(t, "openai_responses")) + if got.BaseURL != testHost+"/v1" || got.API != "openai-responses" || got.APIKey != "not-needed" { + t.Errorf("buildProvider = %+v", got) + } + if len(got.Models) != 1 || got.Models[0].ID != "gpt-5.6-sol" || got.Models[0].MaxTokens == 0 || len(got.Models[0].Input) == 0 { + t.Errorf("models = %+v", got.Models) + } +} + +func TestExtensionSource(t *testing.T) { + p := config.ProviderInfo{ID: "openai-api", Models: []string{"gpt-5.6-sol"}} + src, err := extensionSource(p.ID, buildProvider(testHost, p, backendByIDOrFatal(t, "openai_responses"))) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"export default function", "pi.registerProvider(", `"aperture-openai-api"`, `"openai-responses"`} { + if !strings.Contains(src, want) { + t.Errorf("extension missing %q:\n%s", want, src) + } + } + if strings.Contains(src, "null") { + t.Errorf("extension contains null:\n%s", src) + } +} + +func TestWriteProviderExtension(t *testing.T) { + tmp := t.TempDir() + t.Setenv("HOME", tmp) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(tmp, ".config")) + p := config.ProviderInfo{ID: "openai-api", Models: []string{"gpt-5.6-sol"}} + path, cleanup, err := writeProviderExtension(testHost, p, backendByIDOrFatal(t, "openai_responses")) + if err != nil { + t.Fatal(err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if info.Mode().Perm() != 0o600 { + t.Errorf("perm = %o, want 600", info.Mode().Perm()) + } + cleanup() + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Error("extension still exists after cleanup") + } +} + +func TestBuildArgs(t *testing.T) { + want := []string{"-e", "/tmp/ext.js", "--model", "aperture-openai-api/gpt-5.6-sol", "--auto-approve"} + got := buildArgs("/tmp/ext.js", "openai-api", "openai-api/gpt-5.6-sol", true) + if !slices.Equal(got, want) { + t.Errorf("buildArgs = %v, want %v", got, want) + } + if got := buildArgs("/tmp/ext.js", "openai-api", "", false); !slices.Equal(got, []string{"-e", "/tmp/ext.js"}) { + t.Errorf("buildArgs without model = %v", got) + } +} + +func TestBackendsFor(t *testing.T) { + p := config.ProviderInfo{Compatibility: map[string]bool{ + "openai_responses": true, "anthropic_messages": true, "openai_chat": true, "google_raw_predict": true, + }} + got := backendsFor(p) + ids := make([]string, len(got)) + for i, b := range got { + ids[i] = b.id + } + want := []string{"openai_responses", "anthropic", "openai_chat", "vertex"} + if !slices.Equal(ids, want) { + t.Errorf("backendsFor = %v, want %v", ids, want) + } +} + +func TestCompatibleProviders(t *testing.T) { + provs := []config.ProviderInfo{ + {ID: "match", Models: []string{"model"}, Compatibility: map[string]bool{"openai_responses": true}}, + {ID: "empty", Compatibility: map[string]bool{"openai_responses": true}}, + {ID: "other", Models: []string{"model"}, Compatibility: map[string]bool{"bedrock_converse": true}}, + } + got := compatibleProviders(provs) + if len(got) != 1 || got[0].ID != "match" { + t.Errorf("compatibleProviders = %+v", got) + } +} + +func TestResolveReplay(t *testing.T) { + p := config.ProviderInfo{ID: "openai-api", Models: []string{"gpt-5.6-sol"}, Compatibility: map[string]bool{"openai_responses": true}} + g := &config.Global{ + Providers: []config.ProviderInfo{p}, + LastLaunch: config.LaunchState{LastClientName: name, LastBackendType: "openai_responses", LastProviderID: p.ID, LastModel: "openai-api/gpt-5.6-sol"}, + } + _, _, model, ok := resolveReplay(g) + if !ok || model != "openai-api/gpt-5.6-sol" { + t.Fatalf("resolveReplay = %q, %v", model, ok) + } + g.LastLaunch.LastModel = "openai-api/stale" + if _, _, _, ok := resolveReplay(g); ok { + t.Error("resolveReplay accepted a stale model") + } +} + +func TestInstallUninstall(t *testing.T) { + c := &Client{} + if got := c.Install(&config.Global{}); got.Hint != installCmd || got.Run == nil { + t.Errorf("Install = %+v", got) + } + if got := c.Uninstall(); got.Hint != uninstallCmd || got.Run == nil { + t.Errorf("Uninstall = %+v", got) + } +}