Skip to content

Clamp session DB pagination limits for search and listing APIs - #32564

Closed
aydnOktay wants to merge 2 commits into
NousResearch:mainfrom
aydnOktay:feat/session-db-pagination-bounds
Closed

Clamp session DB pagination limits for search and listing APIs#32564
aydnOktay wants to merge 2 commits into
NousResearch:mainfrom
aydnOktay:feat/session-db-pagination-bounds

Conversation

@aydnOktay

Copy link
Copy Markdown
Contributor

hardens hermes_state.py by clamping limit and offset for search_messages, search_sessions, and list_sessions_rich so invalid or excessive pagination values cannot overload SQLite FTS/list queries, and adds focused regression coverage in tests/test_hermes_state.py for helper clamping behavior and end-to-end session search pagination bounds.

aydnOktay and others added 2 commits May 26, 2026 14:08
Bound LIMIT/OFFSET values in SessionDB search_messages, search_sessions, and list_sessions_rich so invalid or excessive pagination cannot overload SQLite queries, with regression tests in test_hermes_state.py.

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/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 26, 2026
@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 13, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Summary

Two open PRs address the same pagination-input problem by adding a shared clamp to three SessionDB methods, but neither has a recorded pipeline verify verdict. #32564 also removes unrelated query behavior, while #35308 preserves it but still applies the clamp at the wrong abstraction boundary and would truncate legitimate full-history callers.

Related pull requests

  • #32564 related — (+60/-32) — supersede: The diff clamps search_messages, search_sessions, and list_sessions_rich, but it also deletes the existing message-sort normalization/ORDER BY construction and the enriched last_active session-query construction, changes unrelated to the reported pagination cause and potentially breaking their downstream use.
  • #35308 duplicate — (+60/-0) — keep open for revision: This is the cleaner duplicate because it adds the same clamp without deleting adjacent query logic. The visible keep_open review on #35308 should be followed, but its blocking findings must be resolved before merge: clamping inside SessionDB changes the established LIMIT -1 unlimited contract and truncates export/cleanup callers requesting 100000 or 10000 rows, while the tests cover neither list_sessions_rich() nor datasets exceeding 200 rows.

Duplicates

#32564 and #35308 implement essentially the same shared SessionDB pagination clamp across the same three methods; #35308 is the cleaner current-main implementation despite the earlier contributor comment identifying it as a duplicate of #32564.

Suggested consolidation

Revise, then merge #35308 — move pagination validation to the untrusted HTTP/UI boundaries identified by the keep_open review, preserve SessionDB's unlimited and high-limit internal contracts, and add boundary plus full-store regression coverage. Despite the contributor duplicate comment favoring the earlier #32564, the actual #32564 diff removes unrelated sorting and enriched-session query construction; once the corrected #35308 is ready, close #32564 as superseded.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup32564 ["PRs duplicating each other"]
        P32564["PR #32564 (open)"]
        P35308["PR #35308 (open)"]
    end
    class P32564 open
    class P35308 open
    class P32564 target
    click P32564 "https://github.com/NousResearch/hermes-agent/pull/32564"
    click P35308 "https://github.com/NousResearch/hermes-agent/pull/35308"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 10 kB of PR diffs, 2 kB of issue/PR text, 1 kB of discussion (2 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@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

area/sessions Session lifecycle, resume, persistence, history comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants