Skip to content

Commit 89a8115

Browse files
authored
client: keep pagination metadata without --all (#45)
## Description Replaces PR #31 (which added `--all` auto-pagination) with a minimal version that keeps only what downstream PRs depend on. James flagged `--all` as an anti-pattern ripe for abuse — agents that need multiple pages should paginate explicitly with `--token`. **What changed:** The `Spec` struct previously carried `TokenField` and `DataField` — per-spec strings that named the cursor field and data envelope field. In practice, every HeyGen v3 endpoint uses the same values (`"next_token"` and `"data"`), so these were redundant. This PR: 1. **Replaces `TokenField`/`DataField` with `Paginated bool`** on `command.Spec`. Codegen sets `Paginated: true` for list endpoints (detected by the presence of a `next_token` response field + `token` query param). The bool is metadata only — it marks which commands support cursor pagination but doesn't drive any runtime behavior yet. 2. **Exports `client.APIDataField` constant** (`"data"`) — a single source of truth for the response envelope field name. The builder passes this to `formatter.Data()` for `--human` table rendering. Downstream PRs (polling, auth status) also use it. 3. **Updates codegen** — `detectPagination()` now returns `bool` instead of `(bool, string, string)`. Checks both response schema (has `next_token`) and request params (has `token` query param) before marking as paginated. Golden files updated. 4. **Documents the pagination contract** in CLAUDE.md and AGENTS.md: manual pagination via `--limit` and `--token`, no `--all`. **What was removed vs PR #31:** `ExecuteAll()`, `extractPage()`, `marshalItems()`, `cloneInvocation()`, `--all` flag registration, `--all`/`--token` mutual exclusion, and all associated tests. **What was kept:** `Paginated bool` (used by downstream PRs for flag presence detection), `APIDataField` (used by builder, polling, and auth status), and `detectPagination()` in codegen. ## Testing - All existing tests pass — `make test` across linux/macos/windows - `TestGroupEndpoints_Pagination` updated to assert `Paginated: true` instead of `TokenField`/`DataField` strings - `videoListSpec` in builder tests updated to use `Paginated: true` - Golden files regenerated and verified - `./bin/heygen video list --all` → unknown flag error (confirmed removed) - `./bin/heygen video list --limit 5` → works (manual pagination unchanged)
1 parent 3a59061 commit 89a8115

21 files changed

Lines changed: 77 additions & 102 deletions

AGENTS.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,15 @@ Files in `gen/` are deleted and regenerated by `make generate`. Never put hand-w
5757
```go
5858
Data(v json.RawMessage, dataField string, columns []command.Column) error
5959
```
60-
- **Generated commands** pass `spec.DataField` and `defaultColumnsForSpec(spec)`.
60+
- **Generated commands** pass `client.APIDataField` and `defaultColumnsForSpec(spec)`.
6161
- **Hand-written commands** (auth, config) pass `"", nil`.
6262
- `JSONFormatter` ignores `dataField` and `columns` — always renders the full response. No contract change from the agent's perspective.
6363
- `HumanFormatter` uses `dataField` to unwrap the envelope, then renders a table (array) or key-value (object).
6464

65+
### Pagination
66+
- List commands paginate manually via API query flags like `--limit` and `--token`.
67+
- Do not add a generic `--all` mode. If an agent needs multiple pages, it should read `next_token` from the JSON response and request the next page explicitly.
68+
6569
### Auth and `skipAuth`
6670
`initContext()` resolves credentials via `ChainCredentialResolver` (env → file). Commands that don't need auth (e.g., `auth login`, `config set`) annotate with `skipAuth: true` in Cobra `Annotations`. Check `skipAuth` before adding new non-auth commands.
6771

CLAUDE.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,15 @@ Files in `gen/` are deleted and regenerated by `make generate`. Never put hand-w
5555
```go
5656
Data(v json.RawMessage, dataField string, columns []command.Column) error
5757
```
58-
- **Generated commands** pass `spec.DataField` and `defaultColumnsForSpec(spec)`.
58+
- **Generated commands** pass `client.APIDataField` and `defaultColumnsForSpec(spec)`.
5959
- **Hand-written commands** (auth, config) pass `"", nil`.
6060
- `JSONFormatter` ignores `dataField` and `columns` — always renders the full response. No contract change from the agent's perspective.
6161
- `HumanFormatter` uses `dataField` to unwrap the envelope, then renders a table (array) or key-value (object).
6262

63+
### Pagination
64+
- List commands paginate manually via API query flags like `--limit` and `--token`.
65+
- Do not add a generic `--all` mode. If an agent needs multiple pages, it should read `next_token` from the JSON response and request the next page explicitly.
66+
6367
### Auth and `skipAuth`
6468
`initContext()` resolves credentials via `ChainCredentialResolver` (env → file). Commands that don't need auth (e.g., `auth login`, `config set`) annotate with `skipAuth: true` in Cobra `Annotations`. Check `skipAuth` before adding new non-auth commands.
6569

cmd/heygen/builder.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"strconv"
99
"strings"
1010

11+
"github.com/heygen-com/heygen-cli/internal/client"
1112
"github.com/heygen-com/heygen-cli/internal/command"
1213
clierrors "github.com/heygen-com/heygen-cli/internal/errors"
1314
"github.com/spf13/cobra"
@@ -46,7 +47,7 @@ func buildCobraCommand(spec *command.Spec, ctx *cmdContext) *cobra.Command {
4647
return err
4748
}
4849

49-
return ctx.formatter.Data(result, spec.DataField, defaultColumnsForSpec(spec))
50+
return ctx.formatter.Data(result, client.APIDataField, defaultColumnsForSpec(spec))
5051
},
5152
}
5253

cmd/heygen/builder_test.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,12 @@ import (
1717
// videoListSpec mirrors the hand-written video list command as a Spec,
1818
// proving the generic builder produces identical behavior.
1919
var videoListSpec = &command.Spec{
20-
Group: "video",
21-
Name: "list",
22-
Summary: "List videos",
23-
Endpoint: "/v3/videos",
24-
Method: "GET",
25-
// TokenField/DataField for pagination (used by paginator, not tested here)
26-
TokenField: "next_token",
27-
DataField: "data",
20+
Group: "video",
21+
Name: "list",
22+
Summary: "List videos",
23+
Endpoint: "/v3/videos",
24+
Method: "GET",
25+
Paginated: true,
2826
Flags: []command.FlagSpec{
2927
{Name: "limit", Type: "int", Source: "query", JSONName: "limit"},
3028
{Name: "token", Type: "string", Source: "query", JSONName: "token"},

codegen/grouper.go

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import (
5353
// → sessions → sub-group, {session_id} → arg, stop → sub-group
5454
// → x-cli-action: true → no terminal verb appended
5555
// → result: heygen video-agent sessions stop <session-id>
56+
//
5657
// GroupDescriptions maps group name → description from the OpenAPI tag.
5758
// Used by the builder for group command help text.
5859
type GroupDescriptions map[string]string
@@ -156,8 +157,9 @@ func buildSpec(
156157
spec.BodyEncoding = "multipart"
157158
}
158159

159-
// Pagination
160-
_, spec.TokenField, spec.DataField = detectPagination(op)
160+
// Pagination — only sets Paginated bool. The cursor field names and data
161+
// field are API conventions hardcoded in the client package.
162+
spec.Paginated = detectPagination(op, pathItem)
161163

162164
// Flags from query params
163165
for _, paramRef := range collectParams(pathItem, op) {
@@ -439,27 +441,46 @@ func isComplexField(s *openapi3.Schema) bool {
439441

440442
// --- Response analysis ---
441443

442-
func detectPagination(op *openapi3.Operation) (hasMore bool, tokenField, dataField string) {
444+
// detectPagination returns true if the endpoint supports cursor-based pagination.
445+
// An endpoint is paginated when both: (1) the response has a cursor field
446+
// (next_token) and (2) the request has a cursor query param (token).
447+
func detectPagination(op *openapi3.Operation, pathItem *openapi3.PathItem) bool {
443448
respSchema := successResponseSchema(op)
444449
if respSchema == nil {
445-
return
446-
}
447-
if dataProp := respSchema.Properties["data"]; dataProp != nil && dataProp.Value != nil {
448-
dataField = "data"
450+
return false
449451
}
450-
// Check for token at root level and inside data wrapper
452+
453+
// Check for cursor field in response (at root or inside data wrapper).
454+
hasCursorField := false
451455
schemasToCheck := []*openapi3.Schema{respSchema}
452-
if dataField != "" {
453-
schemasToCheck = append(schemasToCheck, respSchema.Properties["data"].Value)
456+
if dataProp := respSchema.Properties["data"]; dataProp != nil && dataProp.Value != nil {
457+
schemasToCheck = append(schemasToCheck, dataProp.Value)
454458
}
455459
for _, schema := range schemasToCheck {
456-
for _, candidate := range []string{"next_token", "token", "cursor"} {
457-
if _, ok := schema.Properties[candidate]; ok {
458-
return true, candidate, dataField
459-
}
460+
if _, ok := schema.Properties["next_token"]; ok {
461+
hasCursorField = true
462+
break
460463
}
461464
}
462-
return
465+
if !hasCursorField {
466+
return false
467+
}
468+
469+
return detectCursorParam(pathItem, op) != ""
470+
}
471+
472+
func detectCursorParam(pathItem *openapi3.PathItem, op *openapi3.Operation) string {
473+
params := collectParams(pathItem, op)
474+
for _, paramRef := range params {
475+
param := paramRef.Value
476+
if param == nil || param.In != "query" {
477+
continue
478+
}
479+
if param.Name == "token" {
480+
return param.Name
481+
}
482+
}
483+
return ""
463484
}
464485

465486
func successResponseSchema(op *openapi3.Operation) *openapi3.Schema {

codegen/grouper_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,8 @@ func TestGroupEndpoints_Pagination(t *testing.T) {
156156
if s.Name != "list" {
157157
continue
158158
}
159-
if s.TokenField != "token" {
160-
t.Errorf("TokenField = %q, want 'token'", s.TokenField)
161-
}
162-
if s.DataField != "data" {
163-
t.Errorf("DataField = %q, want 'data'", s.DataField)
159+
if !s.Paginated {
160+
t.Error("Paginated = false, want true")
164161
}
165162
return
166163
}

codegen/templates/command.go.tmpl

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,8 @@ var {{pascalCase .Group}}{{pascalCase .Name}} = &command.Spec{
1414
Endpoint: {{quote .Endpoint}},
1515
Method: {{quote .Method}},
1616
BodyEncoding: {{quote .BodyEncoding}},
17-
{{- if .TokenField}}
18-
TokenField: {{quote .TokenField}},
19-
{{- end}}
20-
{{- if .DataField}}
21-
DataField: {{quote .DataField}},
17+
{{- if .Paginated}}
18+
Paginated: true,
2219
{{- end}}
2320
{{- if .Examples}}
2421
Examples: []string{

codegen/testdata/golden/upload.go

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

codegen/testdata/golden/widget.go

Lines changed: 1 addition & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

codegen/testdata/test_spec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ paths:
3737
type: array
3838
items:
3939
type: object
40-
token:
40+
next_token:
4141
type: string
4242
post:
4343
tags: [Videos]

0 commit comments

Comments
 (0)