Guard against pagination loops and silent truncation in execute()

[marcusfaust]

What

Two safety guards in RSCClient.execute() auto-pagination:

1. Non-advancing-cursor guard — if the cursor doesn't change between fetches, stop instead of looping. Kills the infinite loop when a connection query selects pageInfo but doesn't declare/pass $after (the server keeps returning page 1 with hasNextPage: true).
2. Truncation warning — when only a first page is returned but more exists, warn so it isn't mistaken for the full set. Covers nodes+count without pageInfo, and a data list with hasMore / total / nextCursor.

Correctly-wired pagination and the existing max_records cap are unchanged.

Why

execute() auto-paginates based on the query's shape, so an under-wired query could hang (loop) or silently return a partial result. These guards make both failure modes safe without changing correct behavior.

Tests

tests/test_pagination.py (runs under the existing pytest tests/ CI job): loop guard (incl. endCursor=None), correct pagination, both truncation shapes, per-field, max_records cap, and no-false-positive cases (complete result; final page with a populated cursor).

[jakerobinson]

Review

This PR adds two safety guards to RSCClient.execute()'s auto-pagination: a non-advancing-cursor check that stops an infinite loop when a query omits $after wiring, and a truncation warning for responses that return only a first page. Both guards are well-tested for the cases the new test suite covers, but the design has a structural blind spot that undermines the PR's own stated goal, plus a more subtle issue specific to how this client's main real-world consumer (the Rubrik MCP) will actually experience the "fix."

1. A response with one paginated connection field and one separately-truncated field silently drops the second field's excess data with zero warning

src/rsc/client.py, the conn_key is None gate (~line 48) vs. the pagination path (~line 85)

The truncation-check loop only runs when conn_key is None — i.e., when no field anywhere in the response looks like a Relay connection. If a query selects a connection field alongside an unrelated data/hasMore-shaped field (a normal, foreseeable GraphQL shape — e.g. vms { nodes pageInfo {...} } events { data hasMore total }), the connection auto-paginates fine, but events returns 100-of-5000 records with no warning printed at all — exactly the silent-truncation failure this PR exists to catch, just in the one case its own gating structure can't see. Not covered by any test — test_truncation_warns_per_field only exercises two truncated fields together, never a connection alongside a truncated one.

2. The non-advancing-cursor guard converts a loud, easy-to-catch bug into a silent one, specifically for this client's main consumer

Same file, ~lines 97-113

Before this PR, an under-wired query (missing $after) hung forever — an obvious, in-your-face failure during development. After this PR, execute() returns normally with only page 1, and the only signal that anything was cut short is a print(..., file=sys.stderr). The Rubrik MCP's tool calls wrap this client, and MCP tool results only carry back the function's JSON return value — not the process's stderr. An agent calling a workflow built on execute() gets a normal-looking, "successful" truncated result with no way to know it's incomplete, and no field in the returned dict flags it. That's a straight trade of a hard failure for a silent one, at exactly the layer this PR's title claims to fix.

Relatedly: the returned pageInfo/count always reflect the first page fetched, never updated when a guard or max_records stops the loop early — so even a caller that inspects the returned dict directly, ignoring stderr, gets no structured signal that pagination stopped short of the real end.

3. next((k for k, v in data.items() if "nodes" in v and "pageInfo" in v), None) (~line 41) — only the first connection-shaped field in a response is ever auto-paginated

If a query selects two connection fields, the second is left completely unpaginated (page 1 only) and un-warned, since it doesn't hit the truncation loop either (that loop is skipped once any conn_key is found — see #1). Structural consequence of a single-slot design applied to a protocol with no cap on top-level field count.

4. if conn["pageInfo"]["endCursor"] == after: (~line 105) — a non-unique cursor encoding could false-positive on a legitimately advancing multi-page result

If a backend's opaque cursor is derived from something coarser than a unique offset (e.g., a date field at day granularity), two genuinely different pages could share an endCursor value, triggering the guard to stop and silently drop everything past that point — with, again, no warning reaching the returned result (per #1/#2). Depends on backend cursor implementation details this PR can't control.

5. flag = val.get("hasMore"); has_more = flag is True or (flag is None and bool(val.get("nextCursor"))) (~lines 71-72) — a truthy-but-non-bool hasMore is silently treated as "not more"

flag is True requires strict identity with the Python bool. A loosely-typed backend returning hasMore: "true" (string) with a real nextCursor present would report the result as complete — the opposite of the doc comment's stated intent to "trust hasMore when present."

Lower-severity cleanup, non-blocking

• The truncation-guard's count-or-total check is more permissive than the pre-existing, unrelated total = conn.get("count") check a few lines below it for the auto-paginated path — a connection exposing only total (no count) would silently skip the existing "Note: N records found" progress message. Worth a shared _extract_total() helper.
• Three near-identical print(..., file=sys.stderr) call sites now exist in this one function with inconsistent prefixes ("Warning:" vs. "Note:") and no shared helper.
• Each dict key in the new shape-sniffing block is fetched via .get() twice — once for the isinstance check, once for the value. A small _first(val, keys, kind) helper would fix this and the nested-ternary readability issue in one move.
• Pre-existing, not introduced by this PR but adjacent to it: max_records=0 is falsy, so both if max_records and ... and [:max_records] if max_records else ... silently treat an explicit "give me zero records" the same as "no limit." Worth closing while this exact code is already being touched and documented.

[marcusfaust]

Thanks for the review — all points addressed in 3cec386:

• Accurate pageInfo on return: after auto-pagination the returned connection's
  pageInfo now reflects the last fetched page, so an incomplete result (record
  cap hit, or the non-advancing-cursor guard firing) is detectable from the
  return value, not just from stderr.
• Truncation scan covers all fields: moved into _warn_if_truncated, which runs
  over every top-level field regardless of whether a connection was
  auto-paginated — so a truncated data/hasMore field, or a second connection
  returned alongside the paginated one, is still reported.
• max_records=0 is now a real cap (0 records) via is not None, not a falsy
  "no cap".
• Non-bool hasMore ("true"/"false" strings) handled.

Added tests for each (18 pass).