Skip to content

refactor: decouple command discovery and reject pagination cycles - #48

Merged
thedogwiththedataonit merged 26 commits into
mainfrom
codex/cli-catalog-pagination
Oct 20, 2026
Merged

thedogwiththedataonit merged 26 commits into
mainfrom
codex/cli-catalog-pagination

Conversation

@thedogwiththedataonit

@thedogwiththedataonit thedogwiththedataonit commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

Discovery commands import the command registry, which imports those handlers back. Inject a projection of command metadata from CLI composition into the execution context, then build docs --agent and doctor from that catalog. Command registration and confirmation remain in the existing composition layer.

--all rejects identical or cyclic cursors with exit code 1 and CLI_PAGINATION on stderr. Every paginated command passes its starting cursor, including JSON input; nested comment-thread message walks share the cycle check. Partial rows stay out of stdout, and errors report fetched rows/pages without offering a cyclic cursor as a resume point. Empty advancing pages, ordinary transport errors, cancellation, and the command surface retain their existing contracts. Pin the local package manager to Bun 1.3.10, matching upstream workflows.

types now uses the shared complete-page drain instead of silently stopping after 50 pages. It finishes pagination before writing or checking a contract file, rejects repeated cursors, and checks cancellation after the final successful read before touching the artifact. A failed or interrupted read preserves the existing file and cannot certify it. Regression tests cover identical/multi-page cycles in write and check modes, the 51st page, and interruption at the final read boundary.

Validation at ef725d684baf40ff75f49bbc1371e518b3f2afc7: Bun typecheck, full 219-file Biome check, all 588 tests in 44 files, generated command-doc freshness, and the package build passed. Coverage includes all 23 paginated command families, JSON input cursors, and identical/multi-page/resumed comment-thread cycles. The built binary passed version/help/144-command agent-manifest checks and real SIGINT/SIGTERM smoke tests. Parity and confirmation tests remain enabled. CI is green on this head.

Latest main is merged, including the complete Figma JSON input contract. Regenerate the OpenAPI mirror and types from merged application Stripe PR #2035: six contact-response consent-source unions add integration, documenting provenance without asserting an opt-in. Request consent-source enums retain their existing contract. The four coordinated application, docs, SDK, and CLI OpenAPI mirrors match SHA-256 4d68ab8d4877bca19f206e2153fa2cde65b3240767f033d051fb1b6667aff75f; application Figma PR #2029 is merged. Package versions and the published SDK dependency are unchanged. Release the SDK before a future CLI dependency update adopts its new API; this CLI fix uses its own error.

Companion SDK PR #59; coordinated docs contract-sync PR #200; application review PR #2027.

RetriggerView in GreptileConfidence Score: 5/5

The PR appears safe to merge; no new actionable issues were found.

Summary

The PR separates command discovery from executable handlers and rejects pagination cycles without printing partial results.

  • Paginated reads stop when a list or comment thread repeats a cursor.
  • brew-cli types reads every trigger page before it checks or writes a contract file.
  • docs --agent and doctor get command details from the CLI's metadata catalog.
  • Write commands confirm resolved stdin requests and keep their rerun input intact.
  • Table output handles large result sets without building a huge temporary argument list.
  • The package now pins its package manager to Bun 1.3.10.

Both earlier unnumbered findings are addressed: resumed lists track their starting cursor, and comment-thread walks use the shared cycle check. The previous and current heads have different merge bases with the target commit, so this review used the full PR diff. No new actionable issues were found. Checks were limited to source and test inspection; no tests were run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[CLI composition] --> B[Command metadata]
  B --> C[Execution context]
  C --> D[docs and doctor]
  E[Paginated read] --> F{Cursor already fetched?}
  F -->|Yes| G[Fail without partial stdout]
  F -->|No| H{Cursor is null?}
  H -->|No| E
  H -->|Yes| I[Return complete rows]
Loading

Reviews (3) · Last reviewed commit: "Merge branch 'audit/cli-50' into audit/c..." · Reviewed by Greptile

Comment thread src/lib/paginate.ts Outdated
Comment thread src/lib/paginate.ts Outdated
@thedogwiththedataonit
thedogwiththedataonit merged commit 557883f into main Oct 20, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant