Skip to content

test: guard session-owned runtime invariants#1753

Closed
Michaelyklam wants to merge 1 commit intonesquena:masterfrom
Michaelyklam:fix/issue-1694-session-runtime-invariants
Closed

test: guard session-owned runtime invariants#1753
Michaelyklam wants to merge 1 commit intonesquena:masterfrom
Michaelyklam:fix/issue-1694-session-runtime-invariants

Conversation

@Michaelyklam
Copy link
Copy Markdown
Contributor

Thinking Path

  • Issue design(streaming): clarify session-scoped runtime ownership for running WebUI sessions #1694 asks to preserve the boundary where running state belongs to the owning session, not whichever pane is currently active.
  • The current code already has session-keyed browser/runtime maps and stream-keyed transport state, so the safest slice is regression coverage around that boundary.
  • While adding the tests, the live done / settled-session terminal paths exposed a small leak: background completion could still call setBusy(false) and stop the active pane's approval/clarify pollers.
  • This PR adds narrow source-level invariant coverage and fixes that terminal cleanup leak without introducing a broader runtime rewrite.

What Changed

  • Added tests/test_session_runtime_ownership_invariants.py covering:
    • sidebar row cancellation uses row-owned session.active_stream_id;
    • live done and settled-session fallback completion do not idle an unrelated active pane;
    • approval/clarify pollers are stopped by owner session;
    • LIVE_STREAMS and INFLIGHT remain session-keyed.
  • Updated static/messages.js so background terminal events:
    • only clear active-pane busy/composer state when the completed stream belongs to the active pane, or no other active-pane inflight runtime exists;
    • stop approval/clarify polling through owner-session guards instead of blindly stopping the currently viewed pane's poller.

Refs #1694.

Why It Matters

This protects the core Milestone 2 streaming invariant: a long-running turn can finish, cancel, or error in the background without tearing down the runtime state for the session the user is currently viewing.

Verification

  • RED: python -m pytest tests/test_session_runtime_ownership_invariants.py -q failed on the unguarded done/poller cleanup paths before the implementation.
  • GREEN focused: /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_session_runtime_ownership_invariants.py -q -> 5 passed.
  • Targeted regressions: /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_session_runtime_ownership_invariants.py tests/test_sprint36.py tests/test_sidebar_first_turn_visibility.py tests/test_streaming_session_sidebar.py -q -> 29 passed.
  • Full isolated suite: env -u HERMES_CONFIG_PATH -u HERMES_WEBUI_HOST /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/ -q -> 4601 passed, 2 skipped, 3 xpassed, 1 warning, 8 subtests passed.
  • git diff --check passed.

Risks / Follow-ups

Model Used

OpenAI Codex gpt-5.5 via Hermes Agent CLI, with terminal/file/GitHub tooling.

@nesquena-hermes
Copy link
Copy Markdown
Collaborator

Thanks @Michaelyklam — this shipped in v0.51.12 (commit 34f2243) as part of a 3-PR full-sweep batch release. Stage rebased your branch onto current master, ran the full pre-release gate (4632 pytest, browser tests, Opus advisor verdict SHIP), and merged via release PR #1755.

GitHub didn't auto-close because the merge commit only references the squash-merged stage branch, not your fork's commit directly — closing manually for hygiene.

Live now on https://get-hermes.ai/ and on existing installs after git pull + restart.

Release notes: https://github.com/nesquena/hermes-webui/releases/tag/v0.51.12

pull Bot pushed a commit to soitun/hermes-webui that referenced this pull request May 6, 2026
pull Bot pushed a commit to soitun/hermes-webui that referenced this pull request May 6, 2026
… custom provider routing + session runtime invariants)

Constituent PRs:
- nesquena#1746 (@Michaelyklam) — shorten cron profile lock for manual runs (closes nesquena#1574, RETURNS from v0.51.11 deferral with queue-drain blocker fixed)
- nesquena#1752 (@Michaelyklam) — route custom provider models dict selections (slice of nesquena#1240 umbrella)
- nesquena#1753 (@Michaelyklam) — guard session-owned runtime invariants (refs nesquena#1694)

nesquena#1746 v2 fix: result_queue.get(timeout=...) BEFORE process.join()
(drain-then-join), with queue.Empty recovery + 200,000-char regression test.
Opus stage-306 verified the fix correct + complete; the prior fork→spawn
SHOULD-FIX filed as follow-up issue nesquena#1754 (separate architectural change).

Tests: 4622 → 4632 passing (+10). 0 regressions. Stably green on first try.

Pre-release verification:
- All 3 PRs CI-green individually + rebased onto master with NO conflicts
  (disjoint files: api/config.py + static/messages.js + api/routes.py)
- pytest 4632 passed, 0 failed
- node -c clean on static/messages.js
- 11/11 browser API endpoints PASS
- Opus advisor: SHIP all 3, 0 MUST-FIX, 1 SHOULD-FIX filed as nesquena#1754

Closes nesquena#1574.
iosub pushed a commit to iosub/HERMES-hermes-webui2 that referenced this pull request May 6, 2026
…unct from PR nesquena#1753

PR nesquena#1753 (shipped v0.51.12) introduced the 3-way OR guard in done/error/cancel
handlers: 'isActiveSession || !S.session || !INFLIGHT[S.session.session_id]'.
The third disjunct ('no other inflight on the active pane') is the permissive
fallback Opus stage-306 verified — it allows the active pane to idle when no
other session is running, even when the completing stream is from a different
session. PR nesquena#1761's centralizing helper _setActivePaneIdleIfOwner inadvertently
dropped this disjunct, so a user viewing pane A (idle) while pane B completes
in the background would not get pane A's composer state cleared.

Restored: _setActivePaneIdleIfOwner now checks the same 3-way OR.

Verified via:
- node -c static/messages.js — clean
- pytest tests/test_session_runtime_ownership_invariants.py
       tests/test_1694_terminal_cleanup_ownership.py — 9 passed

Co-authored-by: dso2ng <dso2ng@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants