Skip to content

test(gateway): guard every /stop path against session suspension - #9242

Open
r266-tech wants to merge 1 commit into
NousResearch:mainfrom
r266-tech:test/stop-no-suspend-regression
Open

test(gateway): guard every /stop path against session suspension#9242
r266-tech wants to merge 1 commit into
NousResearch:mainfrom
r266-tech:test/stop-no-suspend-regression

Conversation

@r266-tech

@r266-tech r266-tech commented Apr 13, 2026

Copy link
Copy Markdown
Contributor

Summary

/stop intentionally interrupts work without suspending the conversation. This
rebases the regression guard onto current main and covers every current route:

  • existing sentinel, active-agent, and pending-message tests now assert that
    suspend_session() was not called;
  • a no-active-agent test enters through the production message dispatcher and
    checks that the current session remains visible;
  • both authorized and unauthorized sibling-thread /stop paths assert session
    preservation.

The patch reuses the existing scenario setup instead of keeping the original
aggregate test. Related PR #9457 overlaps the three older assertions, but its
current head includes unrelated changes and does not cover the no-active or
sibling-thread paths requested in the maintainer review.

Test plan

  • uv run pytest -q tests/gateway/test_session_race_guard.py tests/gateway/test_stop_thread_sibling.py — 27 passed
  • uv run ruff check tests/gateway/test_session_race_guard.py tests/gateway/test_stop_thread_sibling.py

Follow-up to #9241.

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery labels Apr 27, 2026
@alt-glitch

alt-glitch commented Apr 27, 2026

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Updated after the live rework: related to #9457, not a duplicate. This branch additionally covers the no-active-agent and authorized/unauthorized sibling-thread /stop routes; maintainer review can choose whether to consolidate the complementary regression coverage.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for adding a direct regression guard for the session-preservation behavior introduced by 952a885.

Problems

  • Current main has an additional authorized sibling-thread /stop path at gateway/slash_commands.py:1089-1110; the proposed three-path test does not exercise it.
  • The sentinel and active-agent scenarios are already exercised in tests/gateway/test_session_race_guard.py:409-520. The missing piece is specifically the suspend_session() assertion.

Suggested changes

  • Add runner.session_store.suspend_session.assert_not_called() to the existing sentinel, active-agent, and pending-message tests, as open PR test(gateway): guard /stop against session suspension regression #9457 does, rather than duplicating their setup in one aggregate test.
  • Add sibling-thread coverage if this test is intended to guard every current /stop path.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 12, 2026
@r266-tech
r266-tech force-pushed the test/stop-no-suspend-regression branch from 4809f7f to 3277e94 Compare July 13, 2026 19:21
@r266-tech r266-tech changed the title test(gateway): regression test for /stop not suspending session (#9241) test(gateway): guard every /stop path against session suspension Jul 13, 2026
@r266-tech

Copy link
Copy Markdown
Contributor Author

Reworked this onto current main following the maintainer review:

  • moved the preservation assertions into the existing sentinel, active-agent,
    and pending-message tests;
  • added the missing no-active route through _handle_message();
  • covered both authorized and unauthorized sibling-thread /stop branches.

The branch is now a focused two-test-file diff (31 additions, one deletion).
Validation: 27 targeted tests and Ruff pass; the exact reworked diff received a
Codex adversarial-review verdict of approve.

@alt-glitch alt-glitch added the needs-decision Awaiting maintainer decision before any implementation label Jul 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #9457, not a duplicate after the rework: this branch additionally covers the no-active-agent and authorized/unauthorized sibling-thread /stop routes. Maintainer review can choose whether to consolidate the complementary regression coverage.

@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
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/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants