Skip to content

fix(tests): mark routed profile as served in busy-mode test - #83745

Closed
fangliquanflq wants to merge 1 commit into
NousResearch:mainfrom
fangliquanflq:fix/tests-multiplex-busy-route-served
Closed

fangliquanflq wants to merge 1 commit into
NousResearch:mainfrom
fangliquanflq:fix/tests-multiplex-busy-route-served

Conversation

@fangliquanflq

@fangliquanflq fangliquanflq commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

test_profile_route_and_nonmultiplexed_resolution_preserve_boundaries always fails on CI after selective multiplex profile serving landed in c8f235a (feat(gateway): allow selective multiplex profile serving). Route resolution now rejects target profile research when it is not in _multiplex_profile_homes(config), so busy-input mode falls back to default interrupt instead of the snapshotted steer.

This PR monkeypatches _multiplex_profile_homes in that boundary test so research is in the served set. The test keeps asserting the multiplex / non-multiplex busy-mode boundary, not the serving allowlist. Bisect: green at a31be48, red from c8f235a.

Related Issue

Fixes #83743

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Tests (adding or improving test coverage)

Changes Made

  • tests/gateway/test_multiplex_busy_input_mode.py - mark research as served via _multiplex_profile_homes monkeypatch in the profile-route boundary test

How to Test

  1. On this branch, run:
    scripts/run_tests.sh tests/gateway/test_multiplex_busy_input_mode.py -q
  2. Confirm all tests in the file pass, including test_profile_route_and_nonmultiplexed_resolution_preserve_boundaries.
  3. Optional: temporarily remove the monkeypatch and confirm the original assert 'interrupt' == 'steer' failure returns.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the repository test entry on the relevant tests and all tests pass
  • I've added tests for the bug fix (fixture correction for the failing case)
  • I've tested on my platform: Windows 10

Documentation & Housekeeping

  • Relevant documentation: N/A - test-only fixture fix
  • cli-config.yaml.example: N/A - no config key changed
  • CONTRIBUTING.md or AGENTS.md: N/A - no contributor workflow changed
  • Cross-platform impact considered: N/A - pure unit-test fixture
  • Tool descriptions/schemas: N/A - no model tool behavior changed

Route resolution rejects targets outside the served set; the busy-mode boundary test only snapshotted research without marking it served.
@alt-glitch alt-glitch added type/test Test coverage or test infrastructure P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery area/profiles Multi-profile isolation, HERMES_HOME scoping labels Aug 11, 2026
686f6c61 added a commit to 686f6c61/hermes-agent that referenced this pull request Aug 11, 2026
After selective multiplex profile serving (NousResearch#83550), route resolution
rejects target profiles missing from the served set, so this boundary
test fell back to default interrupt on every CI slice (NousResearch#83743). Stamp
research as served so the route-path assertion matches production
rules. Same fixture shape as open NousResearch#83745.
686f6c61 added a commit to 686f6c61/hermes-agent that referenced this pull request Aug 11, 2026
After selective multiplex profile serving (NousResearch#83550), route resolution
rejects target profiles missing from the served set, so this boundary
test fell back to default interrupt on every CI slice (NousResearch#83743). Stamp
research as served so the route-path assertion matches production
rules. Same fixture shape as open NousResearch#83745.
686f6c61 added a commit to 686f6c61/hermes-agent that referenced this pull request Aug 11, 2026
After selective multiplex profile serving (NousResearch#83550), route resolution
rejects target profiles missing from the served set, so this boundary
test fell back to default interrupt on every CI slice (NousResearch#83743). Stamp
research as served so the route-path assertion matches production
rules. Same fixture shape as open NousResearch#83745.
@andrexibiza

Copy link
Copy Markdown
Contributor

This fix unblocks a large batch of open PRs — the same test_profile_route_and_nonmultiplexed_resolution_preserve_boundaries failure currently reds slice 5/12 on at least 10 open PRs (e.g. #83860, #83834, #83771, #83766, #83763, #83761, #83760, #83759, #83697, #83695, #83692), all of which pass the fix-independent checks. Your bisect (green at a31be48, red from c8f235a) matches the root cause we reproduced independently: route acceptance now requires the target profile in the served set, and the synthetic fixture serves none. Checks are green and mergeable is CLEAN — merging this would unblock the whole batch.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(tests): mark routed profile as served in busy-mode test

No blocking issues found. A few minor observations:

  1. The monkeypatch of gateway.run._multiplex_profile_homes hardcodes the served set, bypassing the real config-derived resolution. Appropriate for the test's intent, and monkeypatch.setattr will fail loudly if the patched name changes — just keep the stub minimal if the function gains behavior.

  2. The lambda ignores the _config argument and always serves only the "research" profile. If this test class later needs a different routed profile, the stubbed served set is an implicit assumption — a one-line comment noting the served set is intentionally stubbed would help future edits.

@fangliquanflq

Copy link
Copy Markdown
Contributor Author

Thanks for the review. No additional change is needed for either observation. The stub is intentionally narrow: this boundary test must make research part of the served set so route resolution reaches the busy-mode assertion, while config-derived allowlist behavior is covered separately. The current patch already documents that assumption immediately above the monkeypatch, and monkeypatch.setattr keeps signature/name drift visible. In addition, current main already contains the equivalent test-fixture repair in f51aa6a, using the lower-level profiles_to_serve seam with a fuller explanatory comment. The PR head remains a one-file test-only diff, and its required CI is green, including all 12 Python test slices and Windows/macOS checks.

@teknium1

teknium1 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Thanks @fangliquanflq — reviewed against current origin/main (dbb6acd) in the multiplexed-gateway backlog sweep.

Closing as redundant: the same repair landed in 8d1d193 / e8591ae (#97932, merged 2026-08-29). test_profile_route_and_nonmultiplexed_resolution_preserve_boundaries on main now takes tmp_path, monkeypatch and stubs the served set via hermes_cli.profiles.profiles_to_serve[("research", tmp_path / "research")] (tests/gateway/test_multiplex_busy_input_mode.py:383-390) — the same idea as your _multiplex_profile_homes stub, one layer lower so it survives refactors of the gateway-side helper. Verified just now: scripts/run_tests.sh tests/gateway/test_multiplex_busy_input_mode.py → 17 passed, 0 failed on origin/main.

Credit for the diagnosis: your bisect with @OutThisLife (green at a31be48, red from c8f235a's served-set gate raising ProfileRouteRejected) was correct and unblocked the CI slice for a dozen PRs while the fix was pending. #83743 and #83932 are closed on the same evidence.

@teknium1 teknium1 closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: test_profile_route_and_nonmultiplexed_resolution_preserve_boundaries always fails in CI

5 participants