diff --git a/.claude/commands/c1i.md b/.claude/commands/c1i.md index 6ad6d83..d09e528 100644 --- a/.claude/commands/c1i.md +++ b/.claude/commands/c1i.md @@ -82,7 +82,7 @@ request was sent. | 3 | not authenticated, or API `401`/`403` | | 4 | API `404` (not found) | | 5 | API `429` (rate limited) | -| 6 | C1 failed: API `5xx`, a `200` with a body that isn't JSON, or a redirect loop (`RedirectLoopError`) | +| 6 | C1 failed: API `5xx`, a `200` with a body that isn't JSON, a redirect loop (`RedirectLoopError`), or a stuck pagination cursor (`automations`/`functions` lists, `api --paginate`) | | 7 | MCP tool call completed but the tool itself reported `isError: true` | | 8 | a system beyond C1, or the MCP protocol layer, failed (includes an unreachable gateway) | @@ -90,7 +90,8 @@ Typed internally and classified in one place, `cmd/errors.go`: from the client, `APIError`, `AuthError`, `PathError`, `RedirectError`, `RedirectLoopError`; from the gateway, `mcpgateway.TransportError` (unreachable); plus package-`cmd` wrappers `usageError`, `toolExecutionError`, `nonJSONResponseError`, -`upstreamError`, and `helperLeftError`. +`stuckPaginationCursorError` (a guarded list's cursor stopped +advancing), `upstreamError`, and `helperLeftError`. ## Documentation diff --git a/CHANGELOG.md b/CHANGELOG.md index 20b4082..560856a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,17 @@ credentials fails instead of using the working directory. With a relative ### Fixed +- **`automations list`, `automations executions list`, `functions list`, + `functions commits`, `functions usage`, and `api --paginate` could loop + forever**, re-printing the same rows: on a large enough result set, + `/api/v1/automations`, `/api/v1/automation_executions`, + `/api/v1/functions`, and `/api/v1/functions/{id}/commits` ignore + `page_size`/`page_token` and return the first page under a fresh + `nextPageToken` every time. These commands now stop when the cursor stops + advancing (the same token twice, or two pages in a row of rows already + seen), keep the rows already printed, warn on stderr that results are + INCOMPLETE, and exit `6`. + - **`--debug` and `--max-retries` did nothing on `docs openapi`, `docs endpoints` and `docs endpoint`.** The spec fetch now uses the shared transport, so it traces, refuses a redirect to a different path or host, and diff --git a/CLAUDE.md b/CLAUDE.md index 66c0faf..a0d781f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -228,10 +228,11 @@ tracked root file outside its allowlist. Stage explicit paths rather than `-A`. and `client.RedirectLoopError` (a same-path chain that never settles in 5 hops); `internal/mcpgateway` adds `TransportError` for an unreachable gateway. `cmd/errors.go` maps them — plus its own - `usageError`, `toolExecutionError`, `nonJSONResponseError`, and - `upstreamError` wrappers, for failures with no natural client-level type, - and `helperLeftError` (a login's undeleted temporary credential, always 1) — - to exit codes: + `usageError`, `toolExecutionError`, `nonJSONResponseError`, + `stuckPaginationCursorError` (a guarded list's cursor stopped advancing), + and `upstreamError` wrappers, for failures with no natural + client-level type, and `helperLeftError` (a login's undeleted temporary + credential, always 1) — to exit codes: | Code | Meaning | |------|---------| @@ -241,7 +242,7 @@ tracked root file outside its allowlist. Stage explicit paths rather than `-A`. | `3` | auth (`401`/`403`) | | `4` | not-found (`404`) | | `5` | rate-limited (`429`) | - | `6` | C1 failed (API `5xx`, a redirect loop, or a `200` with a non-JSON body) | + | `6` | C1 failed (API `5xx`, a redirect loop, a `200` with a non-JSON body, or a stuck pagination cursor on a guarded list) | | `7` | tool-execution error (`mcp gateway call` result has `isError: true`) | | `8` | a system beyond C1 or the protocol layer failed (an unreachable gateway, an upstream connector failure, or a protocol-level JSON-RPC error) | diff --git a/README.md b/README.md index 2fb8785..8818ef2 100644 --- a/README.md +++ b/README.md @@ -307,6 +307,12 @@ c1i functions usage [--page-size N] [--page-token TOKEN] [--limit `functions source` auto-resolves the function's published commit (falling back to its head/latest draft) and base64-decodes the source files. Without `--out-dir`, each file is printed to stdout with a `// ===== =====` delimiter; with `--out-dir`, files are written to disk. Fetched source is developer-authored code that commonly inlines credentials, so files are written `0600` and the directory is created — or, if it already exists, tightened, never widened — to at most `0700` (owner-only; group access to the directory would buy nothing since the files inside are already unreadable to group/other, and a filename alone can be informative). Any setuid/setgid/sticky bit on the directory is stripped outright, since none of them mean anything once there is no group or other access left. A tightened pre-existing directory prints a warning to stderr naming its old mode. `functions usage` scans every automation and emits one row per step that calls the given function — useful before deleting a draft. +`functions list`, `functions commits`, and `functions usage` (which reads +`/api/v1/automations`) stop if the server's pagination cursor stops +advancing — a repeated `nextPageToken`, or two pages in a row holding only +rows already seen, which these routes can do on large result sets. The command keeps the rows already printed, warns on stderr that +results are INCOMPLETE and names the route, and exits `6`. + ### Automations ```sh @@ -317,6 +323,9 @@ c1i automations executions list [--state done|error|pending|...] [--template-id Each `automations list` row includes `function_ids` (every distinct function the automation invokes), so `--calls-function` can answer "which automations call function X?". `executions list --state` accepts the short forms (`done`, `error`, `pending`, ...) or the full `AUTOMATION_EXECUTION_STATE_*` enum; state and template filtering are applied client-side, so pair a narrow filter with `--limit` to bound the work. +`automations list` and `automations executions list` carry the same +stuck-cursor guard described under Functions above. + ### Policies ```sh @@ -843,7 +852,7 @@ file, or `-` for stdin) — the two are mutually exclusive. GET and DELETE refus a body by default (a body on either is more likely a mistake than intent); pass `--allow-delete-body` to lift that for DELETE specifically, for the handful of C1 endpoints that require one. `--query key=value` and -`--header key=value` are both repeatable. When `--paginate` is used, each page's first array-valued field is unwrapped and each item is emitted as a single line of NDJSON — the same format used by list commands. This covers both the canonical `list` key and typed keys like `automationExecutions`; use `--list-key ` to force a specific field. If the server returns the same `nextPageToken` twice in a row, `c1i` aborts with an error rather than looping forever. Without `--paginate`, the full JSON response is pretty-printed — and if that response carries a non-empty `nextPageToken`, `c1i` warns on stderr that the result is partial and names `--paginate`, since a truncated page is otherwise indistinguishable from a complete one at exit 0. stdout is unchanged either way. +`--header key=value` are both repeatable. When `--paginate` is used, each page's first array-valued field is unwrapped and each item is emitted as a single line of NDJSON — the same format used by list commands. This covers both the canonical `list` key and typed keys like `automationExecutions`; use `--list-key ` to force a specific field. If the cursor stops advancing — the same `nextPageToken` sent twice, or two pages in a row holding only items already seen (compared by item `id`, else the item's canonical JSON) — `c1i` stops instead of looping, keeps the rows already printed (a single such page is skipped, not re-emitted; id-less items that legitimately repeat across pages can trip this), warns on stderr that results are INCOMPLETE and names the route, and exits `6`. Without `--paginate`, the full JSON response is pretty-printed — and if that response carries a non-empty `nextPageToken`, `c1i` warns on stderr that the result is partial and names `--paginate`, since a truncated page is otherwise indistinguishable from a complete one at exit 0. stdout is unchanged either way. ### API Discovery & Documentation @@ -1010,7 +1019,7 @@ branch on without parsing text: | `3` | not authenticated, or API returned `401`/`403` | | `4` | API returned `404` (not found) | | `5` | API returned `429` (rate limited — back off and retry) | -| `6` | C1 failed: the API returned `5xx`, a redirect chain never settled, or it answered `200` with a body that isn't JSON | +| `6` | C1 failed: the API returned `5xx`, a redirect chain never settled, it answered `200` with a body that isn't JSON, or a guarded list command's pagination cursor stopped advancing | | `7` | `mcp gateway call` completed, but the tool itself reported an error (`isError: true` in its result) | | `8` | a system beyond C1, or the MCP protocol layer, failed — an upstream connector was unreachable or errored, the gateway itself was unreachable (DNS failure, refused connection), or the gateway returned a protocol-level JSON-RPC error | diff --git a/cmd/agents.md b/cmd/agents.md index 96f88d7..d71c94f 100644 --- a/cmd/agents.md +++ b/cmd/agents.md @@ -284,7 +284,7 @@ APP_ID=$(c1i apps create --display-name "Example" | jq -r '.app.id') | 3 | not authenticated, or API `401`/`403` | re-authenticate (`c1i auth login`); a scoped credential needs a role that covers the call | | 4 | API `404` | stop — the resource/path doesn't exist | | 5 | API `429` | back off and retry | -| 6 | C1 failed: API `5xx`, a `200` with a body that isn't JSON, or a redirect chain that never settles | retryable, not your fault | +| 6 | C1 failed: API `5xx`, a `200` with a body that isn't JSON, a redirect chain that never settles, or a stuck pagination cursor (`automations`/`functions` lists, `api --paginate`) | retryable, except a stuck cursor, which persists: the printed rows are real but incomplete | | 7 | `mcp gateway call` succeeded but the tool reported `isError: true` | inspect the printed result, not just the code | | 8 | a system beyond C1, or the protocol layer, failed — including a gateway that is unreachable (DNS failure, refused connection) | retrying the same call usually repeats it. For a connector failure, check the server with `mcp servers get --app-id ` and `mcp servers test-connection`; the upstream may be unreachable or its credentials expired. For a protocol error, it's a version mismatch or a c1i bug — report it, don't work around it | diff --git a/cmd/api.go b/cmd/api.go index af9bec5..0e7b700 100644 --- a/cmd/api.go +++ b/cmd/api.go @@ -1,6 +1,7 @@ package cmd import ( + "bytes" "context" "encoding/json" "fmt" @@ -167,10 +168,14 @@ an intended non-GET REST write.`, } enc := newEmitter(cmd) + guard := newPaginationCursorGuard() pageToken := "" - prevToken := "" + route := fmt.Sprintf("%s %s", method, path) for !limitReached(enc.Written(), limit) { + if guard.tokenAlreadyUsed(pageToken) { + return warnStuckCursor(cmd, route) + } reqPath := path var bodyBytes []byte switch method { @@ -236,7 +241,7 @@ an intended non-GET REST write.`, // Deliberately says "partial" rather than "the first page": // --query page_token=X makes this page N. And --paginate is // offered, not promised — on an endpoint whose cursor never - // advances it aborts (see the prevToken guard below), which is + // advances it aborts (see paginationCursorGuard), which is // still a clear failure rather than this silent one. if tok := nextPageTokenOf(data); tok != "" { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), @@ -250,6 +255,18 @@ an intended non-GET REST write.`, return fmt.Errorf("failed to parse response: %w", err) } + ids := make([]string, len(items)) + for i, item := range items { + ids[i] = itemIdentity(item) + } + emit, stuck := guard.checkPage(ids) + if stuck { + return warnStuckCursor(cmd, route) + } + if !emit { + items = nil + } + for _, item := range items { // Encode the raw item, not a decoded any: decoding to any turns // JSON numbers into float64 and corrupts large integer IDs. The @@ -266,12 +283,7 @@ an intended non-GET REST write.`, if nextToken == "" { break } - // Guard against servers that return the same token forever. - // Without this, c1i would loop indefinitely emitting no progress. - if nextToken == prevToken { - return fmt.Errorf("API returned the same nextPageToken twice in a row for %s; the endpoint may not support pagination via the cursor c1i is sending. Drop --paginate and call the endpoint directly, or report this with the path", path) - } - prevToken = nextToken + guard.markTokenUsed(pageToken) pageToken = nextToken } @@ -414,3 +426,26 @@ func extractListAndToken(data []byte, listKey string) ([]json.RawMessage, string return nil, nextPageToken, nil } + +// itemIdentity identifies a list item for the stuck-cursor guard: its string +// "id" when present, else its canonical JSON (re-marshaling sorts keys; +// UseNumber keeps large integers distinct). +func itemIdentity(item json.RawMessage) string { + var withID struct { + ID string `json:"id"` + } + if err := json.Unmarshal(item, &withID); err == nil && withID.ID != "" { + return withID.ID + } + dec := json.NewDecoder(bytes.NewReader(item)) + dec.UseNumber() + var decoded any + if err := dec.Decode(&decoded); err != nil { + return string(item) + } + canon, err := json.Marshal(decoded) + if err != nil { + return string(item) + } + return string(canon) +} diff --git a/cmd/api_test.go b/cmd/api_test.go index f1261d8..04f241d 100644 --- a/cmd/api_test.go +++ b/cmd/api_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "io" "net/http" "net/http/httptest" @@ -811,3 +812,268 @@ func TestAPIPaginateDrainsCursorWithoutWarning(t *testing.T) { } } } + +// A repeated nextPageToken must stop --paginate before re-fetching, emit the +// row once, warn naming the route, and exit 6. +func TestAPIPaginateSameTokenTwiceReturnsExitServer(t *testing.T) { + var requestCount int + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + if requestCount > 4 { + t.Errorf("server received a 5th request; --paginate did not terminate") + w.WriteHeader(http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprint(w, `{"list":[{"id":"a"}],"nextPageToken":"stuck-tok"}`) + })) + defer srv.Close() + + resetAPICmdFlags(t) + stubNewAPIClient(t, srv) + t.Setenv("C1I_URL", srv.URL) + + var out, errOut bytes.Buffer + apiCmd.SetOut(&out) + apiCmd.SetErr(&errOut) + rootCmd.SetArgs([]string{"api", "--path", "/api/v1/automations", "--paginate"}) + + err := rootCmd.ExecuteContext(context.Background()) + if err == nil { + t.Fatalf("ExecuteContext: want an error (stuck cursor -> exit 6), got nil; stdout=%q stderr=%q", out.String(), errOut.String()) + } + if got, want := exitCode(err), exitServer; got != want { + t.Errorf("exitCode(err) = %d, want %d (exitServer); err=%v", got, want, err) + } + if requestCount != 2 { + t.Fatalf("server received %d requests, want exactly 2 (one to discover the stuck page, one to confirm it repeats)", requestCount) + } + rows := decodeNDJSONRows(t, out.String()) + if len(rows) != 1 || rows[0]["id"] != "a" { + t.Fatalf("got rows %v, want exactly one occurrence of id=a", rows) + } + if !strings.Contains(errOut.String(), "INCOMPLETE") || !strings.Contains(errOut.String(), "GET /api/v1/automations") { + t.Errorf("stderr = %q, want it to say results are INCOMPLETE and name GET /api/v1/automations", errOut.String()) + } +} + +// The observed defect: a fresh token every call over the same page. The +// identical page, not the token, must trip the guard. +func TestAPIPaginateChangingTokenSameRowsReturnsExitServer(t *testing.T) { + var requestCount int + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + if requestCount > 4 { + t.Errorf("server received a 5th request; --paginate did not terminate") + w.WriteHeader(http.StatusBadRequest) + return + } + tok := r.URL.Query().Get("page_token") + w.Header().Set("Content-Type", "application/json") + switch tok { + case "": + _, _ = fmt.Fprint(w, `{"list":[{"id":"a"}],"nextPageToken":"tok-v1"}`) + case "tok-v1", "tok-v2": + _, _ = fmt.Fprintf(w, `{"list":[{"id":"a"}],"nextPageToken":"tok-v%d"}`, requestCount) + default: + t.Errorf("unexpected page_token: %q", tok) + w.WriteHeader(http.StatusInternalServerError) + } + })) + defer srv.Close() + + resetAPICmdFlags(t) + stubNewAPIClient(t, srv) + t.Setenv("C1I_URL", srv.URL) + + var out, errOut bytes.Buffer + apiCmd.SetOut(&out) + apiCmd.SetErr(&errOut) + rootCmd.SetArgs([]string{"api", "--path", "/api/v1/automations", "--paginate"}) + + err := rootCmd.ExecuteContext(context.Background()) + if err == nil { + t.Fatalf("ExecuteContext: want an error (stuck cursor -> exit 6), got nil; stdout=%q stderr=%q", out.String(), errOut.String()) + } + if got, want := exitCode(err), exitServer; got != want { + t.Errorf("exitCode(err) = %d, want %d (exitServer); err=%v", got, want, err) + } + if requestCount != 3 { + t.Fatalf("server received %d requests, want exactly 3 (the page must repeat twice before the cursor counts as stuck)", requestCount) + } + rows := decodeNDJSONRows(t, out.String()) + if len(rows) != 1 || rows[0]["id"] != "a" { + t.Fatalf("got rows %v, want exactly one occurrence of id=a (no duplicate emission)", rows) + } + if !strings.Contains(errOut.String(), "INCOMPLETE") || !strings.Contains(errOut.String(), "GET /api/v1/automations") { + t.Errorf("stderr = %q, want it to say results are INCOMPLETE and name GET /api/v1/automations", errOut.String()) + } +} + +// Items with no "id" are compared by canonical JSON, so a stuck page of them +// is still detected. +func TestAPIPaginateStuckCursorWithoutIDFallsBackToCanonicalJSON(t *testing.T) { + var requestCount int + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + if requestCount > 4 { + t.Errorf("server received a 5th request; --paginate did not terminate") + w.WriteHeader(http.StatusBadRequest) + return + } + tok := r.URL.Query().Get("page_token") + w.Header().Set("Content-Type", "application/json") + switch tok { + case "": + _, _ = fmt.Fprint(w, `{"list":[{"name":"n1","value":1}],"nextPageToken":"tok-v1"}`) + case "tok-v1", "tok-v2": + // Same logical item, same key order even -- a changing token is + // the only thing that looks different. + _, _ = fmt.Fprintf(w, `{"list":[{"name":"n1","value":1}],"nextPageToken":"tok-v%d"}`, requestCount) + default: + t.Errorf("unexpected page_token: %q", tok) + w.WriteHeader(http.StatusInternalServerError) + } + })) + defer srv.Close() + + resetAPICmdFlags(t) + stubNewAPIClient(t, srv) + t.Setenv("C1I_URL", srv.URL) + + var out, errOut bytes.Buffer + apiCmd.SetOut(&out) + apiCmd.SetErr(&errOut) + rootCmd.SetArgs([]string{"api", "--path", "/api/v1/decoys", "--paginate"}) + + err := rootCmd.ExecuteContext(context.Background()) + if err == nil { + t.Fatalf("ExecuteContext: want an error (stuck cursor -> exit 6), got nil; stdout=%q stderr=%q", out.String(), errOut.String()) + } + if got, want := exitCode(err), exitServer; got != want { + t.Errorf("exitCode(err) = %d, want %d (exitServer); err=%v", got, want, err) + } + if requestCount != 3 { + t.Fatalf("server received %d requests, want exactly 3 (the page must repeat twice before the cursor counts as stuck)", requestCount) + } + rows := decodeNDJSONRows(t, out.String()) + if len(rows) != 1 || rows[0]["name"] != "n1" { + t.Fatalf("got rows %v, want exactly one occurrence of name=n1 (no duplicate emission)", rows) + } +} + +// A healthy offset-paginated server that gains exactly one page of rows +// between requests repeats a page once. That must not be read as a stuck +// cursor: the repeat is skipped, pagination continues, and the run exits 0. +func TestAPIPaginateSingleRepeatedPageIsNotStuck(t *testing.T) { + var requestCount int + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + w.Header().Set("Content-Type", "application/json") + switch tok := r.URL.Query().Get("page_token"); tok { + case "": + _, _ = fmt.Fprint(w, `{"list":[{"id":"a"},{"id":"b"}],"nextPageToken":"t1"}`) + case "t1": + _, _ = fmt.Fprint(w, `{"list":[{"id":"a"},{"id":"b"}],"nextPageToken":"t2"}`) + case "t2": + _, _ = fmt.Fprint(w, `{"list":[{"id":"c"}],"nextPageToken":""}`) + default: + t.Errorf("unexpected page_token: %q", tok) + w.WriteHeader(http.StatusBadRequest) + } + })) + defer srv.Close() + + resetAPICmdFlags(t) + stubNewAPIClient(t, srv) + t.Setenv("C1I_URL", srv.URL) + + var out, errOut bytes.Buffer + apiCmd.SetOut(&out) + apiCmd.SetErr(&errOut) + rootCmd.SetArgs([]string{"api", "--path", "/api/v1/things", "--paginate"}) + + if err := rootCmd.ExecuteContext(context.Background()); err != nil { + t.Fatalf("ExecuteContext: %v (exit %d); stderr=%q", err, exitCode(err), errOut.String()) + } + var got []string + for _, row := range decodeNDJSONRows(t, out.String()) { + got = append(got, fmt.Sprint(row["id"])) + } + if strings.Join(got, ",") != "a,b,c" { + t.Fatalf("rows = %v, want [a b c] (the repeated page skipped, not re-emitted)", got) + } + if requestCount != 3 { + t.Fatalf("server received %d requests, want 3", requestCount) + } +} + +// Id-less items that differ only in an integer beyond float64 precision must +// keep distinct identities, or a legitimate page reads as a repeat. +func TestItemIdentityKeepsLargeIntegersDistinct(t *testing.T) { + a := itemIdentity(json.RawMessage(`{"n":9007199254740993}`)) + b := itemIdentity(json.RawMessage(`{"n":9007199254740992}`)) + if a == b { + t.Fatalf("itemIdentity collapsed distinct large integers to %q", a) + } + if x, y := itemIdentity(json.RawMessage(`{"b":1,"a":2}`)), itemIdentity(json.RawMessage(`{"a":2, "b":1}`)); x != y { + t.Fatalf("itemIdentity differs for the same object with reordered keys: %q vs %q", x, y) + } +} + +// Stuck shapes that are not a single page repeated: each must stop with exit +// 6 and print every row once, never cycle (within --limit or without). +func TestAPIPaginateStuckCycleShapesReturnExitServer(t *testing.T) { + cases := []struct { + name string + pages []string // served in rotation, each under a fresh token + limit string + want string + }{ + {"alternating pages", []string{`[{"id":"a"}]`, `[{"id":"b"}]`}, "", "a,b"}, + {"alternating pages under --limit", []string{`[{"id":"a"}]`, `[{"id":"b"}]`}, "7", "a,b"}, + {"page then empty page", []string{`[{"id":"a"}]`, `[]`}, "", "a"}, + {"same rows reordered", []string{`[{"id":"a"},{"id":"b"}]`, `[{"id":"b"},{"id":"a"}]`}, "", "a,b"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var requestCount int + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + if requestCount > 10 { + t.Errorf("server received an 11th request; --paginate did not terminate") + w.WriteHeader(http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"list":%s,"nextPageToken":"tok-%d"}`, tc.pages[(requestCount-1)%len(tc.pages)], requestCount) + })) + defer srv.Close() + + resetAPICmdFlags(t) + stubNewAPIClient(t, srv) + t.Setenv("C1I_URL", srv.URL) + + var out, errOut bytes.Buffer + apiCmd.SetOut(&out) + apiCmd.SetErr(&errOut) + args := []string{"api", "--path", "/api/v1/things", "--paginate"} + if tc.limit != "" { + args = append(args, "--limit", tc.limit) + } + rootCmd.SetArgs(args) + + err := rootCmd.ExecuteContext(context.Background()) + if got := exitCode(err); got != exitServer { + t.Fatalf("exitCode = %d, want %d (exitServer); err=%v stderr=%q", got, exitServer, err, errOut.String()) + } + var got []string + for _, row := range decodeNDJSONRows(t, out.String()) { + got = append(got, fmt.Sprint(row["id"])) + } + if strings.Join(got, ",") != tc.want { + t.Fatalf("rows = %v, want %s", got, tc.want) + } + }) + } +} diff --git a/cmd/automations_executions_list.go b/cmd/automations_executions_list.go index cea5efa..d8f0d87 100644 --- a/cmd/automations_executions_list.go +++ b/cmd/automations_executions_list.go @@ -71,8 +71,12 @@ the server returns. Combine with --limit to bound the work.`, clientFilter := stateFilter != "" || templateID != "" enc := newEmitter(cmd) - prevToken := "" + guard := newPaginationCursorGuard() for !limitReached(enc.Written(), limit) { + if guard.tokenAlreadyUsed(pageToken) { + return warnStuckCursor(cmd, "GET /api/v1/automation_executions") + } + pageSize := requestedPageSize if !clientFilter && !enc.Filtered() { pageSize = effectivePageSize(requestedPageSize, limit, enc.Written()) @@ -97,6 +101,18 @@ the server returns. Combine with --limit to bound the work.`, return fmt.Errorf("failed to parse response: %w", err) } + ids := make([]string, len(resp.AutomationExecutions)) + for i, e := range resp.AutomationExecutions { + ids[i] = e.ID + } + emit, stuck := guard.checkPage(ids) + if stuck { + return warnStuckCursor(cmd, "GET /api/v1/automation_executions") + } + if !emit { + resp.AutomationExecutions = nil + } + for _, e := range resp.AutomationExecutions { if stateFilter != "" && e.State != stateFilter { continue @@ -113,13 +129,7 @@ the server returns. Combine with --limit to bound the work.`, if resp.NextPageToken == "" || manualPaging { break } - // Some pagination paths on this endpoint have been seen returning - // the same cursor twice. Catch that and abort rather than spin — - // the caller can drop --page-token and retry. - if resp.NextPageToken == prevToken { - return fmt.Errorf("automation_executions returned the same nextPageToken twice in a row; pagination on this endpoint may be flaky for the current cursor — re-run without --page-token or report the issue") - } - prevToken = resp.NextPageToken + guard.markTokenUsed(pageToken) pageToken = resp.NextPageToken } diff --git a/cmd/automations_list.go b/cmd/automations_list.go index 12f75c8..c2cf682 100644 --- a/cmd/automations_list.go +++ b/cmd/automations_list.go @@ -101,7 +101,12 @@ var automationsListCmd = &cobra.Command{ clientFilter := enabledOnly || callsFunction != "" enc := newEmitter(cmd) + guard := newPaginationCursorGuard() for !limitReached(enc.Written(), limit) { + if guard.tokenAlreadyUsed(pageToken) { + return warnStuckCursor(cmd, "GET /api/v1/automations") + } + pageSize := requestedPageSize if !clientFilter && !enc.Filtered() { pageSize = effectivePageSize(requestedPageSize, limit, enc.Written()) @@ -126,6 +131,18 @@ var automationsListCmd = &cobra.Command{ return fmt.Errorf("failed to parse response: %w", err) } + ids := make([]string, len(resp.List)) + for i, a := range resp.List { + ids[i] = a.ID + } + emit, stuck := guard.checkPage(ids) + if stuck { + return warnStuckCursor(cmd, "GET /api/v1/automations") + } + if !emit { + resp.List = nil + } + for _, a := range resp.List { if enabledOnly && !a.Enabled { continue @@ -143,6 +160,7 @@ var automationsListCmd = &cobra.Command{ if resp.NextPageToken == "" || manualPaging { break } + guard.markTokenUsed(pageToken) pageToken = resp.NextPageToken } diff --git a/cmd/automations_list_pagination_test.go b/cmd/automations_list_pagination_test.go new file mode 100644 index 0000000..b8624bd --- /dev/null +++ b/cmd/automations_list_pagination_test.go @@ -0,0 +1,171 @@ +package cmd + +import ( + "bytes" + "context" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/ConductorOne/c1i/internal/client" + "github.com/spf13/cobra" +) + +// runAutomationsListCmd runs automationsListCmd against srv. +func runAutomationsListCmd(t *testing.T, srv *httptest.Server) (stdout, stderr string, err error) { + t.Helper() + orig := newListClient + newListClient = func(_ *cobra.Command, _ string) (*client.Client, error) { + return client.NewForTesting(srv.URL, srv.Client()), nil + } + t.Cleanup(func() { newListClient = orig }) + t.Setenv("C1I_URL", "https://example.invalid") + + var out, errOut bytes.Buffer + automationsListCmd.SetOut(&out) + automationsListCmd.SetErr(&errOut) + automationsListCmd.SetContext(context.Background()) + runErr := automationsListCmd.RunE(automationsListCmd, []string{}) + return out.String(), errOut.String(), runErr +} + +// automationPage builds a GET /api/v1/automations response body. +func automationPage(ids []string, nextToken string) string { + items := make([]string, 0, len(ids)) + for _, id := range ids { + items = append(items, fmt.Sprintf(`{"id":%q}`, id)) + } + return fmt.Sprintf(`{"list":[%s],"nextPageToken":%q}`, strings.Join(items, ","), nextToken) +} + +// A page that returns its own token again must stop without re-emitting rows. +func TestAutomationsListStuckCursorSameTokenTerminates(t *testing.T) { + var requestCount int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + if requestCount > 4 { + t.Errorf("server received a 5th request; auto-pagination did not terminate") + w.WriteHeader(http.StatusBadRequest) + return + } + tok := r.URL.Query().Get("page_token") + w.Header().Set("Content-Type", "application/json") + switch tok { + case "": + _, _ = fmt.Fprint(w, automationPage([]string{"auto-a", "auto-b"}, "stuck-tok")) + case "stuck-tok": + // The final page: same rows, same cursor, forever. + _, _ = fmt.Fprint(w, automationPage([]string{"auto-a", "auto-b"}, "stuck-tok")) + default: + t.Errorf("unexpected page_token: %q", tok) + w.WriteHeader(http.StatusInternalServerError) + } + })) + defer srv.Close() + + resetCmdFlags(t, automationsListCmd) + out, errOut, err := runAutomationsListCmd(t, srv) + if err == nil { + t.Fatalf("RunE: want an error (stuck cursor -> exit 6), got nil; stdout=%q stderr=%q", out, errOut) + } + if got, want := exitCode(err), exitServer; got != want { + t.Errorf("exitCode(err) = %d, want %d (exitServer, \"C1 failed\"); err=%v", got, want, err) + } + + rows := decodeNDJSONRows(t, out) + if len(rows) != 2 || rows[0]["id"] != "auto-a" || rows[1]["id"] != "auto-b" { + t.Fatalf("got rows %v, want exactly one occurrence each of auto-a, auto-b (a stuck cursor must not re-emit them)", rows) + } + if requestCount != 2 { + t.Fatalf("server received %d requests, want exactly 2 (one to discover the stuck page, one to confirm it repeats)", requestCount) + } + if !strings.Contains(errOut, "INCOMPLETE") || !strings.Contains(errOut, "GET /api/v1/automations") { + t.Errorf("stderr warning = %q, want it to say results are INCOMPLETE and name GET /api/v1/automations", errOut) + } +} + +// A fresh token every call over an unchanged page: the repeated page, not the +// token, must trip the guard. +func TestAutomationsListStuckCursorChangingTokenTerminates(t *testing.T) { + var requestCount int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + if requestCount > 4 { + t.Errorf("server received a 5th request; auto-pagination did not terminate") + w.WriteHeader(http.StatusBadRequest) + return + } + tok := r.URL.Query().Get("page_token") + w.Header().Set("Content-Type", "application/json") + switch tok { + case "": + _, _ = fmt.Fprint(w, automationPage([]string{"auto-a", "auto-b"}, "tok-v1")) + case "tok-v1", "tok-v2": + // Same rows again, but a freshly minted, never-before-seen token. + _, _ = fmt.Fprint(w, automationPage([]string{"auto-a", "auto-b"}, fmt.Sprintf("tok-v%d", requestCount))) + default: + t.Errorf("unexpected page_token: %q", tok) + w.WriteHeader(http.StatusInternalServerError) + } + })) + defer srv.Close() + + resetCmdFlags(t, automationsListCmd) + out, errOut, err := runAutomationsListCmd(t, srv) + if err == nil { + t.Fatalf("RunE: want an error (stuck cursor -> exit 6), got nil; stdout=%q stderr=%q", out, errOut) + } + if got, want := exitCode(err), exitServer; got != want { + t.Errorf("exitCode(err) = %d, want %d (exitServer, \"C1 failed\"); err=%v", got, want, err) + } + + rows := decodeNDJSONRows(t, out) + if len(rows) != 2 || rows[0]["id"] != "auto-a" || rows[1]["id"] != "auto-b" { + t.Fatalf("got rows %v, want exactly one occurrence each of auto-a, auto-b", rows) + } + if requestCount != 3 { + t.Fatalf("server received %d requests, want exactly 3 (the page must repeat twice before the cursor counts as stuck)", requestCount) + } + if !strings.Contains(errOut, "INCOMPLETE") || !strings.Contains(errOut, "GET /api/v1/automations") { + t.Errorf("stderr warning = %q, want it to say results are INCOMPLETE and name GET /api/v1/automations", errOut) + } +} + +// Empty pages never count as repeats, so a repeated token over them must be +// what stops the loop. +func TestAutomationsListStuckCursorEmptyPagesTerminates(t *testing.T) { + var requestCount int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + if requestCount > 4 { + t.Errorf("server received a 5th request; auto-pagination did not terminate") + w.WriteHeader(http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprint(w, automationPage(nil, "forever-tok")) + })) + defer srv.Close() + + resetCmdFlags(t, automationsListCmd) + out, errOut, err := runAutomationsListCmd(t, srv) + if err == nil { + t.Fatalf("RunE: want an error (stuck cursor -> exit 6), got nil; stdout=%q stderr=%q", out, errOut) + } + if got, want := exitCode(err), exitServer; got != want { + t.Errorf("exitCode(err) = %d, want %d (exitServer, \"C1 failed\"); err=%v", got, want, err) + } + + rows := decodeNDJSONRows(t, out) + if len(rows) != 0 { + t.Fatalf("got %d rows, want 0", len(rows)) + } + if requestCount != 2 { + t.Fatalf("server received %d requests, want exactly 2 (the token repeats on the 2nd response, caught before a 3rd request)", requestCount) + } + if !strings.Contains(errOut, "INCOMPLETE") || !strings.Contains(errOut, "GET /api/v1/automations") { + t.Errorf("stderr warning = %q, want it to say results are INCOMPLETE and name GET /api/v1/automations", errOut) + } +} diff --git a/cmd/errors.go b/cmd/errors.go index c94cc87..caf5225 100644 --- a/cmd/errors.go +++ b/cmd/errors.go @@ -26,7 +26,7 @@ const ( exitAuth = 3 // not authenticated, or API returned 401/403 exitNotFound = 4 // API returned 404 exitRateLimited = 5 // API returned 429 - exitServer = 6 // C1 failed: 5xx, a redirect loop, or a 200 with a non-JSON body + exitServer = 6 // C1 failed: 5xx, a redirect loop, a 200 with a non-JSON body, or a stuck pagination cursor exitToolError = 7 // an MCP tool call completed (transport/protocol succeeded) but the tool itself reported isError exitUpstream = 8 // an upstream system, or the protocol layer itself, failed -- distinct from the C1 API failing (e.g. an unreachable MCP connector, or a JSON-RPC protocol-level error) ) @@ -65,6 +65,15 @@ type nonJSONResponseError struct{ err error } func (e *nonJSONResponseError) Error() string { return e.err.Error() } func (e *nonJSONResponseError) Unwrap() error { return e.err } +// stuckPaginationCursorError: auto-pagination stopped because the cursor +// stopped advancing (see paginationCursorGuard). Rows already printed stand; +// the result is incomplete, so it maps to exitServer. +type stuckPaginationCursorError struct{ route string } + +func (e *stuckPaginationCursorError) Error() string { + return fmt.Sprintf("results from %s are INCOMPLETE: pagination cursor stopped advancing", e.route) +} + // upstreamError marks a failure beyond C1, or in the protocol layer, that did // not arrive as a classifiable HTTP status — an upstream connector failing, or // a JSON-RPC protocol error. It maps to exitUpstream (8), distinct from C1 @@ -208,6 +217,10 @@ func exitCode(err error) int { if errors.As(err, &upstreamErr) { return exitUpstream } + var stuckErr *stuckPaginationCursorError + if errors.As(err, &stuckErr) { + return exitServer + } var nonJSONErr *nonJSONResponseError if errors.As(err, &nonJSONErr) { return exitServer diff --git a/cmd/flags.go b/cmd/flags.go index 4cd9c89..ed18e7c 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -215,6 +215,75 @@ func pageSizeFlag(cmd *cobra.Command) int { return maxSize } +// stuckPageRepeats is how many consecutive stale pages (every item already +// seen) mark a cursor as stuck. One can be a healthy offset-paginated server +// that gained exactly a page of rows between requests; two in a row is +// unlikely unless rows arrive faster than a page per request (tiny +// --page-size on a busy feed). +const stuckPageRepeats = 2 + +// paginationCursorGuard detects auto-pagination that stopped making progress: +// a page_token sent twice, or stuckPageRepeats stale pages in a row under +// fresh-looking tokens. Construct with newPaginationCursorGuard. +type paginationCursorGuard struct { + seenTokens map[string]bool + seenIDs map[string]bool + stale int +} + +func newPaginationCursorGuard() *paginationCursorGuard { + return &paginationCursorGuard{seenTokens: map[string]bool{}, seenIDs: map[string]bool{}} +} + +// tokenAlreadyUsed reports whether token was already sent. Check it before +// the request so the repeat is caught without re-fetching the page. +func (g *paginationCursorGuard) tokenAlreadyUsed(token string) bool { + return token != "" && g.seenTokens[token] +} + +// markTokenUsed records the token the request just sent. +func (g *paginationCursorGuard) markTokenUsed(token string) { + if token != "" { + g.seenTokens[token] = true + } +} + +// checkPage classifies a fetched page by its item ids. A stale page is not +// emitted (it holds nothing new); stuck is true once stuckPageRepeats stale +// pages arrive in a row. Empty pages, and pages with no non-empty id, neither +// count nor reset the run: a live token may precede one, and a missing id is +// no evidence of a repeat. +func (g *paginationCursorGuard) checkPage(ids []string) (emit, stuck bool) { + fresh, known := false, false + for _, id := range ids { + if id == "" { + continue + } + known = true + if !g.seenIDs[id] { + fresh = true + g.seenIDs[id] = true + } + } + if !known { + return true, false + } + if !fresh { + g.stale++ + return false, g.stale >= stuckPageRepeats + } + g.stale = 0 + return true, false +} + +// warnStuckCursor prints the stuck-cursor warning and returns the error that +// maps to exit 6. route is "METHOD /path", for the user to report. +func warnStuckCursor(cmd *cobra.Command, route string) error { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), + "Warning: results from %s are INCOMPLETE: the server's pagination cursor stopped advancing (a repeated nextPageToken, or repeated pages of rows already seen). Rows already printed are kept.\n", route) + return &stuckPaginationCursorError{route: route} +} + // requireNonEmpty errors when any of the named string flags has an empty // value. Cobra's required-flag enforcement only checks that the flag was // set, not that the value is non-empty — so `--app-id ""` silently passes. diff --git a/cmd/flags_test.go b/cmd/flags_test.go index e1a3273..4eee869 100644 --- a/cmd/flags_test.go +++ b/cmd/flags_test.go @@ -145,3 +145,14 @@ func TestRequireNonEmpty(t *testing.T) { } }) } + +// Rows with no id carry no evidence of a repeat, so pages of them must never +// count toward a stuck cursor. +func TestPaginationCursorGuardIgnoresEmptyIDs(t *testing.T) { + g := newPaginationCursorGuard() + for i := 0; i < 5; i++ { + if emit, stuck := g.checkPage([]string{"", ""}); !emit || stuck { + t.Fatalf("page %d of empty ids: emit=%v stuck=%v, want emit=true stuck=false", i, emit, stuck) + } + } +} diff --git a/cmd/functions_commits.go b/cmd/functions_commits.go index 978b5dd..d1ef339 100644 --- a/cmd/functions_commits.go +++ b/cmd/functions_commits.go @@ -31,7 +31,13 @@ var functionsCommitsCmd = &cobra.Command{ limit := getIntFlag(cmd, "limit") enc := newEmitter(cmd) + guard := newPaginationCursorGuard() + route := fmt.Sprintf("GET /api/v1/functions/%s/commits", functionID) for !limitReached(enc.Written(), limit) { + if guard.tokenAlreadyUsed(pageToken) { + return warnStuckCursor(cmd, route) + } + pageSize := requestedPageSize if !enc.Filtered() { pageSize = effectivePageSize(requestedPageSize, limit, enc.Written()) @@ -62,6 +68,18 @@ var functionsCommitsCmd = &cobra.Command{ return fmt.Errorf("failed to parse response: %w", err) } + ids := make([]string, len(resp.List)) + for i, item := range resp.List { + ids[i] = item.ID + } + emit, stuck := guard.checkPage(ids) + if stuck { + return warnStuckCursor(cmd, route) + } + if !emit { + resp.List = nil + } + for _, c := range resp.List { _ = enc.Encode(map[string]string{ "id": c.ID, @@ -78,6 +96,7 @@ var functionsCommitsCmd = &cobra.Command{ if resp.NextPageToken == "" || manualPaging { break } + guard.markTokenUsed(pageToken) pageToken = resp.NextPageToken } diff --git a/cmd/functions_list.go b/cmd/functions_list.go index 0db07ff..a0c950b 100644 --- a/cmd/functions_list.go +++ b/cmd/functions_list.go @@ -77,7 +77,12 @@ var functionsListCmd = &cobra.Command{ clientFilter := publishedOnly || draftOnly enc := newEmitter(cmd) + guard := newPaginationCursorGuard() for !limitReached(enc.Written(), limit) { + if guard.tokenAlreadyUsed(pageToken) { + return warnStuckCursor(cmd, "GET /api/v1/functions") + } + pageSize := requestedPageSize if !clientFilter && !enc.Filtered() { pageSize = effectivePageSize(requestedPageSize, limit, enc.Written()) @@ -102,6 +107,18 @@ var functionsListCmd = &cobra.Command{ return fmt.Errorf("failed to parse response: %w", err) } + ids := make([]string, len(resp.List)) + for i, f := range resp.List { + ids[i] = f.ID + } + emit, stuck := guard.checkPage(ids) + if stuck { + return warnStuckCursor(cmd, "GET /api/v1/functions") + } + if !emit { + resp.List = nil + } + for _, f := range resp.List { published := functionIsPublished(f) if publishedOnly && !published { @@ -119,6 +136,7 @@ var functionsListCmd = &cobra.Command{ if resp.NextPageToken == "" || manualPaging { break } + guard.markTokenUsed(pageToken) pageToken = resp.NextPageToken } diff --git a/cmd/functions_usage.go b/cmd/functions_usage.go index eb16703..adbf94c 100644 --- a/cmd/functions_usage.go +++ b/cmd/functions_usage.go @@ -55,7 +55,12 @@ last_executed_at, args.`, // (see emitter.Filtered), so emitted alone would overcount rows // actually written to stdout. emitted := 0 + guard := newPaginationCursorGuard() for !limitReached(enc.Written(), limit) { + if guard.tokenAlreadyUsed(pageToken) { + return warnStuckCursor(cmd, "GET /api/v1/automations") + } + params := map[string]string{"page_size": strconv.Itoa(pageSize)} if pageToken != "" { params["page_token"] = pageToken @@ -86,6 +91,18 @@ last_executed_at, args.`, return fmt.Errorf("failed to parse response: %w", err) } + ids := make([]string, len(resp.List)) + for i, a := range resp.List { + ids[i] = a.ID + } + emit, stuck := guard.checkPage(ids) + if stuck { + return warnStuckCursor(cmd, "GET /api/v1/automations") + } + if !emit { + resp.List = nil + } + for _, a := range resp.List { for _, step := range a.AutomationSteps { if step.CallFunction == nil || step.CallFunction.FunctionID != functionID { @@ -113,6 +130,7 @@ last_executed_at, args.`, if resp.NextPageToken == "" || manualPaging { break } + guard.markTokenUsed(pageToken) pageToken = resp.NextPageToken } diff --git a/cmd/list_pagination_test.go b/cmd/list_pagination_test.go index 3dded2b..fb6e98b 100644 --- a/cmd/list_pagination_test.go +++ b/cmd/list_pagination_test.go @@ -40,6 +40,10 @@ type listPaginationCase struct { // rowIDs extracts, in emission order, the identifying value each item() // call varied per row from the command's decoded NDJSON output. rowIDs func(rows []map[string]any) []string + // stuckCursorRoute ("METHOD /path") opts a case into + // TestListCommandsStuckCursorReturnsExitServer; set it only for commands + // that use paginationCursorGuard. + stuckCursorRoute string } // Every case's command is a package-level singleton shared by the whole test @@ -86,7 +90,7 @@ func extractPageToken(t *testing.T, method string, r *http.Request) string { return tok } -func runListPaginationCase(t *testing.T, tc listPaginationCase, srv *httptest.Server) (string, error) { +func runListPaginationCase(t *testing.T, tc listPaginationCase, srv *httptest.Server) (stdout, stderr string, err error) { t.Helper() orig := newListClient newListClient = func(_ *cobra.Command, _ string) (*client.Client, error) { @@ -97,16 +101,17 @@ func runListPaginationCase(t *testing.T, tc listPaginationCase, srv *httptest.Se resetCmdFlags(t, tc.cmd) for name, val := range tc.extraFlags { - if err := tc.cmd.Flags().Set(name, val); err != nil { - t.Fatalf("setting --%s=%s: %v", name, val, err) + if setErr := tc.cmd.Flags().Set(name, val); setErr != nil { + t.Fatalf("setting --%s=%s: %v", name, val, setErr) } } - var out bytes.Buffer + var out, errOut bytes.Buffer tc.cmd.SetOut(&out) + tc.cmd.SetErr(&errOut) tc.cmd.SetContext(context.Background()) - err := tc.cmd.RunE(tc.cmd, tc.args) - return out.String(), err + runErr := tc.cmd.RunE(tc.cmd, tc.args) + return out.String(), errOut.String(), runErr } // jstr renders a Go string as a JSON string literal for hand-built fixture @@ -185,12 +190,13 @@ func listPaginationCases() []listPaginationCase { rowIDs: idRows("id"), }, { - name: "automations executions list", - cmd: automationsExecutionsListCmd, - method: http.MethodGet, - wantPath: "/api/v1/automation_executions", - page: flatPage("automationExecutions", "id"), - rowIDs: idRows("id"), + name: "automations executions list", + cmd: automationsExecutionsListCmd, + method: http.MethodGet, + wantPath: "/api/v1/automation_executions", + page: flatPage("automationExecutions", "id"), + rowIDs: idRows("id"), + stuckCursorRoute: "GET /api/v1/automation_executions", }, { name: "automations list", @@ -234,21 +240,23 @@ func listPaginationCases() []listPaginationCase { rowIDs: idRows("id"), }, { - name: "functions commits", - cmd: functionsCommitsCmd, - method: http.MethodGet, - wantPath: "/api/v1/functions/func1/commits", - args: []string{"func1"}, - page: flatPage("list", "id"), - rowIDs: idRows("id"), + name: "functions commits", + cmd: functionsCommitsCmd, + method: http.MethodGet, + wantPath: "/api/v1/functions/func1/commits", + args: []string{"func1"}, + page: flatPage("list", "id"), + rowIDs: idRows("id"), + stuckCursorRoute: "GET /api/v1/functions/func1/commits", }, { - name: "functions list", - cmd: functionsListCmd, - method: http.MethodGet, - wantPath: "/api/v1/functions", - page: flatPage("list", "id"), - rowIDs: idRows("id"), + name: "functions list", + cmd: functionsListCmd, + method: http.MethodGet, + wantPath: "/api/v1/functions", + page: flatPage("list", "id"), + rowIDs: idRows("id"), + stuckCursorRoute: "GET /api/v1/functions", }, { name: "functions usage", @@ -267,7 +275,8 @@ func listPaginationCases() []listPaginationCase { } return fmt.Sprintf(`{"list":[%s],"nextPageToken":%s}`, strings.Join(items, ","), jstr(next)) }, - rowIDs: idRows("automation_id"), + rowIDs: idRows("automation_id"), + stuckCursorRoute: "GET /api/v1/automations", }, { name: "grants list", @@ -443,7 +452,7 @@ func TestListCommandsPaginateAcrossTwoPages(t *testing.T) { })) defer srv.Close() - out, err := runListPaginationCase(t, tc, srv) + out, _, err := runListPaginationCase(t, tc, srv) if err != nil { t.Fatalf("RunE: %v", err) } @@ -486,7 +495,7 @@ func TestListCommandsEmptyPageWithTokenDoesNotStopEarly(t *testing.T) { })) defer srv.Close() - out, err := runListPaginationCase(t, tc, srv) + out, _, err := runListPaginationCase(t, tc, srv) if err != nil { t.Fatalf("RunE: %v", err) } @@ -524,7 +533,7 @@ func TestListCommandsExplicitPageTokenDisablesAutoPagination(t *testing.T) { tc.extraFlags = map[string]string{} } tc.extraFlags["page-token"] = "manual-tok" - out, err := runListPaginationCase(t, tc, srv) + out, _, err := runListPaginationCase(t, tc, srv) if err != nil { t.Fatalf("RunE: %v", err) } @@ -540,3 +549,55 @@ func TestListCommandsExplicitPageTokenDisablesAutoPagination(t *testing.T) { }) } } + +// Every guarded command, against a server that reseals a fresh token over the +// same page: stop, emit the row once, warn naming the route, exit 6. +func TestListCommandsStuckCursorReturnsExitServer(t *testing.T) { + for _, tc := range listPaginationCases() { + if tc.stuckCursorRoute == "" { + continue + } + t.Run(tc.name, func(t *testing.T) { + var requestCount int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestCount++ + if requestCount > 4 { + t.Errorf("server received a 5th request; auto-pagination did not terminate") + w.WriteHeader(http.StatusBadRequest) + return + } + tok := extractPageToken(t, tc.method, r) + w.Header().Set("Content-Type", "application/json") + switch tok { + case "": + _, _ = fmt.Fprint(w, tc.page([]string{"stuck-item"}, "tok-v1")) + case "tok-v1", "tok-v2": + _, _ = fmt.Fprint(w, tc.page([]string{"stuck-item"}, fmt.Sprintf("tok-v%d", requestCount))) + default: + t.Errorf("unexpected page token: %q", tok) + w.WriteHeader(http.StatusInternalServerError) + } + })) + defer srv.Close() + + out, errOut, err := runListPaginationCase(t, tc, srv) + if err == nil { + t.Fatalf("RunE: want an error (stuck cursor -> exit 6), got nil; stdout=%q stderr=%q", out, errOut) + } + if got, want := exitCode(err), exitServer; got != want { + t.Errorf("exitCode(err) = %d, want %d (exitServer, \"C1 failed\"); err=%v", got, want, err) + } + if requestCount != 3 { + t.Fatalf("server received %d requests, want exactly 3 (the page must repeat twice before the cursor counts as stuck)", requestCount) + } + rows := decodeNDJSONRows(t, out) + ids := tc.rowIDs(rows) + if len(ids) != 1 || ids[0] != "stuck-item" { + t.Fatalf("got ids %v, want exactly one occurrence of stuck-item (a stuck cursor must not re-emit it)", ids) + } + if !strings.Contains(errOut, "INCOMPLETE") || !strings.Contains(errOut, tc.stuckCursorRoute) { + t.Errorf("stderr warning = %q, want it to say results are INCOMPLETE and name %s", errOut, tc.stuckCursorRoute) + } + }) + } +}