feat: add cursor-first pagination (--limit/--continue)

[jpage-godaddy]

Summary

• Implements docs/proposals/cursor-first-pagination.md: CommandSpec::with_cursor/CursorConfig register --limit/--continue as a new, parallel pagination mechanism. CommandSpec::with_pagination/PaginationConfig (--limit/--offset) is untouched and fully supported — a command opts into at most one of the two.
• Unlike offset pagination, the engine never slices or measures a cursor itself (a cursor is backend-opaque) — the handler reports what it learned from its own backend call via CommandResult::with_cursor(CursorContinuation), which surfaces as a new envelope.cursor field (CursorMeta) and an automatic next_actions "next page" suggestion, mirroring the offset-pagination machinery end to end (flag registration, middleware state, envelope construction, human-output rendering, docs).
• CursorContinuation::with_limit lets a handler report an effective page size that differs from the parsed --limit (e.g. derived from the --continue token itself, so a caller can resume with --continue alone without repeating --limit) — this also signals the engine to omit --limit from the auto-generated next-page command, since the token is then self-sufficient about size.
• raw_output remains mutually exclusive with both pagination styles.
• Middleware and MiddlewareRequest are now #[non_exhaustive] (addressed a Copilot review finding): both structs already grew a field once without any compatibility escape hatch for external callers. MiddlewareRequest gains a new() constructor plus with_auth/with_view_id/with_raw_output/with_pagination_command/with_cursor_command builders, since #[non_exhaustive] forbids struct-literal construction (even ..Default::default() spread) from outside this crate.
• PaginationConfig/CursorConfig are also now #[non_exhaustive] with a plain new(default_limit, max_limit) constructor each, for the same reason and for API consistency between the two sibling pagination configs.
• Cursor commands are rejected at construction time when paired with a non-context (RuntimeCommandSpec::new/new_typed) or streaming constructor — neither can act on --continue — mirroring the existing handles_dry_run misuse guard. A cursor command's response is also never client-sliced by stale offset-pagination state, even if a pre_run hook or a direct Middleware::run caller sets limit/offset after flag parsing. A backend-supplied --continue token with control characters (e.g. an ANSI escape sequence) is escaped before it's ever printed in a suggested next-page command or a raw --output toon field, and a bare ! is spliced into its own single-quoted segment so a copy-pasted suggestion can't trigger interactive Bash history expansion.
• envelope.cursor.count reflects the handler's actual returned page size, not a stale/--expr-reshaped display count — captured before the output pipeline runs, mirroring how offset pagination already captures PaginationMeta.count. CursorMeta.self_sufficient_limit records whether continue_from alone (via CursorContinuation::with_limit) is enough to resume at limit, so the human "so far" hint always matches what the generated next-page command actually needs.

Test plan

• cargo fmt --all --check
• cargo clippy --all-targets -- -D warnings
• RUSTDOCFLAGS='-D warnings' cargo doc --no-deps
• cargo test --all-targets (new tests/cursor_pagination.rs, 26 end-to-end cases, plus updates to existing pagination/human-output/TOON tests)
• cargo rustdoc --lib -- -W missing-docs (zero missing docs)
• ./cli-engine/scripts/check-module-size.sh

Manual verification

Exercised end-to-end against gddy with its cli-engine dependency patched to this branch. Migrated all 8 of its .with_pagination commands to .with_cursor, covering every backend shape this feature needs to support:

• A genuine opaque forward cursor (domain list's v3 pageToken)
• Fixed page/pageSize backends synthesizing their own continuation token (email list, dns list)
• A real GraphQL Relay cursor (platform app list's first/after)
• Fully in-memory/static collections (platform actions list, api search/response list/parameter list)

Confirmed via --help and live runs that: --continue shows up only for commands that opted in; --limit/--continue are rejected as unknown args otherwise; a suggested next-page command round-trips correctly across multiple pages; and --limit is correctly omitted from the suggestion exactly when (and only when) a handler's token is self-sufficient about page size.

[Copilot]

🟡 Changes recommended

Unresolved public API compatibility breaks and cursor validation/rendering issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds opt-in cursor-first pagination alongside offset pagination, including cursor flags, middleware state, envelope metadata, next-page actions, human rendering, documentation, and tests.

Changes:

• Adds CursorConfig, CursorContinuation, and --limit/--continue.
• Extends envelopes and human output with cursor information.
• Adds end-to-end cursor pagination coverage and authoring guidance.

File summaries

File	Summary
cli-engine/tests/foundation.rs	Updates middleware request fixtures.
cli-engine/tests/cursor_pagination.rs	Adds end-to-end cursor pagination coverage.
cli-engine/src/output/mod.rs	Re-exports cursor metadata.
cli-engine/src/output/human/tests.rs	Updates renderer tests.
cli-engine/src/output/human/mod.rs	Integrates cursor rendering.
cli-engine/src/output/human/footer.rs	Adds cursor summaries; token quoting and scalar fallback handling need changes.
cli-engine/src/output/human/body.rs	Adds cursor-aware table rendering.
cli-engine/src/output/envelope.rs	Adds CursorMeta; the public struct field is a downstream compatibility break.
cli-engine/src/middleware/run.rs	Builds cursor metadata and next actions; metadata should be restricted to array data.
cli-engine/src/middleware/mod.rs	Adds cursor state; the public field is a downstream compatibility break.
cli-engine/src/lib.rs	Exposes cursor APIs.
cli-engine/src/flags/register.rs	Registers and validates cursor arguments.
cli-engine/src/flags/mod.rs	Updates cursor flag handling.
cli-engine/src/command/spec.rs	Adds cursor configuration; the derived default currently permits an invalid zero limit.
cli-engine/src/command/mod.rs	Adds cursor continuation result APIs.
cli-engine/src/cli/schema_tree.rs	Registers cursor flags and validates combinations.
cli-engine/src/cli/run.rs	Applies cursor configuration during dispatch.
cli-engine/src/cli/mod.rs	Exposes command quoting helpers.
cli-engine/src/cli/flags_apply.rs	Parses and replays cursor flags.
cli-engine/docs/concepts.md	Documents cursor pagination; contains a spacing typo.
AGENTS.md	Updates cursor pagination authoring guidance.

Review details

Suppressed comments (4)

cli-engine/src/command/spec.rs:194

• CursorConfig derives Default, but that default has default_limit == 0 even though this type documents zero as invalid. In optimized builds the debug_assert! in with_cursor is removed, so CursorConfig::default() reaches the parser with a default --limit 0 that the cursor parser rejects, leaving the command unusable. Make the default valid or enforce the invariant in release builds as well.

#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
pub struct CursorConfig {
    /// Page size sent to the backend when the user passes no `--limit`. Must
    /// be greater than zero.
    pub default_limit: i64,
    /// Upper bound a user can request with an explicit `--limit`. `0` (the
    /// default) means uncapped. Does not affect `default_limit` itself.
    pub max_limit: i64,

cli-engine/src/middleware/mod.rs:611

• MiddlewareRequest is a public, non-#[non_exhaustive] struct; adding cursor_command makes downstream struct literals fail to compile. This needs to be handled as a breaking API change or moved behind a compatibility-preserving constructor/extension mechanism.

    pub cursor_command: Option<String>,

cli-engine/src/output/envelope.rs:188

• CursorMeta.limit can be supplied by CursorContinuation::with_limit, so it is the effective page size applied by the handler, not necessarily the requested page size. The field documentation should reflect the override semantics exercised by the new API.

    /// Requested page size.
    pub limit: i64,

cli-engine/src/output/human/footer.rs:174

• Unlike append_pagination_summary immediately above, this fallback substitutes cursor.count when shown is None. If --expr length(@) turns the data into a scalar, human output therefore claims Showing 0 (or a cursor "so far" count) beside non-list data; use a neutral cursor summary when no rendered array count exists, matching the pagination fallback.

    let count = shown.unwrap_or(cursor.count);
    out.push_str(&format!(
        "\n{}\n",
        cursor_summary_text(SummaryStyle::Standalone, count, cursor)

• Files reviewed: 21/21 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Unresolved moderate findings affect streaming support, constructor usability, request invariants, and continuation instructions.

Review details

Suppressed comments (8)

Previously missed (3) — in code that hasn't changed since the last review.

cli-engine/src/cli/run.rs:519

• This forwards cursor_command to the streaming path, but run_streaming_command wraps every successful streaming handler result as CommandResult::new(Value::Null), so render_envelope never receives an array or CursorContinuation and can never emit cursor metadata or a next-page action. A streaming command with .with_cursor therefore advertises flags that do nothing; reject this combination or add a completion-metadata path for streaming handlers.
  cli-engine/docs/concepts.md:524
• This paragraph repeats an inaccurate contract: it says limit is always the parsed --limit, but CursorContinuation::with_limit intentionally replaces that value and causes the generated next action to omit --limit. Document this supported override here as well, otherwise consumers may assume the envelope always echoes the request.
  cli-engine/src/output/envelope.rs:184
• The new CursorMeta documentation says both limit and count are always computed from the parsed request, but CursorContinuation::with_limit intentionally overrides CursorMeta.limit and also changes next-action generation. Please document that handler-supplied effective limit here so consumers do not rely on an inaccurate contract.

cli-engine/docs/concepts.md:528

• The guide says every cursor next_actions command appends both --limit and --continue, but CursorContinuation::with_limit deliberately omits --limit when the token carries the effective page size. Document that exception so the guide matches the generated command.

When `continue_from` is present (`has_more`), the engine appends a `next_actions` entry replaying the command with `--limit`/`--continue <token>` for the next page.

cli-engine/src/cli/schema_tree.rs:220

• A streaming RuntimeCommandSpec can also carry this CommandSpec, so this registration exposes cursor flags for a handler that returns only Result<()>. run_streaming_command wraps that result as CommandResult::new(Value::Null), leaving no way to attach CursorContinuation; the stream therefore never gets envelope.cursor or an automatic next-page action even though with_cursor advertised the feature. Reject cursor pagination for streaming constructors (or add a streaming continuation/envelope path) instead of silently accepting this unsupported combination.

    command = apply_cursor_args(command, spec);

cli-engine/src/command/spec.rs:413

• with_cursor can be attached to RuntimeCommandSpec::new/new_typed, but those handlers cannot access CommandContext, and command_args_from_matches excludes these framework-owned flags. Such a command exposes --continue but has no way to send the parsed cursor state to its backend, so pagination cannot resume. Reject cursor specs in the non-context constructors (as handles_dry_run does) or provide a handler API carrying the cursor state.

    pub fn with_cursor(mut self, config: CursorConfig) -> Self {

cli-engine/src/middleware/mod.rs:681

• These public builder methods do not enforce the documented mutual-exclusion invariant: chaining MiddlewareRequest::new(...).with_pagination_command(...).with_cursor_command(...) leaves both options set, and render_envelope can then run both pagination paths and emit both metadata forms/actions. Clear the opposite option in both setters, or reject the combination, so externally constructed requests cannot violate the contract.

    pub fn with_cursor_command(mut self, cursor_command: impl Into<String>) -> Self {
        self.cursor_command = Some(cursor_command.into());
        self

cli-engine/src/output/human/footer.rs:100

• The continue_from summary tells users to run only --continue <token>, but this also covers opaque tokens where continuation.limit is None; the generated next_actions deliberately retains --limit {effective_limit} because resuming without it can change or invalidate the backend request. With an explicit non-default --limit, the human instruction therefore does not replay the same page and may fail. Carry the self-sufficient/limit information into the human summary (including the standalone branch) so it matches the generated next-page command.

    match (style, cursor.total, cursor.remaining, &cursor.continue_from) {
        (SummaryStyle::TableFooter, Some(total), _, _) => format!("{count} of {total} rows"),
        (SummaryStyle::TableFooter, None, Some(remaining), _) => {
            format!("{count} rows, {remaining} remaining")
        }

• Files reviewed: 21/21 changed files
• Comments generated: 0 new
• Review effort level: Lite

[jpage-godaddy]

Addressed in 8127a6b, in response to the second Copilot review pass (5226659693), whose 8 findings were reported as "suppressed comments" in the review body rather than posted as separate review threads — replying here since there's no thread to attach to per-item.

1. src/cli/run.rs:519 / src/cli/schema_tree.rs:220 — a streaming command paired with with_cursor would expose --continue even though run_streaming_command always wraps its result as CommandResult::new(Value::Null), so cursor metadata could never attach. Fixed by rejecting the pairing at construction time in RuntimeCommandSpec::new_streaming/new_typed_streaming (debug_assert!(spec.cursor.is_none())), mirroring the existing raw_output+streaming guard already there.
2. src/command/spec.rs:413 — with_cursor on RuntimeCommandSpec::new/new_typed gave the handler no way to read back middleware.cursor_limit/continue_token, so --continue couldn't actually resume anything. Fixed the same way as the existing handles_dry_run misuse guard: debug_assert!(spec.cursor.is_none()) in both non-context constructors, pointing callers at new_with_context/new_typed_with_context.
3. src/middleware/mod.rs:681 — MiddlewareRequest::with_pagination_command/with_cursor_command didn't enforce mutual exclusion; chaining both left both set. Fixed: each setter now clears the other field.
4. src/output/envelope.rs:184 / docs/concepts.md:524 — CursorMeta.limit's docs claimed it's always the parsed --limit, but CursorContinuation::with_limit intentionally overrides it. Fixed both docs to describe the override.
5. docs/concepts.md:528 — the guide said next_actions always appends both --limit and --continue, missing the with_limit exception that omits --limit. Fixed.
6. src/output/human/footer.rs:100 — the human "so far" summary told the user to run --continue <token> alone, but the actual generated next_actions command keeps --limit too whenever the token isn't self-sufficient about page size (no with_limit call) — resuming with a non-default --limit and only --continue could silently change page size. Fixed: the "so far" line now includes --limit (cursor.limit, the same effective value the generated command uses), so the two never disagree. Updated the three tests asserting the old wording.

Full verification suite (fmt, clippy, tests incl. tests/cursor_pagination.rs, doc tests, missing-docs, module-size) passes on 8127a6b.

[Copilot]

🟡 Changes recommended

A critical cursor/offset state issue and a moderate next-page hint inconsistency remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 22/22 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Unresolved moderate issues affect cursor replay safety, field preservation, public API compatibility, and pipeline behavior.

Review details

Suppressed comments (4)

cli-engine/src/cli/flags_apply.rs:216

• The cursor token is now sourced from a backend and passed through this helper for both next_actions and human output, but the quoted branch leaves control characters such as ESC, CR, and LF unchanged. A backend-supplied token containing an ANSI sequence can alter the terminal, and embedded newlines make the copy-paste command ambiguous even though shell metacharacters are escaped. Escape non-printable/control characters before rendering the token and add a regression test for this path.

pub(crate) fn quote_pagination_value(value: &str) -> String {
    let safe_unquoted =
        |c: char| c.is_ascii_alphanumeric() || matches!(c, '-' | '_' | '.' | '/' | ':' | '@');
    if value.is_empty() || !value.chars().all(safe_unquoted) {
        let escaped = value
            .replace('\\', "\\\\")
            .replace('"', "\\\"")
            .replace('$', "\\$")
            .replace('`', "\\`");
        format!("\"{escaped}\"")

cli-engine/src/cli/flags_apply.rs:113

• command_replay_base now also feeds cursor next_actions, but it ignores flags.fields_explicit. For a command with default_fields, flags.fields is non-empty even when the user did not pass --fields, so the suggested cursor command turns an author-controlled default into an explicit projection; the first run skips validation for that default, while the replay can fail when an optional field is absent. An explicit --fields "" is also dropped and reverts to the default on the next page. Only replay an explicit fields value, preserving an explicit empty value.

pub(super) fn command_replay_base(
    binary_name: &str,
    command_path: &str,

cli-engine/src/command/spec.rs:197

• CursorConfig is a new public configuration struct, but it is exhaustive and the documentation encourages consumers to construct it with a literal. Any future configuration field will therefore break every consumer, unlike the non-exhaustive public authoring types introduced elsewhere in this PR. Please make this type non-exhaustive and provide a constructor/builder (or another future-compatible construction pattern) before exposing it.

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct CursorConfig {
    /// Page size sent to the backend when the user passes no `--limit`. Must
    /// be greater than zero.
    pub default_limit: i64,
    /// Upper bound a user can request with an explicit `--limit`. `0` (the
    /// default) means uncapped. Does not affect `default_limit` itself.
    pub max_limit: i64,

cli-engine/src/middleware/run.rs:574

• Although apply_cursor_flags clears middleware.limit/offset, run_pre_run executes after that reset and can legally mutate the public middleware state; callers of Middleware::run can also preset those fields. apply_pipeline will then slice the backend page before this cursor block, potentially dropping rows and even producing both pagination and cursor metadata. Make the pipeline use zero limit/offset whenever cursor_command.is_some() (rather than relying only on the earlier reset).

        if let Some(base) = cursor_command
            && let Some(data) = &envelope.data
            && let Some(items) = data.as_array()

• Files reviewed: 22/22 changed files
• Comments generated: 0 new
• Review effort level: Lite

[jpage-godaddy]

Addressed 2 of 4 findings from the fourth Copilot review pass (5228862286, also reported as "suppressed comments" with no separate threads) in 7b75d7a:

1. src/cli/flags_apply.rs:216 (quote_pagination_value) — control characters (raw newlines, ANSI escape sequences) in a backend-supplied cursor token passed through unescaped, which could make the printed next-page suggestion look like multiple lines or repaint the terminal on display. Fixed: control characters now render as \n/\r/\t or a \xHH hex placeholder. Added 3 unit tests directly on the function (quote_pagination_value_tests module), including one for an ESC/ANSI sequence.
2. src/middleware/run.rs:574 — even after the previous fix (apply_cursor_flags zeroing middleware.limit/.offset), a pre_run hook runs after that reset and can still mutate those public fields, and a caller driving Middleware::run directly bypasses apply_cursor_flags entirely. Fixed by making the actual pipeline call site itself authoritative: PipelineOpts.limit/.offset are now forced to 0 whenever cursor_command.is_some(), regardless of how self.limit/self.offset got set.

The remaining 2 findings (CursorConfig non-exhaustiveness, and command_replay_base ignoring fields_explicit) are scope/consistency questions I'm checking with the repo owner on before deciding — will follow up.

Full verification suite passes on 7b75d7a.

[jpage-godaddy]

Resolving the 2 remaining findings from the fourth Copilot review pass (5228862286):

src/command/spec.rs:197 (CursorConfig non-exhaustiveness) — Fixed in d68b1e2. Since PaginationConfig is CursorConfig's exact sibling (same shape, same purpose, also documented/used via literal construction), I checked with the repo owner rather than fixing only the one Copilot flagged: they chose to mark both #[non_exhaustive] for API consistency, not just CursorConfig. Both now have a plain new(default_limit, max_limit) constructor (no with_* builders — both fields are always required, nothing to build up incrementally), and every literal-construction call site (tests, AGENTS.md, docs) is converted, since #[non_exhaustive] forbids struct-literal syntax entirely for external callers, spread syntax included — the same lesson this PR already hit once with Middleware/MiddlewareRequest.

src/cli/flags_apply.rs:113 (command_replay_base ignoring fields_explicit) — Confirmed with the repo owner: not fixing, as it's pre-existing. This exact behavior (replaying a command's default_fields as if --fields were explicit, and dropping an explicit --fields "") already existed for offset pagination's next_actions before this PR, under the helper's old name (pagination_command_base) — cursor pagination just reuses the same shared helper as-is, it doesn't introduce the gap. Fixing it would change already-shipped offset-pagination replay behavior, which is a separate concern from adding cursor support and out of scope here.

Full verification suite passes on d68b1e2.

[Copilot]

🟡 Changes recommended

Unresolved API compatibility, opaque-token parsing/replay, and custom-renderer output issues block approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

cli-engine/src/cli/flags_apply.rs:227

• quote_pagination_value is also used by command_replay_base for ordinary user-argument replay, not only for cursor tokens. Replacing a control byte with the text \\n/\\xHH means the shell passes those literal characters (the CLI does not decode them), so a generated next-page command cannot round-trip an argument or opaque token containing a control character. Either emit a representation that reconstructs the original value or suppress the replay action when exact reproduction is impossible.

                '\n' => escaped.push_str("\\n"),
                '\r' => escaped.push_str("\\r"),
                '\t' => escaped.push_str("\\t"),
                c if c.is_control() => escaped.push_str(&format!("\\x{:02x}", c as u32)),

cli-engine/src/flags/register.rs:307

• Because this accepts arbitrary hyphen-prefixed tokens, --continue --schema is a valid cursor invocation, but Cli::run calls the raw has_true_schema_flag scan before clap parses the command and mistakes that token for the global schema flag. The handler is then skipped and schema output is returned instead of fetching the requested cursor page. The raw pre-scan must skip the value following --continue (and other per-command value flags), or schema detection must be deferred until parsed matches; add a regression for a token such as --schema.

            Arg::new("continue")
                .long("continue")
                .value_name("TOKEN")
                .allow_hyphen_values(true)

cli-engine/src/flags/register.rs:307

• Cursor tokens are documented as opaque strings, but the command-line normalization pass runs before clap and does not know that --continue consumes the next token. For example, my-cli list --continue --verbose x is rewritten as if --verbose were a global flag (and --continue --schema becomes --schema=true), so the token is changed or the invocation fails before the handler sees it. The pre-parser must consume cursor values using the command tree's value-argument rules before applying optional-global normalization.

            Arg::new("continue")
                .long("continue")
                .value_name("TOKEN")
                .allow_hyphen_values(true)

cli-engine/src/flags/register.rs:307

• When environments are configured, Cli::new also runs prescan_env_flag over the raw argv before the command is parsed. A valid opaque token such as --continue --env dev is therefore treated as the invocation's global environment override, which can select/prune a different command tree before dispatch. That prescan must likewise know that --continue consumes its following token (or cursor tokens must be constrained/encoded).

            Arg::new("continue")
                .long("continue")
                .value_name("TOKEN")
                .allow_hyphen_values(true)

cli-engine/src/output/human/mod.rs:447

• The new cursor footer is only reached through render_human_with_view, but render_human_with_registry_selected returns early for a registered custom renderer after appending only next_actions. A valid cursor command using register_func therefore gets no documented standalone cursor summary in human output, even though custom output is a non-table response. Apply the cursor summary footer in that custom-renderer path as well (using the array length) before appending next actions.

        append_cursor_summary(&mut body, envelope.cursor.as_ref(), shown);

• Files reviewed: 25/25 changed files
• Comments generated: 1
• Review effort level: Lite

[jpage-godaddy]

Resolving the 3 findings from the fifth Copilot review pass (5229065721), all confirmed with the repo owner:

src/cli/flags_apply.rs:227 (control-char escaping breaks round-trip) — Confirmed and documented in e876802, not changed further: quote_pagination_value's escaping is deliberately display-safe (no raw control byte reaches the terminal), not round-trip-safe (a plain shell doesn't decode \n/\xHH back into the original byte). The alternative that would round-trip — ANSI-C $'...' quoting — isn't POSIX and would make every other, ordinary replayed value non-portable, to gain exact reproduction for a case only a malformed or adversarial backend cursor token would ever hit. Added a doc comment spelling out this trade-off explicitly.

src/flags/register.rs:307 (×3 — --schema/--env/general prescan collision with an opaque --continue value) — Confirmed real, not fixing in this PR. The underlying mechanism (raw argv pre-scans running before clap sees the full command tree, with no notion of which upcoming token is consumed as a value rather than being another flag) predates cursor pagination — --limit/--offset/--timeout already use allow_hyphen_values(true) too. What's new is that --continue's value is a fully opaque, backend-controlled, unconstrained string (by design — see AGENTS.md's cursor authoring guidance), so a real backend token could collide with a global flag's exact text by chance, with no user intent involved — unlike the pre-existing flags, whose values are shape-constrained (numeric/duration) and would need a user to deliberately contrive a collision. Properly fixing this means teaching every raw pre-scan about the full command tree's value-consuming args before its textual heuristic runs — real, scoped framework work, but separate from adding cursor pagination itself. Happy to open a tracked follow-up if you'd like one; didn't file a GitHub issue unprompted.

src/output/human/mod.rs:447 (custom-renderer path skips the cursor summary footer) — Checked: render_human_with_registry_selected's custom-renderer early-return already skips append_pagination_summary for offset pagination too (calls only append_next_actions), predating this PR. Cursor pagination inherits the identical, pre-existing gap rather than introducing a new one — same category as the fields_explicit finding from the previous round, so not fixing here for the same reason.

Full verification suite passes on e876802.

[Copilot]

🟡 Changes recommended

Unresolved findings cover TOON control-character escaping, Bash-safe ! replay quoting, explicit empty --fields replay, PaginationConfig compatibility, and stale cursor envelope documentation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

cli-engine/docs/proposals/cursor-first-pagination.md:87

• This example update leaves the proposal's envelope section stale: it still describes cursor metadata as making offset optional, while the implemented public CursorMeta has no offset and instead adds remaining and self_sufficient_limit. Please update that section as part of this proposal change so it documents the actual API consumers receive.

CommandSpec::new("list", "List things").with_cursor(CursorConfig::new(25, 500))

cli-engine/src/cli/flags_apply.rs:232

• The quoted branch still leaves ! unescaped inside a double-quoted value. In interactive Bash, history expansion is performed inside double quotes, so a valid backend token such as a!b can expand to a history entry or fail with event not found; the suggested next-page command is then not safely copy-pastable or an exact replay. Use a quoting form that also suppresses Bash history expansion (for example POSIX single-quote escaping for values containing !).

                '\\' => escaped.push_str("\\\\"),
                '"' => escaped.push_str("\\\""),
                '$' => escaped.push_str("\\$"),
                '`' => escaped.push_str("\\`"),

cli-engine/src/cli/flags_apply.rs:111

• command_replay_base cannot distinguish an explicit --fields "" from no --fields: it only emits non-empty flags.fields and does not consult flags.fields_explicit. For a command with default_fields, the current invocation keeps all fields while the suggested next-page command falls back to the command default, so the cursor replay is not equivalent. Preserve an explicit empty --fields in the replay.

pub(super) fn command_replay_base(

cli-engine/src/command/spec.rs:156

• Marking the existing public PaginationConfig as #[non_exhaustive] is a source-breaking change: all of its fields were public and the previous docs explicitly supported PaginationConfig { ..., ..Default::default() }, but downstream struct literals can no longer compile. This conflicts with the proposal's additive/non-breaking framework claim; keep this pre-existing type constructible (and reserve #[non_exhaustive] for the new cursor type), or explicitly version and document the breaking change.

#[non_exhaustive]

• Files reviewed: 25/25 changed files
• Comments generated: 1
• Review effort level: Lite

[jpage-godaddy]

Resolving the 4 findings from the sixth Copilot review pass (5229195247):

src/cli/flags_apply.rs:232 (Bash ! history expansion) — Fixed in e7ee3c5, and it's a real, distinct issue from the round-5 control-character fix: interactive Bash performs history expansion on an unescaped ! even inside double quotes, and (verified against a real Bash) backslash-escaping it there doesn't help — \! leaves the literal backslash in the resulting argument, its own round-trip failure. Fixed properly this time: each ! is spliced into its own single-quoted segment ("a"'!'"b" parses as the single argument a!b) — single quotes are immune to history expansion, and this round-trips exactly, unlike the control-character trade-off from round 5. Added unit tests, and updated the one pre-existing offset-pagination test whose --filter value happened to contain ! (this fix applies to quote_pagination_value generally, not just cursor tokens).

docs/proposals/cursor-first-pagination.md:87 (stale envelope description) — Fixed in e7ee3c5: that section still described the design as adding an optional offset to PaginationMeta, not the shipped CursorMeta shape (no offset at all — there's no "skip N" concept for a cursor — plus remaining/self_sufficient_limit). Updated to match what's actually implemented, with a pointer to concepts.md for the full contract.

src/cli/flags_apply.rs:111 (fields_explicit replay) — Same finding as round 4; already addressed there (pre-existing, shared by offset pagination, out of scope for this PR) — not re-litigating.

src/command/spec.rs:156 (PaginationConfig non-exhaustive is a breaking change) — Same trade-off already decided in round 4: making PaginationConfig (not just CursorConfig) #[non_exhaustive] was a deliberate choice for API consistency between the two sibling configs, made explicitly aware it's source-breaking for any external literal-construction call site. This crate is pre-1.0 (0.9.5); a breaking change here is expected to ship as a feat!/fix! with a BREAKING CHANGE footer per this repo's own commit conventions, not something that needs to be avoided.

Full verification suite passes on e7ee3c5.

[Copilot]

🟡 Changes recommended

Unresolved API compatibility, TOON control-character escaping, and custom-renderer summary issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

cli-engine/src/command/spec.rs:156

• Adding #[non_exhaustive] to the existing public PaginationConfig is source-breaking: consumers following the previous documented PaginationConfig { ... ..Default::default() } pattern can no longer construct it, even though the proposal still describes cli-engine as additive/non-breaking and the summary says offset pagination is untouched. Either preserve the old construction path, or explicitly treat this as a semver/migration change and update the proposal/release docs.

#[non_exhaustive]

cli-engine/src/output/envelope.rs:204

• continue_from is backend-controlled and is still serialized into every output format, but the TOON renderer's is_safe_unquoted/escape_string path does not reject or escape control characters such as ESC. Thus --output toon can print an ANSI sequence from this new field directly to a terminal, even though the suggested command is escaped; make the TOON string encoding reject/escape all control characters (or otherwise sanitize this field) before emitting it.

    pub continue_from: Option<String>,

cli-engine/src/output/human/mod.rs:447

• The new cursor summary is only appended after render_data_body, but render_human_with_registry_selected returns early for a custom human renderer before reaching this path. A cursor command using register_global_human_view_func/HumanViewRegistry::register_func therefore shows no cursor count, limit, or resume hint (including on the last page), even though the feature's human-output contract says these facts are rendered; append the cursor summary in the custom-renderer branch as well, using the final array count, before its existing next-actions footer.

        append_cursor_summary(&mut body, envelope.cursor.as_ref(), shown);

• Files reviewed: 25/25 changed files
• Comments generated: 1
• Review effort level: Lite

[jpage-godaddy]

Resolving the 3 findings from the seventh Copilot review pass (5229319150):

src/output/envelope.rs:204 (TOON control-character escaping) — Fixed in b529d41, a real and distinct finding: is_safe_unquoted/escape_string in the TOON renderer only special-cased \n/\r/\t (plus \/"), so any other control character (an ESC byte, the start of most ANSI escape sequences) rendered completely unquoted and unescaped straight to the terminal under --output toon. Not specific to continue_from — this is a generic string encoder used for every field in every command's output — but cursor pagination's opaque, backend-controlled token is what surfaced it, since it's the first field genuinely likely to carry an unexpected byte. Fixed both functions to treat/escape any control character generally (matching the \xHH placeholder style already used for the human-output replay fix), with a regression test.

src/command/spec.rs:156 (PaginationConfig non-exhaustive breaking change) and src/output/human/mod.rs:447 (custom-renderer footer gap) — Same two findings as rounds 4/6 and round 5 respectively; both already addressed there (an explicitly-accepted trade-off, and a pre-existing gap shared with offset pagination) — no new information, not re-litigating.

Full verification suite passes on b529d41.

[Copilot]

🟡 Changes recommended

Unresolved critical runtime validation and moderate compatibility, metadata-count, and TOON-escaping findings block approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

cli-engine/docs/design.md:296

• This overview still describes --limit/--offset as the only per-command pagination exception, but this PR adds the parallel with_cursor/--continue API. The output-envelope and pipeline summaries later in design.md likewise omit cursor, so readers of this public design document get an incomplete contract. Please update this section and the output summary to describe both mutually exclusive styles.

for itself with `CommandSpec::with_pagination(PaginationConfig::new(default_limit, max_limit))`;

cli-engine/src/command/runtime.rs:137

• In release builds this debug_assert! is removed, so a streaming command can still advertise --limit/--continue; however, the streaming path always returns Value::Null and has no CursorContinuation to drive a next page. That makes the advertised cursor flags silently ineffective. Reject this pairing unconditionally (including new_typed_streaming).

        debug_assert!(
            spec.cursor.is_none(),
            "command {:?} sets with_cursor but a streaming handler's result is always wrapped \
             as CommandResult::new(Value::Null) — there is no array or CursorContinuation to \
             report, so --continue would advertise resumption that can never happen; \
             with_cursor is only supported on non-streaming commands",
            spec.name
        );

cli-engine/src/command/spec.rs:156

• Marking the existing PaginationConfig as #[non_exhaustive] is a source-breaking change: consumers following the previously documented PaginationConfig { default_limit, max_limit, ..Default::default() } pattern can no longer compile, even though this PR describes the offset pagination API as untouched/additive. Keep this existing public struct constructible or explicitly version/document the breaking migration rather than treating the new constructor as compatibility-preserving.

#[non_exhaustive]

cli-engine/src/command/spec.rs:124

• This field's None description is now misleading: a cursor command has pagination == None but still registers the shared --limit. The sibling cursor description has the analogous ambiguity for offset commands. Please document these as independent offset/cursor policies (for example, pagination == None means no --offset, not no pagination), otherwise generated rustdoc gives incorrect guidance for the new API.

    /// [`with_pagination`](CommandSpec::with_pagination). Mutually exclusive
    /// with [`cursor`](CommandSpec::cursor).

cli-engine/src/middleware/run.rs:601

• This count is taken after apply_pipeline, so an expression that keeps an array (for example --expr "[?name=='alpha']") changes cursor.count from the backend page size to the post-expression display size. The cursor metadata is meant to describe the handler's returned page (and the footer comments already distinguish that from the displayed count); capture the count before --expr and use the post-pipeline length only for human rendering.

            let count = items.len() as i64;
            let continuation = cursor_continuation.unwrap_or_default();
            let has_more = continuation.continue_from.is_some();

cli-engine/src/output/toon.rs:324

• The new \xHH escape is not a JSON-style string escape, while this renderer's quoted strings otherwise use JSON-style escapes (\\, \", \n, etc.). A cursor token containing ESC therefore renders as invalid TOON such as "...\\x1b...", defeating machine-readable TOON output. Encode arbitrary control characters with a TOON-supported Unicode escape (for example \\u{:04x}) instead.

            c if c.is_control() => escaped.push_str(&format!("\\x{:02x}", c as u32)),

• Files reviewed: 26/26 changed files
• Comments generated: 1
• Review effort level: Lite

[jpage-godaddy]

Resolving the 6 findings from the eighth Copilot review pass (5229600632):

src/middleware/run.rs:601 (cursor.count measured after --expr) — Fixed in 8ec8c7e, a real and important bug: count was taken from envelope.data after apply_pipeline ran, so an --expr that filters but keeps the result an array (e.g. a JMESPath predicate) silently substituted the post-expression display count for the page the handler's backend call actually returned. PaginationMeta.count already avoided this — apply_pipeline captures it internally, before --expr runs — but cursor metadata is built entirely outside apply_pipeline, so it needed its own pre-pipeline snapshot. Added a regression test (filters a 2-item page to 1 displayed row, asserts cursor.count still reads 2).

src/output/toon.rs:324 (\xHH is not a valid TOON/JSON escape) — Fixed in 8ec8c7e: correct catch on my own round-7 fix. Every other escape in that encoder is JSON-style (\\, \", \n, ...), and \xHH isn't valid JSON string syntax, so it would itself make the rendered TOON unparseable — exactly defeating the point of a machine-readable format. Switched to \uXXXX, matching the rest of the encoder; updated the existing regression test.

src/command/spec.rs:124 (pagination/cursor field docs ambiguous about --limit) — Fixed in 8ec8c7e: both docs previously implied their own None meant "no --limit at all," which is wrong now that the sibling policy can register its own --limit. Reworded both to be precise about which specific flag (--offset vs --continue) each None actually rules out.

docs/design.md:296 — Fixed in 8ec8c7e: that overview section and the output-envelope/pipeline summary further down still described only --limit/--offset, omitting the parallel with_cursor/--continue API entirely. Both now describe both styles, with a pointer to concepts.md for the full cursor contract.

src/command/runtime.rs:137 (streaming+cursor guard is debug_assert-only) — Not changing: this is the exact same pattern as the pre-existing handles_dry_run misuse guard and the raw_output+streaming guard, both also debug_assert-only and both predating this PR (their own doc comments already call this out explicitly — e.g. RuntimeCommandSpec's "Construct with one of the new* constructors — never as a struct literal... a development-time safety net, not the actual guarantee"). Making the cursor guard unconditional while leaving those two as debug_assert would be inconsistent; making all three unconditional would be a real but separate change to this crate's established misuse-guard convention, not something scoped to adding cursor pagination.

src/command/spec.rs:156 (PaginationConfig non-exhaustive breaking change) — Same finding as rounds 4/6/7; already addressed, no new information, not re-litigating.

Full verification suite passes on 8ec8c7e.

[Copilot]

🟡 Changes recommended

Release-only cursor guards, an API compatibility break, missing #[must_use] annotations, and cursor validation issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (6)

Previously missed (1) — in code that hasn't changed since the last review.

cli-engine/src/middleware/run.rs:607

• This guard only checks the post-pipeline value. If a handler returns an object or scalar with with_cursor and --expr transforms it into an array, raw_cursor_array_len is None but this block still attaches CursorMeta and can emit a next-page action for data that was never a cursor page. Require the pre-pipeline array snapshot to be present before entering this block.

cli-engine/src/command/runtime.rs:74

• debug_assert! is removed from release builds, so a production consumer can still construct a cursor command with RuntimeCommandSpec::new. This handler only receives the resolver and spec.args; the cursor flags are registered separately and are not in that map, so it cannot observe --continue and the generated next-page action cannot actually resume. Make this rejection unconditional (or reject the spec during registration).

        debug_assert!(

cli-engine/src/command/runtime.rs:180

• This has the same release-build problem as the untyped constructor: debug_assert! disappears, allowing RuntimeCommandSpec::new_typed to register cursor flags even though its handler receives no CommandContext and cannot read the parsed continuation token. A generated next-page command will not be resumable; enforce the rejection in all builds.

        debug_assert!(

cli-engine/src/command/runtime.rs:282

• debug_assert! disappears in release builds here as well, allowing a typed streaming cursor command whose wrapped result is always Null and can never report a continuation. In that build the command accepts --continue but silently ignores the cursor contract; enforce this constructor guard unconditionally or reject it at registration.

        debug_assert!(

cli-engine/src/command/spec.rs:164

• PaginationConfig was previously an exhaustive public struct with public fields, and its documented construction pattern used a struct literal with ..Default::default(). Adding #[non_exhaustive] makes existing downstream offset-pagination code fail to compile, so this is a breaking change to the existing API even though the new cursor type is the only type that needs future-proofing. Keep PaginationConfig exhaustive (and retain the old literal-compatible documentation), or version and explicitly announce this migration.

#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
#[non_exhaustive]

cli-engine/src/middleware/mod.rs:635

• The new consuming constructor/builders omit #[must_use], unlike the existing public constructors/builders (for example Middleware::new in src/middleware/run.rs:21-24 and CommandSpec::with_long in src/command/spec.rs:245-249). Ignoring MiddlewareRequest::new(...).with_auth(...) or another with_* call silently discards the configured request; add #[must_use] to the constructor and all of these fluent methods.

impl<'request> MiddlewareRequest<'request> {
    /// Builds a request from the fields every caller needs to set, with
    /// everything else defaulted (`auth: AuthRequirement::Required`,
    /// `view_id`/`pagination_command`/`cursor_command`: `None`, `raw_output:
    /// false`). Chain the `with_*` methods below for anything else.
    pub fn new(

• Files reviewed: 26/26 changed files
• Comments generated: 1
• Review effort level: Lite

[jpage-godaddy]

Resolving the 6 findings from the ninth Copilot review pass (5229715613):

src/middleware/run.rs:607 (cursor metadata could attach when --expr synthesizes an array from a non-array result) — Fixed in 3f9898c: a real gap in the previous round's own fix. The guard only checked the post-pipeline shape, so a handler that returned a non-array result (never a real cursor page) could still get CursorMeta/a next-page action if --expr happened to synthesize an array from it (e.g. [@], wrapping a scalar/object in a single-element list). Both checks are now required: the pre-pipeline snapshot must exist (the handler's raw result really was an array) and the post-pipeline data must still be an array (the existing, opposite-direction case — --expr reshaping a real page into a scalar). Added a regression test for the newly-caught direction.

src/middleware/mod.rs:635 (MiddlewareRequest::new/with_* missing #[must_use]) — Fixed in 3f9898c: confirmed the crate's convention (Middleware::new, PaginationConfig::new, CursorConfig::new all have it) and added it to the constructor and all 5 builder methods.

src/command/runtime.rs:74,180,282 (debug_assert-only cursor/streaming-or-non-context guards) — Same category as the previous round's runtime.rs:137 finding, just three more call sites (new, new_typed, new_typed_streaming); already explained there — consistent with this crate's existing, documented debug_assert-only misuse-guard convention (handles_dry_run, raw_output+streaming), not something scoped to cursor pagination to change unilaterally.

src/command/spec.rs:164 (PaginationConfig non-exhaustive breaking change) — Same finding as rounds 4/6/7/8; already addressed, no new information.

Full verification suite passes on 3f9898c.

[Copilot]

🔵 Needs a closer look

Resolve the PaginationConfig compatibility break and prevent completed continuations from being marked self-sufficient without a token.

Review details

Suppressed comments (2)

cli-engine/src/command/spec.rs:164

• Marking the existing public PaginationConfig as #[non_exhaustive] is a source-breaking change: downstream callers that used the previously documented PaginationConfig { default_limit: ..., ..Default::default() } form can no longer compile, even though this PR describes offset pagination and PaginationConfig as untouched/fully supported. A constructor does not preserve that API. Please either leave this existing type exhaustive (only the new CursorConfig needs future-proofing) or explicitly treat this as a breaking release/migration.

#[non_exhaustive]

cli-engine/src/middleware/run.rs:655

• self_sufficient_limit is documented as describing whether continue_from alone can resume, but this is set from continuation.limit even when the continuation is done() and continue_from is None (for example, CursorContinuation::done().with_limit(2)). That produces cursor metadata claiming a nonexistent token is self-sufficient. Set this flag only when a continuation token is present, or reject/ignore with_limit for a completed page.

                self_sufficient_limit: continuation.limit.is_some(),

• Files reviewed: 26/26 changed files
• Comments generated: 0 new
• Review effort level: Lite

[jpage-godaddy]

Resolving the 2 findings from the tenth Copilot review pass (5229778642):

src/middleware/run.rs:655 (self_sufficient_limit set without a token) — Fixed in 08018f2, a real edge case: CursorContinuation::done().with_limit(n) is a handler misuse (with_limit only means anything relative to a token to resume with, and done() has no continue_from), but self_sufficient_limit was set purely from continuation.limit.is_some(), so it would claim a nonexistent token is self-sufficient about page size. Gated it on has_more too. Added a regression test.

src/command/spec.rs:164 (PaginationConfig non-exhaustive breaking change) — Same finding as rounds 4/6/7/8/9; already addressed, no new information.

Full verification suite passes on 08018f2.

[Copilot]

🔵 Needs a closer look

Address the PaginationConfig compatibility issue and the two documentation nits.

Review details

Suppressed comments (3)

cli-engine/docs/concepts.md:517

• This paragraph says self_sufficient_limit is true exactly when the handler called with_limit, but the implementation deliberately sets it false for CursorContinuation::done().with_limit(...) because no continue_from token exists. Please document the actual has_more && with_limit condition, which the new completed-page test asserts.

A command that opted into `--limit`/`--continue` cursor pagination via `CommandSpec::with_cursor` gets a top-level `cursor` field on the envelope instead of `pagination` — `limit`, `count`, `total`, `remaining`, `continue_from`, `has_more`, and `self_sufficient_limit` — whenever it returned array data. Unlike `pagination`, the engine cannot compute this itself: a cursor is opaque to everything except the handler that called the backend, so `count` is the only piece the engine derives itself (the returned array's length); `limit` defaults to the parsed `--limit`, but a handler can override it via `CursorContinuation::with_limit` to report the effective page size it actually resumed with (e.g. one decoded from `continue_from` itself) — `self_sufficient_limit` is `true` exactly when that override happened, meaning `continue_from` alone is enough to resume and a replay command can omit `--limit`; `total`/`remaining`/`continue_from` come from whatever the handler reported via `CommandResult::with_cursor(CursorContinuation::more(token).with_total(n).with_remaining(n))` — or `CursorContinuation::done()` (or no call at all) to report the end of iteration. `total`/`remaining` are `None` when the backend never reports them, which a pure opaque-cursor API is not obligated to do.

cli-engine/src/command/spec.rs:164

• Marking the existing PaginationConfig as #[non_exhaustive] is source-breaking for downstream commands that use the previously documented PaginationConfig { default_limit, max_limit, ..Default::default() } construction; adding new does not preserve that API. This also contradicts the proposal's additive/non-breaking contract for with_pagination. Please keep the existing config constructible or make this breaking change explicit in the release/versioning plan.

#[non_exhaustive]

cli-engine/src/output/envelope.rs:212

• This says self_sufficient_limit is true when the handler called with_limit, but the implementation deliberately sets it false for CursorContinuation::done().with_limit(...) because no continue_from token exists. Please document the actual has_more && with_limit condition, which the new completed-page test asserts.

    /// Whether `continue_from` alone is sufficient to resume at `limit`
    /// (the handler called
    /// [`CursorContinuation::with_limit`](crate::CursorContinuation::with_limit)),
    /// so a replay command can omit `--limit` — the same condition the
    /// engine uses to decide whether `next_actions` includes it. `false` for
    /// a plain [`CursorContinuation::more`](crate::CursorContinuation::more)

• Files reviewed: 26/26 changed files
• Comments generated: 0 new
• Review effort level: Lite

[jpage-godaddy]

Resolving the 3 findings from the eleventh Copilot review pass (5229847606):

src/output/envelope.rs:212 and docs/concepts.md:517 — Fixed in f33aa2a: both direct fallout of the previous round's fix (self_sufficient_limit is now has_more && continuation.limit.is_some(), not just the latter) — the field doc and the concepts guide still described the old, incomplete condition. Both now describe the actual has_more && with_limit requirement and why (a completed page has no token for with_limit to describe as self-sufficient).

src/command/spec.rs:164 (PaginationConfig non-exhaustive breaking change) — Same finding as rounds 4/6/7/8/9/10; already addressed, no new information.

Full verification suite passes on f33aa2a.

[Copilot]

🔵 Needs a closer look

Unresolved moderate findings remain in replay-value quoting, release-build guards, and PaginationConfig API compatibility.

Review details

Suppressed comments (6)

cli-engine/src/cli/flags_apply.rs:254

• These control-character escapes are applied by the shared helper to every replayed value, not just backend cursor tokens. command_replay_base uses it for user-supplied command args and --filter/--expr/--fields; a value containing a newline, carriage return, or tab was previously preserved inside the double-quoted shell word, but now becomes the literal characters \\n/\\r/\\t, so the suggested next-page command no longer replays the request. Keep control sanitization in a cursor-token-specific quoting path (or parameterize this helper) while preserving round-trip quoting for user arguments.

                '\n' => segment.push_str("\\n"),
                '\r' => segment.push_str("\\r"),
                '\t' => segment.push_str("\\t"),
                c if c.is_control() => segment.push_str(&format!("\\x{:02x}", c as u32)),

cli-engine/src/command/runtime.rs:83

• This guard disappears in optimized builds, so a release consumer can register with_cursor with RuntimeCommandSpec::new. That handler only receives spec.args (the generated cursor flags are not in that list), so it cannot observe cursor_limit or continue_token and will generally ignore --continue, repeatedly fetching the initial page despite the advertised cursor flags. The PR contract says this pair is rejected at construction time; make the rejection unconditional or otherwise prevent this invalid pairing in release builds.

        debug_assert!(
            spec.cursor.is_none(),
            "command {:?} sets with_cursor but RuntimeCommandSpec::new's handler \
             (CredentialResolver, args) has no CommandContext and can never read back \
             middleware.cursor_limit/continue_token to drive its own backend call, so \
             --continue would advertise resumption the handler cannot perform; use \
             RuntimeCommandSpec::new_with_context (or new_typed_with_context to keep typed \
             args) instead",
            spec.name
        );

cli-engine/src/command/runtime.rs:189

• The same release-build hole exists for new_typed: after debug_assert! is removed, a cursor command can be constructed with a handler that receives only the typed args and has no middleware context. It therefore cannot consume --continue, while the command still registers and advertises cursor pagination. This guard needs to be enforced in release builds or the invalid constructor pairing must be made impossible.

        debug_assert!(
            spec.cursor.is_none(),
            "command {:?} sets with_cursor but RuntimeCommandSpec::new_typed's handler \
             (CredentialResolver, args) has no CommandContext and can never read back \
             middleware.cursor_limit/continue_token to drive its own backend call, so \
             --continue would advertise resumption the handler cannot perform; use \
             RuntimeCommandSpec::new_with_context (or new_typed_with_context to keep typed \
             args) instead",
            spec.name
        );

cli-engine/src/command/runtime.rs:137

• This streaming guard is also debug-only. In an optimized build, a streaming command can opt into cursor flags, but the streaming adapter always returns CommandResult::new(Value::Null), so no cursor envelope or next-page action can ever be produced. That leaves --continue advertised without the promised cursor behavior; enforce the restriction outside debug_assert! or remove the unsupported pairing from the public API.

        debug_assert!(
            spec.cursor.is_none(),
            "command {:?} sets with_cursor but a streaming handler's result is always wrapped \
             as CommandResult::new(Value::Null) — there is no array or CursorContinuation to \
             report, so --continue would advertise resumption that can never happen; \
             with_cursor is only supported on non-streaming commands",
            spec.name
        );

cli-engine/src/command/runtime.rs:289

• This typed streaming guard is debug-only as well. In an optimized build, the command still registers --continue, but the streaming adapter wraps the handler's completion as CommandResult::new(Value::Null), so the cursor metadata and automatic next-page action can never be emitted. Enforce this unsupported pairing in release builds or make it impossible through the constructor API.

        debug_assert!(
            spec.cursor.is_none(),
            "command {:?} sets with_cursor but a streaming handler's result is always wrapped \
             as CommandResult::new(Value::Null) — there is no array or CursorContinuation to \
             report, so --continue would advertise resumption that can never happen; \
             with_cursor is only supported on non-streaming commands",
            spec.name
        );

cli-engine/src/command/spec.rs:164

• Adding #[non_exhaustive] to the existing PaginationConfig is an unnecessary source-breaking change: downstream callers that currently use PaginationConfig { ... } (including ..Default::default()) will stop compiling even though this type gained no field in this PR. Keep the existing config constructible (only the new CursorConfig needs future-field protection), or explicitly treat this release as a breaking API version.

#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
#[non_exhaustive]

• Files reviewed: 26/26 changed files
• Comments generated: 0 new
• Review effort level: Lite

[jpage-godaddy]

The twelfth Copilot review pass (5229910963) raised 6 findings; all 6 are repeats of categories already explained in earlier rounds, with no new actionable information:

• src/cli/flags_apply.rs:254 (control-character escaping trades off round-trip fidelity) — same trade-off explicitly decided in round 5 ("keep display-safe escaping, document the limitation"), just re-framed as affecting ordinary replayed args (not only cursor tokens) rather than the control-character case specifically. Same underlying decision either way: quote_pagination_value is shared by design, and the display-safety goal (no raw control byte reaching a terminal) is worth the narrow round-trip cost for the rare case of a literal \n/\r/\t in a replayed value.
• src/command/runtime.rs:83,189,137,289 (four debug_assert-only cursor/streaming-or-non-context guards) — the exact same four call sites already explained in rounds 8 and 9 (line numbers shifted slightly from intervening edits, but same guards, same text). Consistent with this crate's pre-existing, documented debug_assert-only misuse-guard convention (handles_dry_run, raw_output+streaming) — not something scoped to cursor pagination to change unilaterally.
• src/command/spec.rs:164 (PaginationConfig non-exhaustive breaking change) — same finding as rounds 4/6/7/8/9/10/11; already addressed.

No code change accompanies this reply — per this loop's own convention (only re-request a Copilot review when a round actually changes code), this is the natural stopping point. All genuinely new, actionable findings across the previous 11 rounds have been fixed and verified; what remains are explicitly-decided trade-offs and an established crate-wide convention this PR is consistent with, not open bugs.