fix(cli): hide cron sessions by default in sessions list and browse - #11537
fix(cli): hide cron sessions by default in sessions list and browse#11537dso2ng wants to merge 1 commit into
Conversation
|
Heads up: the workflow runs for this fork PR are currently showing |
eec8337 to
8e4551f
Compare
|
Maintainer note: this fork-based PR now has a clean single-commit history on top of current
Current blocker appears to be GitHub Actions approval for fork workflows: the PR checks are still showing If a maintainer approves / runs the PR workflows in the GitHub UI, I can follow up on any real CI failures after that. |
|
@kshitijk4poor @teknium1 friendly ping on this one: the branch has been rebased/cleaned up into a single focused commit on top of current The remaining blocker appears to be fork-workflow approval ( |
8e4551f to
f9f29ed
Compare
f9f29ed to
37d771b
Compare
|
Refresh update: I force-pushed this branch as a clean single commit on top of current Updated head: The only refresh-specific change was aligning the new browse assertions with the current CLI default browse limit ( Focused local verification on the refreshed branch:
GitHub metadata now reports the PR as mergeable again. There are still no checks reported on this fork branch, so CI appears not to have run yet. |
|
Refresh update: I merged current Updated head: Focused local verification on the refreshed head:
GitHub now reports the PR as mergeable again. The new workflow runs are still at the fork workflow approval gate ( |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused CLI regression coverage. The premise remains valid on current main: hermes_cli/main.py:13825-13831 and hermes_cli/main.py:14405-14411 exclude only tool, while hermes_state.py:3075-3081 supports the proposed default exclusion plus explicit --source cron opt-in.
Problems
- The added
action = args.sessions_actionis stale-context duplication: current main already initializes it athermes_cli/main.py:13770. Keeping the PR line would add a second, unnecessary assignment.
Suggested changes
- Omit that redundant assignment when salvaging; the exclusion changes and four behavior-contract tests are otherwise appropriately scoped to
sessions listandsessions browse.
Automated hermes-sweeper review.
| @@ -11290,9 +11290,12 @@ def cmd_sessions(args): | |||
| print(f"Error: Could not open session database: {e}") | |||
| return | |||
|
|
|||
| # Hide third-party tool sessions by default, but honour explicit --source | |||
| action = args.sessions_action | |||
There was a problem hiding this comment.
Current main already assigns action = args.sessions_action at hermes_cli/main.py:13770; omit this duplicate assignment when salvaging the exclusion changes.
Exclude cron sessions from the default session list and browse views while still honoring explicit --source cron queries.
33dff23 to
863a2cf
Compare
|
Thanks — addressed on the current head I removed the stale duplicate Focused verification on the updated head: Could you please re-review the updated head? |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Four PRs are associated with this complex, but they cover two distinct causes: #11537 and #31499 reduce cron-session clutter, while #16370 and #54711 address raw arrow-key input in curses pickers. The current diffs show #11537 as the focused CLI list/browse fix and #54711 as the effective raw-sequence decoder integration.
Related pull requests
- #11537
related— (+74/-5) — merge after CI: Addscronto the default exclusions for bothsessions listandsessions browse, preserves explicit--source cron, and tests all four behavior paths. The current diff addresses the keep_open review on #11537 by relying on the existingactioninitialization rather than adding the criticized duplicate assignment. - #16370
related— (+24/-0) — superseded by #54711: Although it targets the arrow-navigation symptom, the diff only enables keypad mode and does not decode rawESC [ A/Bsequences before the ESC-cancel branch. Despite the keep_open review on #16370, its documented required fix path and raw-input regression coverage are implemented by #54711 instead. - #31499 [closed]
duplicate— (+5/-2) — closed duplicate of #11537: It remains relevant as evidence for the same cron-clutter policy, but modifies legacy CLI and gateway resume paths rather than #11537's currenthermes sessions listandbrowsepaths; the author closed it in favor of #11537. - #54711
related— (+197/-31) — merge separately: Routes the session browser and the sibling plugins picker through the existing raw-sequence decoder, with regression tests provingESC [ A/Bnavigation and selection behavior. It satisfies the keep_open review on #54711 by extending the fix and tests to the contributor-identified sibling picker.
Duplicates
#31499 substantially duplicates the cron-session exclusion intent of #11537 and is already closed in its favor. #16370 and #54711 target the same arrow-navigation failure, but #54711 supersedes #16370 with the decoder-based implementation required by review.
Suggested consolidation
Merge #11537 after its fork CI is approved and passes, and retain #31499 as its closed duplicate. Merge #54711 as the separate raw-arrow fix and close #16370 as superseded, because keypad initialization alone does not handle the reported raw escape sequences whereas #54711 decodes them and covers both affected pickers.
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 Dup11537 ["PRs duplicating each other"]
P11537["PR #11537 (open)"]
P31499["PR #31499 (closed)"]
end
class P11537 open
class P31499 closed
class P11537 target
click P11537 "https://github.com/NousResearch/hermes-agent/pull/11537"
click P31499 "https://github.com/NousResearch/hermes-agent/pull/31499"
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 4 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 21 kB of PR diffs, 8 kB of issue/PR text, 9 kB of discussion (14 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
Summary
cronsessions by default inhermes sessions listcronsessions by default inhermes sessions browse--source cronas an explicit opt-in for inspecting cron sessionsWhy
Frequent cron jobs create many background sessions that clutter the default interactive session views. This change keeps manual session navigation focused on user-facing sessions while preserving explicit access to cron sessions when requested.
Behavior before
hermes sessions listhidtoolsessions by default, but still showedcronhermes sessions browsehidtoolsessions by default, but still showedcronBehavior after
hermes sessions listhides bothtoolandcronby defaulthermes sessions browsehides bothtoolandcronby defaulthermes sessions list --source cronstill shows cron sessionshermes sessions browse --source cronstill shows cron sessionsHow to test
source venv/bin/activate && pytest tests/hermes_cli/test_session_browse.py -qhermes sessions listhermes sessions list --source cronhermes sessions browsehermes sessions browse --source cronPlatforms tested
Notes