fix: batch CLI issue fixes#14
Merged
Merged
Conversation
PR Summary by QodoFix CLI list output, pagination, and workspace selection; refresh docs
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
Code Review by Qodo
Context used 1. Workspace recovery blocked
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+79
to
+101
| if (opts.all) { | ||
| const rows: unknown[] = []; | ||
| let cursor: string | undefined; | ||
| let page: { nextCursor?: string | null; hasMore?: boolean } | undefined; | ||
| do { | ||
| const body = unwrap(await list({ query: { ...query, cursor } })); | ||
| rows.push(...(body.data ?? [])); | ||
| page = body.page; | ||
| cursor = page?.nextCursor ?? undefined; | ||
| } while (page?.hasMore && cursor); | ||
| if (opts.raw) { | ||
| print( | ||
| { data: rows, page: { nextCursor: null, hasMore: false } }, | ||
| { json: true }, | ||
| ); | ||
| return; | ||
| } | ||
| if (opts.csv) { | ||
| process.stdout.write(`${toCsv(rows, columns)}\n`); | ||
| return; | ||
| } | ||
| print(rows, { json: opts.json, columns }); | ||
| return; |
There was a problem hiding this comment.
1. --all lacks guardrails 📎 Requirement gap ☼ Reliability
The new --all auto-pagination loop can fetch an unbounded number of pages with no limits or safeguards, risking very large downloads and rate-limit/time issues. This does not meet the requirement that --all be safe/guardrailed for large lists/exports.
Agent Prompt
## Issue description
The new `--all` implementation auto-paginates without any guardrails (max pages/items, timeout/abort, or confirmation), which can lead to unbounded requests and very large outputs.
## Issue Context
Compliance requires `--all` to be safe and consistent for large lists/exports. Currently, the loop continues while `page.hasMore` and `nextCursor` are truthy, with no upper bound.
## Fix Focus Areas
- src/commands/resource.ts[79-101]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+98
to
103
| > **Write operations** (`create`/`update`/`delete` and actions like `publish`, | ||
| > `cancel`, `refund`) send a JSON body via `--data <inline-json>` or | ||
| > `--data @file.json`. `delete` prompts for confirmation unless you pass `--yes`. | ||
|
|
||
| ### Common flags | ||
|
|
There was a problem hiding this comment.
2. Readme missing minimum cli version 📎 Requirement gap ⚙ Maintainability
The updated README documents new/updated CLI behaviors (writes, --all, --raw, workspace switching) but does not specify a minimum CLI version or add troubleshooting/update steps to prevent version drift. Users running an older cached/global version may not have these behaviors, causing operational confusion and support churn.
Agent Prompt
## Issue description
README describes behaviors that may not exist in older installed/cached CLI versions, but it does not state a minimum required CLI version nor provide upgrade/troubleshooting steps.
## Issue Context
Compliance requires documentation to avoid version drift, including minimum version guidance and troubleshooting (e.g., checking version and using `@latest`). The README was updated for writes/pagination/workspace behavior but still lacks those guardrails.
## Fix Focus Areas
- README.md[98-113]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+45
to
+47
| configureClient(); | ||
| const me = unwrap(await getMe({})).data; | ||
| const match = me.workspaces.find( |
There was a problem hiding this comment.
3. Workspace recovery blocked 🐞 Bug ☼ Reliability
ft workspace list|use|show calls configureClient() which automatically applies the persisted workspaceId as X-Workspace-Id; if that ID is stale/unauthorized, getMe() can fail and the user cannot use these commands to recover. workspace show also prefers the persisted workspaceId even when it doesn’t exist in me.workspaces, causing an unnecessary hard failure.
Agent Prompt
### Issue description
The new `workspace` commands are intended to help users switch workspaces, but they currently depend on the existing configured workspace header being valid. Because `configureClient()` injects `X-Workspace-Id` from config by default, a stale/unauthorized `workspaceId` can prevent `getMe()` from succeeding, leaving users unable to run `ft workspace list/use/show` to fix the configuration.
### Issue Context
- `configureClient()` chooses `workspaceOverride ?? cfg.workspaceId` and sends it as `X-Workspace-Id`.
- Workspace commands call `configureClient()` with no override before calling `getMe()`.
- `workspace show` uses `cfg.workspaceId ?? me.activeWorkspaceId` and fails if that ID isn’t found in `me.workspaces`, instead of falling back.
### Fix Focus Areas
- src/lib/api.ts[9-21]
- src/commands/workspace.ts[23-79]
### Suggested fix
1. Add a supported way to configure the client **without** the workspace header (e.g., `configureClient({ workspaceOverride?: string, disableWorkspaceHeader?: boolean })` or accept `null` as “do not send”).
2. In `workspace list/use/show`, call the “no workspace header” configuration so `GET /me` works even when the saved workspace is broken.
3. In `workspace show` (and optionally `list`), if a configured workspaceId is not present in `me.workspaces`, fall back to `me.activeWorkspaceId` and/or print a warning that the local config is stale and needs `ft workspace use <slug>`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+79
to
+93
| if (opts.all) { | ||
| const rows: unknown[] = []; | ||
| let cursor: string | undefined; | ||
| let page: { nextCursor?: string | null; hasMore?: boolean } | undefined; | ||
| do { | ||
| const body = unwrap(await list({ query: { ...query, cursor } })); | ||
| rows.push(...(body.data ?? [])); | ||
| page = body.page; | ||
| cursor = page?.nextCursor ?? undefined; | ||
| } while (page?.hasMore && cursor); | ||
| if (opts.raw) { | ||
| print( | ||
| { data: rows, page: { nextCursor: null, hasMore: false } }, | ||
| { json: true }, | ||
| ); |
There was a problem hiding this comment.
4. Autopagination can stop early 🐞 Bug ≡ Correctness
In --all mode, pagination stops when page.hasMore is true but page.nextCursor is
missing/empty, returning partial results without any error. The --raw output in --all mode also
hardcodes { hasMore: false, nextCursor: null }, which can mask that early termination.
Agent Prompt
### Issue description
The `--all` autopagination loop assumes that `page.hasMore === true` always comes with a usable `page.nextCursor`. If that assumption is violated (or if `nextCursor` is accidentally empty), the loop stops early and returns incomplete results with no warning.
### Issue Context
Current loop:
- sets `cursor = page?.nextCursor ?? undefined`
- continues while `(page?.hasMore && cursor)`
So `hasMore=true` + missing cursor will terminate.
Also, `--raw` in `--all` mode emits a fabricated `page` object with `hasMore:false`, which can hide that termination condition.
### Fix Focus Areas
- src/commands/resource.ts[79-101]
### Suggested fix
1. After each page, if `page?.hasMore` is true but `page?.nextCursor` is falsy, abort with `fail()` explaining the pagination metadata is inconsistent.
2. Consider tracking seen cursors and aborting if a cursor repeats to prevent potential infinite loops.
3. For `--raw --all`, either:
- return the last real `page` from the server plus a flag like `aggregated: true`, or
- include a warning field indicating the page metadata is no longer meaningful after aggregation.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Batch of client-side CLI fixes (no contract changes). Build passed; lint passed.
Fixed
README.md: removed the stale "writes return 501 / phase 2" claim, documented every write command (create/update/delete/publish/cancel/refund/set-role) and the new workspace group in the Commands table, added the new list/write flags, and updated Quickstart + Pagination sections. Also replaced the stale 501 hint text insrc/lib/api.ts. Leftdocs/PRD.mdanddocs/BACKLOG.mduntouched as historical roadmap artifacts.src/commands/workspace.tswithft workspace list|use|show, registered inindex.ts. ReadsgetMe(workspaces[]+activeWorkspaceId), persists the selection through the existingsaveConfig({workspaceId})whichapi.tsalready sends as theX-Workspace-Idheader. Pure local config, no contract change.--rawflag to list commands inresource.tsthat prints the full{ data, page }envelope as JSON, surfacing the existingpage.nextCursor/hasMoremetadata that--json(data-only) omits.--allto list commands: a client-side loop over the existing cursor query param that fetches every page (whilepage.hasMore) and concatenates the results before output; works with--json/--raw/--csv/table.index.tsagainst the DTOs: ticket-typesstock->capacity, eventsstartsAt->createdAt(startsAtlives onEventDate), and plansinterval->billingCycle(same-class latent bug). Added--columns/--fullvia a newresolveColumns()helper inoutput.ts; CSV now honors the resolved columns too.Closes #13
Closes #12
Closes #10
Closes #8
Closes #6
Contract-blocked (not in this PR)
These issues require endpoints/params the OpenAPI contract does not yet expose, so they are out of scope here. They are tracked in
CONTRACT-GAPS.mdand needendpoint-requesterto open the backend requests infree-admin:GET /salesquery params (event,eventDate,reference,buyer,from,to,channel).GET /reports/inventoryaggregate endpoint (free-admin#165).GET /reports/exports/buyersfilters + ticket/event fields, or a newGET /reports/exports/attendees./reports/inventory,/salesfilters,/reports/exportsfilters).🤖 Generated with Claude Code