From 8ff2e6ddb2dddad60aa288ed3c5e0c756f81eeba Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 4 Mar 2026 19:01:14 -0800 Subject: [PATCH 1/5] Add benchmarks CI job with regression detection Runs on push to main when Go files change. Uses benchstat to compare against cached baseline; >20% regression emits ::error:: and fails. Results posted to step summary and uploaded as artifact. --- .github/workflows/test.yml | 80 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8d2f531..687e117 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -77,3 +77,83 @@ jobs: - name: Test with race detector run: go test -race -v ./... + + benchmarks: + name: Benchmarks + runs-on: ubuntu-latest + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + fetch-depth: 2 + + - name: Check for benchmark-relevant changes + id: filter + uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3 + with: + filters: | + bench: + - '**/*.go' + - 'go.mod' + - 'go.sum' + + - name: Set up Go + if: steps.filter.outputs.bench == 'true' + uses: actions/setup-go@4b73464bb391d4059bd26b0524d20df3927bd417 # v6 + with: + go-version-file: go.mod + + - name: Run benchmarks + if: steps.filter.outputs.bench == 'true' + run: | + set -o pipefail + go test -bench=. -benchmem -count=3 ./... | tee benchmarks.txt + + - name: Download previous benchmark baseline + if: steps.filter.outputs.bench == 'true' + uses: actions/cache/restore@cdf6c1fa76f9f475f3d7449005a359c84ca0f306 # v5 + with: + path: benchmarks-baseline.txt + key: benchmarks-baseline-${{ github.sha }} + restore-keys: | + benchmarks-baseline- + + - name: Install benchstat + if: steps.filter.outputs.bench == 'true' + run: go install golang.org/x/perf/cmd/benchstat@v0.0.0-20250207232725-2c7781eb8b4a + + - name: Compare benchmarks + if: steps.filter.outputs.bench == 'true' && hashFiles('benchmarks-baseline.txt') != '' + run: | + echo "## Benchmark Comparison" >> "$GITHUB_STEP_SUMMARY" + echo '```' >> "$GITHUB_STEP_SUMMARY" + benchstat benchmarks-baseline.txt benchmarks.txt >> "$GITHUB_STEP_SUMMARY" 2>&1 || true + echo '```' >> "$GITHUB_STEP_SUMMARY" + + - name: Check for significant regression + if: steps.filter.outputs.bench == 'true' && hashFiles('benchmarks-baseline.txt') != '' + run: | + benchstat benchmarks-baseline.txt benchmarks.txt > comparison.txt 2>&1 || true + if grep -E '\+[2-9][0-9]\.[0-9]+%|\+[1-9][0-9][0-9]+' comparison.txt; then + echo "::error::Performance regression detected (≥20% slower). See benchmark comparison in step summary." + exit 1 + fi + + - name: Save benchmark baseline + if: steps.filter.outputs.bench == 'true' + run: cp benchmarks.txt benchmarks-baseline.txt + + - name: Cache benchmark baseline + if: steps.filter.outputs.bench == 'true' + uses: actions/cache/save@cdf6c1fa76f9f475f3d7449005a359c84ca0f306 # v5 + with: + path: benchmarks-baseline.txt + key: benchmarks-baseline-${{ github.sha }} + + - name: Upload benchmark results + if: steps.filter.outputs.bench == 'true' + uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f # v7.0.0 + with: + name: benchmarks + path: benchmarks.txt + retention-days: 30 From f5d5b54a0f55e1272af7e49c3943eafbe5b1c903 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 4 Mar 2026 19:01:16 -0800 Subject: [PATCH 2/5] Add AI PR classification and breaking change detection Two-job workflow: classify labels PRs as bug/enhancement/documentation for release note categorization; breaking detects exported Go API surface changes and posts a PR comment when found. --- .github/prompts/classify-pr.prompt.yml | 24 +++ .github/prompts/detect-breaking.prompt.yml | 36 ++++ .github/workflows/ai-labeler.yml | 224 +++++++++++++++++++++ 3 files changed, 284 insertions(+) create mode 100644 .github/prompts/classify-pr.prompt.yml create mode 100644 .github/prompts/detect-breaking.prompt.yml create mode 100644 .github/workflows/ai-labeler.yml diff --git a/.github/prompts/classify-pr.prompt.yml b/.github/prompts/classify-pr.prompt.yml new file mode 100644 index 0000000..0d87499 --- /dev/null +++ b/.github/prompts/classify-pr.prompt.yml @@ -0,0 +1,24 @@ +messages: + - role: system + content: | + You classify pull requests for release note categorization. + Respond with exactly one word: bug, enhancement, or documentation. + + - bug: corrects wrong behavior, broken defaults, incorrect error codes, + retry/backoff defects, auth handling bugs, compatibility regressions. + Test-only changes that fix assertions for previously-wrong behavior + count as bug. + - enhancement: new API coverage, new SDK features, new configuration + options, new test coverage, generator/tooling improvements. + If only generated files changed with no bug claim, default to + enhancement. + - documentation: README, CONTRIBUTING, SECURITY, or other docs-only + changes with no runtime behavior change. SDK README updates that + accompany code changes don't count — label the code change. + + When a PR mixes categories: bug > enhancement > documentation. + Prefer diff evidence over the PR title. +model: openai/gpt-4o-mini +modelParameters: + maxCompletionTokens: 10 + temperature: 0 diff --git a/.github/prompts/detect-breaking.prompt.yml b/.github/prompts/detect-breaking.prompt.yml new file mode 100644 index 0000000..41252c6 --- /dev/null +++ b/.github/prompts/detect-breaking.prompt.yml @@ -0,0 +1,36 @@ +messages: + - role: system + content: | + You analyze Go library diffs for breaking changes to the public API. + A breaking change is: + - Removal or rename of an exported type, function, method, or constant + - Change to an exported function or method signature (parameters, return types) + - Removal of a package + - Breaking an interface contract (adding methods to an exported interface) + - Removal of exported struct fields + + NOT breaking: adding new exported types/functions/methods/constants, + adding new packages, internal refactors, test changes, documentation, + adding new struct fields, changes to unexported identifiers. + + Respond with a JSON object: + {"breaking": true/false, "items": ["description of each breaking change"]} +model: openai/gpt-4o-mini +responseFormat: json_schema +jsonSchema: |- + { + "name": "breaking_analysis", + "strict": true, + "schema": { + "type": "object", + "properties": { + "breaking": { "type": "boolean" }, + "items": { "type": "array", "items": { "type": "string" } } + }, + "required": ["breaking", "items"], + "additionalProperties": false + } + } +modelParameters: + maxCompletionTokens: 500 + temperature: 0 diff --git a/.github/workflows/ai-labeler.yml b/.github/workflows/ai-labeler.yml new file mode 100644 index 0000000..de1c201 --- /dev/null +++ b/.github/workflows/ai-labeler.yml @@ -0,0 +1,224 @@ +name: Classify PR + +on: + pull_request_target: + types: [opened, synchronize, reopened] + +concurrency: + group: classify-pr-${{ github.event.pull_request.number }} + cancel-in-progress: true + +permissions: + contents: read + issues: write + models: read + pull-requests: write + +jobs: + classify: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + + - name: Build prompt + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + PR=${{ github.event.pull_request.number }} + gh pr diff "$PR" > /tmp/pr.diff + gh pr view "$PR" --json title --jq .title > /tmp/pr-title.txt + gh pr view "$PR" --json body --jq '.body // ""' > /tmp/pr-body.txt + + # Compose user message + { + printf 'PR #%s: %s\n' "$PR" "$(cat /tmp/pr-title.txt)" + echo "" + cat /tmp/pr-body.txt + echo "" + echo "Diff (truncated):" + head -c 100000 /tmp/pr.diff + } > /tmp/user-message.txt + + # Build full prompt YAML: splice user message into the messages array + python3 -c " + with open('.github/prompts/classify-pr.prompt.yml') as f: + lines = f.readlines() + with open('/tmp/user-message.txt') as f: + user_msg = f.read() + + insert_at = len(lines) + for i, line in enumerate(lines): + if i == 0: + continue + if line.strip() and not line[0].isspace(): + insert_at = i + break + + entry = [' - role: user\n', ' content: |\n'] + for ln in user_msg.splitlines(): + entry.append(' ' + ln + '\n') + + lines[insert_at:insert_at] = entry + with open('/tmp/prompt.yml', 'w') as f: + f.writelines(lines) + + try: + import yaml + doc = yaml.safe_load(open('/tmp/prompt.yml')) + assert doc['messages'][-1]['role'] == 'user', 'prompt splice failed' + except ImportError: + pass + " + + - name: Classify + id: classify + uses: actions/ai-inference@e09e65981758de8b2fdab13c2bfb7c7d5493b0b6 # v2.0.7 + with: + prompt-file: /tmp/prompt.yml + + - name: Apply label + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + LABEL=$(echo "${{ steps.classify.outputs.response }}" | tr -d '[:space:]' | tr '[:upper:]' '[:lower:]') + case "$LABEL" in + bug|enhancement|documentation) ;; + *) echo "Unexpected: $LABEL — skipping"; exit 0 ;; + esac + PR=${{ github.event.pull_request.number }} + CURRENT=$(gh pr view "$PR" --json labels --jq '.labels[].name') + for L in bug enhancement documentation; do + if [ "$L" != "$LABEL" ] && echo "$CURRENT" | grep -qx "$L"; then + gh pr edit "$PR" --remove-label "$L" 2>/dev/null || true + fi + done + if ! echo "$CURRENT" | grep -qx "$LABEL"; then + gh pr edit "$PR" --add-label "$LABEL" 2>/dev/null || true + fi + + breaking: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + + - name: Build prompt + id: api-diff + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + PR=${{ github.event.pull_request.number }} + gh pr diff "$PR" > /tmp/full.diff + + # Filter diff to exported Go library files (not tests, seed, or non-package dirs) + python3 -c " + import sys, re + diff = open('/tmp/full.diff').read() + dir_exclude = ('seed/', 'internal/', 'actions/', 'prompts/', 'skills/') + sections = re.split(r'(?=^diff --git)', diff, flags=re.MULTILINE) + for s in sections: + m = re.match(r'diff --git a/(\S+)', s) + if m: + path = m.group(1) + if path.endswith('.go') and not path.endswith('_test.go') and not any(path.startswith(d) for d in dir_exclude): + sys.stdout.write(s) + " > /tmp/api.diff + + if [ ! -s /tmp/api.diff ]; then + echo "skip=true" >> "$GITHUB_OUTPUT" + else + TITLE=$(gh pr view "$PR" --json title --jq .title) + + { + printf 'PR #%s: %s\n' "$PR" "$TITLE" + echo "" + echo "Diff of exported Go library files:" + head -c 100000 /tmp/api.diff + } > /tmp/user-message.txt + + python3 -c " + with open('.github/prompts/detect-breaking.prompt.yml') as f: + lines = f.readlines() + with open('/tmp/user-message.txt') as f: + user_msg = f.read() + + insert_at = len(lines) + for i, line in enumerate(lines): + if i == 0: + continue + if line.strip() and not line[0].isspace(): + insert_at = i + break + + entry = [' - role: user\n', ' content: |\n'] + for ln in user_msg.splitlines(): + entry.append(' ' + ln + '\n') + + lines[insert_at:insert_at] = entry + with open('/tmp/prompt.yml', 'w') as f: + f.writelines(lines) + + try: + import yaml + doc = yaml.safe_load(open('/tmp/prompt.yml')) + assert doc['messages'][-1]['role'] == 'user', 'prompt splice failed' + except ImportError: + pass + " + echo "skip=false" >> "$GITHUB_OUTPUT" + fi + + - name: Detect breaking changes + if: steps.api-diff.outputs.skip != 'true' + id: detect + uses: actions/ai-inference@e09e65981758de8b2fdab13c2bfb7c7d5493b0b6 # v2.0.7 + with: + prompt-file: /tmp/prompt.yml + + - name: Apply breaking label + if: steps.api-diff.outputs.skip != 'true' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + RESPONSE_FILE="${{ steps.detect.outputs.response-file }}" + if [ -z "$RESPONSE_FILE" ] || [ ! -f "$RESPONSE_FILE" ]; then + echo "::warning::Model response file is missing; skipping breaking label." + exit 0 + fi + if ! jq empty "$RESPONSE_FILE" 2>/dev/null; then + echo "::warning::Model response is not valid JSON; skipping breaking label." + { + echo "## Breaking change detection failed" + echo "Model returned invalid JSON. Breaking label was **not** applied." + if [ -s "$RESPONSE_FILE" ]; then + echo '```' + cat "$RESPONSE_FILE" + echo '```' + fi + } >> "$GITHUB_STEP_SUMMARY" + exit 0 + fi + BREAKING=$(jq -r '.breaking' "$RESPONSE_FILE") + PR=${{ github.event.pull_request.number }} + + if [ "$BREAKING" = "true" ]; then + ITEMS=$(jq -r '.items[]' "$RESPONSE_FILE" | sed 's/^/- /') + gh label create breaking --color "B60205" 2>/dev/null || true + gh pr edit "$PR" --add-label "breaking" + + { + echo "**Potential breaking changes detected:**" + echo "" + echo "$ITEMS" + echo "" + echo "_Review carefully before merging. Consider a major version bump._" + } > /tmp/breaking-comment.md + + EXISTING=$(gh pr view "$PR" --json comments --jq '.comments[] | select(.body | startswith("**Potential breaking")) | .id' | head -1) + if [ -n "$EXISTING" ]; then + gh api graphql -f query="mutation { updateIssueComment(input: {id: \"$EXISTING\", body: $(jq -Rs . /tmp/breaking-comment.md)}) { issueComment { id } } }" + else + gh pr comment "$PR" --body-file /tmp/breaking-comment.md + fi + else + gh pr edit "$PR" --remove-label "breaking" 2>/dev/null || true + fi From 69df84c6da81b3025c578d3a5b75c43ef3a9cb7d Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 4 Mar 2026 19:01:18 -0800 Subject: [PATCH 3/5] Add path-based PR labeling Labels PRs by changed paths: ci, tests, docs, deps, skills, seed, actions, prompts. Feeds into release.yml changelog categories. --- .github/labeler.yml | 24 ++++++++++++++++++++++++ .github/workflows/labeler.yml | 17 +++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 .github/labeler.yml create mode 100644 .github/workflows/labeler.yml diff --git a/.github/labeler.yml b/.github/labeler.yml new file mode 100644 index 0000000..14d98ce --- /dev/null +++ b/.github/labeler.yml @@ -0,0 +1,24 @@ +ci: + - changed-files: + - any-glob-to-any-file: ['.github/**'] +tests: + - changed-files: + - any-glob-to-any-file: ['**/*_test.go'] +docs: + - changed-files: + - any-glob-to-any-file: ['*.md', 'docs/**'] +deps: + - changed-files: + - any-glob-to-any-file: ['go.mod', 'go.sum'] +skills: + - changed-files: + - any-glob-to-any-file: ['skills/**'] +seed: + - changed-files: + - any-glob-to-any-file: ['seed/**'] +actions: + - changed-files: + - any-glob-to-any-file: ['actions/**'] +prompts: + - changed-files: + - any-glob-to-any-file: ['.github/prompts/**', 'prompts/**'] diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml new file mode 100644 index 0000000..955c4d8 --- /dev/null +++ b/.github/workflows/labeler.yml @@ -0,0 +1,17 @@ +name: Label PRs + +on: + pull_request_target: + types: [opened, synchronize, reopened] + +permissions: + contents: read + pull-requests: write + +jobs: + label: + runs-on: ubuntu-latest + steps: + - uses: actions/labeler@634933edcd8ababfe52f92936142cc22ac488b1b # v6.0.1 + with: + sync-labels: true From 08ab3bba7e17f8f715008b3c47d18eb21a6ef9ff Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 4 Mar 2026 23:59:59 -0800 Subject: [PATCH 4/5] Add check-toolchain, test-coverage, and coverage to seed Makefile check-toolchain detects PATH go vs GOROOT go mismatches (mise environments); wired as prerequisite to build and test. test-coverage generates coverage.out and coverage.html. coverage alias auto-opens the report in a browser. Document all three in MAKEFILE-CONVENTION.md "Optional (recommended)" table. --- MAKEFILE-CONVENTION.md | 3 +++ seed/Makefile | 38 ++++++++++++++++++++++++++++++-------- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/MAKEFILE-CONVENTION.md b/MAKEFILE-CONVENTION.md index f654121..1cb2077 100644 --- a/MAKEFILE-CONVENTION.md +++ b/MAKEFILE-CONVENTION.md @@ -31,6 +31,9 @@ Standard Makefile targets for 37signals Go CLIs and libraries. The seed template | `bench` | `go test -bench=. -benchmem ./...` — benchmarks. | | `bench-cpu` | Benchmarks with CPU profile output. | | `bench-mem` | Benchmarks with memory profile output. | +| `check-toolchain` | Guard against Go toolchain mismatch (PATH go vs GOROOT go). Wired as prereq to `build` and `test`. | +| `test-coverage` | `go test -coverprofile=coverage.out` + generate `coverage.html`. | +| `coverage` | Alias for `test-coverage` that auto-opens the report in a browser. | | `check-all` | Full CI suite: fmt-check + vet + lint + test-race + test-e2e + bench. | ## Composition Rules diff --git a/seed/Makefile b/seed/Makefile index 760046e..2b420c0 100644 --- a/seed/Makefile +++ b/seed/Makefile @@ -15,15 +15,28 @@ LEGACY_PATTERN ?= .DEFAULT_GOAL := check -.PHONY: check build test test-race test-e2e vet lint fmt fmt-check bench bench-cpu bench-mem \ +.PHONY: check check-toolchain build test test-race test-e2e vet lint fmt fmt-check bench bench-cpu bench-mem \ check-all clean tidy tidy-check check-naming replace-check check-surface check-surface-diff \ check-surface-compat vuln secrets security provenance-check release-check release \ - sync-skills sync-skills-remote collect-profile build-pgo + sync-skills sync-skills-remote collect-profile build-pgo test-coverage coverage # Default target: fast checks suitable for pre-commit / inner-loop dev. check: fmt-check vet test test-e2e tidy-check -build: +# Guard against Go toolchain mismatch (mise environment) +check-toolchain: + @GOV=$$(go version | awk '{print $$3}'); \ + ROOT=$$(go env GOROOT); \ + ROOTV=$$($$ROOT/bin/go version | awk '{print $$3}'); \ + if [ "$$GOV" != "$$ROOTV" ]; then \ + echo "ERROR: Go toolchain mismatch"; \ + echo " PATH go: $$GOV ($$(which go))"; \ + echo " GOROOT go: $$ROOTV ($$ROOT/bin/go)"; \ + echo "Fix: eval \"\$$(mise hook-env)\" && make "; \ + exit 1; \ + fi + +build: check-toolchain go build -o $(BUILD_DIR)/$(BINARY_NAME) ./cmd/$(BINARY_NAME) # Build with PGO optimization (requires default.pgo) @@ -37,10 +50,10 @@ build-pgo: go build $(PGO_FLAGS) -o $(BUILD_DIR)/$(BINARY_NAME) ./cmd/$(BINARY_NAME); \ fi -test: +test: check-toolchain go test ./... -test-race: +test-race: check-toolchain go test -race ./... test-e2e: @@ -58,13 +71,22 @@ fmt: fmt-check: @test -z "$$(gofmt -l .)" || (echo "Run 'make fmt' to fix formatting" && gofmt -l . && exit 1) -bench: +# Run tests with coverage +test-coverage: check-toolchain + go test -v -coverprofile=coverage.out ./... + go tool cover -html=coverage.out -o coverage.html + +# Coverage with browser open +coverage: test-coverage + @command -v open >/dev/null 2>&1 && open coverage.html || true + +bench: check-toolchain go test -bench=. -benchmem ./... -bench-cpu: +bench-cpu: check-toolchain go test -bench=. -cpuprofile=cpu.prof ./... -bench-mem: +bench-mem: check-toolchain go test -bench=. -memprofile=mem.prof ./... tidy: From 6c2dc3cdcacf83ad137b5d2e0b5468ddb5346340 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Thu, 5 Mar 2026 00:00:05 -0800 Subject: [PATCH 5/5] Add editor package for $EDITOR integration Open(initialContent) launches $EDITOR (default vi) with a temp file, waits for close, returns the edited text. Supports editors with arguments (e.g. EDITOR="code --wait") via strings.Fields splitting. Guards against whitespace-only $EDITOR and empty results. --- AGENTS.md | 1 + editor/editor.go | 60 ++++++++++++++++++++++++++++++++++ editor/editor_test.go | 75 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+) create mode 100644 editor/editor.go create mode 100644 editor/editor_test.go diff --git a/AGENTS.md b/AGENTS.md index 5b1e855..1ce9894 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,6 +33,7 @@ All packages import from `github.com/basecamp/cli/`. | `oauthcallback` | `WaitForCallback()` starts local server, returns authorization code | | `profile` | Named profiles (`--profile`, `APP_PROFILE`), base URL + app-specific settings | | `surface` | `Snapshot()` walks Cobra tree; `Diff()` detects breaking removals | +| `editor` | `Open(initialContent)` launches `$EDITOR`, returns edited text | ## Testing diff --git a/editor/editor.go b/editor/editor.go new file mode 100644 index 0000000..b49ed47 --- /dev/null +++ b/editor/editor.go @@ -0,0 +1,60 @@ +// Package editor provides $EDITOR integration for composing content. +package editor + +import ( + "fmt" + "os" + "os/exec" + "strings" +) + +// Open launches $EDITOR with initialContent and returns the edited text. +// Falls back to vi if $EDITOR is not set. +// Returns an error if the editor exits non-zero or the result is empty. +func Open(initialContent string) (string, error) { + editorCmd := strings.TrimSpace(os.Getenv("EDITOR")) + if editorCmd == "" { + editorCmd = "vi" + } + + tmp, err := os.CreateTemp("", "cli-edit-*.md") + if err != nil { + return "", fmt.Errorf("creating temp file: %w", err) + } + defer func() { _ = os.Remove(tmp.Name()) }() + + if initialContent != "" { + _, writeErr := tmp.WriteString(initialContent) + if writeErr != nil { + _ = tmp.Close() + return "", fmt.Errorf("writing initial content: %w", writeErr) + } + } + closeErr := tmp.Close() + if closeErr != nil { + return "", fmt.Errorf("closing temp file: %w", closeErr) + } + + // Use sh -c to handle quoted arguments and paths with spaces in $EDITOR + cmd := exec.Command("sh", "-c", editorCmd+` "$1"`, "_", tmp.Name()) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + runErr := cmd.Run() + if runErr != nil { + return "", fmt.Errorf("editor exited with error: %w", runErr) + } + + data, err := os.ReadFile(tmp.Name()) + if err != nil { + return "", fmt.Errorf("reading edited file: %w", err) + } + + result := strings.TrimSpace(string(data)) + if result == "" { + return "", fmt.Errorf("empty content — aborting") + } + + return result, nil +} diff --git a/editor/editor_test.go b/editor/editor_test.go new file mode 100644 index 0000000..7ebe18a --- /dev/null +++ b/editor/editor_test.go @@ -0,0 +1,75 @@ +package editor + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestOpenReturnsContent(t *testing.T) { + script := filepath.Join(t.TempDir(), "noop-editor") + if err := os.WriteFile(script, []byte("#!/bin/sh\n# no-op\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("EDITOR", script) + + got, err := Open("hello world") + if err != nil { + t.Fatalf("Open() error = %v", err) + } + if got != "hello world" { + t.Errorf("Open() = %q, want %q", got, "hello world") + } +} + +func TestOpenEmptyResultErrors(t *testing.T) { + script := filepath.Join(t.TempDir(), "empty-editor") + if err := os.WriteFile(script, []byte("#!/bin/sh\n: > \"$1\"\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("EDITOR", script) + + _, err := Open("initial") + if err == nil { + t.Error("Open() should error on empty result") + } +} + +func TestOpenEditorNotFound(t *testing.T) { + t.Setenv("EDITOR", "/nonexistent/editor") + _, err := Open("") + if err == nil { + t.Error("Open() should error when editor not found") + } +} + +func TestOpenEditorWhitespaceFallsBack(t *testing.T) { + // Whitespace-only EDITOR should be treated as unset (fall back to vi), + // not panic or return a whitespace-specific error. + t.Setenv("EDITOR", " ") + t.Setenv("PATH", "/nonexistent") + _, err := Open("") + if err == nil { + t.Fatal("expected error when vi is not found") + } + if strings.Contains(err.Error(), "whitespace") { + t.Error("whitespace-only EDITOR should fall back to vi, not error about whitespace") + } +} + +func TestOpenEditorWithArgs(t *testing.T) { + script := filepath.Join(t.TempDir(), "append-editor") + if err := os.WriteFile(script, []byte("#!/bin/sh\necho \" edited\" >> \"$2\"\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("EDITOR", script+" --flag") + + got, err := Open("hello") + if err != nil { + t.Fatalf("Open() error = %v", err) + } + if !strings.Contains(got, "edited") { + t.Errorf("Open() = %q, want content containing 'edited'", got) + } +}