Skip to content

Clamp session.list / spawn_tree.list limit before slicing - #79302

Closed
aydnOktay wants to merge 1 commit into
NousResearch:mainfrom
aydnOktay:fix/session-list-limit-clamp
Closed

Clamp session.list / spawn_tree.list limit before slicing#79302
aydnOktay wants to merge 1 commit into
NousResearch:mainfrom
aydnOktay:fix/session-list-limit-clamp

Conversation

@aydnOktay

Copy link
Copy Markdown
Contributor

Summary

  • session.list and spawn_tree.list sliced with the raw JSON-RPC limit.
  • A negative limit is valid Python (rows[:-n]) and silently drops the newest entries from the resume picker / spawn-tree listing.
  • Huge limits also inflate DB fetches and snapshot scans.
  • Clamp both to 1..500 and fall back to defaults on non-int values.

…licing

A negative JSON-RPC limit is a valid Python slice (rows[:-n]) and silently
drops the newest entries from the resume picker / spawn-tree listing.
Huge limits also inflate DB fetches and snapshot scans. Clamp to 1..500
and fall back on non-int values.

Co-authored-by: Cursor <cursoragent@cursor.com>
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) needs-decision Awaiting maintainer decision before any implementation labels Aug 5, 2026
@spfcraze

spfcraze commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary:
This PR's spawn_tree.list change duplicates the author's own open #78394, and the two differ on limit=0: here the or 50 fallback returns the default page, there 0 clamps to 1.

Problems:

  • Both PRs edit the same limit line of the spawn_tree.list handler in tui_gateway/methods_session.py and both add tests/tui_gateway/test_spawn_tree_list_limit_clamp.py; the description does not reference Clamp spawn_tree.list limit to avoid unbounded payloads #78394, so the overlap is not visible from this thread alone.
  • limit=0 resolves differently in the two diffs: this PR keeps int(params.get("limit") or 50), so a falsy 0 becomes the 50-entry default page; Clamp spawn_tree.list limit to avoid unbounded payloads #78394 handles 0 explicitly and clamps it to 1, with test_spawn_tree_list_clamps_zero_limit asserting a single entry and a comment explaining why the or 50 shape was avoided.

Solution:
Consolidate the spawn_tree.list clamp into one of the two PRs, carrying over #78394's explicit 0-handling (0 clamps to 1, None/"" fall back to 50), since this PR's or 50 leaves limit=0 returning the default page.


Checked against f93c2bc — the tip of fix/session-list-limit-clamp when this was written — and 241605d, main at the same moment.

@aydnOktay

Copy link
Copy Markdown
Contributor Author

Closing to concentrate review on a single linked product bug: #89979.

@aydnOktay aydnOktay closed this Aug 19, 2026
@aydnOktay

Copy link
Copy Markdown
Contributor Author

Closing to concentrate review on a single linked product bug: #89979.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants