From 0df5f1e83aac98be038c7fc77d54cdf0de53a477 Mon Sep 17 00:00:00 2001 From: Guy J Grigsby Date: Tue, 22 Sep 2026 17:56:02 +0000 Subject: [PATCH 1/3] aperture: version source builds from the release tag A make install or go build binary reported a commit height (B29), so a running binary could not be matched to the release it came from. The goreleaser build already stamped the tag, so the height scheme only ever showed on source builds, which is every build a dev runs. git describe gives the tag on a release commit and the tag plus distance past it otherwise, for both the Makefile ldflags and the init() fallback plain go build exercises. Keeping B-numbers would have cost nothing to write and stayed unmatchable against the release list forever. --- Makefile | 4 ++-- cmd/aperture/main.go | 26 ++++++++++---------------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/Makefile b/Makefile index 7525a73..5f61486 100644 --- a/Makefile +++ b/Makefile @@ -1,14 +1,14 @@ .PHONY: build test lint check clean install BUILD_DATE := $(shell date -u +"%Y-%m-%dT%H:%M:%SZ") -GIT_HEIGHT := $(shell git rev-list --count HEAD 2>/dev/null || echo 0) +GIT_VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo dev) GIT_DESC := $(shell git describe --always) ifneq ($(shell git status --porcelain),) GIT_DESC := $(GIT_DESC)-dirty endif -LDFLAGS := -X main.buildVersion=B$(GIT_HEIGHT) -X main.buildCommit=$(GIT_DESC) -X main.buildDate=$(BUILD_DATE) +LDFLAGS := -X main.buildVersion=$(GIT_VERSION) -X main.buildCommit=$(GIT_DESC) -X main.buildDate=$(BUILD_DATE) build: go build -ldflags "$(LDFLAGS)" -o .build/aperture ./cmd/aperture diff --git a/cmd/aperture/main.go b/cmd/aperture/main.go index f37997e..b928cf3 100644 --- a/cmd/aperture/main.go +++ b/cmd/aperture/main.go @@ -47,8 +47,8 @@ func init() { } if buildVersion == "B0-dev" { - if height := gitCommitHeight(); height != "" { - buildVersion = "B" + height + if desc := gitDescribe(); desc != "" { + buildVersion = desc } else if info.Main.Version != "" && info.Main.Version != "(devel)" { buildVersion = info.Main.Version } @@ -79,14 +79,17 @@ func init() { } } -func gitCommitHeight() string { +// gitDescribe reports the release version of the checkout containing this +// source file: the tag on an exact release commit, or the nearest tag with +// the distance and commit appended. It returns "" outside a checkout. +func gitDescribe() string { _, file, _, ok := runtime.Caller(0) if !ok { return "" } for dir := filepath.Dir(file); ; dir = filepath.Dir(dir) { if _, err := os.Stat(filepath.Join(dir, ".git")); err == nil { - return gitCommitHeightInDir(dir) + return gitDescribeInDir(dir) } parent := filepath.Dir(dir) if parent == dir { @@ -95,23 +98,14 @@ func gitCommitHeight() string { } } -func gitCommitHeightInDir(dir string) string { - cmd := exec.Command("git", "rev-list", "--count", "HEAD") +func gitDescribeInDir(dir string) string { + cmd := exec.Command("git", "describe", "--tags", "--always", "--dirty") cmd.Dir = dir out, err := cmd.Output() if err != nil { return "" } - height := strings.TrimSpace(string(out)) - if height == "" { - return "" - } - for _, r := range height { - if r < '0' || r > '9' { - return "" - } - } - return height + return strings.TrimSpace(string(out)) } // startRunLog points slog at the run log and returns a function that closes From 4c261fa44496900da56d1d606df8fcd839b6caed Mon Sep 17 00:00:00 2001 From: Guy J Grigsby Date: Tue, 22 Sep 2026 18:10:32 +0000 Subject: [PATCH 2/3] e2e: add end-to-end suite driving the built binary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every existing test exercises internals in-process; nothing executed the shipped binary, so the flag-before-TUI ordering, the PTY handoff to a launched agent, and the return to the picker after the child exits were all untested at the level users hit them. Four tests against the real build: -version, a bad endpoint failing before the TUI takes the terminal, the Pi happy path against a fake Aperture (httptest /v1/models) with a stub pi on PATH recording argv and capturing the generated provider extension while it exists, and the unreachable-endpoint banner. Runs hermetically: temp HOME/XDG, PATH of the stub dir plus system dirs only so host agent binaries cannot leak into the picker. creack/pty is promoted from the module graph; no new third-party code. TERM=dumb because startup asks the terminal its color profile and a PTY never answers — with xterm each run paid the five-second query timeout. Skipped: bridge flows (need a control plane), install flows (network), Windows PTYs. --- e2e/e2e_test.go | 128 ++++++++++++++++++++++++++++++++ e2e/harness.go | 193 ++++++++++++++++++++++++++++++++++++++++++++++++ go.mod | 1 + 3 files changed, 322 insertions(+) create mode 100644 e2e/e2e_test.go create mode 100644 e2e/harness.go diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go new file mode 100644 index 0000000..fbb9aac --- /dev/null +++ b/e2e/e2e_test.go @@ -0,0 +1,128 @@ +package e2e + +import ( + "io" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +var apertureBin string + +// TestMain builds the binary under test once. The tests exercise what we +// ship, so they run the real build rather than recompiling main's guts +// into the test binary. +func TestMain(m *testing.M) { + dir, err := os.MkdirTemp("", "aperture-e2e") + if err != nil { + panic(err) + } + apertureBin = filepath.Join(dir, "aperture") + build := exec.Command("go", "build", "-o", apertureBin, "../cmd/aperture") + if out, err := build.CombinedOutput(); err != nil { + panic(string(out)) + } + code := m.Run() + _ = os.RemoveAll(dir) + os.Exit(code) +} + +// modelsJSON is the smallest GET /v1/models payload that walks one provider +// through discovery: one provider, one model, one wire endpoint. With one +// of each, the launch flow asks no follow-up questions — selecting the +// client launches it. +const modelsJSON = `{ + "object": "list", + "data": [ + { + "id": "test-model", + "supported_endpoints": ["/v1/responses"], + "metadata": {"provider": {"id": "test-provider", "name": "Test Provider", "upstream": "test"}} + } + ] +}` + +// fakeAperture serves the discovery contract and nothing else. +func fakeAperture(t *testing.T) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/models" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, modelsJSON) + })) + t.Cleanup(srv.Close) + return srv +} + +func TestVersion(t *testing.T) { + stdout, _, err := run(t, apertureBin, hermeticEnv(t, ""), "-version") + if err != nil { + t.Fatalf("-version: %v", err) + } + if stdout == "" || stdout == "B0-dev" { + t.Errorf("version output = %q, want a release version", stdout) + } +} + +// A bad endpoint URL has to fail before the TUI takes the terminal: the +// script that passed it reads stderr and the exit code, not a painted error. +func TestBadEndpointExitsBeforeTUI(t *testing.T) { + _, stderr, err := run(t, apertureBin, hermeticEnv(t, ""), "-endpoint", "://nope") + exitErr, ok := err.(*exec.ExitError) + if !ok || exitErr.ExitCode() != 1 { + t.Fatalf("exit = %v, want exit code 1", err) + } + if !strings.Contains(stderr, "aperture:") { + t.Errorf("stderr = %q, want the failure reported", stderr) + } +} + +// The happy path: connect to a fake Aperture, pick the only installed +// client, and watch the launch reach the stub binary with the generated +// provider extension pointing back at the fake Aperture. +func TestLaunchPi(t *testing.T) { + srv := fakeAperture(t) + binDir := t.TempDir() + recordDir := installStubPi(t, binDir) + env := append(hermeticEnv(t, binDir), "APERTURE_E2E_RECORD="+recordDir) + + term := spawn(t, apertureBin, []string{"-endpoint", srv.URL}, env) + // With the stub as the only installed client, the picker opens with Pi + // as row [1] and no quick-select. + term.waitFor(t, "Which editor do you want to use?") + term.waitFor(t, "[1] Pi") + term.send("1") + + // One provider, one backend, one model: no follow-up menus, the + // selection launches straight into the stub. + argv := waitForFile(t, filepath.Join(recordDir, "argv")) + extension := waitForFile(t, filepath.Join(recordDir, "extension")) + + // "q" quits only from the root menu: a clean exit here also proves the + // TUI took the terminal back after the child exited. + term.send("q") + term.waitExit(t) + + if !strings.Contains(argv, "-e\n") { + t.Errorf("stub argv = %q, want the extension loaded with -e", argv) + } + if !strings.Contains(extension, srv.URL) { + t.Errorf("extension routes to %q, want the Aperture at %s", extension, srv.URL) + } +} + +// An unreachable endpoint paints the failure banner rather than hanging or +// exiting; the launcher stays up so the user can pick another endpoint. +func TestUnreachableEndpoint(t *testing.T) { + term := spawn(t, apertureBin, []string{"-endpoint", "http://127.0.0.1:1"}, hermeticEnv(t, "")) + term.waitFor(t, "Could not reach") + term.send("\x03") + term.waitExit(t) +} diff --git a/e2e/harness.go b/e2e/harness.go new file mode 100644 index 0000000..ece45ac --- /dev/null +++ b/e2e/harness.go @@ -0,0 +1,193 @@ +// Package e2e exercises the built aperture binary end to end: real process, +// real PTY, a fake Aperture over HTTP, and stub agent binaries on PATH. +// Nothing here touches the user's real home, config, or tailnet. +package e2e + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/charmbracelet/x/ansi" + "github.com/creack/pty" +) + +// terminal is a running aperture process attached to a PTY, with the output +// stream captured for assertions. +type terminal struct { + cmd *exec.Cmd + ptmx *os.File + + mu sync.Mutex + out bytes.Buffer +} + +// spawn starts bin on a PTY with args and env and begins capturing output. +func spawn(t *testing.T, bin string, args []string, env []string) *terminal { + t.Helper() + cmd := exec.Command(bin, args...) + cmd.Env = env + ptmx, err := pty.StartWithSize(cmd, &pty.Winsize{Rows: 40, Cols: 120}) + if err != nil { + t.Fatalf("spawning %s: %v", bin, err) + } + term := &terminal{cmd: cmd, ptmx: ptmx} + go func() { + var buf [4096]byte + for { + n, err := ptmx.Read(buf[:]) + if n > 0 { + term.mu.Lock() + term.out.Write(buf[:n]) + term.mu.Unlock() + } + if err != nil { + return + } + } + }() + return term +} + +// send types keys into the terminal. +func (term *terminal) send(keys string) { + if _, err := term.ptmx.WriteString(keys); err != nil { + // The process may have exited between the last waitFor and this + // send; waitExit reports the real outcome. + return + } +} + +// screen is everything the process has printed so far, escape sequences +// stripped, so assertions match words rather than terminal control bytes. +func (term *terminal) screen() string { + term.mu.Lock() + defer term.mu.Unlock() + return ansi.Strip(term.out.String()) +} + +// waitFor blocks until substr appears on screen, failing with the full +// screen after a generous timeout. The timeout covers bridge bring-up on a +// slow CI box; locally every screen arrives in milliseconds. +func (term *terminal) waitFor(t *testing.T, substr string) { + t.Helper() + deadline := time.Now().Add(15 * time.Second) + for time.Now().Before(deadline) { + if strings.Contains(term.screen(), substr) { + return + } + time.Sleep(20 * time.Millisecond) + } + t.Fatalf("timed out waiting for %q; screen:\n%s", substr, term.screen()) +} + +// waitExit expects the process to exit 0 within the timeout. +func (term *terminal) waitExit(t *testing.T) { + t.Helper() + done := make(chan error, 1) + go func() { done <- term.cmd.Wait() }() + select { + case err := <-done: + term.ptmx.Close() + if err != nil { + t.Fatalf("exit: %v; screen:\n%s", err, term.screen()) + } + case <-time.After(15 * time.Second): + _ = term.cmd.Process.Kill() + t.Fatalf("process still running; screen:\n%s", term.screen()) + } +} + +// run executes bin without a PTY and returns its streams. For the flag +// paths that resolve before the TUI takes the terminal. +func run(t *testing.T, bin string, env []string, args ...string) (string, string, error) { + t.Helper() + cmd := exec.Command(bin, args...) + cmd.Env = env + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Run() + return stdout.String(), stderr.String(), err +} + +// hermeticEnv builds the environment for one run: a throwaway home so +// config, state, the run log and generated extensions land in a temp dir, +// and a PATH of binDir plus the system dirs only. The system dirs keep git +// reachable for version stamping while hiding every agent binary the dev +// box happens to have installed, so the client picker shows exactly the +// stubs the test installed. +// +// TERM=dumb, not xterm: startup asks the terminal its color profile and a +// PTY never answers, which costs every run a five-second query timeout. +// Assertions strip escape sequences anyway, so color buys nothing here. +func hermeticEnv(t *testing.T, binDir string) []string { + t.Helper() + home := t.TempDir() + drop := map[string]bool{ + "HOME": true, "XDG_CONFIG_HOME": true, "TERM": true, "PATH": true, + "APERTURE_ENDPOINT": true, "APERTURE_BRIDGE": true, + } + var env []string + for _, e := range os.Environ() { + k, _, _ := strings.Cut(e, "=") + if !drop[k] { + env = append(env, e) + } + } + path := string(filepath.ListSeparator) + "/usr/bin" + string(filepath.ListSeparator) + "/bin" + if binDir != "" { + path = binDir + path + } + return append(env, + "HOME="+home, + "XDG_CONFIG_HOME="+filepath.Join(home, ".config"), + "TERM=dumb", + "PATH="+path, + ) +} + +// stubPi is a stand-in for the pi binary: it records its argv and +// environment, and captures the generated provider extension while it still +// exists — aperture removes the extension when the child exits. +const stubPi = `#!/bin/sh +printf '%s\n' "$@" > "$APERTURE_E2E_RECORD/argv" +env > "$APERTURE_E2E_RECORD/env" +prev="" +for a in "$@"; do + if [ "$prev" = "-e" ]; then + cat "$a" > "$APERTURE_E2E_RECORD/extension" + fi + prev="$a" +done +` + +// installStubPi writes the stub into binDir and returns the directory the +// stub records into. +func installStubPi(t *testing.T, binDir string) string { + t.Helper() + recordDir := t.TempDir() + if err := os.WriteFile(filepath.Join(binDir, "pi"), []byte(stubPi), 0o755); err != nil { + t.Fatal(err) + } + return recordDir +} + +// waitForFile blocks until path exists and is non-empty. +func waitForFile(t *testing.T, path string) string { + t.Helper() + deadline := time.Now().Add(15 * time.Second) + for time.Now().Before(deadline) { + if data, err := os.ReadFile(path); err == nil && len(data) > 0 { + return string(data) + } + time.Sleep(20 * time.Millisecond) + } + t.Fatalf("timed out waiting for %s", path) + return "" +} diff --git a/go.mod b/go.mod index 3a697de..0af0e28 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/lipgloss v1.1.0 github.com/charmbracelet/x/ansi v0.11.6 + github.com/creack/pty v1.1.24 golang.org/x/sys v0.47.0 tailscale.com v1.102.3 ) From 492405b38f007e236a992bd5347864b44a1e3b00 Mon Sep 17 00:00:00 2001 From: Guy J Grigsby Date: Tue, 22 Sep 2026 18:11:56 +0000 Subject: [PATCH 3/3] docs: require unit and e2e tests with new functionality --- AGENTS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 4321ff0..4cffa00 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -120,6 +120,8 @@ license-compatible, no GPL/AGPL; name the one chosen, or why none fit. never poll the control plane or the LocalAPI in a tight loop. - Work consciously skipped is said out loud, never left as a TODO comment or as speculative code. +- Unit tests and e2e tests need to be included with all new functionality. + Test the expected ideal behavior, not the current implementation details. Being lazy about the solution is the goal. Being lazy about understanding it is not: trace the flow a change touches before picking an approach, because the