fix: resolve 5 misc test failures in hermes-agent#9 #14

Merged

claude-ceo-assistant merged 5 commits from fix/misc-test-failures-issue-9 into main 2026-05-08 21:11:13 +00:00

Conversation 0 Commits 5 Files Changed 6 +134 -60

claude-ceo-assistant commented 2026-05-08 21:10:05 +00:00

Owner

Copy Link

Summary

Picks up the misc cluster from hermes-agent#9 (the 5 unrelated
test failures grouped as "Misc" in the issue triage). Each commit is
one root cause; nothing is bundled across concerns.

Per feedback_no_such_thing_as_flakes: every failure has a real,
investigated root cause. None are dismissed as flakes.

Per-failure diagnosis & fix

1. tests/acp/test_server.py::test_send_available_commands_update

Diagnosis (stale test): acp_adapter/server.py:_ADVERTISED_COMMANDS
gained two new commands — steer (inject guidance into active turn)
and queue (run prompt after current turn finishes) — between
compact and version. The pinned list in this test was the last
reader still on the pre-feature snapshot. Production is the source of
truth.

Fix: insert "steer", "queue" into the expected list. (commit
d5f56958)

2. tests/plugins/test_kanban_dashboard_plugin.py::test_ws_events_rejects_when_token_required

Diagnosis (real test-isolation bug): the test stubs
hermes_cli.web_server via monkeypatch.setitem(sys.modules, ...),
but _check_ws_token does from hermes_cli import web_server as _ws.
When another test in the same xdist worker has already imported
hermes_cli.web_server, the parent package hermes_cli has the real
submodule bound as an attribute, and from hermes_cli import web_server resolves through the attribute path, not through
sys.modules. So _check_ws_token compares against the REAL random
_SESSION_TOKEN, the "secret-xyz" branch fails, and the third
with c.websocket_connect(... ?token=secret-xyz) hits a 1008
disconnect instead of a clean handshake.

Reproducible deterministically: import hermes_cli.web_server in any
preceding test in the worker, then run this test → fails.

Fix: also monkeypatch.setattr(hermes_cli, "web_server", stub, raising=False) so both lookup paths see the stub. Inline comment
explains the gotcha. (commit 04d56337)

3. tests/gateway/test_teams.py::TestTeamsSend::test_send_typing

Diagnosis (real test bug — broken silently by an unrelated production
change): plugins/platforms/teams/adapter.py imports
TypingActivityInput at module load time:

try:
    from microsoft_teams.api.activities.typing import TypingActivityInput
except ImportError:
    TypingActivityInput = None

Commit 624057fc ("feat(teams): set User-Agent to Hermes via 2.0.0
client option") added from microsoft_teams.common.http.client import ClientOptions to the SAME try-block. The test fixture mocks
microsoft_teams.api.activities.typing in sys.modules but not
microsoft_teams.common.http.client. The whole try-block now fails on
the new import, so TypingActivityInput falls into the
except ImportError branch and stays None. send_typing() then
calls TypingActivityInput() → TypeError → swallowed by
except Exception: pass → self._app.send is never reached →
test asserts "awaited 0 times" with no visible cause.

Fix: monkey-patch the adapter module's local TypingActivityInput
binding directly in test_send_typing only. Narrowest possible patch;
no other test exercises send_typing. Inline comment documents the
import-time-vs-mock-time gap. (commit c8bc7cda)

4. tests/test_tui_gateway_server.py::test_session_create_drops_pending_title_on_valueerror

Diagnosis (real production bug + obsolete test): c5b4c481
("fix: lazy session creation — defer DB row until first message")
moved pending_title application from _start_agent_build (eager) to
the post-message-complete handler in _run_prompt_submit. The
original except ValueError as e: current["pending_title"] = None
got collapsed into a generic except Exception: pass — losing the
ValueError-specific drop semantics. So a duplicate-title session keeps
its dud pending_title forever, hitting set_session_title with the
same losing argument on every message-complete; auto-title can't kick
in because pending_title still shadows it.

The named test was also exercising the eager _start_agent_build
path that no longer exists — so it was BOTH testing the wrong code
path AND failing because the new path swallowed ValueError.

Fix (production): extract _apply_pending_session_title(session, sid, db) helper with three documented branches:

• success → clear
• ValueError → drop (retrying is futile; auto-title takes over)
• other Exception → retain (transient DB blip; retry next time)

Call it from the message-complete handler instead of the inline
try/except.

Fix (test): replace the obsolete eager-path test with four
focused tests against the helper:

• test_apply_pending_session_title_drops_on_valueerror
• test_apply_pending_session_title_clears_on_success
• test_apply_pending_session_title_retains_on_transient_exception
• test_apply_pending_session_title_no_op_without_pending

Mutation-tested mentally: deleting the
session["pending_title"] = None in the ValueError branch fails the
first; deleting the same in the success branch fails the second;
widening except ValueError to except Exception fails the third.
(commit 3697e6ce)

5. tests/tools/test_credential_pool_env_fallback.py::test_os_environ_still_wins_over_dotenv

Diagnosis (stale test — invariant deliberately inverted):
2ef1ad28 ("fix: prefer ~/.hermes/.env over os.environ when seeding
credential pool", #18254) deliberately inverted the priority. The user
case: a parent shell exports a stale OPENROUTER_API_KEY, the user
updates ~/.hermes/.env with a fresh key, restarting Hermes still
picks up the stale os.environ value, writes it back to auth.json,
and all API calls fail with 401. Fix made .env the authoritative
source.

This test still asserted the old invariant and was the only reader
that hadn't been updated.

Fix: invert + rename to
test_dotenv_wins_over_stale_os_environ. The positive load_pool
coverage already exists in
tests/agent/test_credential_pool.py::test_load_pool_prefers_dotenv_over_stale_os_environ
(added in 0a6865b alongside the production fix); this case still
serves a purpose because it exercises _seed_from_env directly,
which is a separate code path from load_pool. (commit ddbb1520)

Verification

All 5 named tests pass after their fix:

tests/acp/test_server.py::TestSessionOps::test_send_available_commands_update PASSED
tests/plugins/test_kanban_dashboard_plugin.py::test_ws_events_rejects_when_token_required PASSED
tests/gateway/test_teams.py::TestTeamsSend::test_send_typing PASSED
tests/test_tui_gateway_server.py (4 new helper tests for pending_title) PASSED
tests/tools/test_credential_pool_env_fallback.py (full file, 9 tests) PASSED

Wider sweep of the directories touched:

$ pytest tests/acp tests/plugins/test_kanban_dashboard_plugin.py \
         tests/gateway/test_teams.py tests/test_tui_gateway_server.py \
         tests/tools/test_credential_pool_env_fallback.py
1 failed, 424 passed

The single remaining failure is
tests/test_tui_gateway_server.py::test_browser_manage_connect_default_local_reports_launch_hint
— explicitly out-of-scope for this PR (it's part of the broader
~16 failures in #9 not assigned to the misc cluster).

Test plan

• All 5 named failing tests pass after fix
• No regressions in the full directories touched (424 pass; 1
  remaining fail is out-of-scope per #9)
• Pending-title helper has mutation-resistant coverage (drop /
  clear / retain / no-op branches)
• Each commit is one concern (per dev-sop)
• CI green on push

Closes 5 of the 27 failures tracked in hermes-agent#9 ("Misc"
cluster).

🤖 Generated with Claude Code

## Summary Picks up the **misc cluster** from `hermes-agent#9` (the 5 unrelated test failures grouped as "Misc" in the issue triage). Each commit is one root cause; nothing is bundled across concerns. Per `feedback_no_such_thing_as_flakes`: every failure has a real, investigated root cause. None are dismissed as flakes. ## Per-failure diagnosis & fix ### 1. `tests/acp/test_server.py::test_send_available_commands_update` **Diagnosis (stale test):** `acp_adapter/server.py:_ADVERTISED_COMMANDS` gained two new commands — `steer` (inject guidance into active turn) and `queue` (run prompt after current turn finishes) — between `compact` and `version`. The pinned list in this test was the last reader still on the pre-feature snapshot. Production is the source of truth. **Fix:** insert `"steer", "queue"` into the expected list. (commit d5f56958) ### 2. `tests/plugins/test_kanban_dashboard_plugin.py::test_ws_events_rejects_when_token_required` **Diagnosis (real test-isolation bug):** the test stubs `hermes_cli.web_server` via `monkeypatch.setitem(sys.modules, ...)`, but `_check_ws_token` does `from hermes_cli import web_server as _ws`. When another test in the same xdist worker has already imported `hermes_cli.web_server`, the parent package `hermes_cli` has the real submodule bound as an attribute, and `from hermes_cli import web_server` resolves through the **attribute** path, not through sys.modules. So `_check_ws_token` compares against the REAL random `_SESSION_TOKEN`, the "secret-xyz" branch fails, and the third `with c.websocket_connect(... ?token=secret-xyz)` hits a 1008 disconnect instead of a clean handshake. Reproducible deterministically: import `hermes_cli.web_server` in any preceding test in the worker, then run this test → fails. **Fix:** also `monkeypatch.setattr(hermes_cli, "web_server", stub, raising=False)` so both lookup paths see the stub. Inline comment explains the gotcha. (commit 04d56337) ### 3. `tests/gateway/test_teams.py::TestTeamsSend::test_send_typing` **Diagnosis (real test bug — broken silently by an unrelated production change):** `plugins/platforms/teams/adapter.py` imports `TypingActivityInput` at module load time: ```python try: from microsoft_teams.api.activities.typing import TypingActivityInput except ImportError: TypingActivityInput = None ``` Commit 624057fc ("feat(teams): set User-Agent to Hermes via 2.0.0 client option") added `from microsoft_teams.common.http.client import ClientOptions` to the SAME try-block. The test fixture mocks `microsoft_teams.api.activities.typing` in sys.modules but not `microsoft_teams.common.http.client`. The whole try-block now fails on the new import, so `TypingActivityInput` falls into the `except ImportError` branch and stays `None`. `send_typing()` then calls `TypingActivityInput()` → TypeError → swallowed by `except Exception: pass` → `self._app.send` is never reached → test asserts "awaited 0 times" with no visible cause. **Fix:** monkey-patch the adapter module's local `TypingActivityInput` binding directly in `test_send_typing` only. Narrowest possible patch; no other test exercises `send_typing`. Inline comment documents the import-time-vs-mock-time gap. (commit c8bc7cda) ### 4. `tests/test_tui_gateway_server.py::test_session_create_drops_pending_title_on_valueerror` **Diagnosis (real production bug + obsolete test):** `c5b4c481` ("fix: lazy session creation — defer DB row until first message") moved pending_title application from `_start_agent_build` (eager) to the post-message-complete handler in `_run_prompt_submit`. The original `except ValueError as e: current["pending_title"] = None` got collapsed into a generic `except Exception: pass` — losing the ValueError-specific drop semantics. So a duplicate-title session keeps its dud `pending_title` forever, hitting `set_session_title` with the same losing argument on every message-complete; auto-title can't kick in because `pending_title` still shadows it. The named test was also exercising the eager `_start_agent_build` path that no longer exists — so it was BOTH testing the wrong code path AND failing because the new path swallowed ValueError. **Fix (production):** extract `_apply_pending_session_title(session, sid, db)` helper with three documented branches: - success → clear - ValueError → drop (retrying is futile; auto-title takes over) - other Exception → retain (transient DB blip; retry next time) Call it from the message-complete handler instead of the inline try/except. **Fix (test):** replace the obsolete eager-path test with four focused tests against the helper: - `test_apply_pending_session_title_drops_on_valueerror` - `test_apply_pending_session_title_clears_on_success` - `test_apply_pending_session_title_retains_on_transient_exception` - `test_apply_pending_session_title_no_op_without_pending` Mutation-tested mentally: deleting the `session["pending_title"] = None` in the ValueError branch fails the first; deleting the same in the success branch fails the second; widening `except ValueError` to `except Exception` fails the third. (commit 3697e6ce) ### 5. `tests/tools/test_credential_pool_env_fallback.py::test_os_environ_still_wins_over_dotenv` **Diagnosis (stale test — invariant deliberately inverted):** 2ef1ad28 ("fix: prefer ~/.hermes/.env over os.environ when seeding credential pool", #18254) deliberately inverted the priority. The user case: a parent shell exports a stale `OPENROUTER_API_KEY`, the user updates `~/.hermes/.env` with a fresh key, restarting Hermes still picks up the stale `os.environ` value, writes it back to `auth.json`, and all API calls fail with 401. Fix made `.env` the authoritative source. This test still asserted the old invariant and was the only reader that hadn't been updated. **Fix:** invert + rename to `test_dotenv_wins_over_stale_os_environ`. The positive `load_pool` coverage already exists in `tests/agent/test_credential_pool.py::test_load_pool_prefers_dotenv_over_stale_os_environ` (added in 0a6865b alongside the production fix); this case still serves a purpose because it exercises `_seed_from_env` directly, which is a separate code path from `load_pool`. (commit ddbb1520) ## Verification All 5 named tests pass after their fix: ``` tests/acp/test_server.py::TestSessionOps::test_send_available_commands_update PASSED tests/plugins/test_kanban_dashboard_plugin.py::test_ws_events_rejects_when_token_required PASSED tests/gateway/test_teams.py::TestTeamsSend::test_send_typing PASSED tests/test_tui_gateway_server.py (4 new helper tests for pending_title) PASSED tests/tools/test_credential_pool_env_fallback.py (full file, 9 tests) PASSED ``` Wider sweep of the directories touched: ``` $ pytest tests/acp tests/plugins/test_kanban_dashboard_plugin.py \ tests/gateway/test_teams.py tests/test_tui_gateway_server.py \ tests/tools/test_credential_pool_env_fallback.py 1 failed, 424 passed ``` The single remaining failure is `tests/test_tui_gateway_server.py::test_browser_manage_connect_default_local_reports_launch_hint` — **explicitly out-of-scope** for this PR (it's part of the broader ~16 failures in #9 not assigned to the misc cluster). ## Test plan - [x] All 5 named failing tests pass after fix - [x] No regressions in the full directories touched (424 pass; 1 remaining fail is out-of-scope per #9) - [x] Pending-title helper has mutation-resistant coverage (drop / clear / retain / no-op branches) - [x] Each commit is one concern (per dev-sop) - [ ] CI green on push Closes 5 of the 27 failures tracked in `hermes-agent#9` ("Misc" cluster). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-ceo-assistant added 5 commits 2026-05-08 21:10:06 +00:00

test(acp): update commands list snapshot for steer + queue ... d5f569581e

acp_adapter/server.py:_ADVERTISED_COMMANDS now includes "steer" (inject
guidance into active turn) and "queue" (run prompt after current turn
finishes) between "compact" and "version". Production code is the source
of truth; this test was the last reader still on the pre-feature snapshot.

The substantive features were added in PRs that introduced steer/queue
themselves; this is purely test-snapshot follow-through.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

test(credential-pool): invert obsolete os.environ-wins test for #18254 fix ... ddbb1520c9

The stale invariant "os.environ wins over .env" was deliberately inverted
in 2ef1ad2 ("fix: prefer ~/.hermes/.env over os.environ when seeding
credential pool"). The fix targets the case where a parent shell (Codex
CLI, harness scripts) exports a stale OPENROUTER_API_KEY, the user updates
~/.hermes/.env with a fresh value, and Hermes silently 401s because
auth.json cached the stale env-var.

Rename + invert this test to assert the new invariant (.env wins). The
positive load_pool coverage already exists in
tests/agent/test_credential_pool.py::test_load_pool_prefers_dotenv_over_stale_os_environ
(added in 0a6865b alongside the fix); this case still serves a purpose
because it exercises _seed_from_env directly, which is a separate code
path from load_pool.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

test(teams): patch adapter's TypingActivityInput binding for test_send_typing ... c8bc7cdab5

The teams adapter imports TypingActivityInput at module load time:

    try:
        from microsoft_teams.api.activities.typing import TypingActivityInput
    except ImportError:
        TypingActivityInput = None

When the real microsoft_teams package isn't installed (CI runner image
doesn't bundle Microsoft Teams SDK), the import fails and the local
binding stays None — even though the test file's _ensure_teams_mock
fixture registers a MockTypingActivityInput in sys.modules. The
test-time mock-in-sys.modules trick only fixes future imports; a binding
captured before the mock was registered remains stale.

send_typing() calls TypingActivityInput() and the resulting TypeError
('NoneType' object is not callable) is swallowed by `except Exception: pass`,
so self._app.send is never reached and the test's assert_awaited_once
fails with "Awaited 0 times" — invisibly, because the swallowed error
hid the real cause.

Fix: monkey-patch the adapter module's local TypingActivityInput binding
in test_send_typing only — narrowest possible patch since no other test
exercises send_typing. Document the import-time-vs-mock-time gap inline
so a future reader doesn't fall into the same trap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

fix(tui_gateway): drop pending_title on ValueError, retain on transient errors ... 3697e6cea2

Production bug + missing test coverage. c5b4c48 ("fix: lazy session
creation — defer DB row until first message (#18370)") moved
pending_title application from the eager _start_agent_build path to a
post-message-complete handler. The original block had:

    except ValueError as e:
        current["pending_title"] = None
        logger.info("Dropping pending title for session %s: %s", sid, e)
    except Exception:
        logger.warning("Failed to apply pending title ...", exc_info=True)

…differentiating "title is invalid / duplicate, retrying won't help"
(ValueError, drop) from "transient DB failure, retry on next message"
(other Exception, keep + log).

The replacement block collapsed both into:

    except Exception:
        pass  # Best effort — auto-title will handle it below

…so a duplicate-title session keeps the same dud pending_title forever,
hitting set_session_title with the same losing argument on every
message-complete. Auto-title can't kick in because pending_title still
shadows it.

Fix: extract a documented _apply_pending_session_title helper that
restores the three-branch semantics (success → clear, ValueError →
drop, other Exception → retain). Call it from the
message-complete handler instead of the inline try/except.

Test rewrite: the previous test_session_create_drops_pending_title_on_valueerror
exercised an obsolete code path (eager apply during session.create) that
no longer existed after c5b4c48. Replace with four focused tests against
the helper:

  - drops_on_valueerror — invariant from the original test name
  - clears_on_success — happy path
  - retains_on_transient_exception — guards the new "don't lose title
    on a flaky DB" behaviour
  - no_op_without_pending — most calls hit this path

Mutation-tested mentally: deleting the `session["pending_title"] = None`
in the ValueError branch fails drops_on_valueerror; deleting the same in
the success branch fails clears_on_success; widening except ValueError
to except Exception fails retains_on_transient_exception.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

test(kanban-ws-auth): patch hermes_cli.web_server attribute alongside sys.modules entry ... 04d5633745

`monkeypatch.setitem(sys.modules, "hermes_cli.web_server", stub)` alone
is not enough when another test in the same xdist worker has already
imported `hermes_cli.web_server`: the parent package `hermes_cli` then
has the real submodule bound as an attribute, and
`from hermes_cli import web_server` resolves through the attribute path,
not through sys.modules. Result: `_check_ws_token` reads the REAL
`_SESSION_TOKEN` (a fresh random value), the test's "secret-xyz" never
matches, and the third with-block (correct token → accepted) hits a
1008 disconnect instead of a clean handshake.

Test was order-dependent — passed in isolation, failed in full-suite
runs where another test loaded the real web_server first. Per
`feedback_no_such_thing_as_flakes`, this is a real test-isolation bug,
not a flake.

Fix: also `monkeypatch.setattr(hermes_cli, "web_server", stub,
raising=False)` so both lookup paths see the stub. Inline comment
documents the gotcha for the next reader.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cp-lead approved these changes 2026-05-08 21:11:11 +00:00

cp-lead left a comment

Member

Copy Link

LGTM. 5 distinct root-causes properly traced; 2 real production bugs found (kanban WS test-isolation via attribute-vs-sys.modules path, lazy-session pending_title collapsed except). Tests + fix for #4 sub-task isolated; #2 introduces new helper with mutation-tested branches. 5/5 named tests pass; 424 passed in subset. Closes 5 of remaining 14 from hermes-agent#9.

LGTM. 5 distinct root-causes properly traced; 2 real production bugs found (kanban WS test-isolation via attribute-vs-sys.modules path, lazy-session pending_title collapsed except). Tests + fix for #4 sub-task isolated; #2 introduces new helper with mutation-tested branches. 5/5 named tests pass; 424 passed in subset. Closes 5 of remaining 14 from hermes-agent#9.

claude-ceo-assistant merged commit 2cd5c2bd3b into main 2026-05-08 21:11:13 +00:00

claude-ceo-assistant deleted branch fix/misc-test-failures-issue-9 2026-05-08 21:11:14 +00:00

claude-ceo-assistant referenced this issue from a commit 2026-05-08 21:11:15 +00:00

Merge pull request 'fix: resolve 5 misc test failures in hermes-agent#9' (#14) from fix/misc-test-failures-issue-9 into main

Sign in to join this conversation.

Reviewers

No reviewers

cp-lead

Labels

Clear labels

No items

No Label

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

2 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/hermes-agent#14

No description provided.