From 1f7423f90288703d2cd110c1eb6dac7f134d4a96 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 00:40:11 +0200 Subject: [PATCH 01/28] [Config] Save the API key only under HOME, via a private temp file login wrote the key through config.toml.tmp, a fixed name: a 0644 file left there made the saved key world-readable, a symlink there sent the key to its target, and 7 of 8 concurrent logins failed. Without HOME, the key was saved to and read from ./.config/serpapi in the current directory. The temp file is now created with O_EXCL and mode 0600, and with no config directory login says to set HOME instead of saving. Co-Authored-By: Claude Opus 5.5 --- pkg/config/config.go | 63 +++++++++++++----- pkg/config/config_test.go | 122 ++++++++++++++++++++++++++++++++++- pkg/config/config_unix.go | 33 ---------- pkg/config/config_windows.go | 13 ---- 4 files changed, 166 insertions(+), 65 deletions(-) delete mode 100644 pkg/config/config_unix.go delete mode 100644 pkg/config/config_windows.go diff --git a/pkg/config/config.go b/pkg/config/config.go index 8d8c306..14111ec 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -17,21 +17,27 @@ type configFile struct { APIKey string `toml:"api_key"` } -// Dir returns the directory where the CLI stores its config. +// Dir returns the directory where the CLI stores its config, or "" when +// neither the user config directory nor the home directory is known: a +// relative directory would put the key in, or take it from, wherever the +// command runs. func Dir() string { if dir, err := os.UserConfigDir(); err == nil { return filepath.Join(dir, "serpapi") } - home, err := os.UserHomeDir() - if err != nil { - return filepath.Join(".config", "serpapi") + if home, err := os.UserHomeDir(); err == nil { + return filepath.Join(home, ".config", "serpapi") } - return filepath.Join(home, ".config", "serpapi") + return "" } -// Path returns the full path to the config file. +// Path returns the full path to the config file, or "" when Dir is "". func Path() string { - return filepath.Join(Dir(), "config.toml") + dir := Dir() + if dir == "" { + return "" + } + return filepath.Join(dir, "config.toml") } // ErrNoAPIKey is returned by ResolveAPIKey when no source has a key. @@ -43,7 +49,11 @@ var ErrNoAPIKey = &clierrors.UsageError{ // is none. A file that can't be read or parsed is an error naming the file // and line; it never quotes the file, which holds the key. func LoadAPIKey() (string, error) { - data, err := os.ReadFile(Path()) + path := Path() + if path == "" { + return "", nil + } + data, err := os.ReadFile(path) if errors.Is(err, fs.ErrNotExist) { return "", nil } @@ -64,10 +74,15 @@ func LoadAPIKey() (string, error) { return strings.TrimSpace(cfg.APIKey), nil } -// SaveConfig writes the API key to the config file atomically. +// SaveConfig writes the API key to the config file atomically, through a +// temporary file that only this call creates, so a file left or planted next +// to the config can't widen its permissions or receive the key. func SaveConfig(apiKey string) error { dir := Dir() - if err := mkdirSecure(dir); err != nil { + if dir == "" { + return &clierrors.UsageError{Message: "No directory to save the API key in: HOME is not set; set HOME, or pass the key with SERPAPI_KEY instead of saving it"} + } + if err := os.MkdirAll(dir, 0o700); err != nil { return &clierrors.UsageError{Message: "Failed to create config dir: " + err.Error()} } @@ -77,23 +92,37 @@ func SaveConfig(apiKey string) error { return &clierrors.UsageError{Message: err.Error()} } - finalPath := Path() - tmpPath := finalPath + ".tmp" - - if err := writeFileSecure(tmpPath, content); err != nil { - return &clierrors.UsageError{Message: err.Error()} + // CreateTemp opens a new file with O_EXCL and mode 0600. + tmp, err := os.CreateTemp(dir, "config.toml.*.tmp") + if err != nil { + return &clierrors.UsageError{Message: "Failed to save config: " + err.Error()} + } + if err := writeSynced(tmp, content); err != nil { + os.Remove(tmp.Name()) + return &clierrors.UsageError{Message: "Failed to save config: " + err.Error()} } // os.Rename replaces an existing file on Windows too (MoveFileEx with // MOVEFILE_REPLACE_EXISTING), so the old key stays until the new one lands. - if err := os.Rename(tmpPath, finalPath); err != nil { - os.Remove(tmpPath) + if err := os.Rename(tmp.Name(), Path()); err != nil { + os.Remove(tmp.Name()) return &clierrors.UsageError{Message: "Failed to save config: " + err.Error()} } return nil } +func writeSynced(f *os.File, data []byte) error { + _, err := f.Write(data) + if err == nil { + err = f.Sync() + } + if closeErr := f.Close(); err == nil { + err = closeErr + } + return err +} + func tomlMarshal(cfg configFile) ([]byte, error) { buf := new(bytes.Buffer) enc := toml.NewEncoder(buf) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 664163f..62e2a7c 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -5,7 +5,9 @@ import ( "os" "path/filepath" "runtime" + "slices" "strings" + "sync" "testing" clierrors "github.com/serpapi/serpapi-cli/pkg/errors" @@ -68,7 +70,123 @@ func TestSaveConfigRoundTripAndOverwrite(t *testing.T) { if runtime.GOOS != "windows" && info.Mode().Perm() != 0o600 { t.Errorf("config mode = %v, want 0600", info.Mode().Perm()) } - if _, err := os.Stat(Path() + ".tmp"); !os.IsNotExist(err) { - t.Errorf("temporary file left behind: %v", err) + wantNoTempFiles(t) +} + +func wantNoTempFiles(t *testing.T) { + t.Helper() + left, err := filepath.Glob(filepath.Join(Dir(), "config.toml*tmp")) + if err != nil { + t.Fatal(err) + } + if len(left) > 0 { + t.Errorf("temporary files left behind: %v", left) + } +} + +// A file someone else left or planted at the old fixed temporary name must +// not decide the saved key's permissions or where the key is written. +func TestSaveConfigIgnoresFilesAtTheOldTempName(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("file modes and symlinks differ on Windows") + } + isolate(t) + if err := os.MkdirAll(Dir(), 0o700); err != nil { + t.Fatal(err) + } + stale := Path() + ".tmp" + if err := os.WriteFile(stale, nil, 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chmod(stale, 0o644); err != nil { + t.Fatal(err) + } + + if err := SaveConfig("stale-key"); err != nil { + t.Fatal(err) + } + if info, err := os.Stat(Path()); err != nil || info.Mode().Perm() != 0o600 { + t.Fatalf("config = %v, %v; want mode 0600 despite a 0644 %s", info, err, filepath.Base(stale)) + } + + victim := filepath.Join(t.TempDir(), "victim") + if err := os.WriteFile(victim, []byte("untouched\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Remove(stale); err != nil { + t.Fatal(err) + } + if err := os.Symlink(victim, stale); err != nil { + t.Fatal(err) + } + + if err := SaveConfig("symlink-key"); err != nil { + t.Fatal(err) + } + if b, _ := os.ReadFile(victim); string(b) != "untouched\n" { + t.Errorf("symlink target = %q, want it untouched", b) + } + if info, err := os.Lstat(Path()); err != nil || !info.Mode().IsRegular() { + t.Errorf("config = %v, %v; want a regular file", info, err) + } + if got, err := ResolveAPIKey(""); err != nil || got != "symlink-key" { + t.Errorf("ResolveAPIKey = %q, %v; want symlink-key", got, err) + } +} + +func TestSaveConfigConcurrently(t *testing.T) { + isolate(t) + keys := []string{"key-0", "key-1", "key-2", "key-3", "key-4", "key-5", "key-6", "key-7"} + errs := make(chan error, len(keys)) + var wg sync.WaitGroup + for _, key := range keys { + wg.Add(1) + go func() { + defer wg.Done() + errs <- SaveConfig(key) + }() + } + wg.Wait() + close(errs) + for err := range errs { + if err != nil { + t.Errorf("SaveConfig: %v", err) + } + } + got, err := ResolveAPIKey("") + if err != nil || !slices.Contains(keys, got) { + t.Errorf("ResolveAPIKey = %q, %v; want one of the saved keys", got, err) + } + wantNoTempFiles(t) +} + +// Without HOME there is no config directory, and a relative one would put the +// key in, or take it from, whatever directory the command runs in. +func TestNoConfigDirWithoutHome(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows finds the config directory through other variables") + } + t.Setenv("HOME", "") + t.Setenv("XDG_CONFIG_HOME", "") + cwd := t.TempDir() + t.Chdir(cwd) + planted := filepath.Join(cwd, ".config", "serpapi", "config.toml") + if err := os.MkdirAll(filepath.Dir(planted), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(planted, []byte(`api_key = "planted-key"`+"\n"), 0o600); err != nil { + t.Fatal(err) + } + + if key, err := ResolveAPIKey(""); err != ErrNoAPIKey { + t.Errorf("ResolveAPIKey = %q, %v; want ErrNoAPIKey, not the key in the current directory", key, err) + } + err := SaveConfig("new-key") + var usageErr *clierrors.UsageError + if !errors.As(err, &usageErr) || !strings.Contains(usageErr.Message, "HOME") { + t.Errorf("SaveConfig = %v; want a UsageError naming HOME", err) + } + if b, _ := os.ReadFile(planted); !strings.Contains(string(b), "planted-key") { + t.Errorf("the planted file was overwritten: %q", b) } } diff --git a/pkg/config/config_unix.go b/pkg/config/config_unix.go deleted file mode 100644 index e7c72ef..0000000 --- a/pkg/config/config_unix.go +++ /dev/null @@ -1,33 +0,0 @@ -//go:build !windows - -package config - -import ( - "fmt" - "os" -) - -func mkdirSecure(path string) error { - return os.MkdirAll(path, 0700) -} - -func writeFileSecure(path string, data []byte) error { - f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) - if err != nil { - return err - } - n, err := f.Write(data) - if err != nil { - f.Close() - return err - } - if n != len(data) { - f.Close() - return fmt.Errorf("incomplete write: wrote %d of %d bytes", n, len(data)) - } - if err := f.Sync(); err != nil { - f.Close() - return err - } - return f.Close() -} diff --git a/pkg/config/config_windows.go b/pkg/config/config_windows.go deleted file mode 100644 index 2be07cc..0000000 --- a/pkg/config/config_windows.go +++ /dev/null @@ -1,13 +0,0 @@ -//go:build windows - -package config - -import "os" - -func mkdirSecure(path string) error { - return os.MkdirAll(path, 0700) -} - -func writeFileSecure(path string, data []byte) error { - return os.WriteFile(path, data, 0600) -} From e0af7a7231662c700c4b635d9cd14c508f99bbd0 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 00:41:03 +0200 Subject: [PATCH 02/28] [Search] Run --jq on the pages fetched before Ctrl-C in --all-pages Ctrl-C or SIGTERM during --all-pages --jq printed nothing: --jq ran with the canceled context, so it stopped before its first step, and the pages already fetched, each a counted search, were lost. README and --help promise them. --jq now runs on them for at most the 3 s grace period after the signal, then the run exits 130 as without --jq. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/edge_test.go | 52 ++++++++++++++++++++++++++++++++++++++++++++ pkg/cmd/root.go | 10 ++++----- pkg/cmd/search.go | 21 +++++++++++++----- 3 files changed, 72 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go index a344ca9..3e4e0fb 100644 --- a/pkg/cmd/edge_test.go +++ b/pkg/cmd/edge_test.go @@ -411,6 +411,58 @@ func TestInterruptDuringAllPagesPrintsFetchedPages(t *testing.T) { } } +// interruptAtPage3 serves pages 1 and 2 and cancels ctx once page 3 is asked +// for, as Ctrl-C during a slow page would. +func interruptAtPage3(t *testing.T, cancel context.CancelFunc) { + t.Helper() + reached := make(chan struct{}) + fakeAPI(t, func(w http.ResponseWriter, r *http.Request) { + if n := pageNumber(r); n < 3 { + fmt.Fprint(w, page(n, true)) + return + } + close(reached) + <-r.Context().Done() + }) + t.Setenv("SERPAPI_KEY", "envkey") + go func() { + <-reached + cancel() + }() +} + +// The pages fetched before Ctrl-C were billed, so --jq still runs on them. +func TestInterruptDuringAllPagesWithJQPrintsFetchedPages(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + interruptAtPage3(t, cancel) + + res := executeContext(t, ctx, "", searchWith("--all-pages", "--jq", ".organic_results | length")...) + + wantError(t, res, 130, "interrupted", "Stopped while fetching page 3; pages 1-2 were printed") + if res.stdout != "2\n" { + t.Errorf("stdout = %q, want --jq's result on the 2 fetched pages", res.stdout) + } +} + +// --jq on the fetched pages gets interruptGrace after the signal, no more. +func TestInterruptDuringAllPagesBoundsAnEndlessJQ(t *testing.T) { + grace := interruptGrace + interruptGrace = 100 * time.Millisecond + t.Cleanup(func() { interruptGrace = grace }) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + interruptAtPage3(t, cancel) + + start := time.Now() + res := executeContext(t, ctx, "", searchWith("--all-pages", "--jq", "reduce range(1e12) as $i (0; . + 1)")...) + + wantError(t, res, 130, "interrupted", "Stopped while running --jq") + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Errorf("took %v, want --jq stopped about interruptGrace after the signal", elapsed) + } +} + func TestInterruptBeforeTheResponse(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 373697b..c174c4e 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -318,12 +318,13 @@ func handleOutput(cmd *cobra.Command, raw []byte, next func(id string) string) e if err := dec.Decode(&v); err != nil { return &clierrors.APIError{Message: "Failed to parse response: " + err.Error()} } - return printJQ(cmd, v) + return printJQ(cmd.Context(), cmd, v) } -// printJQ runs --jq on v and prints each result as it is produced. A filter -// that prints only null gets a warning listing the keys v has. -func printJQ(cmd *cobra.Command, v any) error { +// printJQ runs --jq on v until ctx ends, printing each result as it is +// produced. A filter that prints only null gets a warning listing the keys v +// has. +func printJQ(ctx context.Context, cmd *cobra.Command, v any) error { code, err := jq.Compile(jqFlag) if err != nil { return &clierrors.UsageError{Message: err.Error()} @@ -333,7 +334,6 @@ func printJQ(cmd *cobra.Command, v any) error { bw := bufio.NewWriter(w) var writeErr error printed, nulls := 0, 0 - ctx := cmd.Context() if ctx == nil { ctx = context.Background() } diff --git a/pkg/cmd/search.go b/pkg/cmd/search.go index 6514da6..a15beae 100644 --- a/pkg/cmd/search.go +++ b/pkg/cmd/search.go @@ -3,6 +3,7 @@ package cmd import ( "bufio" "bytes" + "context" "encoding/json" "errors" "fmt" @@ -290,7 +291,15 @@ func searchAllPages(cmd *cobra.Command, client *api.Client, first map[string]str warnNothingMatched(cmd) } - if err := printMerged(cmd, merged); err != nil { + // Pages fetched before Ctrl-C were billed, so --jq still runs on them, for + // at most interruptGrace; the process exits about then anyway. + printCtx := ctx + if pageErr != nil && ctx.Err() != nil { + var cancel context.CancelFunc + printCtx, cancel = context.WithTimeout(context.WithoutCancel(ctx), interruptGrace) + defer cancel() + } + if err := printMerged(printCtx, cmd, merged); err != nil { return err } if pageErr == nil { @@ -310,12 +319,12 @@ func searchAllPages(cmd *cobra.Command, client *api.Client, first map[string]str return annotatePageError(pageErr, pages) } -// printMerged prints the merged pages. --jq runs on them directly, and a -// pipe gets them encoded as they are written, so no encoded copy of every -// page is ever held in memory. -func printMerged(cmd *cobra.Command, merged map[string]any) error { +// printMerged prints the merged pages. --jq runs on them directly until ctx +// ends, and a pipe gets them encoded as they are written, so no encoded copy +// of every page is ever held in memory. +func printMerged(ctx context.Context, cmd *cobra.Command, merged map[string]any) error { if jqFlag != "" { - return printJQ(cmd, merged) + return printJQ(ctx, cmd, merged) } w := cmd.OutOrStdout() if output.IsTerminal(w) { From 48e4d287def3669741b33e8f171ddd3d0e24741e Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 00:42:32 +0200 Subject: [PATCH 03/28] [Hints] Don't promise a cache hit when the search used no_cache A large result without a search id, such as merged --all-pages pages, suggested repeating the command with --jq because "the same search within an hour comes from the cache". With no_cache=true the repeat is a new, counted search, for every page with --all-pages; an agent following the hint paid again. The hint now says so. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/edge_test.go | 20 +++++++++++++------- pkg/cmd/hint.go | 22 ++++++++++++++++------ pkg/cmd/hint_test.go | 2 +- pkg/cmd/search.go | 11 ++++++----- 4 files changed, 36 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go index 3e4e0fb..d9c9ebb 100644 --- a/pkg/cmd/edge_test.go +++ b/pkg/cmd/edge_test.go @@ -64,6 +64,13 @@ func searchWith(args ...string) []string { return append(append([]string{}, sear func edgeCases() []edgeCase { invalidKey := `{"error":"Invalid API key. Your API key should be here: https://serpapi.com/manage-api-key"}` pages := func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, page(pageNumber(r), pageNumber(r) < 3)) } + bigPages := func(w http.ResponseWriter, r *http.Request) { + next := "" + if pageNumber(r) == 1 { + next = `"serpapi_pagination":{"next":"https://serpapi.com/search.json?engine=google&q=coffee&start=10"},` + } + fmt.Fprint(w, bigResult(next+`"inline_images":[{"position":1}],`, 700)) + } return []edgeCase{ {name: "argv/no parameters", args: []string{"search"}, exit: 2, code: "usage_error", msg: "No search parameters given"}, {name: "argv/q and bare words", args: searchWith("tea"), exit: 2, code: "usage_error", msg: "Got both q=... and bare words"}, @@ -202,17 +209,16 @@ func edgeCases() []edgeCase { warn: "keys by size: organic_results 52 KB, search_metadata 24 B. To print only what you need, read the stored search for free: serpapi archive abc123 --jq '.KEY'"}, {name: "output/large result without an id", args: search, api: respond(bigResult("", 1500)), warn: "repeat the command with --jq '.KEY': the same search within an hour comes from the cache"}, + {name: "output/large no_cache result promises no cache hit", args: searchWith("no_cache=true"), api: respond(bigResult("", 1500)), + warn: "repeat the command with --jq '.KEY'; no_cache=true makes the repeat a new, counted search"}, {name: "output/large result with --jq is quiet", args: searchWith("--jq", ".organic_results | length"), api: respond(bigResult("", 1500)), stdout: "1500\n", quiet: true}, {name: "output/large result with --fields still hints", args: searchWith("--fields", "organic_results"), api: respond(bigResult("", 1500)), warn: "Large result: 52 KB"}, - {name: "output/large merged pages", args: searchWith("--all-pages"), reqs: 2, api: func(w http.ResponseWriter, r *http.Request) { - next := "" - if pageNumber(r) == 1 { - next = `"serpapi_pagination":{"next":"https://serpapi.com/search.json?engine=google&q=coffee&start=10"},` - } - fmt.Fprint(w, bigResult(next+`"inline_images":[{"position":1}],`, 700)) - }, warn: "Large result: 48 KB; top-level keys by size: organic_results 48 KB, inline_images 70 B. To print only what you need, repeat the command"}, + {name: "output/large merged pages", args: searchWith("--all-pages"), reqs: 2, api: bigPages, + warn: "Large result: 48 KB; top-level keys by size: organic_results 48 KB, inline_images 70 B. To print only what you need, repeat the command"}, + {name: "output/large no_cache merged pages", args: searchWith("no_cache=true", "--all-pages"), reqs: 2, api: bigPages, + warn: "no_cache=true makes the repeat a new, counted search for each page"}, {name: "output/large archive", args: []string{"archive", "abc123"}, api: respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500)), warn: "add --jq '.KEY'; reading the archive is free"}, {name: "output/jq printing only null lists the keys", args: searchWith("--jq", ".organic"), api: oneResult, stdout: "\n", diff --git a/pkg/cmd/hint.go b/pkg/cmd/hint.go index 3ca433d..9387235 100644 --- a/pkg/cmd/hint.go +++ b/pkg/cmd/hint.go @@ -112,13 +112,23 @@ func warnRepeated(cmd *cobra.Command, parsed []params.Param) { // archiveHint is the next step for a large stored search. func archiveHint(string) string { return "add --jq '.KEY'; reading the archive is free" } -// searchHint is the next step for a large search result: re-reading the -// stored search is free, and so is the same search within the cache hour. -func searchHint(id string) string { - if id != "" { - return fmt.Sprintf("read the stored search for free: serpapi archive %s --jq '.KEY'", id) +// searchHint returns the next step for a large result of the search sent +// with p: re-reading the stored search is free, and so is the same search +// within the cache hour, unless no_cache asked for a new one. +func searchHint(p map[string]string) func(id string) string { + return func(id string) string { + if id != "" { + return fmt.Sprintf("read the stored search for free: serpapi archive %s --jq '.KEY'", id) + } + if v := p["no_cache"]; v != "" && v != "false" && v != "0" { + hint := "repeat the command with --jq '.KEY'; no_cache=" + v + " makes the repeat a new, counted search" + if allPagesFlag { + hint += " for each page" + } + return hint + } + return "repeat the command with --jq '.KEY': the same search within an hour comes from the cache" } - return "repeat the command with --jq '.KEY': the same search within an hour comes from the cache" } func sizeText(n int) string { diff --git a/pkg/cmd/hint_test.go b/pkg/cmd/hint_test.go index daa1264..28d1314 100644 --- a/pkg/cmd/hint_test.go +++ b/pkg/cmd/hint_test.go @@ -28,7 +28,7 @@ func TestLargeResultHintListsTwelveKeysLargestFirst(t *testing.T) { cmd := &cobra.Command{} cmd.SetOut(&stdout) cmd.SetErr(&stderr) - hintLargeJSON(cmd, raw, searchHint) + hintLargeJSON(cmd, raw, searchHint(nil)) var got struct { Warning struct { diff --git a/pkg/cmd/search.go b/pkg/cmd/search.go index a15beae..e0d5e1b 100644 --- a/pkg/cmd/search.go +++ b/pkg/cmd/search.go @@ -168,7 +168,7 @@ func sendSearch(cmd *cobra.Command, client *api.Client, p map[string]string, raw if restricted && isEmptyObject(result) { warnNothingMatched(cmd) } - return handleOutput(cmd, result, searchHint) + return handleOutput(cmd, result, searchHint(p)) } // explainMissingKey adds to a 401 for a search sent without a key how to give @@ -299,7 +299,7 @@ func searchAllPages(cmd *cobra.Command, client *api.Client, first map[string]str printCtx, cancel = context.WithTimeout(context.WithoutCancel(ctx), interruptGrace) defer cancel() } - if err := printMerged(printCtx, cmd, merged); err != nil { + if err := printMerged(printCtx, cmd, merged, searchHint(first)("")); err != nil { return err } if pageErr == nil { @@ -321,8 +321,9 @@ func searchAllPages(cmd *cobra.Command, client *api.Client, first map[string]str // printMerged prints the merged pages. --jq runs on them directly until ctx // ends, and a pipe gets them encoded as they are written, so no encoded copy -// of every page is ever held in memory. -func printMerged(ctx context.Context, cmd *cobra.Command, merged map[string]any) error { +// of every page is ever held in memory. next is the large-result hint's next +// step. +func printMerged(ctx context.Context, cmd *cobra.Command, merged map[string]any, next string) error { if jqFlag != "" { return printJQ(ctx, cmd, merged) } @@ -343,7 +344,7 @@ func printMerged(ctx context.Context, cmd *cobra.Command, merged map[string]any) return &clierrors.APIError{Message: "Failed to encode result: " + err.Error()} } sortKeySizes(keys) - hintLargeResult(cmd, int(size), keys, searchHint("")) + hintLargeResult(cmd, int(size), keys, next) bw := bufio.NewWriterSize(w, 64<<10) if _, err := output.WriteObject(bw, merged, nil); err != nil { return outputError(err) From 92590fb91b6c9854c28fc65bba6177e6c33c3787 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 00:47:37 +0200 Subject: [PATCH 04/28] [Login] Stop at once on Ctrl-C at a prompt, with echo turned back on Since #17's signal handling, Ctrl-C at login's hidden key prompt waited the 3 s grace period, said "the output may be cut off", and left the terminal not echoing; the [y/N] prompt and a key piped through an open stdin waited too. In a pseudo-terminal, both prompts now exit 130 at once, the hidden one with echo restored, and nothing is saved. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/cmd_test.go | 32 ++++++++++++++++++++++- pkg/cmd/login.go | 62 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index eada884..f06556c 100644 --- a/pkg/cmd/cmd_test.go +++ b/pkg/cmd/cmd_test.go @@ -9,10 +9,12 @@ import ( "net/http" "net/http/httptest" "net/url" + "os" "strconv" "strings" "sync" "testing" + "time" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -102,12 +104,18 @@ func execute(t *testing.T, stdin string, args ...string) result { // executeContext is execute with a context the test can cancel, as Ctrl-C does. func executeContext(t *testing.T, ctx context.Context, stdin string, args ...string) result { + t.Helper() + return executeInput(t, ctx, strings.NewReader(stdin), args...) +} + +// executeInput is executeContext with any stdin, such as a pipe that stays open. +func executeInput(t *testing.T, ctx context.Context, stdin io.Reader, args ...string) result { t.Helper() resetFlags(rootCmd) var stdout, stderr bytes.Buffer rootCmd.SetOut(&stdout) rootCmd.SetErr(&stderr) - rootCmd.SetIn(strings.NewReader(stdin)) + rootCmd.SetIn(stdin) // A nil slice would make cobra parse the test binary's own arguments. rootCmd.SetArgs(append([]string{}, args...)) code := run(ctx) @@ -606,6 +614,28 @@ func TestLoginWithoutKey(t *testing.T) { wantError(t, execute(t, "\n", "login"), 2, "usage_error", "No API key given") } +// An agent's timeout sends SIGTERM to a login still waiting on an open pipe; +// it has to stop then, not when the grace period runs out. +func TestLoginStopsAtOnceWhenInterrupted(t *testing.T) { + fakeAPI(t, nil) + stdin, writer := io.Pipe() + t.Cleanup(func() { writer.Close() }) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + time.AfterFunc(50*time.Millisecond, cancel) + + start := time.Now() + res := executeInput(t, ctx, stdin, "login") + + wantError(t, res, 130, "interrupted", "Stopped while waiting for the API key on stdin; nothing was saved") + if elapsed := time.Since(start); elapsed > time.Second { + t.Errorf("login took %v to stop, want well under the %v grace period", elapsed, interruptGrace) + } + if _, err := os.Stat(config.Path()); !os.IsNotExist(err) { + t.Errorf("config file after an interrupted login: %v", err) + } +} + // login shows the account's email on a terminal, so it must be as safe there // as a search result: a real PTY run printed ESC ] 52 and ESC [ 2J from it. func TestAccountEmailIsSafeOnATerminal(t *testing.T) { diff --git a/pkg/cmd/login.go b/pkg/cmd/login.go index 648c613..ed21d79 100644 --- a/pkg/cmd/login.go +++ b/pkg/cmd/login.go @@ -67,30 +67,55 @@ func runLogin(cmd *cobra.Command, _ []string) error { } // readAPIKey prompts on a terminal and otherwise reads the first line of -// stdin. It returns "" and no error when the user keeps the saved key. +// stdin. It returns "" and no error when the user keeps the saved key. Ctrl-C +// or SIGTERM stops it at once with an interrupted error. func readAPIKey(cmd *cobra.Command) (string, error) { + ctx := cmd.Context() stderr := cmd.ErrOrStderr() var key string if f, ok := cmd.InOrStdin().(*os.File); ok && term.IsTerminal(int(f.Fd())) { if saved, err := config.LoadAPIKey(); err == nil && saved != "" { - if email, err := accountEmail(cmd.Context(), saved); err == nil { + if email, err := accountEmail(ctx, saved); err == nil { fmt.Fprintf(stderr, "Already logged in as %s. Replace the saved key? [y/N] ", email) - line, _ := bufio.NewReader(f).ReadString('\n') + line, err := readInterruptibly(ctx, func() (string, error) { return bufio.NewReader(f).ReadString('\n') }, + "Stopped at the prompt; the saved key is unchanged") + if isInterrupted(err) { + fmt.Fprintln(stderr) + return "", err + } if !strings.EqualFold(strings.TrimSpace(line), "y") { fmt.Fprintln(stderr, "Kept the saved key.") return "", nil } } } + const stopped = "Stopped at the API key prompt; nothing was saved" + if ctx.Err() != nil { + return "", &clierrors.InterruptedError{Message: stopped} + } + // ReadPassword turns echo off until it returns, which it doesn't on + // Ctrl-C, so the state saved here is put back then. + fd := int(f.Fd()) + state, _ := term.GetState(fd) fmt.Fprint(stderr, "SerpApi API key (input hidden): ") - b, err := term.ReadPassword(int(f.Fd())) + b, err := readInterruptibly(ctx, func() ([]byte, error) { return term.ReadPassword(fd) }, stopped) + if isInterrupted(err) && state != nil { + _ = term.Restore(fd, state) + } fmt.Fprintln(stderr) + if isInterrupted(err) { + return "", err + } if err != nil { return "", &clierrors.UsageError{Message: "Failed to read the API key: " + err.Error()} } key = string(b) } else { - line, err := bufio.NewReader(cmd.InOrStdin()).ReadString('\n') + line, err := readInterruptibly(ctx, func() (string, error) { return bufio.NewReader(cmd.InOrStdin()).ReadString('\n') }, + "Stopped while waiting for the API key on stdin; nothing was saved") + if isInterrupted(err) { + return "", err + } if err != nil && !errors.Is(err, io.EOF) { return "", &clierrors.UsageError{Message: "Failed to read the API key: " + err.Error()} } @@ -102,6 +127,33 @@ func readAPIKey(cmd *cobra.Command) (string, error) { return key, nil } +// readInterruptibly returns what read returns, or an interrupted error with +// the stopped message as soon as ctx is canceled. A read from a terminal or a +// pipe can't be canceled itself; it ends with the process. +func readInterruptibly[T any](ctx context.Context, read func() (T, error), stopped string) (T, error) { + type result struct { + v T + err error + } + done := make(chan result, 1) + go func() { + v, err := read() + done <- result{v, err} + }() + select { + case r := <-done: + return r.v, r.err + case <-ctx.Done(): + var zero T + return zero, &clierrors.InterruptedError{Message: stopped} + } +} + +func isInterrupted(err error) bool { + var interrupted *clierrors.InterruptedError + return errors.As(err, &interrupted) +} + func accountEmail(ctx context.Context, apiKey string) (string, error) { raw, err := newClient(apiKey).Account(ctx) if err != nil { From 7ae9a22c4492d7f8f30d0aa1caafb09452e1018d Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 00:48:28 +0200 Subject: [PATCH 05/28] [CLI] Report an unknown help topic as a usage error serpapi help serach printed "Unknown help topic" and the whole usage as plain text on stderr and exited 0, though exit 2 means the command is wrong and stderr carries JSON lines. It now exits 2 with one usage_error line that names the nearest command, or points to serpapi --help when none is close; help with a known topic or none prints as before. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/cmd_test.go | 22 ++++++++++++++++++++++ pkg/cmd/root.go | 19 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index f06556c..df6daad 100644 --- a/pkg/cmd/cmd_test.go +++ b/pkg/cmd/cmd_test.go @@ -712,6 +712,7 @@ func TestHelpAndVersionGoToStdout(t *testing.T) { }{ {[]string{"--help"}, "SERPAPI_KEY"}, {[]string{"search", "--help"}, "--all-pages"}, + {[]string{"help", "search"}, "--all-pages"}, {[]string{"--version"}, "serpapi version "}, } { res := execute(t, "", tt.args...) @@ -720,3 +721,24 @@ func TestHelpAndVersionGoToStdout(t *testing.T) { } } } + +// A help topic that doesn't exist is a wrong command: exit 2 and one JSON +// line, not the usage as plain text with exit 0. +func TestHelpWithAnUnknownTopic(t *testing.T) { + fakeAPI(t, nil) + + for topic, want := range map[string]string{ + "serach": `Unknown help topic "serach"; did you mean search?`, + "nope": `Unknown help topic "nope"; see 'serpapi --help'`, + } { + res := execute(t, "", "help", topic) + + errObj := wantError(t, res, 2, "usage_error", want) + if errObj["message"] != want { + t.Errorf("help %s: message = %q, want %q", topic, errObj["message"], want) + } + if res.stdout != "" { + t.Errorf("help %s: stdout = %q, want empty", topic, res.stdout) + } + } +} diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index c174c4e..446feec 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -171,6 +171,25 @@ func init() { rootCmd.PersistentPreRunE = validateGlobalFlags + // cobra's help command prints the usage and exits 0 for an unknown topic. + rootCmd.SetHelpCommand(&cobra.Command{ + Use: "help [command]", + Short: "Help about any command", + RunE: func(c *cobra.Command, args []string) error { + target, _, err := c.Root().Find(args) + if len(args) > 0 && (err != nil || target == c.Root()) { + msg := fmt.Sprintf("Unknown help topic %q", strings.Join(args, " ")) + if s := c.Root().SuggestionsFor(args[0]); len(s) > 0 { + return &clierrors.UsageError{Message: msg + "; did you mean " + strings.Join(s, " or ") + "?"} + } + return &clierrors.UsageError{Message: msg + "; see 'serpapi --help'"} + } + target.InitDefaultHelpFlag() + target.InitDefaultVersionFlag() + return target.Help() + }, + }) + // Wrap cobra flag-parsing errors as UsageError so they get exit code 2. rootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { return &clierrors.UsageError{Message: flagErrorHint(cmd, err.Error())} From 81d84dd52f278fb64dbd621c4caf485a83a875ce Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 00:53:20 +0200 Subject: [PATCH 06/28] [Login] Fix a test-only data race after Ctrl-C on piped stdin The read goroutine looked up the command's stdin itself, and after an interrupt nothing waits for it, so go test -race flagged that lookup against the next test's SetIn (go1.27.2 and go1.25.0). The caller now builds the reader. The binary exits at that point, so only tests could race; -race over pkg/cmd: 20 of 20 runs pass. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/login.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/login.go b/pkg/cmd/login.go index ed21d79..a24afff 100644 --- a/pkg/cmd/login.go +++ b/pkg/cmd/login.go @@ -111,7 +111,8 @@ func readAPIKey(cmd *cobra.Command) (string, error) { } key = string(b) } else { - line, err := readInterruptibly(ctx, func() (string, error) { return bufio.NewReader(cmd.InOrStdin()).ReadString('\n') }, + in := bufio.NewReader(cmd.InOrStdin()) + line, err := readInterruptibly(ctx, func() (string, error) { return in.ReadString('\n') }, "Stopped while waiting for the API key on stdin; nothing was saved") if isInterrupted(err) { return "", err @@ -129,7 +130,8 @@ func readAPIKey(cmd *cobra.Command) (string, error) { // readInterruptibly returns what read returns, or an interrupted error with // the stopped message as soon as ctx is canceled. A read from a terminal or a -// pipe can't be canceled itself; it ends with the process. +// pipe can't be canceled itself; it ends with the process. read outlives an +// interrupted call, so it must not touch the command. func readInterruptibly[T any](ctx context.Context, read func() (T, error), stopped string) (T, error) { type result struct { v T From 803cda6c2ab4b8f96c88709104781c92f81601e0 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 01:26:17 +0200 Subject: [PATCH 07/28] [CLI] Keep help's completion and text with the unknown-topic error 7ae9a22 replaced cobra's help command, which dropped its completion (serpapi help s offered files instead of search) and its long help text. cobra's own help command is back; only its run step is replaced, so an unknown topic still exits 2 with one JSON line. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/cmd_test.go | 12 ++++++++++++ pkg/cmd/root.go | 44 +++++++++++++++++++++++++------------------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index df6daad..830d6cd 100644 --- a/pkg/cmd/cmd_test.go +++ b/pkg/cmd/cmd_test.go @@ -714,6 +714,7 @@ func TestHelpAndVersionGoToStdout(t *testing.T) { {[]string{"search", "--help"}, "--all-pages"}, {[]string{"help", "search"}, "--all-pages"}, {[]string{"--version"}, "serpapi version "}, + {[]string{"help", "help"}, "Help provides help for any command"}, } { res := execute(t, "", tt.args...) if res.code != 0 || res.stderr != "" || !strings.Contains(res.stdout, tt.want) { @@ -742,3 +743,14 @@ func TestHelpWithAnUnknownTopic(t *testing.T) { } } } + +// The help command still completes command names, and only them. +func TestHelpCompletesCommands(t *testing.T) { + fakeAPI(t, nil) + + res := execute(t, "", "__complete", "help", "se") + + if res.code != 0 || !strings.HasPrefix(res.stdout, "search\t") || !strings.Contains(res.stdout, "\n:4\n") { + t.Errorf("exit %d, stdout %q; want search and directive 4 (no file completion)", res.code, res.stdout) + } +} diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 446feec..ac247cd 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -148,6 +148,7 @@ func exitAfterGrace(ctx context.Context, done <-chan struct{}, stop func(), stde // run executes the root command, reports any error on its stderr as JSON and // returns the exit code. A command stopped by canceling ctx exits 130. func run(ctx context.Context) int { + initHelpCommand() err := rootCmd.ExecuteContext(ctx) if err == nil { return 0 @@ -160,6 +161,30 @@ func run(ctx context.Context) int { return clierrors.ExitCode(err) } +// initHelpCommand keeps cobra's help command, with its text and completion, +// but makes an unknown topic a usage error; cobra prints the usage and exits 0. +// It runs once every command has been added, which cobra needs. +func initHelpCommand() { + rootCmd.InitDefaultHelpCmd() + if help, _, err := rootCmd.Find([]string{"help"}); err == nil && help != rootCmd { + help.RunE = runHelp + } +} + +func runHelp(c *cobra.Command, args []string) error { + target, _, err := c.Root().Find(args) + if len(args) > 0 && (err != nil || target == c.Root()) { + msg := fmt.Sprintf("Unknown help topic %q", strings.Join(args, " ")) + if s := c.Root().SuggestionsFor(args[0]); len(s) > 0 { + return &clierrors.UsageError{Message: msg + "; did you mean " + strings.Join(s, " or ") + "?"} + } + return &clierrors.UsageError{Message: msg + "; see 'serpapi --help'"} + } + target.InitDefaultHelpFlag() + target.InitDefaultVersionFlag() + return target.Help() +} + func init() { pf := rootCmd.PersistentFlags() pf.StringVar(&apiKeyFlag, "api-key", "", "SerpApi API key; prefer SERPAPI_KEY, since flags show up in ps and shell history") @@ -171,25 +196,6 @@ func init() { rootCmd.PersistentPreRunE = validateGlobalFlags - // cobra's help command prints the usage and exits 0 for an unknown topic. - rootCmd.SetHelpCommand(&cobra.Command{ - Use: "help [command]", - Short: "Help about any command", - RunE: func(c *cobra.Command, args []string) error { - target, _, err := c.Root().Find(args) - if len(args) > 0 && (err != nil || target == c.Root()) { - msg := fmt.Sprintf("Unknown help topic %q", strings.Join(args, " ")) - if s := c.Root().SuggestionsFor(args[0]); len(s) > 0 { - return &clierrors.UsageError{Message: msg + "; did you mean " + strings.Join(s, " or ") + "?"} - } - return &clierrors.UsageError{Message: msg + "; see 'serpapi --help'"} - } - target.InitDefaultHelpFlag() - target.InitDefaultVersionFlag() - return target.Help() - }, - }) - // Wrap cobra flag-parsing errors as UsageError so they get exit code 2. rootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { return &clierrors.UsageError{Message: flagErrorHint(cmd, err.Error())} From 571ac25a2292ab1478a7e17466b0375e529e264b Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 01:27:15 +0200 Subject: [PATCH 08/28] [Config] Ignore a relative home directory for the saved key 1f7423f stopped using the current directory when HOME is unset, but HOME=. or any relative HOME still put the key under, and read it from, wherever the command ran. Dir now accepts only absolute paths, so login stops with exit 2 and search, account and archive ignore any config file there; the error names %AppData% on Windows. Co-Authored-By: Claude Opus 5.5 --- pkg/config/config.go | 16 ++++++--- pkg/config/config_test.go | 76 ++++++++++++++++++++++++++------------- 2 files changed, 63 insertions(+), 29 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 14111ec..7e7709a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -7,6 +7,7 @@ import ( "io/fs" "os" "path/filepath" + "runtime" "strings" "github.com/BurntSushi/toml" @@ -18,14 +19,14 @@ type configFile struct { } // Dir returns the directory where the CLI stores its config, or "" when -// neither the user config directory nor the home directory is known: a -// relative directory would put the key in, or take it from, wherever the +// neither the user config directory nor the home directory is an absolute +// path: a relative one would put the key in, or take it from, wherever the // command runs. func Dir() string { - if dir, err := os.UserConfigDir(); err == nil { + if dir, err := os.UserConfigDir(); err == nil && filepath.IsAbs(dir) { return filepath.Join(dir, "serpapi") } - if home, err := os.UserHomeDir(); err == nil { + if home, err := os.UserHomeDir(); err == nil && filepath.IsAbs(home) { return filepath.Join(home, ".config", "serpapi") } return "" @@ -80,7 +81,12 @@ func LoadAPIKey() (string, error) { func SaveConfig(apiKey string) error { dir := Dir() if dir == "" { - return &clierrors.UsageError{Message: "No directory to save the API key in: HOME is not set; set HOME, or pass the key with SERPAPI_KEY instead of saving it"} + home := "HOME" + if runtime.GOOS == "windows" { + home = "%AppData%" + } + return &clierrors.UsageError{Message: "No directory to save the API key in: " + home + + " is not set to an absolute path; set it, or pass the key with SERPAPI_KEY instead of saving it"} } if err := os.MkdirAll(dir, 0o700); err != nil { return &clierrors.UsageError{Message: "Failed to create config dir: " + err.Error()} diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 62e2a7c..cbf753d 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -162,31 +162,59 @@ func TestSaveConfigConcurrently(t *testing.T) { // Without HOME there is no config directory, and a relative one would put the // key in, or take it from, whatever directory the command runs in. -func TestNoConfigDirWithoutHome(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Windows finds the config directory through other variables") - } - t.Setenv("HOME", "") - t.Setenv("XDG_CONFIG_HOME", "") - cwd := t.TempDir() - t.Chdir(cwd) - planted := filepath.Join(cwd, ".config", "serpapi", "config.toml") - if err := os.MkdirAll(filepath.Dir(planted), 0o700); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(planted, []byte(`api_key = "planted-key"`+"\n"), 0o600); err != nil { - t.Fatal(err) - } +// Without an absolute home directory, the key is neither read from nor saved +// to a directory relative to wherever the command runs. +func TestNoConfigDirWithoutAnAbsoluteHome(t *testing.T) { + for _, home := range []string{"", ".", filepath.Join("relative", "home")} { + for _, name := range []string{"HOME", "XDG_CONFIG_HOME", "AppData", "USERPROFILE"} { + t.Setenv(name, home) + } + cwd := t.TempDir() + t.Chdir(cwd) + var planted []string + for _, dir := range []string{filepath.Join(home, ".config"), filepath.Join(home, "Library", "Application Support"), home} { + path := filepath.Join(dir, "serpapi", "config.toml") + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(`api_key = "planted-key"`+"\n"), 0o600); err != nil { + t.Fatal(err) + } + planted = append(planted, path) + } + before := filesUnder(t, cwd) - if key, err := ResolveAPIKey(""); err != ErrNoAPIKey { - t.Errorf("ResolveAPIKey = %q, %v; want ErrNoAPIKey, not the key in the current directory", key, err) - } - err := SaveConfig("new-key") - var usageErr *clierrors.UsageError - if !errors.As(err, &usageErr) || !strings.Contains(usageErr.Message, "HOME") { - t.Errorf("SaveConfig = %v; want a UsageError naming HOME", err) + if Dir() != "" || Path() != "" { + t.Errorf("home %q: Dir() = %q, Path() = %q; want both empty", home, Dir(), Path()) + } + if key, err := ResolveAPIKey(""); err != ErrNoAPIKey { + t.Errorf("home %q: ResolveAPIKey = %q, %v; want ErrNoAPIKey, not a key under the current directory", home, key, err) + } + err := SaveConfig("new-key") + var usageErr *clierrors.UsageError + if !errors.As(err, &usageErr) || !strings.Contains(usageErr.Message, "absolute") || !strings.Contains(usageErr.Message, "SERPAPI_KEY") { + t.Errorf("home %q: SaveConfig = %v; want a UsageError that asks for an absolute home or SERPAPI_KEY", home, err) + } + for _, path := range planted { + if b, _ := os.ReadFile(path); !strings.Contains(string(b), "planted-key") { + t.Errorf("home %q: %s was overwritten: %q", home, path, b) + } + } + if after := filesUnder(t, cwd); !slices.Equal(before, after) { + t.Errorf("home %q: files under the current directory changed from %v to %v", home, before, after) + } } - if b, _ := os.ReadFile(planted); !strings.Contains(string(b), "planted-key") { - t.Errorf("the planted file was overwritten: %q", b) +} + +func filesUnder(t *testing.T, root string) []string { + t.Helper() + var files []string + err := filepath.WalkDir(root, func(path string, _ os.DirEntry, err error) error { + files = append(files, path) + return err + }) + if err != nil { + t.Fatal(err) } + return files } From 77b5926d166515a6562292596a45f68a16d10442 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 01:29:22 +0200 Subject: [PATCH 09/28] [Hints] Read zero_trace and no_cache as the API does A large ZeroTrace result was told to re-read its stored search for free, but ZeroTrace stores nothing and skips the cache (serpapi/SerpApi api_controller.rb: no_cache = to_bool(zero_trace || no_cache)). The hint now says the repeat is a new, counted search. It reads both flags the way the API does through Mongoid 7.5's Boolean.evolve: no_cache=False is off and served from the cache; a word in neither of Mongoid's lists gets "may", since Mongoid 8 takes it as true. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/edge_test.go | 14 +++++++++++ pkg/cmd/hint.go | 55 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go index d9c9ebb..763c7fb 100644 --- a/pkg/cmd/edge_test.go +++ b/pkg/cmd/edge_test.go @@ -211,6 +211,18 @@ func edgeCases() []edgeCase { warn: "repeat the command with --jq '.KEY': the same search within an hour comes from the cache"}, {name: "output/large no_cache result promises no cache hit", args: searchWith("no_cache=true"), api: respond(bigResult("", 1500)), warn: "repeat the command with --jq '.KEY'; no_cache=true makes the repeat a new, counted search"}, + {name: "output/large no_cache result keeps the free archive", args: searchWith("no_cache=true"), api: respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500)), + warn: "read the stored search for free: serpapi archive abc123 --jq '.KEY'"}, + {name: "output/large no_cache in the API's true words", args: searchWith("no_cache=YES"), api: respond(bigResult("", 1500)), + warn: "no_cache=YES makes the repeat a new, counted search"}, + {name: "output/large no_cache in the API's false words", args: searchWith("no_cache=False"), api: respond(bigResult("", 1500)), + warn: "repeat the command with --jq '.KEY': the same search within an hour comes from the cache"}, + {name: "output/large no_cache in neither list", args: searchWith("no_cache=maybe"), api: respond(bigResult("", 1500)), + warn: "repeat the command with --jq '.KEY'; with no_cache=maybe the repeat may be a new, counted search"}, + {name: "output/large zero_trace result has no archive", args: searchWith("zero_trace=true"), api: respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500)), + warn: "repeat the command with --jq '.KEY'; zero_trace=true stores nothing and makes the repeat a new, counted search"}, + {name: "output/large zero_trace=0 keeps the archive", args: searchWith("zero_trace=0"), api: respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500)), + warn: "read the stored search for free: serpapi archive abc123 --jq '.KEY'"}, {name: "output/large result with --jq is quiet", args: searchWith("--jq", ".organic_results | length"), api: respond(bigResult("", 1500)), stdout: "1500\n", quiet: true}, {name: "output/large result with --fields still hints", args: searchWith("--fields", "organic_results"), @@ -219,6 +231,8 @@ func edgeCases() []edgeCase { warn: "Large result: 48 KB; top-level keys by size: organic_results 48 KB, inline_images 70 B. To print only what you need, repeat the command"}, {name: "output/large no_cache merged pages", args: searchWith("no_cache=true", "--all-pages"), reqs: 2, api: bigPages, warn: "no_cache=true makes the repeat a new, counted search for each page"}, + {name: "output/large zero_trace merged pages", args: searchWith("zero_trace=1", "--all-pages"), reqs: 2, api: bigPages, + warn: "zero_trace=1 stores nothing and makes the repeat a new, counted search for each page"}, {name: "output/large archive", args: []string{"archive", "abc123"}, api: respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500)), warn: "add --jq '.KEY'; reading the archive is free"}, {name: "output/jq printing only null lists the keys", args: searchWith("--jq", ".organic"), api: oneResult, stdout: "\n", diff --git a/pkg/cmd/hint.go b/pkg/cmd/hint.go index 9387235..b6990f7 100644 --- a/pkg/cmd/hint.go +++ b/pkg/cmd/hint.go @@ -114,21 +114,58 @@ func archiveHint(string) string { return "add --jq '.KEY'; reading the archive i // searchHint returns the next step for a large result of the search sent // with p: re-reading the stored search is free, and so is the same search -// within the cache hour, unless no_cache asked for a new one. +// within the cache hour, unless no_cache asked for a new one. ZeroTrace +// stores nothing and skips the cache. func searchHint(p map[string]string) func(id string) string { return func(id string) string { - if id != "" { + zeroTrace, noCache := apiBool(p, "zero_trace"), apiBool(p, "no_cache") + if id != "" && zeroTrace == paramOff { return fmt.Sprintf("read the stored search for free: serpapi archive %s --jq '.KEY'", id) } - if v := p["no_cache"]; v != "" && v != "false" && v != "0" { - hint := "repeat the command with --jq '.KEY'; no_cache=" + v + " makes the repeat a new, counted search" - if allPagesFlag { - hint += " for each page" - } - return hint + hint := "repeat the command with --jq '.KEY'" + switch { + case zeroTrace == paramOn: + hint += "; zero_trace=" + p["zero_trace"] + " stores nothing and makes the repeat a new, counted search" + case zeroTrace == paramUnsure: + hint += "; with zero_trace=" + p["zero_trace"] + " the repeat may be a new, counted search" + case noCache == paramOn: + hint += "; no_cache=" + p["no_cache"] + " makes the repeat a new, counted search" + case noCache == paramUnsure: + hint += "; with no_cache=" + p["no_cache"] + " the repeat may be a new, counted search" + default: + return hint + ": the same search within an hour comes from the cache" } - return "repeat the command with --jq '.KEY': the same search within an hour comes from the cache" + if allPagesFlag { + hint += " for each page" + } + return hint + } +} + +type paramBool int + +const ( + paramOff paramBool = iota + paramOn + paramUnsure +) + +// apiBool reads a boolean parameter as the API does, through Mongoid 7.5's +// Boolean.evolve: its true words, in any case, are on, and anything else is +// off. Mongoid 8 keeps any other word as a string, which Ruby takes as true, +// so only its false words are sure to stay off. +func apiBool(p map[string]string, key string) paramBool { + v, ok := p[key] + if !ok { + return paramOff + } + switch strings.ToLower(v) { + case "true", "t", "yes", "y", "on", "1", "1.0": + return paramOn + case "false", "f", "no", "n", "off", "0", "0.0": + return paramOff } + return paramUnsure } func sizeText(n int) string { From 7f401823e31f6b4daa01d7095f375dffa4a1893b Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 01:33:45 +0200 Subject: [PATCH 10/28] [Search] Print what arrived before Ctrl-C, ahead of the forced exit e0af7a7 ran --jq on the pages fetched before a signal only when a page was still loading. A signal after the last page or a single response arrived made --jq print nothing; now it runs on what arrived and exits 0. After a signal, --jq stops at half of interruptGrace, so it flushes what it printed before exitAfterGrace exits at the full grace; it ran past that, and the forced exit dropped its buffered output. run() says "Stopped before the response arrived" only for a request the signal canceled, so a --jq that fails on fetched pages keeps its exit 2 error. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/edge_test.go | 113 +++++++++++++++++++++++++++++++++++++++++++ pkg/cmd/root.go | 21 ++++++-- pkg/cmd/search.go | 11 ++--- 3 files changed, 133 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go index 763c7fb..542f0e9 100644 --- a/pkg/cmd/edge_test.go +++ b/pkg/cmd/edge_test.go @@ -517,6 +517,119 @@ func TestInterruptDuringJQKeepsWhatItPrinted(t *testing.T) { } } +// cancelAtEndOf cancels ctx as the client reads the last byte of page n: +// after the page arrived, before anything is printed. +func cancelAtEndOf(t *testing.T, n int, cancel context.CancelFunc) { + t.Helper() + orig := http.DefaultTransport + t.Cleanup(func() { http.DefaultTransport = orig }) + http.DefaultTransport = roundTripFunc(func(r *http.Request) (*http.Response, error) { + resp, err := orig.RoundTrip(r) + if err == nil && pageNumber(r) == n { + resp.Body = &onEOF{ReadCloser: resp.Body, do: cancel} + } + return resp, err + }) +} + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } + +// onEOF calls do once the body has been read to the end. +type onEOF struct { + io.ReadCloser + do func() +} + +func (b *onEOF) Read(p []byte) (int, error) { + n, err := b.ReadCloser.Read(p) + if err == io.EOF { + b.do() + } + return n, err +} + +// A signal after the last page arrived, before it was printed, still prints +// every page: all of them were billed. +func TestInterruptAfterTheLastPagePrintsThemAll(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + fakeAPI(t, func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, page(pageNumber(r), pageNumber(r) < 3)) }) + t.Setenv("SERPAPI_KEY", "envkey") + cancelAtEndOf(t, 3, cancel) + + res := executeContext(t, ctx, "", searchWith("--all-pages", "--jq", ".organic_results | length")...) + + wantOK(t, res) + if res.stdout != "3\n" || ctx.Err() == nil { + t.Errorf("stdout = %q, signaled = %v; want --jq's result on all 3 pages after the signal", res.stdout, ctx.Err() != nil) + } +} + +func TestInterruptAfterTheResponseStillRunsJQ(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + fakeAPI(t, func(w http.ResponseWriter, _ *http.Request) { fmt.Fprint(w, page(1, false)) }) + t.Setenv("SERPAPI_KEY", "envkey") + cancelAtEndOf(t, 1, cancel) + + res := executeContext(t, ctx, "", searchWith("--jq", ".organic_results[0].position")...) + + wantOK(t, res) + if res.stdout != "1\n" || ctx.Err() == nil { + t.Errorf("stdout = %q, signaled = %v; want --jq's result after the signal", res.stdout, ctx.Err() != nil) + } +} + +// --jq that fails on the pages fetched before a signal reports its own error: +// those pages did arrive. +func TestJQErrorAfterAnInterruptIsReported(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + interruptAtPage3(t, cancel) + + res := executeContext(t, ctx, "", searchWith("--all-pages", "--jq", ".organic_results[].position, (.missing | keys)")...) + + wantError(t, res, 2, "usage_error", "keys cannot be applied to: null") + if res.stdout != "1\n2\n" { + t.Errorf("stdout = %q, want what --jq printed before its error", res.stdout) + } +} + +// After a signal, --jq on the fetched pages stops, and flushes what it +// printed, before the process would exit at interruptGrace. +func TestInterruptedJQStopsBeforeTheProcessExits(t *testing.T) { + grace := interruptGrace + interruptGrace = time.Second + t.Cleanup(func() { interruptGrace = grace }) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + interruptAtPage3(t, cancel) + var watchdog lockedBuffer + done, returned := make(chan struct{}), make(chan struct{}) + go func() { + exitAfterGrace(ctx, done, func() {}, &watchdog, func(int) {}) + close(returned) + }() + + res := executeContext(t, ctx, "", searchWith("--all-pages", "--jq", ".organic_results[0].position, (reduce range(1e12) as $i (0; . + 1))")...) + close(done) + select { + case <-returned: + case <-time.After(10 * time.Second): + t.Fatal("exitAfterGrace never returned") + } + + if got := watchdog.String(); got != "" { + t.Errorf("the process would have exited before --jq stopped: %s", got) + } + wantError(t, res, 130, "interrupted", "Stopped while running --jq") + if res.stdout != "1\n" { + t.Errorf("stdout = %q, want the value --jq printed before it was stopped", res.stdout) + } +} + // lockedBuffer is a bytes.Buffer safe to write from another goroutine. type lockedBuffer struct { mu sync.Mutex diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index ac247cd..190114c 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -146,15 +146,15 @@ func exitAfterGrace(ctx context.Context, done <-chan struct{}, stop func(), stde } // run executes the root command, reports any error on its stderr as JSON and -// returns the exit code. A command stopped by canceling ctx exits 130. +// returns the exit code. A request stopped by canceling ctx exits 130; an +// error from what had already arrived, such as a failing --jq, keeps its own. func run(ctx context.Context) int { initHelpCommand() err := rootCmd.ExecuteContext(ctx) if err == nil { return 0 } - var interrupted *clierrors.InterruptedError - if ctx.Err() != nil && !errors.As(err, &interrupted) { + if ctx.Err() != nil && errors.Is(err, context.Canceled) { err = &clierrors.InterruptedError{Message: "Stopped before the response arrived"} } clierrors.FprintError(rootCmd.ErrOrStderr(), err) @@ -343,7 +343,20 @@ func handleOutput(cmd *cobra.Command, raw []byte, next func(id string) string) e if err := dec.Decode(&v); err != nil { return &clierrors.APIError{Message: "Failed to parse response: " + err.Error()} } - return printJQ(cmd.Context(), cmd, v) + ctx, cancel := printContext(cmd.Context()) + defer cancel() + return printJQ(ctx, cmd, v) +} + +// printContext is the context for printing a result that has arrived. After +// a signal, the search was billed, so the result still prints, with --jq +// stopped at half of interruptGrace: it then flushes and reports what it +// printed before exitAfterGrace exits the process. +func printContext(ctx context.Context) (context.Context, context.CancelFunc) { + if ctx == nil || ctx.Err() == nil { + return ctx, func() {} + } + return context.WithTimeout(context.WithoutCancel(ctx), interruptGrace/2) } // printJQ runs --jq on v until ctx ends, printing each result as it is diff --git a/pkg/cmd/search.go b/pkg/cmd/search.go index e0d5e1b..b080502 100644 --- a/pkg/cmd/search.go +++ b/pkg/cmd/search.go @@ -291,14 +291,9 @@ func searchAllPages(cmd *cobra.Command, client *api.Client, first map[string]str warnNothingMatched(cmd) } - // Pages fetched before Ctrl-C were billed, so --jq still runs on them, for - // at most interruptGrace; the process exits about then anyway. - printCtx := ctx - if pageErr != nil && ctx.Err() != nil { - var cancel context.CancelFunc - printCtx, cancel = context.WithTimeout(context.WithoutCancel(ctx), interruptGrace) - defer cancel() - } + // Pages fetched before Ctrl-C were billed, so --jq still runs on them. + printCtx, cancel := printContext(ctx) + defer cancel() if err := printMerged(printCtx, cmd, merged, searchHint(first)("")); err != nil { return err } From 056ab9b986c7be50dabbd899444794a3e29c8789 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 01:38:23 +0200 Subject: [PATCH 11/28] [Login] Keep echo on after a signal at the hidden key prompt 92590fb restored the terminal when a signal stopped the hidden prompt, but term.ReadPassword turns echo off inside the reading goroutine, so a goroutine that got there after the restore left echo off: forced with a barrier, 5 of 5 SIGTERM runs ended with -echo. Raw mode is now set and undone by the caller, and the goroutine only reads, through x/term's Terminal: 5 of 5 kept echo. Ctrl-C is a key in raw mode and still exits 130 at once; Ctrl-D, which ReadPassword ignored, now gives "No API key given", exit 2. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/login.go | 70 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/login.go b/pkg/cmd/login.go index a24afff..9db19f9 100644 --- a/pkg/cmd/login.go +++ b/pkg/cmd/login.go @@ -2,6 +2,7 @@ package cmd import ( "bufio" + "bytes" "context" "encoding/json" "errors" @@ -93,23 +94,12 @@ func readAPIKey(cmd *cobra.Command) (string, error) { if ctx.Err() != nil { return "", &clierrors.InterruptedError{Message: stopped} } - // ReadPassword turns echo off until it returns, which it doesn't on - // Ctrl-C, so the state saved here is put back then. - fd := int(f.Fd()) - state, _ := term.GetState(fd) fmt.Fprint(stderr, "SerpApi API key (input hidden): ") - b, err := readInterruptibly(ctx, func() ([]byte, error) { return term.ReadPassword(fd) }, stopped) - if isInterrupted(err) && state != nil { - _ = term.Restore(fd, state) - } - fmt.Fprintln(stderr) - if isInterrupted(err) { - return "", err - } + line, err := readHidden(ctx, f, stderr, stopped) if err != nil { - return "", &clierrors.UsageError{Message: "Failed to read the API key: " + err.Error()} + return "", err } - key = string(b) + key = line } else { in := bufio.NewReader(cmd.InOrStdin()) line, err := readInterruptibly(ctx, func() (string, error) { return in.ReadString('\n') }, @@ -128,6 +118,58 @@ func readAPIKey(cmd *cobra.Command) (string, error) { return key, nil } +// readHidden reads a line from the terminal f without showing it. Raw mode is +// set and undone here, not in the goroutine that reads, so a read that a +// signal leaves running can't turn echo off again after it was restored. In +// raw mode Ctrl-C is a key rather than a signal, and stops the read the same +// way; Ctrl-D gives no key. +func readHidden(ctx context.Context, f *os.File, stderr io.Writer, stopped string) (string, error) { + fd := int(f.Fd()) + state, err := term.MakeRaw(fd) + if err != nil { + return "", &clierrors.UsageError{Message: "Failed to read the API key: " + err.Error()} + } + in := &ctrlCWatch{r: f} + t := term.NewTerminal(struct { + io.Reader + io.Writer + }{in, stderr}, "") + line, err := readInterruptibly(ctx, func() (string, error) { return t.ReadPassword("") }, stopped) + _ = term.Restore(fd, state) + if errors.Is(err, term.ErrPasteIndicator) { + err = nil + } + if err != nil { + fmt.Fprintln(stderr) + } + switch { + case err == nil: + return line, nil + case isInterrupted(err): + return "", err + case errors.Is(err, io.EOF) && in.ctrlC: + return "", &clierrors.InterruptedError{Message: stopped} + case errors.Is(err, io.EOF): + return "", nil + } + return "", &clierrors.UsageError{Message: "Failed to read the API key: " + err.Error()} +} + +// ctrlCWatch notes a Ctrl-C, which raw mode passes on as a key. Only the +// reading goroutine writes ctrlC; it is read after that goroutine returned. +type ctrlCWatch struct { + r io.Reader + ctrlC bool +} + +func (w *ctrlCWatch) Read(p []byte) (int, error) { + n, err := w.r.Read(p) + if bytes.IndexByte(p[:n], 3) >= 0 { + w.ctrlC = true + } + return n, err +} + // readInterruptibly returns what read returns, or an interrupted error with // the stopped message as soon as ctx is canceled. A read from a terminal or a // pipe can't be canceled itself; it ends with the process. read outlives an From 6ed77ed0e1c2b10541f1594eb7301911b9d4b7d4 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 01:40:28 +0200 Subject: [PATCH 12/28] [Docs] Note login's absolute HOME and what zero_trace costs README and --help say that login needs an absolute HOME (%AppData% on Windows), as 571ac25 made it, and that zero_trace=true skips the cache and leaves nothing for archive to read, as the large-result hint now says (serpapi/SerpApi api_controller.rb and its zero_trace spec). Co-Authored-By: Claude Opus 5.5 --- README.md | 4 ++-- pkg/cmd/root.go | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index db9f2bd..3b51bfc 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ The API key comes from the first of: Without a key, `search` still runs, but the API answers only a few demo queries such as `q=coffee` and returns 401 for the rest. `account` and `archive` need a key. -`serpapi login` saves the key, readable only by you, to `serpapi/config.toml` in the OS config directory: `$XDG_CONFIG_HOME` or `~/.config` on Linux, `~/Library/Application Support` on macOS, `%AppData%` on Windows. +`serpapi login` saves the key, readable only by you, to `serpapi/config.toml` in the OS config directory: `$XDG_CONFIG_HOME` or `~/.config` on Linux, `~/Library/Application Support` on macOS, `%AppData%` on Windows. Without an absolute `HOME` (`%AppData%` on Windows) there is no config file: `login` exits 2, and other commands need `--api-key` or `SERPAPI_KEY`. ```toml api_key = "your_serpapi_key_here" @@ -160,7 +160,7 @@ A config file that can't be read or isn't valid TOML is reported by path and lin | 1 | `network_error` | No answer, such as a timeout. | | 130 | `interrupted` | Stopped by Ctrl-C or SIGTERM after printing what had arrived, such as the `--all-pages` pages fetched so far. | -- **Cost**: each successful search counts against your monthly searches, including each `--all-pages` page; cached, errored and failed ones don't, though errored ones count toward the hourly limit ([pricing FAQ](https://serpapi.com/pricing)). An identical search, `--fields` included, repeated within an hour comes from the cache. `no_cache=true` forces a new, counted search; cached results are at most an hour old, so add it only when that is too old. `account`, `locations` and `archive` are free. +- **Cost**: each successful search counts against your monthly searches, including each `--all-pages` page; cached, errored and failed ones don't, though errored ones count toward the hourly limit ([pricing FAQ](https://serpapi.com/pricing)). An identical search, `--fields` included, repeated within an hour comes from the cache. `no_cache=true` forces a new, counted search; cached results are at most an hour old, so add it only when that is too old. `zero_trace=true` (Enterprise) stores nothing, so it skips the cache too and leaves no search for `archive` to read. `account`, `locations` and `archive` are free. - **No prompts without a terminal.** Only `serpapi login` reads stdin, and it waits for the first line, so pipe the key in (`printf %s "$KEY" | serpapi login`) or, simpler, set `SERPAPI_KEY`. ## Links diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 190114c..ab8bfc1 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -87,8 +87,9 @@ each page of --all-pages; cached, errored and failed ones don't, though errored ones count toward the hourly limit (https://serpapi.com/pricing). An identical search, --fields included, repeated within an hour comes from the cache. no_cache=true forces a new, counted search; cached results are at most an -hour old, so add it only when that is too old. account, locations and archive -are free.`, +hour old, so add it only when that is too old. zero_trace=true (Enterprise) +stores nothing, so it skips the cache too and leaves no search for archive to +read. account, locations and archive are free.`, Example: ` serpapi search engine=google q=coffee --fields "organic_results[0:3]" serpapi search engine=google coffee shops --jq ".organic_results[].link" serpapi search engine=google q=coffee output=md From 46af5c5875dec70294e46cce139e830e9b367f13 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 01:56:26 +0200 Subject: [PATCH 13/28] [CLI] Count the exit and --jq deadlines from the signal itself 7f40182 gave --jq half of interruptGrace from when printing started, so a slow decode after the signal could still push it past the forced exit. Execute now cancels with a cause that records the signal time, and both the watchdog and the print deadline count from it; --jq stops a quarter of interruptGrace before the exit. The cause unwraps to context.Canceled, since net/http reports a canceled request with the context's cause: without that, a signal before the response turned into network_error, exit 1. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/edge_test.go | 62 +++++++++++++++++++++++++++++++++++++++++--- pkg/cmd/root.go | 47 ++++++++++++++++++++++++++------- 2 files changed, 97 insertions(+), 12 deletions(-) diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go index 542f0e9..7390d70 100644 --- a/pkg/cmd/edge_test.go +++ b/pkg/cmd/edge_test.go @@ -603,9 +603,9 @@ func TestInterruptedJQStopsBeforeTheProcessExits(t *testing.T) { grace := interruptGrace interruptGrace = time.Second t.Cleanup(func() { interruptGrace = grace }) - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - interruptAtPage3(t, cancel) + ctx, cancel := context.WithCancelCause(context.Background()) + defer cancel(nil) + interruptAtPage3(t, func() { cancel(&signaled{at: time.Now()}) }) var watchdog lockedBuffer done, returned := make(chan struct{}), make(chan struct{}) go func() { @@ -681,3 +681,59 @@ func TestExitAfterGrace(t *testing.T) { func(code int) { t.Errorf("exit(%d) without a signal", code) }) }) } + +// The process exits interruptGrace after the signal, however long the +// command took to notice it. +func TestExitAfterGraceCountsFromTheSignal(t *testing.T) { + grace := interruptGrace + interruptGrace = 2 * time.Second + t.Cleanup(func() { interruptGrace = grace }) + ctx, cancel := context.WithCancelCause(context.Background()) + cancel(&signaled{at: time.Now().Add(-interruptGrace)}) + exited := make(chan struct{}) + start := time.Now() + + go exitAfterGrace(ctx, make(chan struct{}), func() {}, io.Discard, func(int) { close(exited) }) + + select { + case <-exited: + if elapsed := time.Since(start); elapsed > time.Second { + t.Errorf("exited %v after the watchdog woke, want at once: the grace had run out", elapsed) + } + case <-time.After(5 * time.Second): + t.Fatal("never exited") + } +} + +// --jq after a signal stops a quarter of interruptGrace before the process +// exits, counted from the signal, so a slow decode before printing doesn't +// push it past the exit. +func TestPrintDeadlineCountsFromTheSignal(t *testing.T) { + ctx, cancel := context.WithCancelCause(context.Background()) + at := time.Now().Add(-time.Second) + cancel(&signaled{at: at}) + + printCtx, stop := printContext(ctx) + defer stop() + + want := at.Add(interruptGrace - interruptGrace/4) + if d, ok := printCtx.Deadline(); !ok || !d.Equal(want) { + t.Errorf("deadline %v (set: %v), want %v", d, ok, want) + } +} + +// net/http reports a canceled request with the context's cause, so a signal +// still reads as a stop before the response. +func TestSignalBeforeTheResponse(t *testing.T) { + ctx, cancel := context.WithCancelCause(context.Background()) + defer cancel(nil) + fakeAPI(t, func(_ http.ResponseWriter, r *http.Request) { + cancel(&signaled{at: time.Now()}) + <-r.Context().Done() + }) + t.Setenv("SERPAPI_KEY", "envkey") + + res := executeContext(t, ctx, "", search...) + + wantError(t, res, 130, "interrupted", "Stopped before the response arrived") +} diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index ab8bfc1..c039301 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -112,21 +112,50 @@ read. account, locations and archive are free.`, // signal before the process exits anyway. var interruptGrace = 3 * time.Second +// signaled is the cause Execute cancels the command's context with, so code +// that sees the cancellation knows when the signal came. +type signaled struct{ at time.Time } + +func (*signaled) Error() string { return "stopped by Ctrl-C or SIGTERM" } + +// Unwrap keeps errors.Is(err, context.Canceled) true for a request it +// stopped: net/http reports the context's cause, not context.Canceled. +func (*signaled) Unwrap() error { return context.Canceled } + +// exitTime is when the process exits after the first signal: interruptGrace +// after it. A context canceled without a signal time counts from now. +func exitTime(ctx context.Context) time.Time { + var s *signaled + if errors.As(context.Cause(ctx), &s) { + return s.at.Add(interruptGrace) + } + return time.Now().Add(interruptGrace) +} + // Execute runs the root command. The first Ctrl-C or SIGTERM cancels the // command's context so it can print what it has; a second one kills it. func Execute() { - ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + ctx, cancel := context.WithCancelCause(context.Background()) + stop := func() { cancel(&signaled{at: time.Now()}) } + sigs := make(chan os.Signal, 1) + signal.Notify(sigs, os.Interrupt, syscall.SIGTERM) + go func() { + <-sigs + stop() + }() + restoreDefault := func() { signal.Stop(sigs) } done := make(chan struct{}) - go exitAfterGrace(ctx, done, stop, rootCmd.ErrOrStderr(), os.Exit) + go exitAfterGrace(ctx, done, restoreDefault, rootCmd.ErrOrStderr(), os.Exit) code := run(ctx) close(done) - stop() + restoreDefault() os.Exit(code) } // exitAfterGrace waits for the first signal, restores the default handling // so that a second one kills the process, and exits 130 if the command is -// still running interruptGrace later, as it is when blocked on a full pipe. +// still running interruptGrace after the signal, as it is when blocked on a +// full pipe. func exitAfterGrace(ctx context.Context, done <-chan struct{}, stop func(), stderr io.Writer, exit func(int)) { select { case <-done: @@ -136,7 +165,7 @@ func exitAfterGrace(ctx context.Context, done <-chan struct{}, stop func(), stde stop() select { case <-done: - case <-time.After(interruptGrace): + case <-time.After(time.Until(exitTime(ctx))): // stderr may be the full pipe, so the message gets a moment, not a promise. go clierrors.FprintError(stderr, &clierrors.InterruptedError{ Message: fmt.Sprintf("Stopped %s after the signal; the output may be cut off", interruptGrace), @@ -350,14 +379,14 @@ func handleOutput(cmd *cobra.Command, raw []byte, next func(id string) string) e } // printContext is the context for printing a result that has arrived. After -// a signal, the search was billed, so the result still prints, with --jq -// stopped at half of interruptGrace: it then flushes and reports what it -// printed before exitAfterGrace exits the process. +// a signal the search was billed, so the result still prints, with --jq +// stopped a quarter of interruptGrace before the process exits: that leaves +// it time to flush and report what it printed. func printContext(ctx context.Context) (context.Context, context.CancelFunc) { if ctx == nil || ctx.Err() == nil { return ctx, func() {} } - return context.WithTimeout(context.WithoutCancel(ctx), interruptGrace/2) + return context.WithDeadline(context.WithoutCancel(ctx), exitTime(ctx).Add(-interruptGrace/4)) } // printJQ runs --jq on v until ctx ends, printing each result as it is From 54815f5679bbbd7d27c62489d9dc052ba7140def Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 01:59:01 +0200 Subject: [PATCH 14/28] [Login] Treat Ctrl-C at the hidden prompt as the signal it replaces In raw mode (056ab9b) Ctrl-C reaches the hidden prompt as a key. Typed in the same read as Enter, the line won: the key was saved and login exited 0. And since the key canceled nothing, a stderr pipe that nobody read hung the run with no watchdog. Now any Ctrl-C in the read stops it with exit 130, nothing saved, and cancels the run as SIGINT does, so the process exits within interruptGrace. PTY probe: Enter+^C in one write 130 at once (main: 130 after 3.1 s); ^C with stderr a full pipe exits at 3.1 s with echo on (before: still running at 8 s). Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/login.go | 10 ++++++---- pkg/cmd/root.go | 12 +++++++++++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/login.go b/pkg/cmd/login.go index 9db19f9..2f7edc6 100644 --- a/pkg/cmd/login.go +++ b/pkg/cmd/login.go @@ -121,8 +121,8 @@ func readAPIKey(cmd *cobra.Command) (string, error) { // readHidden reads a line from the terminal f without showing it. Raw mode is // set and undone here, not in the goroutine that reads, so a read that a // signal leaves running can't turn echo off again after it was restored. In -// raw mode Ctrl-C is a key rather than a signal, and stops the read the same -// way; Ctrl-D gives no key. +// raw mode Ctrl-C is a key rather than a signal; it stops the run as the +// signal would, even after Enter in the same read. Ctrl-D gives no key. func readHidden(ctx context.Context, f *os.File, stderr io.Writer, stopped string) (string, error) { fd := int(f.Fd()) state, err := term.MakeRaw(fd) @@ -136,6 +136,10 @@ func readHidden(ctx context.Context, f *os.File, stderr io.Writer, stopped strin }{in, stderr}, "") line, err := readInterruptibly(ctx, func() (string, error) { return t.ReadPassword("") }, stopped) _ = term.Restore(fd, state) + if !isInterrupted(err) && in.ctrlC { + stopRun(ctx) + err = &clierrors.InterruptedError{Message: stopped} + } if errors.Is(err, term.ErrPasteIndicator) { err = nil } @@ -147,8 +151,6 @@ func readHidden(ctx context.Context, f *os.File, stderr io.Writer, stopped strin return line, nil case isInterrupted(err): return "", err - case errors.Is(err, io.EOF) && in.ctrlC: - return "", &clierrors.InterruptedError{Message: stopped} case errors.Is(err, io.EOF): return "", nil } diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index c039301..1401d89 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -132,6 +132,16 @@ func exitTime(ctx context.Context) time.Time { return time.Now().Add(interruptGrace) } +// stopRunKey holds, in the command's context, the func that cancels it as a +// signal does. login calls it for a Ctrl-C that raw mode turns into a key. +type stopRunKey struct{} + +func stopRun(ctx context.Context) { + if stop, ok := ctx.Value(stopRunKey{}).(func()); ok { + stop() + } +} + // Execute runs the root command. The first Ctrl-C or SIGTERM cancels the // command's context so it can print what it has; a second one kills it. func Execute() { @@ -146,7 +156,7 @@ func Execute() { restoreDefault := func() { signal.Stop(sigs) } done := make(chan struct{}) go exitAfterGrace(ctx, done, restoreDefault, rootCmd.ErrOrStderr(), os.Exit) - code := run(ctx) + code := run(context.WithValue(ctx, stopRunKey{}, stop)) close(done) restoreDefault() os.Exit(code) From f849cf4350f156efac2a411467626e250039da1a Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 02:00:48 +0200 Subject: [PATCH 15/28] [Tests] Wait for the watchdog's message before its test ends 46af5c5's test let the watchdog's message goroutine outlive it, so go test -race caught that goroutine reading output.IsTerminal while a later test swapped it (2 of 4 runs). The test now waits for the message. pkg/cmd under -race: 20 of 20 runs pass. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/edge_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go index 7390d70..af372f3 100644 --- a/pkg/cmd/edge_test.go +++ b/pkg/cmd/edge_test.go @@ -691,9 +691,10 @@ func TestExitAfterGraceCountsFromTheSignal(t *testing.T) { ctx, cancel := context.WithCancelCause(context.Background()) cancel(&signaled{at: time.Now().Add(-interruptGrace)}) exited := make(chan struct{}) + var stderr lockedBuffer start := time.Now() - go exitAfterGrace(ctx, make(chan struct{}), func() {}, io.Discard, func(int) { close(exited) }) + go exitAfterGrace(ctx, make(chan struct{}), func() {}, &stderr, func(int) { close(exited) }) select { case <-exited: @@ -703,6 +704,13 @@ func TestExitAfterGraceCountsFromTheSignal(t *testing.T) { case <-time.After(5 * time.Second): t.Fatal("never exited") } + // The message has a goroutine of its own; outliving this test, it would + // race with tests that swap output.IsTerminal. + for deadline := time.Now().Add(5 * time.Second); !strings.Contains(stderr.String(), "Stopped 2s after the signal"); time.Sleep(time.Millisecond) { + if time.Now().After(deadline) { + t.Fatalf("stderr = %q, want the watchdog's message", stderr.String()) + } + } } // --jq after a signal stops a quarter of interruptGrace before the process From 697f41c6e6b59f46b0a4511058b5a6184c1669a8 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 02:01:11 +0200 Subject: [PATCH 16/28] [Hints] Call a repeat counted when either flag surely makes it so With zero_trace=maybe and no_cache=true, the hint said the repeat "may" be counted, though no_cache=true makes it counted under either Mongoid reading of zero_trace. A flag that is surely on now wins over one that is unsure. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/edge_test.go | 3 +++ pkg/cmd/hint.go | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go index af372f3..8c29590 100644 --- a/pkg/cmd/edge_test.go +++ b/pkg/cmd/edge_test.go @@ -223,6 +223,9 @@ func edgeCases() []edgeCase { warn: "repeat the command with --jq '.KEY'; zero_trace=true stores nothing and makes the repeat a new, counted search"}, {name: "output/large zero_trace=0 keeps the archive", args: searchWith("zero_trace=0"), api: respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500)), warn: "read the stored search for free: serpapi archive abc123 --jq '.KEY'"}, + {name: "output/large unsure zero_trace with no_cache=true", args: searchWith("zero_trace=maybe", "no_cache=true"), + api: respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500)), + warn: "repeat the command with --jq '.KEY'; no_cache=true makes the repeat a new, counted search"}, {name: "output/large result with --jq is quiet", args: searchWith("--jq", ".organic_results | length"), api: respond(bigResult("", 1500)), stdout: "1500\n", quiet: true}, {name: "output/large result with --fields still hints", args: searchWith("--fields", "organic_results"), diff --git a/pkg/cmd/hint.go b/pkg/cmd/hint.go index b6990f7..671feab 100644 --- a/pkg/cmd/hint.go +++ b/pkg/cmd/hint.go @@ -126,10 +126,10 @@ func searchHint(p map[string]string) func(id string) string { switch { case zeroTrace == paramOn: hint += "; zero_trace=" + p["zero_trace"] + " stores nothing and makes the repeat a new, counted search" - case zeroTrace == paramUnsure: - hint += "; with zero_trace=" + p["zero_trace"] + " the repeat may be a new, counted search" case noCache == paramOn: hint += "; no_cache=" + p["no_cache"] + " makes the repeat a new, counted search" + case zeroTrace == paramUnsure: + hint += "; with zero_trace=" + p["zero_trace"] + " the repeat may be a new, counted search" case noCache == paramUnsure: hint += "; with no_cache=" + p["no_cache"] + " the repeat may be a new, counted search" default: From de6215bcb0f019be44e914cbd231adfaf0fb7347 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 02:01:37 +0200 Subject: [PATCH 17/28] [Docs] Say exit 130 means the run stopped before it finished A signal that lands after everything arrived and printed leaves exit 0: the output is complete, and 130 would tell a script to retry billed work. The README row and --help said 130 meant "stopped by Ctrl-C or SIGTERM", which a review read as any signal; they now say it stopped before it finished. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- pkg/cmd/root.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 3b51bfc..9bf506c 100644 --- a/README.md +++ b/README.md @@ -158,7 +158,7 @@ A config file that can't be read or isn't valid TOML is reported by path and lin | 2 | `usage_error` | The command is wrong; fix it before retrying. | | 1 | `api_error` | `status` is the HTTP status: 200 the search ran but reported a problem, such as no results; 400 invalid parameters; 401 invalid API key; 403 account not allowed; 429 rate limited or out of searches; 5xx SerpApi failed. No `status`: the response was unreadable. | | 1 | `network_error` | No answer, such as a timeout. | - | 130 | `interrupted` | Stopped by Ctrl-C or SIGTERM after printing what had arrived, such as the `--all-pages` pages fetched so far. | + | 130 | `interrupted` | Ctrl-C or SIGTERM stopped it before it finished, after printing what had arrived, such as the `--all-pages` pages fetched so far. | - **Cost**: each successful search counts against your monthly searches, including each `--all-pages` page; cached, errored and failed ones don't, though errored ones count toward the hourly limit ([pricing FAQ](https://serpapi.com/pricing)). An identical search, `--fields` included, repeated within an hour comes from the cache. `no_cache=true` forces a new, counted search; cached results are at most an hour old, so add it only when that is too old. `zero_trace=true` (Enterprise) stores nothing, so it skips the cache too and leaves no search for `archive` to read. `account`, `locations` and `archive` are free. - **No prompts without a terminal.** Only `serpapi login` reads stdin, and it waits for the first line, so pipe the key in (`printf %s "$KEY" | serpapi login`) or, simpler, set `SERPAPI_KEY`. diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 1401d89..c3e980b 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -79,8 +79,8 @@ limited or out of searches; 5xx SerpApi failed. No status means no usable answer: a network_error, such as a timeout, or an unreadable response. Requests time out after 60s: --timeout or SERPAPI_TIMEOUT sets seconds, 0 waits indefinitely. -Exit 130, code interrupted: stopped by Ctrl-C or SIGTERM, after printing what -had arrived. +Exit 130, code interrupted: Ctrl-C or SIGTERM stopped it before it finished, +after printing what had arrived. Cost: each successful search counts against your monthly searches, including each page of --all-pages; cached, errored and failed ones don't, though errored From d6ba7483a8bde80c8eb8cf2f32fc018028727d5a Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 02:31:27 +0200 Subject: [PATCH 18/28] [CI] Tag and release each merge to main once its tests pass v0.2.3 shipped on 2026-03-31; #7, #9, #15 and #17 merged after it and none reached users, because a release waited for someone to push a tag. Now, after both test legs and the live e2e pass on main, the tag job tags the commit with the next version and calls release.yml: a patch, or what the PR's release:minor or release:major label asks for. A release:skip label, or a merge where nothing that ships changed since the latest release, releases nothing. Outside pull requests a skipped live test fails CI, a published tag is never reused, a commit older than the latest release is never tagged, and runs on main go one at a time. A Release preview check tells each PR what merging releases. Dry run on this history: this branch with release:minor gives v0.3.0. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/release-tag.sh | 109 ++++++++++++++++++++++++++ .github/workflows/ci.yml | 63 ++++++++++++++- .github/workflows/release-preview.yml | 35 +++++++++ .github/workflows/release.yml | 22 +++++- AGENTS.md | 22 +++--- 5 files changed, 236 insertions(+), 15 deletions(-) create mode 100755 .github/scripts/release-tag.sh create mode 100644 .github/workflows/release-preview.yml diff --git a/.github/scripts/release-tag.sh b/.github/scripts/release-tag.sh new file mode 100755 index 0000000..a0c2988 --- /dev/null +++ b/.github/scripts/release-tag.sh @@ -0,0 +1,109 @@ +#!/usr/bin/env bash +# Decides whether a commit on main is released, and as which version, and +# tags it. ci.yml runs it after the tests pass on main; release-preview.yml +# runs it with DRY_RUN=true on a pull request's merge commit, to show what +# merging releases. +# +# Environment: +# SHA the commit +# BUMP auto (default), patch, minor or major. auto takes the bump from +# the pull request's release:major, release:minor or +# release:skip label, else patch, and releases only when +# something that ships changed since the latest release. +# LABELS_JSON the pull request's labels as a JSON array; looked up from the +# commit's merged pull request when unset +# DRY_RUN true to only report +# GITHUB_REPOSITORY, GH_TOKEN for the API +# Outputs, to GITHUB_OUTPUT: tag, the tag created for release.yml (empty when +# nothing is released); next, the version this commit gets or would get; +# reason, one line on what happened. +set -euo pipefail + +sha=$(git rev-parse --verify "${SHA:?SHA is required}^{commit}") +bump=${BUMP:-auto} +dry_run=${DRY_RUN:-false} + +output() { + if [ -n "${GITHUB_OUTPUT:-}" ]; then printf '%s=%s\n' "$1" "$2" >>"$GITHUB_OUTPUT"; fi +} + +finish() { + echo "$1" + output reason "$1" + exit 0 +} + +# Releases are vMAJOR.MINOR.PATCH; pre-release tags such as v1.0.0-rc.1 don't count. +release_tags() { git tag --list 'v*' | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | sort -V || true; } + +existing=$(git tag --points-at "$sha" | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | sort -V | tail -n 1 || true) +if [ -n "$existing" ]; then + output next "$existing" + finish "Not released again: $sha is already tagged $existing" +fi + +latest=$(release_tags | tail -n 1) +since=$(git hash-object -t tree /dev/null) +if [ -n "$latest" ]; then + since=$(git rev-list -n 1 "$latest") + if git merge-base --is-ancestor "$sha" "$since"; then + finish "Not released: $sha is older than $latest" + fi + # v0.2.1 to v0.2.3 were tagged on go-rewrite after it merged, so main + # doesn't contain them; comparing trees doesn't need it to. + if ! git merge-base --is-ancestor "$since" "$sha"; then + echo "$latest is on a commit that $sha doesn't contain; comparing their files" + fi +else + latest=v0.0.0 +fi + +if [ "$bump" = auto ]; then + if [ -n "${LABELS_JSON:-}" ]; then + labels=$(jq -r '.[]' <<<"$LABELS_JSON") + else + labels=$(gh api "repos/${GITHUB_REPOSITORY:?}/commits/$sha/pulls" \ + --jq '.[] | select(.merged_at != null) | .labels[].name') + fi + if grep -qx 'release:skip' <<<"$labels"; then + finish "Not released: the pull request has the release:skip label" + elif grep -qx 'release:major' <<<"$labels"; then + bump="major" + elif grep -qx 'release:minor' <<<"$labels"; then + bump="minor" + else + bump="patch" + fi + shipped=$(git diff --name-only "$since" "$sha" -- cmd pkg internal go.mod go.sum .goreleaser.yml \ + ':(exclude)*_test.go' ':(exclude)*/testdata/*') + if [ -z "$shipped" ]; then + finish "Not released: nothing that ships changed since $latest, only docs, tests or CI" + fi +fi + +IFS=. read -r major minor patch <<<"${latest#v}" +case $bump in +major) next="v$((major + 1)).0.0" ;; +minor) next="v$major.$((minor + 1)).0" ;; +patch) next="v$major.$minor.$((patch + 1))" ;; +*) + echo "BUMP must be auto, patch, minor or major, not $bump" >&2 + exit 2 + ;; +esac +output next "$next" + +if git rev-parse -q --verify "refs/tags/$next" >/dev/null; then + echo "::error::$next already exists on another commit; a published tag is never moved or reused" >&2 + exit 1 +fi + +if [ "$dry_run" = true ]; then + finish "Would release $next ($bump after $latest) at $sha" +fi + +# A lightweight tag, as the earlier releases have. The API refuses a tag that +# already exists, so two runs can't both make it. +gh api "repos/${GITHUB_REPOSITORY:?}/git/refs" -f ref="refs/tags/$next" -f sha="$sha" >/dev/null +output tag "$next" +finish "Tagged $sha as $next ($bump after $latest)" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f2d9c2..f6d0110 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,9 +5,26 @@ on: branches: [main, develop] pull_request: branches: [main] + workflow_dispatch: + inputs: + bump: + description: "Version for main's head: auto follows the merged PR's release:* label and releases only when something that ships changed" + type: choice + options: [auto, patch, minor, major] + default: auto + dry_run: + description: Only report what would be released + type: boolean + default: true permissions: {} +# Runs on main tag and release one at a time, in order; a newer run replaces a +# waiting one, whose commits it contains. Other runs never wait. +concurrency: + group: ${{ github.ref == 'refs/heads/main' && 'release-main' || github.run_id }} + cancel-in-progress: false + jobs: test: runs-on: ubuntu-latest @@ -39,7 +56,51 @@ jobs: go-version: stable check-latest: true # Fork PRs get no secrets, so only the offline contract tests run there. + # Anywhere else a skipped live test fails the job, since main's runs + # release when they pass. - name: Run E2E tests + shell: bash env: SERPAPI_KEY: ${{ secrets.SERPAPI_KEY }} - run: go test -v ./tests/... + REQUIRE_LIVE: ${{ github.event_name != 'pull_request' }} + run: | + go test -v ./tests/... | tee e2e.log + if [ "$REQUIRE_LIVE" = true ] && grep -q -- '--- SKIP' e2e.log; then + echo "::error::Live e2e tests were skipped: is the SERPAPI_KEY secret set?" + exit 1 + fi + + # After the tests pass on main, release-tag.sh tags the commit with the next + # version, and release.yml releases it. A tag pushed with GITHUB_TOKEN + # doesn't start release.yml's tag trigger, so this run calls it. + tag: + needs: [test, e2e] + if: github.ref == 'refs/heads/main' && github.event_name != 'pull_request' + runs-on: ubuntu-latest + permissions: + contents: write + pull-requests: read + outputs: + tag: ${{ steps.tag.outputs.tag }} + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + fetch-depth: 0 + persist-credentials: false + - id: tag + env: + GH_TOKEN: ${{ github.token }} + SHA: ${{ github.sha }} + BUMP: ${{ inputs.bump || 'auto' }} + DRY_RUN: ${{ inputs.dry_run || false }} + run: bash .github/scripts/release-tag.sh + + release: + needs: tag + if: needs.tag.outputs.tag != '' + permissions: + contents: write + uses: ./.github/workflows/release.yml + with: + tag: ${{ needs.tag.outputs.tag }} + secrets: inherit diff --git a/.github/workflows/release-preview.yml b/.github/workflows/release-preview.yml new file mode 100644 index 0000000..b004644 --- /dev/null +++ b/.github/workflows/release-preview.yml @@ -0,0 +1,35 @@ +name: Release preview + +# Says on each pull request what merging it releases, so a missing +# release:minor label shows up before the merge. Reads nothing secret. +on: + pull_request: + branches: [main] + types: [opened, synchronize, reopened, labeled, unlabeled] + +permissions: {} + +jobs: + preview: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + fetch-depth: 0 + persist-credentials: false + - id: tag + env: + SHA: ${{ github.sha }} + LABELS_JSON: ${{ toJSON(github.event.pull_request.labels.*.name) }} + DRY_RUN: "true" + run: bash .github/scripts/release-tag.sh + - env: + REASON: ${{ steps.tag.outputs.reason }} + NEXT: ${{ steps.tag.outputs.next }} + run: | + notes="none: GoReleaser will list the commits" + if [ -n "$NEXT" ] && [ -f ".github/release-notes/$NEXT.md" ]; then notes=".github/release-notes/$NEXT.md"; fi + echo "::notice title=Merging releases::$REASON; release notes: $notes" + printf '### Merging this pull request\n\n%s\n\nRelease notes: %s\n' "$REASON" "$notes" >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 30d63e7..45876d1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -4,6 +4,13 @@ on: push: tags: - "v*" + # ci.yml calls this after it tags a commit on main. + workflow_call: + inputs: + tag: + description: The tag to release + type: string + required: true permissions: {} @@ -12,9 +19,12 @@ jobs: runs-on: ubuntu-latest permissions: contents: write + env: + TAG: ${{ inputs.tag || github.ref_name }} steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: + ref: ${{ env.TAG }} fetch-depth: 0 - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: @@ -22,12 +32,20 @@ jobs: # release binaries need the latest patched Go (v0.2.3 shipped go1.25.0). go-version: stable check-latest: true + # .github/release-notes/vX.Y.Z.md, when the release has one, replaces + # GoReleaser's list of commits. + - id: notes + run: | + if [ -f ".github/release-notes/$TAG.md" ]; then + echo "args=--release-notes .github/release-notes/$TAG.md" >> "$GITHUB_OUTPUT" + fi - uses: goreleaser/goreleaser-action@9ed2f89a662bf1735a48bc8557fd212fa902bebf # v6.2.1 with: version: ~> v2 - args: release --clean + args: release --clean ${{ steps.notes.outputs.args }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GORELEASER_CURRENT_TAG: ${{ env.TAG }} homebrew: runs-on: ubuntu-latest @@ -35,7 +53,7 @@ jobs: steps: - name: Update Homebrew formula env: - TAG: ${{ github.ref_name }} + TAG: ${{ inputs.tag || github.ref_name }} HOMEBREW_TAP_DEPLOY_KEY: ${{ secrets.HOMEBREW_TAP_DEPLOY_KEY }} run: | VERSION="${TAG#v}" diff --git a/AGENTS.md b/AGENTS.md index 28c7aa5..4c60697 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,20 +2,18 @@ ## Releases -- A push or merge to `main` releases nothing. It runs CI (`.github/workflows/ci.yml`), whose e2e job makes real searches with the `SERPAPI_KEY` secret. -- Only a pushed `v*` tag releases. `.github/workflows/release.yml` then: - 1. runs GoReleaser (`.goreleaser.yml`), which publishes a GitHub release with 5 archives (darwin and linux on amd64 and arm64, windows on amd64) and `serpapi__checksums.txt`, and stamps the version into `serpapi --version`; +- A merge to `main` releases the CLI once CI passes on the merge commit, live e2e included; outside pull requests a skipped live test fails CI, so a release never rests on skipped tests. The CI `tag` job runs `.github/scripts/release-tag.sh`, which tags the commit with the next version, then calls `.github/workflows/release.yml` for that tag. +- The next version is a patch after the highest `vX.Y.Z` tag. Label the pull request `release:minor` or `release:major` for those, or `release:skip` to release nothing. A merge releases nothing either when nothing that ships changed since the latest release: `cmd/`, `pkg/`, `go.mod`, `go.sum` and `.goreleaser.yml`, tests excluded. The "Release preview" check on each pull request says what merging it releases. +- A tag reaches every Homebrew and `go install` user, so merging to `main` is the release decision: label the pull request first. +- `release.yml`: + 1. runs GoReleaser (`.goreleaser.yml`), which publishes a GitHub release with 5 archives (darwin and linux on amd64 and arm64, windows on amd64) and `serpapi__checksums.txt`, and stamps the version into `serpapi --version`. `.github/release-notes/.md`, when it exists, is the release body; otherwise GoReleaser lists the commits; 2. writes `Formula/serpapi-cli.rb` in `serpapi/homebrew-tap` from those checksums and pushes it as a `serpapi-cli ` commit. -- A tag reaches every Homebrew and `go install` user, so tagging is a maintainer's call. Tag the merge commit on `main` once its CI has passed, live e2e included: the live tests skip without `SERPAPI_KEY`, so the e2e log must have no `--- SKIP` line. Tags here are lightweight: - ```bash - git fetch origin - git tag v0.3.0 - git rev-list -n1 v0.3.0 # must print that sha - git push origin v0.3.0 - ``` +- To release by hand, run the CI workflow on `main` from the Actions tab with a bump (`dry_run` is on by default), or push a lightweight `v*` tag on a `main` commit whose CI passed with no `--- SKIP` in the e2e log; `release.yml` runs on the pushed tag. +- If a release job fails after the tag was made, re-run the failed jobs of that CI run; the tag stays. - Never move, delete or reuse a published tag: Go's module proxy and checksum database keep the first content they saw for each version, so a moved tag never reaches most `go install` users and fails checksum verification for the rest. -- The release body is GoReleaser's list of commits. To publish written notes instead, run `gh release edit v0.3.0 -R serpapi/serpapi-cli --notes-file ` after the Release run. +- v0.2.1 to v0.2.3 are tagged on `go-rewrite`, which `main` doesn't contain; `release-tag.sh` compares files with the latest release, not history. +- To change a published release's notes: `gh release edit v0.3.0 -R serpapi/serpapi-cli --notes-file `. - Release binaries build with the latest stable Go (`go-version: stable`); the `go` line in `go.mod` is the oldest Go that `go install` users need. - The formula's `test do` block in `release.yml` runs the released binary, e.g. `serpapi account` without a key must exit 2 with "No API key". Change it together with the behavior it checks. -- Check a release: the Release run is green, the release has the 5 archives and the checksums file, the tap has the `serpapi-cli ` commit, and `brew upgrade serpapi-cli && brew test serpapi-cli` passes. +- Check a release: the CI run's `tag` and `release` jobs (or, for a pushed tag, the Release run) are green, the release has the 5 archives and the checksums file, the tap has the `serpapi-cli ` commit, and `brew upgrade serpapi-cli && brew test serpapi-cli` passes. - To pull a bad release, revert the formula commit in the tap so new installs get the previous version, and fix forward: release the next patch version with the bad one retracted in its `go.mod` (`retract v0.3.0` in v0.3.1), since `go install ...@latest` takes the highest tag that isn't retracted. From 006a650fa560c09eac791be4442c2bc884af23d1 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 02:31:27 +0200 Subject: [PATCH 19/28] [Release] Add v0.3.0's notes for the automated release release.yml uses .github/release-notes/.md as the release body when it exists, so the v0.3.0 release gets these notes instead of GoReleaser's list of commits. Co-Authored-By: Claude Opus 5.5 --- .github/release-notes/v0.3.0.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .github/release-notes/v0.3.0.md diff --git a/.github/release-notes/v0.3.0.md b/.github/release-notes/v0.3.0.md new file mode 100644 index 0000000..1be348e --- /dev/null +++ b/.github/release-notes/v0.3.0.md @@ -0,0 +1,28 @@ +serpapi is now predictable for scripts and AI agents, and safe on a terminal. + +- **One error contract.** Errors and warnings are JSON lines on stderr with the HTTP status. Exit 2 means fix the command; it fails before a billed search, except when a `--jq` filter fails on the response itself; 1 is an API or network error; 130 means Ctrl-C or SIGTERM stopped it before it finished, after printing what had arrived. +- **Markdown and other formats.** `output=md`, `html`, `plain` and `csv`, in any case, work for `search` and `archive` (#10). +- **Timeouts.** `--timeout` takes seconds, such as `120`, or a duration, such as `2m`; `SERPAPI_TIMEOUT` sets it for a shell; `0` waits indefinitely. The default is 60 s (#15, @strzibny). +- **`--all-pages`.** `--fields` no longer stops paging after page 1 (#9, @adarshdigievo). `no_cache`, `zero_trace` and `output` apply to every page. Paging stops at a repeated page or at a page a mistyped `--fields` matches nothing on. Pages already fetched are printed, through `--jq` too, when a later page fails or on Ctrl-C. Output streams, so 100 pages of 1 MiB peak at 187–188 MiB instead of 720–735 MiB (3 runs each against v0.2.3 on macOS arm64, from a local fake API). +- **Warnings that name the fix.** A large result lists its top-level keys by size and the free `serpapi archive ID --jq` next step, or says when `no_cache` or `zero_trace` makes a repeat a new, counted search. A `--jq` that prints only null lists the keys the response has. A parameter given twice says which value was sent. +- **Safer.** Terminal output escapes control characters from results and from the account email that `login` prints, so a page title can't set the clipboard or clear the screen. `serpapi account` masks the API key (#7, @ilyazub), and the key is never echoed from a stray argument or a broken config file. `login` saves the key only under an absolute HOME, through a new private temp file, so a file left at the old temp name can't widen its permissions or receive the key. Ctrl-C or SIGTERM at its prompts stops it at once with echo on. +- **Also:** `serpapi version`; `login --api-key`; `login` asks before replacing a saved key (#7); `SERPAPI_API_KEY` as a fallback for `SERPAPI_KEY`; `--help` documents exit codes, errors and costs. +- **Patched Go.** Binaries are built with the latest patched Go at release time; `go version -m "$(command -v serpapi)"` shows which. v0.2.3's binaries were built with go1.25.0, in which `govulncheck -mode=binary` finds 57 (darwin/arm64) and 58 (linux/amd64) known vulnerabilities; a release rehearsal with go1.27.2 found none. + +## Changes scripts may notice + +- These now exit 2 before any request: `q=` with bare words, no parameters, the API key passed as `key`, `apikey`, `serp_api_key` or `serpapi_key` (`api_key` already did), `--fields` outside `search`, `--max-pages 0`, `--all-pages` with `async=true`, `--jq` or `--all-pages` with `output=md`, an unknown `output=`, and `locations` without a query. +- A `--jq` filter that can't run, such as a syntax error, `input` or `debug`, fails before the search; one that fails on the response, such as `tonumber` on a title, still exits 2 after it. `halt` now works. +- Bare words are joined into one `q`: `serpapi search engine=google coffee shops` sends `q=coffee shops`, not `q=shops`. +- `--fields` adds `error` to `json_restrictor`, so an API error still arrives. `--fields ,,` sends no `json_restrictor`; it used to send `,,` and print `{}`. +- A `SERPAPI_KEY` padded with spaces or a newline is trimmed before it's sent. +- `serpapi account` prints `api_key` masked, as `abcd…wxyz` (#7). +- `--all-pages`: when a later page fails, or `serpapi_pagination.next` isn't a valid URL, the pages so far are printed before the error, exit 1; that invalid URL is now `api_error`, not `network_error`. A 200 response with an error on a later page is a warning after the pages so far, exit 0, not exit 1. +- A JSON array body is exit 1 `api_error`, not exit 0. +- `serpapi` with no command prints nothing on stdout, where its usage used to go; stderr has the JSON error, and the usage too on a terminal. It still exits 2. +- `serpapi help TOPIC` with an unknown topic exits 2 with a JSON `usage_error` naming the closest command; it exited 0 with the usage on stderr. +- `serpapi login` exits 2 when HOME (`%AppData%` on Windows) isn't an absolute path, and no command reads a config file then; v0.2.3 used `.config/serpapi` under the current directory. +- At the hidden key prompt, Ctrl-D exits 2 with "No API key given" (v0.2.3 kept waiting), and SIGTERM exits 130 with echo on (v0.2.3: 143, echo off). +- The default timeout is 60 s, not 30 s. + +Thanks to @adarshdigievo (#4, #9), @jvmvik (#6), @strzibny (#15) and @ilyazub (#7). From a7a032f2880b3adc4d3f9401c38fe1bd7be3a0bd Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 02:33:17 +0200 Subject: [PATCH 20/28] [CI] Let maintainers release an existing tag again from Actions If the first automated release fails after its tag is made, re-running the CI run's failed jobs repeats the same workflow. A workflow_dispatch with a tag input runs release.yml from main for that tag once the cause is fixed, without moving or re-pushing the tag. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 7 +++++++ AGENTS.md | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 45876d1..4026909 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -11,6 +11,13 @@ on: description: The tag to release type: string required: true + # To release an existing tag again, e.g. after a failed run. + workflow_dispatch: + inputs: + tag: + description: An existing v* tag to release + type: string + required: true permissions: {} diff --git a/AGENTS.md b/AGENTS.md index 4c60697..22eaa11 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,7 +9,7 @@ 1. runs GoReleaser (`.goreleaser.yml`), which publishes a GitHub release with 5 archives (darwin and linux on amd64 and arm64, windows on amd64) and `serpapi__checksums.txt`, and stamps the version into `serpapi --version`. `.github/release-notes/.md`, when it exists, is the release body; otherwise GoReleaser lists the commits; 2. writes `Formula/serpapi-cli.rb` in `serpapi/homebrew-tap` from those checksums and pushes it as a `serpapi-cli ` commit. - To release by hand, run the CI workflow on `main` from the Actions tab with a bump (`dry_run` is on by default), or push a lightweight `v*` tag on a `main` commit whose CI passed with no `--- SKIP` in the e2e log; `release.yml` runs on the pushed tag. -- If a release job fails after the tag was made, re-run the failed jobs of that CI run; the tag stays. +- If a release job fails after the tag was made, re-run the failed jobs of that CI run, or, once the cause is fixed on `main`, run the Release workflow from the Actions tab with that tag. The tag stays. - Never move, delete or reuse a published tag: Go's module proxy and checksum database keep the first content they saw for each version, so a moved tag never reaches most `go install` users and fails checksum verification for the rest. - v0.2.1 to v0.2.3 are tagged on `go-rewrite`, which `main` doesn't contain; `release-tag.sh` compares files with the latest release, not history. - To change a published release's notes: `gh release edit v0.3.0 -R serpapi/serpapi-cli --notes-file `. From 783655523cad5c1c01ccee98f0b22f2ac9b84031 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 02:34:15 +0200 Subject: [PATCH 21/28] [CI] Check the release tag before it reaches checkout or GoReleaser The Release workflow now takes a tag from a dispatch input, which went straight into actions/checkout's ref, a path and GoReleaser's arguments. It must now look like vX.Y.Z, with an optional pre-release suffix: a branch name, a path or "v0.3.0 --skip=validate" stops the run. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4026909..642d777 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -29,6 +29,14 @@ jobs: env: TAG: ${{ inputs.tag || github.ref_name }} steps: + # TAG may come from a dispatch input; check it before it reaches a ref, + # a path or GoReleaser's arguments. + - name: Check the tag + run: | + if ! [[ $TAG =~ ^v[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z.-]+)?$ ]]; then + echo "::error::$TAG is not a vX.Y.Z tag" + exit 1 + fi - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ env.TAG }} From 6671733848d1d8a1af9f74b7a71a9d2bdbfcfeb9 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 02:58:30 +0200 Subject: [PATCH 22/28] [CI] Bump by every pull request merged since the last release Only the merged pull request's label set the bump, so a release:major merge whose run failed was released by the next, unlabeled merge as a patch. Now a release:minor or release:major label on any pull request merged since the latest release counts; release:skip still applies only to the merge being released. On this history the lookup asks for 7 commits' pull requests, #2 to #15, none labeled. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/release-tag.sh | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/.github/scripts/release-tag.sh b/.github/scripts/release-tag.sh index a0c2988..d80cf92 100755 --- a/.github/scripts/release-tag.sh +++ b/.github/scripts/release-tag.sh @@ -6,10 +6,12 @@ # # Environment: # SHA the commit -# BUMP auto (default), patch, minor or major. auto takes the bump from -# the pull request's release:major, release:minor or -# release:skip label, else patch, and releases only when -# something that ships changed since the latest release. +# BUMP auto (default), patch, minor or major. auto bumps minor or +# major when any pull request merged since the latest release +# has the release:minor or release:major label, else patch; +# releases nothing when the merged pull request has +# release:skip; and releases only when something that ships +# changed since the latest release. # LABELS_JSON the pull request's labels as a JSON array; looked up from the # commit's merged pull request when unset # DRY_RUN true to only report @@ -42,8 +44,13 @@ if [ -n "$existing" ]; then finish "Not released again: $sha is already tagged $existing" fi +pr_labels() { + gh api "repos/${GITHUB_REPOSITORY:?}/commits/$1/pulls" --jq '.[] | select(.merged_at != null) | .labels[].name' +} + latest=$(release_tags | tail -n 1) since=$(git hash-object -t tree /dev/null) +earlier="" if [ -n "$latest" ]; then since=$(git rev-list -n 1 "$latest") if git merge-base --is-ancestor "$sha" "$since"; then @@ -54,6 +61,9 @@ if [ -n "$latest" ]; then if ! git merge-base --is-ancestor "$since" "$sha"; then echo "$latest is on a commit that $sha doesn't contain; comparing their files" fi + if git rev-parse -q --verify "$sha^" >/dev/null; then + earlier=$(git rev-list --first-parent "$sha^" --not "$since") + fi else latest=v0.0.0 fi @@ -62,12 +72,17 @@ if [ "$bump" = auto ]; then if [ -n "${LABELS_JSON:-}" ]; then labels=$(jq -r '.[]' <<<"$LABELS_JSON") else - labels=$(gh api "repos/${GITHUB_REPOSITORY:?}/commits/$sha/pulls" \ - --jq '.[] | select(.merged_at != null) | .labels[].name') + labels=$(pr_labels "$sha") fi if grep -qx 'release:skip' <<<"$labels"; then finish "Not released: the pull request has the release:skip label" - elif grep -qx 'release:major' <<<"$labels"; then + fi + # A release:major or release:minor pull request merged since the latest + # release counts too: its own run may have failed. + while read -r c; do + if [ -n "$c" ]; then labels+=$'\n'$(pr_labels "$c"); fi + done <<<"$earlier" + if grep -qx 'release:major' <<<"$labels"; then bump="major" elif grep -qx 'release:minor' <<<"$labels"; then bump="minor" From bd29dc5f24c04b25e2637bb00929fa099f24514b Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 02:58:30 +0200 Subject: [PATCH 23/28] [CI] Release only the tested commit, one at a time, never backwards From the review of the auto-release: CI passes the tested SHA, and the Release workflow stops if the tag points elsewhere, isn't a tag, or is older than the newest release, which GoReleaser would mark latest and Homebrew would roll back to. Runs on main and every publish wait their turn (concurrency queue: max) instead of a newer one replacing a waiting one. A re-run after a partial upload replaces the assets (replace_existing_artifacts) instead of failing on them. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 6 ++++-- .github/workflows/release.yml | 31 +++++++++++++++++++++++++++++++ .goreleaser.yml | 5 +++++ AGENTS.md | 3 ++- 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f6d0110..2392a22 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,11 +19,12 @@ on: permissions: {} -# Runs on main tag and release one at a time, in order; a newer run replaces a -# waiting one, whose commits it contains. Other runs never wait. +# Runs on main test, tag and release one at a time, in order; each waits its +# turn rather than replacing a waiting one. Other runs never wait. concurrency: group: ${{ github.ref == 'refs/heads/main' && 'release-main' || github.run_id }} cancel-in-progress: false + queue: max jobs: test: @@ -103,4 +104,5 @@ jobs: uses: ./.github/workflows/release.yml with: tag: ${{ needs.tag.outputs.tag }} + sha: ${{ github.sha }} secrets: inherit diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 642d777..31871fd 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -11,6 +11,10 @@ on: description: The tag to release type: string required: true + sha: + description: The tested commit the tag must point at + type: string + required: true # To release an existing tag again, e.g. after a failed run. workflow_dispatch: inputs: @@ -26,8 +30,15 @@ jobs: runs-on: ubuntu-latest permissions: contents: write + # Publishing waits for any other release, from CI, a tag push or a + # dispatch, so their uploads and Homebrew pushes never interleave. + concurrency: + group: release-publish + cancel-in-progress: false + queue: max env: TAG: ${{ inputs.tag || github.ref_name }} + SHA: ${{ inputs.sha || (github.event_name == 'push' && github.sha) || '' }} steps: # TAG may come from a dispatch input; check it before it reaches a ref, # a path or GoReleaser's arguments. @@ -41,6 +52,22 @@ jobs: with: ref: ${{ env.TAG }} fetch-depth: 0 + - name: Check what the tag points at + run: | + if ! git rev-parse -q --verify "refs/tags/$TAG" >/dev/null; then + echo "::error::$TAG is not a tag" + exit 1 + fi + at=$(git rev-parse "$TAG^{commit}") + if [ -n "$SHA" ] && [ "$at" != "$SHA" ]; then + echo "::error::$TAG points at $at, not at the tested $SHA" + exit 1 + fi + newest=$(git tag --list 'v*' | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | sort -V | tail -n 1) + if [ "$(printf '%s\n%s\n' "$newest" "$TAG" | sort -V | tail -n 1)" != "$TAG" ]; then + echo "::error::$newest is newer than $TAG; releasing $TAG would mark it latest and point Homebrew back at it" + exit 1 + fi - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: # Not go.mod: its go line is the minimum for go install users, while @@ -65,6 +92,10 @@ jobs: homebrew: runs-on: ubuntu-latest needs: goreleaser + concurrency: + group: release-publish + cancel-in-progress: false + queue: max steps: - name: Update Homebrew formula env: diff --git a/.goreleaser.yml b/.goreleaser.yml index b96ff76..ee1efac 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -34,6 +34,11 @@ archives: checksum: name_template: "serpapi_{{.Version}}_checksums.txt" +release: + # A re-run for the same tag, after a partial upload, replaces the assets + # already there instead of failing on them. + replace_existing_artifacts: true + snapshot: version_template: "{{.Version}}-next" diff --git a/AGENTS.md b/AGENTS.md index 22eaa11..aecdee9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -3,13 +3,14 @@ ## Releases - A merge to `main` releases the CLI once CI passes on the merge commit, live e2e included; outside pull requests a skipped live test fails CI, so a release never rests on skipped tests. The CI `tag` job runs `.github/scripts/release-tag.sh`, which tags the commit with the next version, then calls `.github/workflows/release.yml` for that tag. -- The next version is a patch after the highest `vX.Y.Z` tag. Label the pull request `release:minor` or `release:major` for those, or `release:skip` to release nothing. A merge releases nothing either when nothing that ships changed since the latest release: `cmd/`, `pkg/`, `go.mod`, `go.sum` and `.goreleaser.yml`, tests excluded. The "Release preview" check on each pull request says what merging it releases. +- The next version is a patch after the highest `vX.Y.Z` tag. Label the pull request `release:minor` or `release:major` for those; the label counts for every later merge until a release goes out, so a failed release run doesn't lose it. `release:skip` releases nothing for that merge. A merge releases nothing either when nothing that ships changed since the latest release: `cmd/`, `pkg/`, `go.mod`, `go.sum` and `.goreleaser.yml`, tests excluded. The "Release preview" check on each pull request says what merging it releases. - A tag reaches every Homebrew and `go install` user, so merging to `main` is the release decision: label the pull request first. - `release.yml`: 1. runs GoReleaser (`.goreleaser.yml`), which publishes a GitHub release with 5 archives (darwin and linux on amd64 and arm64, windows on amd64) and `serpapi__checksums.txt`, and stamps the version into `serpapi --version`. `.github/release-notes/.md`, when it exists, is the release body; otherwise GoReleaser lists the commits; 2. writes `Formula/serpapi-cli.rb` in `serpapi/homebrew-tap` from those checksums and pushes it as a `serpapi-cli ` commit. - To release by hand, run the CI workflow on `main` from the Actions tab with a bump (`dry_run` is on by default), or push a lightweight `v*` tag on a `main` commit whose CI passed with no `--- SKIP` in the e2e log; `release.yml` runs on the pushed tag. - If a release job fails after the tag was made, re-run the failed jobs of that CI run, or, once the cause is fixed on `main`, run the Release workflow from the Actions tab with that tag. The tag stays. +- `release.yml` refuses a tag that doesn't point at the tested commit or is older than the newest release (it would become "latest" and roll Homebrew back), and publishes one release at a time. - Never move, delete or reuse a published tag: Go's module proxy and checksum database keep the first content they saw for each version, so a moved tag never reaches most `go install` users and fails checksum verification for the rest. - v0.2.1 to v0.2.3 are tagged on `go-rewrite`, which `main` doesn't contain; `release-tag.sh` compares files with the latest release, not history. - To change a published release's notes: `gh release edit v0.3.0 -R serpapi/serpapi-cli --notes-file `. From 2344e90cfcb4a689949af7c6f6342885481d4bcc Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 03:11:10 +0200 Subject: [PATCH 24/28] [CI] Count a release label only from a pull request that ships A docs-only pull request labeled release:major releases nothing, but its label then turned the next unlabeled code merge into a major release. An earlier pull request's release:minor or release:major label now counts only if its own change touched what ships. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/release-tag.sh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/.github/scripts/release-tag.sh b/.github/scripts/release-tag.sh index d80cf92..c4f769d 100755 --- a/.github/scripts/release-tag.sh +++ b/.github/scripts/release-tag.sh @@ -48,6 +48,12 @@ pr_labels() { gh api "repos/${GITHUB_REPOSITORY:?}/commits/$1/pulls" --jq '.[] | select(.merged_at != null) | .labels[].name' } +# The files between two commits that ship in the binary, tests excluded. +shipped() { + git diff --name-only "$1" "$2" -- cmd pkg internal go.mod go.sum .goreleaser.yml \ + ':(exclude)*_test.go' ':(exclude)*/testdata/*' +} + latest=$(release_tags | tail -n 1) since=$(git hash-object -t tree /dev/null) earlier="" @@ -78,9 +84,10 @@ if [ "$bump" = auto ]; then finish "Not released: the pull request has the release:skip label" fi # A release:major or release:minor pull request merged since the latest - # release counts too: its own run may have failed. + # release counts too, since its own run may have failed, but only if it + # changed what ships: a docs-only one released nothing. while read -r c; do - if [ -n "$c" ]; then labels+=$'\n'$(pr_labels "$c"); fi + if [ -n "$c" ] && [ -n "$(shipped "$c^" "$c")" ]; then labels+=$'\n'$(pr_labels "$c"); fi done <<<"$earlier" if grep -qx 'release:major' <<<"$labels"; then bump="major" @@ -89,9 +96,7 @@ if [ "$bump" = auto ]; then else bump="patch" fi - shipped=$(git diff --name-only "$since" "$sha" -- cmd pkg internal go.mod go.sum .goreleaser.yml \ - ':(exclude)*_test.go' ':(exclude)*/testdata/*') - if [ -z "$shipped" ]; then + if [ -z "$(shipped "$since" "$sha")" ]; then finish "Not released: nothing that ships changed since $latest, only docs, tests or CI" fi fi From 0f890c0c372c6e629965faeabc4caf0af2b36288 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 03:11:10 +0200 Subject: [PATCH 25/28] [CI] Give the release preview a token for its label lookups The preview looks up the labels of pull requests merged since the last release through gh, which needs a token in Actions; without one it failed whenever main had such merges, as it has now. It gets the run's read-only token and pull-requests: read. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release-preview.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/release-preview.yml b/.github/workflows/release-preview.yml index b004644..f3ba4a0 100644 --- a/.github/workflows/release-preview.yml +++ b/.github/workflows/release-preview.yml @@ -14,6 +14,7 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + pull-requests: read steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: @@ -21,6 +22,8 @@ jobs: persist-credentials: false - id: tag env: + # gh needs a token in Actions; this one only reads. + GH_TOKEN: ${{ github.token }} SHA: ${{ github.sha }} LABELS_JSON: ${{ toJSON(github.event.pull_request.labels.*.name) }} DRY_RUN: "true" From 6faf9f7d1539dda2dbdc9b41a340523ebec1260c Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 03:11:10 +0200 Subject: [PATCH 26/28] [CI] Never build a complete release again bd29dc5's replace_existing_artifacts let a re-run replace a published release's binaries, built with whatever Go is newest then, under the same version. It is gone. Release now skips GoReleaser when the tag's release has every file its checksums list, so a re-run only refreshes Homebrew, and stops on a partial release until its assets are deleted. On this repo's releases: v0.2.3 and v0.2.2 skip, v9.9.9 builds. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 22 ++++++++++++++++++++++ .goreleaser.yml | 5 ----- AGENTS.md | 2 +- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 31871fd..76b95f3 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -68,6 +68,27 @@ jobs: echo "::error::$newest is newer than $TAG; releasing $TAG would mark it latest and point Homebrew back at it" exit 1 fi + # A complete release is never built again: a newer Go would change the + # binaries users already have under that version. A partial one, from a + # failed upload, needs its assets deleted first. + - id: existing + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + run: | + if ! gh release view "$TAG" >/dev/null 2>&1; then + exit 0 + fi + sums=$(gh release download "$TAG" --pattern 'serpapi_*_checksums.txt' --output - 2>/dev/null || true) + assets=$(gh release view "$TAG" --json assets --jq '.assets[].name') + missing=$(awk '{print $2}' <<<"$sums" | grep -vxF -e "$assets" || true) + if [ -n "$sums" ] && [ -z "$missing" ]; then + echo "skip=true" >> "$GITHUB_OUTPUT" + echo "::notice::$TAG is already released with every file in its checksums; not building it again" + else + echo "::error::$TAG has an incomplete release; delete its assets with gh release delete-asset, then run this again" + exit 1 + fi - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: # Not go.mod: its go line is the minimum for go install users, while @@ -82,6 +103,7 @@ jobs: echo "args=--release-notes .github/release-notes/$TAG.md" >> "$GITHUB_OUTPUT" fi - uses: goreleaser/goreleaser-action@9ed2f89a662bf1735a48bc8557fd212fa902bebf # v6.2.1 + if: steps.existing.outputs.skip != 'true' with: version: ~> v2 args: release --clean ${{ steps.notes.outputs.args }} diff --git a/.goreleaser.yml b/.goreleaser.yml index ee1efac..b96ff76 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -34,11 +34,6 @@ archives: checksum: name_template: "serpapi_{{.Version}}_checksums.txt" -release: - # A re-run for the same tag, after a partial upload, replaces the assets - # already there instead of failing on them. - replace_existing_artifacts: true - snapshot: version_template: "{{.Version}}-next" diff --git a/AGENTS.md b/AGENTS.md index aecdee9..c1ca44f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,7 @@ 2. writes `Formula/serpapi-cli.rb` in `serpapi/homebrew-tap` from those checksums and pushes it as a `serpapi-cli ` commit. - To release by hand, run the CI workflow on `main` from the Actions tab with a bump (`dry_run` is on by default), or push a lightweight `v*` tag on a `main` commit whose CI passed with no `--- SKIP` in the e2e log; `release.yml` runs on the pushed tag. - If a release job fails after the tag was made, re-run the failed jobs of that CI run, or, once the cause is fixed on `main`, run the Release workflow from the Actions tab with that tag. The tag stays. -- `release.yml` refuses a tag that doesn't point at the tested commit or is older than the newest release (it would become "latest" and roll Homebrew back), and publishes one release at a time. +- `release.yml` refuses a tag that doesn't point at the tested commit or is older than the newest release (it would become "latest" and roll Homebrew back), and publishes one release at a time. It never builds a complete release again, since a newer Go would change binaries users already have under that version: a re-run only refreshes Homebrew. A partial release, from a failed upload, stops it until its assets are deleted with `gh release delete-asset`. - Never move, delete or reuse a published tag: Go's module proxy and checksum database keep the first content they saw for each version, so a moved tag never reaches most `go install` users and fails checksum verification for the rest. - v0.2.1 to v0.2.3 are tagged on `go-rewrite`, which `main` doesn't contain; `release-tag.sh` compares files with the latest release, not history. - To change a published release's notes: `gh release edit v0.3.0 -R serpapi/serpapi-cli --notes-file `. From 90ced260c77327c653f7a8878ac25c718d1adca2 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 03:21:25 +0200 Subject: [PATCH 27/28] [CI] Publish in one locked job from the exact tag, never rebuilding From the advisor's review of the auto-release. Checkout takes refs/tags/TAG, so a branch with the same name can't stand in for it, and HEAD must equal the tag's commit. Building, uploading and the Homebrew update share one job and one lock, so releases can't interleave. Only a confirmed 404 means "no release yet": an API or network error stops the run. A partial release is never rebuilt with a newer Go; the next version fixes it forward. Checked against this repo's releases: v0.2.3 and v0.2.2 skip, v9.9.9 builds, v0.1.1 (Rust-era assets) stops, an unreachable API stops. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 41 +++++++++++++++-------------------- AGENTS.md | 3 ++- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 76b95f3..f662379 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -26,12 +26,12 @@ on: permissions: {} jobs: - goreleaser: + # One job builds, uploads and updates Homebrew while holding the lock, so + # releases from CI, a tag push or a dispatch never interleave. + publish: runs-on: ubuntu-latest permissions: contents: write - # Publishing waits for any other release, from CI, a tag push or a - # dispatch, so their uploads and Homebrew pushes never interleave. concurrency: group: release-publish cancel-in-progress: false @@ -50,7 +50,7 @@ jobs: fi - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: - ref: ${{ env.TAG }} + ref: refs/tags/${{ env.TAG }} fetch-depth: 0 - name: Check what the tag points at run: | @@ -59,6 +59,10 @@ jobs: exit 1 fi at=$(git rev-parse "$TAG^{commit}") + if [ "$(git rev-parse HEAD)" != "$at" ]; then + echo "::error::checked out $(git rev-parse HEAD), not $TAG at $at" + exit 1 + fi if [ -n "$SHA" ] && [ "$at" != "$SHA" ]; then echo "::error::$TAG points at $at, not at the tested $SHA" exit 1 @@ -68,25 +72,25 @@ jobs: echo "::error::$newest is newer than $TAG; releasing $TAG would mark it latest and point Homebrew back at it" exit 1 fi - # A complete release is never built again: a newer Go would change the - # binaries users already have under that version. A partial one, from a - # failed upload, needs its assets deleted first. + # A published version is never built again: a newer Go would change + # binaries users may already have. A complete release only gets its + # Homebrew formula refreshed; a partial one is left for a fix forward. - id: existing env: GH_TOKEN: ${{ github.token }} - GH_REPO: ${{ github.repository }} run: | - if ! gh release view "$TAG" >/dev/null 2>&1; then - exit 0 + if ! assets=$(gh api "repos/$GITHUB_REPOSITORY/releases/tags/$TAG" --jq '.assets[].name' 2>&1); then + if grep -q 'HTTP 404' <<<"$assets"; then exit 0; fi + echo "::error::Couldn't check for a $TAG release: $assets" + exit 1 fi - sums=$(gh release download "$TAG" --pattern 'serpapi_*_checksums.txt' --output - 2>/dev/null || true) - assets=$(gh release view "$TAG" --json assets --jq '.assets[].name') + sums=$(gh release download "$TAG" --repo "$GITHUB_REPOSITORY" --pattern 'serpapi_*_checksums.txt' --output - 2>/dev/null || true) missing=$(awk '{print $2}' <<<"$sums" | grep -vxF -e "$assets" || true) if [ -n "$sums" ] && [ -z "$missing" ]; then echo "skip=true" >> "$GITHUB_OUTPUT" - echo "::notice::$TAG is already released with every file in its checksums; not building it again" + echo "::notice::$TAG is already released with every file in its checksums; only Homebrew is refreshed" else - echo "::error::$TAG has an incomplete release; delete its assets with gh release delete-asset, then run this again" + echo "::error::$TAG has an incomplete release, which is never rebuilt; release the next version instead (Actions, CI, Run workflow on main with bump patch)" exit 1 fi - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 @@ -111,17 +115,8 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} GORELEASER_CURRENT_TAG: ${{ env.TAG }} - homebrew: - runs-on: ubuntu-latest - needs: goreleaser - concurrency: - group: release-publish - cancel-in-progress: false - queue: max - steps: - name: Update Homebrew formula env: - TAG: ${{ inputs.tag || github.ref_name }} HOMEBREW_TAP_DEPLOY_KEY: ${{ secrets.HOMEBREW_TAP_DEPLOY_KEY }} run: | VERSION="${TAG#v}" diff --git a/AGENTS.md b/AGENTS.md index c1ca44f..cfeddd2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,8 @@ 2. writes `Formula/serpapi-cli.rb` in `serpapi/homebrew-tap` from those checksums and pushes it as a `serpapi-cli ` commit. - To release by hand, run the CI workflow on `main` from the Actions tab with a bump (`dry_run` is on by default), or push a lightweight `v*` tag on a `main` commit whose CI passed with no `--- SKIP` in the e2e log; `release.yml` runs on the pushed tag. - If a release job fails after the tag was made, re-run the failed jobs of that CI run, or, once the cause is fixed on `main`, run the Release workflow from the Actions tab with that tag. The tag stays. -- `release.yml` refuses a tag that doesn't point at the tested commit or is older than the newest release (it would become "latest" and roll Homebrew back), and publishes one release at a time. It never builds a complete release again, since a newer Go would change binaries users already have under that version: a re-run only refreshes Homebrew. A partial release, from a failed upload, stops it until its assets are deleted with `gh release delete-asset`. +- `release.yml` refuses a tag that doesn't point at the tested commit or is older than the newest release (it would become "latest" and roll Homebrew back), and publishes one release at a time. A published version is never built again, since a newer Go would change binaries users may already have: a complete release only gets its Homebrew formula refreshed, and a partial one, from a failed upload, stops it. Release the next version instead: run CI on `main` from the Actions tab with bump `patch`. +- `.github/scripts/release-tag-test.py` tests `release-tag.sh` offline in scratch repos; CI runs it before the `tag` job. - Never move, delete or reuse a published tag: Go's module proxy and checksum database keep the first content they saw for each version, so a moved tag never reaches most `go install` users and fails checksum verification for the rest. - v0.2.1 to v0.2.3 are tagged on `go-rewrite`, which `main` doesn't contain; `release-tag.sh` compares files with the latest release, not history. - To change a published release's notes: `gh release edit v0.3.0 -R serpapi/serpapi-cli --notes-file `. From 8f1c581fb3e4c9e712028529215ecd5e5533cadf Mon Sep 17 00:00:00 2001 From: ilyazub Date: Tue, 13 Oct 2026 03:21:25 +0200 Subject: [PATCH 28/28] [CI] Run release-tag.sh's offline tests before tagging The 27 cases that shaped release-tag.sh, from labels and the files that ship to tags off main and API failures, lived outside the repo. They now sit beside it and run in CI on every pull request and push; the tag job waits for them, so a broken script can't tag. Co-Authored-By: Claude Opus 5.5 --- .github/scripts/release-tag-test.py | 186 ++++++++++++++++++++++++++++ .github/workflows/ci.yml | 10 +- 2 files changed, 195 insertions(+), 1 deletion(-) create mode 100755 .github/scripts/release-tag-test.py diff --git a/.github/scripts/release-tag-test.py b/.github/scripts/release-tag-test.py new file mode 100755 index 0000000..d475cef --- /dev/null +++ b/.github/scripts/release-tag-test.py @@ -0,0 +1,186 @@ +#!/usr/bin/env python3 +"""Tests release-tag.sh in scratch git repos, offline: a stub gh answers the +two API calls the script makes, a merged pull request's labels and creating a +tag ref (a local git tag, refused when it exists, as the API refuses with 422). +Prints one line per case and exits 1 if any fails. CI runs it before tagging. +Usage: release-tag-test.py [SCRIPT]""" +import os +import shutil +import stat +import subprocess +import sys +import tempfile + +STUB = r"""#!/usr/bin/env bash +echo "gh $*" >> "$STUB_LOG" +if [ "$1 $2" = "api repos/serpapi/serpapi-cli/git/refs" ]; then + ref=${4#ref=refs/tags/}; sha=${6#sha=} + if [ "$ref" = "${STUB_EXISTS:-}" ] || git rev-parse -q --verify "refs/tags/$ref" >/dev/null; then echo "Reference already exists (HTTP 422)" >&2; exit 1; fi + [ -n "${STUB_FAIL_CREATE:-}" ] && { echo "HTTP 500" >&2; exit 1; } + git tag "$ref" "$sha"; exit 0 +fi +case "$2" in repos/serpapi/serpapi-cli/commits/*/pulls) + [ -n "${STUB_FAIL_LABELS:-}" ] && { echo "HTTP 502" >&2; exit 1; } + sha=${2#repos/serpapi/serpapi-cli/commits/}; sha=${sha%/pulls} + if [ -f "${STUB_DIR:-/nonexistent}/labels-$sha" ]; then cat "$STUB_DIR/labels-$sha"; else printf '%s\n' "${STUB_LABELS:-}"; fi + exit 0 ;; +esac +echo "unexpected gh call: $*" >&2; exit 3 +""" + + +def git(repo, *args): + return subprocess.run(["git", "-C", repo, *args], check=True, capture_output=True, text=True).stdout.strip() + + +def commit(repo, path, text="x"): + full = os.path.join(repo, path) + os.makedirs(os.path.dirname(full) or repo, exist_ok=True) + with open(full, "a") as f: + f.write(text + "\n") + git(repo, "add", "-A") + git(repo, "commit", "-q", "-m", f"change {path}") + return git(repo, "rev-parse", "HEAD") + + +def fresh(root, tags=True): + repo = tempfile.mkdtemp(dir=root) + git(repo, "init", "-q", "-b", "main") + git(repo, "config", "user.email", "t@example.com") + git(repo, "config", "user.name", "t") + base = commit(repo, "pkg/cmd/root.go", "package cmd") + if tags: + git(repo, "tag", "v0.2.2", base) + base = commit(repo, "pkg/cmd/search.go", "package cmd") + git(repo, "tag", "v0.2.3", base) + return repo, base + + +def run(script, repo, root, **env): + out = os.path.join(root, "out") + open(out, "w").close() + log = os.path.join(root, "gh.log") + open(log, "w").close() + e = {"PATH": f"{root}/bin:{os.environ['PATH']}", "GITHUB_OUTPUT": out, "GITHUB_REPOSITORY": "serpapi/serpapi-cli", + "STUB_LOG": log, "HOME": root} + e.update({k: v for k, v in env.items() if v is not None}) + p = subprocess.run(["bash", script], cwd=repo, env=e, capture_output=True, text=True) + outputs = dict(line.split("=", 1) for line in open(out).read().splitlines() if "=" in line) + return p.returncode, p.stdout.strip(), p.stderr.strip(), outputs, open(log).read() + + +def main(): + script = os.path.abspath(sys.argv[1] if len(sys.argv) > 1 else os.path.join(os.path.dirname(__file__), "release-tag.sh")) + root = tempfile.mkdtemp(prefix="release-tag-test-") + os.makedirs(f"{root}/bin") + with open(f"{root}/bin/gh", "w") as f: + f.write(STUB) + os.chmod(f"{root}/bin/gh", stat.S_IRWXU) + failures = 0 + + def case(name, want_code, want_reason, want_tag, repo, sha, created=None, **env): + nonlocal failures + before = set(git(repo, "tag", "--list").split()) + code, stdout, stderr, outputs, log = run(script, repo, root, SHA=sha, **env) + new_tags = sorted(set(git(repo, "tag", "--list").split()) - before) + ok = (code == want_code and want_reason in (stdout + stderr) and outputs.get("tag", "") == want_tag + and new_tags == ([created] if created else [])) + failures += not ok + print(f"{'ok ' if ok else 'FAIL'} {name:52} exit={code} tag={outputs.get('tag', '')!r} next={outputs.get('next', '')!r} " + f"created={new_tags} :: {(stdout or stderr)[:95]}") + return log + + repo, _ = fresh(root) + code = commit(repo, "pkg/api/client.go", "package api") + case("code change, no labels: patch", 0, "Tagged", "v0.2.4", repo, code, created="v0.2.4", LABELS_JSON="[]") + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + case("release:minor label", 0, "minor after v0.2.3", "v0.3.0", repo, code, created="v0.3.0", LABELS_JSON='["release:minor"]') + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + case("release:major label", 0, "major after v0.2.3", "v1.0.0", repo, code, created="v1.0.0", LABELS_JSON='["release:major"]') + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + case("release:skip label", 0, "release:skip", "", repo, code, LABELS_JSON='["release:skip","release:minor"]') + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + case("multi-word label beside release:minor", 0, "minor", "v0.3.0", repo, code, created="v0.3.0", + LABELS_JSON='["good first issue","release:minor"]') + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + case("label that only contains release:minor", 0, "patch after", "v0.2.4", repo, code, created="v0.2.4", + LABELS_JSON='["not release:minor"]') + repo, _ = fresh(root) + for path in ["README.md", ".github/workflows/ci.yml", "pkg/cmd/x_test.go", "tests/e2e_test.go", "pkg/cmd/testdata/a.json", "AGENTS.md"]: + docs = commit(repo, path) + case("only docs, tests, testdata and CI changed", 0, "nothing that ships", "", repo, docs, LABELS_JSON='["release:minor"]') + case("explicit BUMP=minor releases them anyway", 0, "minor after v0.2.3", "v0.3.0", repo, docs, created="v0.3.0", BUMP="minor") + repo, _ = fresh(root); sha = commit(repo, "go.mod", "module x") + case("go.mod change ships", 0, "Tagged", "v0.2.4", repo, sha, created="v0.2.4", LABELS_JSON="[]") + repo, _ = fresh(root); sha = commit(repo, ".goreleaser.yml", "version: 2") + case(".goreleaser.yml change ships", 0, "Tagged", "v0.2.4", repo, sha, created="v0.2.4", LABELS_JSON="[]") + repo, _ = fresh(root); old = commit(repo, "pkg/a.go"); new = commit(repo, "pkg/b.go"); git(repo, "tag", "v0.2.4", new) + case("commit older than the latest release", 0, "older than v0.2.4", "", repo, old, LABELS_JSON="[]") + case("commit already tagged", 0, "already tagged v0.2.4", "", repo, new, LABELS_JSON="[]") + repo, base = fresh(root); git(repo, "checkout", "-q", "-b", "side", base); side = commit(repo, "pkg/side.go") + git(repo, "tag", "v0.2.4", side); git(repo, "checkout", "-q", "main"); git(repo, "tag", "-d", "v0.2.4") + git(repo, "tag", "v0.2.4-not-a-release", side) + main_sha = commit(repo, "pkg/main.go") + case("pre-release style tags don't count", 0, "patch after v0.2.3", "v0.2.4", repo, main_sha, created="v0.2.4", LABELS_JSON="[]") + repo, base = fresh(root); git(repo, "checkout", "-q", "-b", "side", base); side = commit(repo, "pkg/side.go") + git(repo, "checkout", "-q", "main"); main_sha = commit(repo, "pkg/main.go") + git(repo, "tag", "v0.2.4", side) + case("latest release on a branch main left (go-rewrite)", 0, "comparing their files", "v0.2.5", repo, main_sha, created="v0.2.5", + LABELS_JSON="[]") + repo, base = fresh(root); git(repo, "checkout", "-q", "-b", "side", base); side = commit(repo, "pkg/side.go") + git(repo, "tag", "v0.2.4", side); git(repo, "checkout", "-q", "main"); git(repo, "merge", "-q", "--no-edit", "side") + docs = commit(repo, "README.md") + case("main has the release's files, plus docs", 0, "nothing that ships", "", repo, docs, LABELS_JSON="[]") + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + case("next tag made meanwhile by another run", 1, "already exists", "", repo, code, LABELS_JSON="[]", STUB_EXISTS="v0.2.4") + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + case("dry run creates nothing", 0, "Would release v0.3.0", "", repo, code, LABELS_JSON='["release:minor"]', DRY_RUN="true") + repo, _ = fresh(root, tags=False); code = commit(repo, "pkg/api/client.go") + case("no tags yet: first release", 0, "patch after v0.0.0", "v0.0.1", repo, code, created="v0.0.1", LABELS_JSON="[]") + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + log = case("labels looked up from the merged PR", 0, "minor after", "v0.3.0", repo, code, created="v0.3.0", STUB_LABELS="release:minor") + if f"commits/{code}/pulls" not in log: + failures += 1 + print("FAIL the label lookup didn't ask for the commit's pull requests:", log.strip()[:120]) + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + case("unknown BUMP", 2, "BUMP must be", "", repo, code, BUMP="bogus") + repo, _ = fresh(root); major_pr = commit(repo, "pkg/api/client.go"); head = commit(repo, "pkg/cmd/root.go") + with open(f"{root}/labels-{major_pr}", "w") as f: + f.write("release:major\n") + log = case("earlier release:major merge, unlabeled head", 0, "major after v0.2.3", "v1.0.0", repo, head, created="v1.0.0", + STUB_DIR=root) + if f"commits/{major_pr}/pulls" not in log: + failures += 1 + print("FAIL the earlier merge's labels weren't looked up:", log.strip()[:160]) + repo, _ = fresh(root); skip_pr = commit(repo, "pkg/api/client.go"); head = commit(repo, "pkg/cmd/root.go") + with open(f"{root}/labels-{skip_pr}", "w") as f: + f.write("release:skip\n") + case("earlier release:skip doesn't hold back the head", 0, "patch after v0.2.3", "v0.2.4", repo, head, created="v0.2.4", + STUB_DIR=root) + repo, _ = fresh(root); minor_pr = commit(repo, "pkg/api/client.go"); head = commit(repo, "pkg/cmd/root.go") + with open(f"{root}/labels-{minor_pr}", "w") as f: + f.write("release:minor\n") + case("preview: PR labels plus an earlier minor merge", 0, "Would release v0.3.0", "", repo, head, LABELS_JSON="[]", + DRY_RUN="true", STUB_DIR=root) + repo, _ = fresh(root); docs_major = commit(repo, "README.md"); head = commit(repo, "pkg/cmd/root.go") + with open(f"{root}/labels-{docs_major}", "w") as f: + f.write("release:major\n") + case("earlier docs-only release:major doesn't count", 0, "patch after v0.2.3", "v0.2.4", repo, head, created="v0.2.4", + STUB_DIR=root) + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + case("tag API failure fails the job", 1, "HTTP 500", "", repo, code, LABELS_JSON="[]", STUB_FAIL_CREATE="1") + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + case("label lookup failure fails the job", 1, "HTTP 502", "", repo, code, STUB_FAIL_LABELS="1") + repo, _ = fresh(root); code = commit(repo, "pkg/api/client.go") + log = case("dry run with labels makes no API call", 0, "Would release", "", repo, code, LABELS_JSON="[]", DRY_RUN="true") + if log.strip(): + failures += 1 + print("FAIL dry run called gh:", log.strip()[:120]) + + shutil.rmtree(root) + print(f"{failures} failed") + sys.exit(1 if failures else 0) + + +if __name__ == "__main__": + main() diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2392a22..6671e48 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -74,8 +74,16 @@ jobs: # After the tests pass on main, release-tag.sh tags the commit with the next # version, and release.yml releases it. A tag pushed with GITHUB_TOKEN # doesn't start release.yml's tag trigger, so this run calls it. + release-script: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + persist-credentials: false + - run: python3 .github/scripts/release-tag-test.py + tag: - needs: [test, e2e] + needs: [test, e2e, release-script] if: github.ref == 'refs/heads/main' && github.event_name != 'pull_request' runs-on: ubuntu-latest permissions: