fix(cli): make refresh_interval configurable, default to 0 (disabled) - #48319
fix(cli): make refresh_interval configurable, default to 0 (disabled)#48319Elshayib wants to merge 2 commits into
Conversation
…le lock healing When _release_session_guard skips cleanup due to guard mismatch (e.g., reset/new command swapped the interrupt_event), the finally block in _process_message_background was deleting _session_tasks[key] before attempting to release the guard. This left _active_sessions with a stale guard but no _session_tasks entry, making _session_task_is_stale() return False (task=None → not stale). The stale lock was never healed, causing permanent deadlock: subsequent messages were received but never dispatched to the agent. Fix: only delete _session_tasks[key] if _release_session_guard actually removed _active_sessions[key]. When the guard mismatches, the _session_tasks entry is preserved so _session_task_is_stale can detect the done task and _heal_stale_session_lock clears both entries on the next inbound message. Regression test: test_guard_mismatch_preserves_session_task_for_stale_detection Refs: #48300
prompt_toolkit's refresh_interval=1.0 causes a full UI redraw every second in non-fullscreen mode. Terminal emulators with "auto-scroll on output" interpret this as new content and snap the viewport to the bottom, fighting the user's manual scroll position. Fix: add display.cli_refresh_interval config key (default 0 = disabled). Users who want the idle clock ticking can set a positive value in config.yaml. Fixes #48309.
alt-glitch
added
type/bug
|
Competing with #48321 for the same fix (#48309). This PR makes the idle redraw configurable via a new |
|
Thanks for flagging. I'm aware of #48321 — both PRs address the same root cause (the auto-scroll fight from idle redraw), but the approaches differ:
This PR is strictly a superset of #48321's behavior — same default, but configurable. I'd suggest we merge this one and close #48321, since anyone who wants the clock ticking can set |
|
Resolved via the config knob added in #49056 and finalized in #49087 (commit d7bff94). We kept the default at |
Follow-up to the salvaged NousResearch#48312 — adds the config-default test (ported from NousResearch#48319) and the AUTHOR_MAP entry for the cherry-picked commit.
Follow-up to the salvaged NousResearch#48312 — adds the config-default test (ported from NousResearch#48319) and the AUTHOR_MAP entry for the cherry-picked commit.
Follow-up to the salvaged NousResearch#48312 — adds the config-default test (ported from NousResearch#48319) and the AUTHOR_MAP entry for the cherry-picked commit.
Follow-up to the salvaged NousResearch#48312 — adds the config-default test (ported from NousResearch#48319) and the AUTHOR_MAP entry for the cherry-picked commit.
Follow-up to the salvaged NousResearch#48312 — adds the config-default test (ported from NousResearch#48319) and the AUTHOR_MAP entry for the cherry-picked commit.
Follow-up to the salvaged NousResearch#48312 — adds the config-default test (ported from NousResearch#48319) and the AUTHOR_MAP entry for the cherry-picked commit.
4 participants
Summary
prompt_toolkit
refresh_interval=1.0(introduced in6724daa2cfor idle clock ticking) causes a full UI redraw every second in non-fullscreen CLI mode. Terminal emulators with "auto-scroll on output" (Xshell, iTerm2, Windows Terminal) interpret this as new content and snap the viewport to the bottom, fighting the user's manual scroll position.Fixes #48309.
Changes
hermes_cli/config.py: Added"cli_refresh_interval": 0to thedisplayconfig defaults. Default0= disabled (no background redraw), preserving pre-6724daa2cbehavior.cli.py: Replaced hardcodedrefresh_interval=1.0withfloat(CLI_CONFIG.get("display", {}).get("cli_refresh_interval", 0)). Users who want the idle clock ticking can setdisplay.cli_refresh_interval: 1.0inconfig.yaml.tests/hermes_cli/test_config.py: AddedTestCliRefreshIntervalConfigverifying the default is0.Test plan
tests/hermes_cli/test_config.py::TestCliRefreshIntervalConfig— PASSEDtests/hermes_cli/test_config.py— 101/101 PASSED