fix(gateway): await AsyncSessionDB in preflight-compression warning

[tyoon10]

Fixes #70966.

Problem

Every gateway /model switch silently loses the preflight-compression warning. The user is never told the next message will compress, and nothing surfaces at default log level.

The gateway holds its session DB as AsyncSessionDB (gateway/run.py), whose generic __getattr__ forwarder returns an awaitable for every method call so blocking SQLite work is offloaded via asyncio.to_thread. enrich_model_switch_warnings_for_gateway called it without await:

messages = db.get_messages_as_conversation(entry.session_id)   # coroutine, never awaited

messages was therefore a coroutine, not a list, and _estimate_tokens raised:

TypeError: object of type 'coroutine' has no len()

Both gateway call sites wrap the helper in a bare except Exception logged at debug level, so the feature failed closed and silently. The only visible artifact was a misleading RuntimeWarning: coroutine 'AsyncSessionDB.__getattr__.<locals>._offloaded' was never awaited. The warning added for #23767 has never fired on the gateway.

Fix

• Make enrich_model_switch_warnings_for_gateway a coroutine and await the facade call.
• await it at both call sites in gateway/slash_commands.py (both already async def).
• Guard with inspect.isawaitable so the helper still works when handed a plain sync SessionDB.
• On failure, reset messages = None instead of pass, so a DB error degrades to "no warning" rather than forwarding a stale coroutine into the estimator.

Scope

AsyncSessionDB is constructed only at gateway/run.py, and gateway/run.py itself awaits every facade call — context_switch_guard.py was the only leak. The many self._session_db.<method>() calls in cli.py / run_agent.py / agent/* operate on a plain sync SessionDB and are correct as written; I checked them rather than assuming. CLI and TUI call merge_preflight_compression_warning directly and are untouched, as is its signature (existing tests/hermes_cli/test_context_switch_guard.py passes unchanged).

Tests

tests/gateway/test_model_switch_preflight_warning.py drives the real helper against a real on-disk SessionDB — no mock of the code under test — and asserts the behaviour contract rather than a frozen string:

• warning fires through the AsyncSessionDB facade (the regression guard)
• same helper works with a plain sync SessionDB
• no warning when the session is below threshold (it is threshold-driven, not unconditional)
• a DB error degrades to "no warning" instead of propagating

Verified failing before the change with the exact production TypeError, and passing after:

# before
E  TypeError: object of type 'coroutine' has no len()
   hermes_cli/context_switch_guard.py:41: TypeError
   4 failed

# after
   4 passed

Neighbouring suites green: tests/gateway/test_async_session_db.py, test_model_command_async_offload.py, test_model_switch_persistence.py, test_model_command_expensive_confirm.py, test_48031_model_switch_after_auto_reset.py (38 passed) and tests/hermes_cli/test_context_switch_guard.py (5 passed).

[alt-glitch]

This was generated by AI during triage.

Related: #64780, #64832, and #63799. Current main still uses session_store; this patch awaits its AsyncSessionDB facade at both live gateway call sites and retains sync-DB compatibility. The earlier alternatives target the same warning path but are not a current-path duplicate.

[tyoon10]

Thanks @alt-glitch — I dug into the three you linked. Correcting the record on my own PR, and flagging a gap none of the three close.

#63799 and #64832 are genuine prior art on the same bug, opened 9-11 days before mine, same two production files. I searched issues before filing (enrich_model_switch_warnings_for_gateway, get_messages_as_conversation, "preflight compression") and turned up nothing, but I never searched for the facade term, so I missed #63712 and both PRs. That's on me. If maintainers prefer to land either of those, I have no objection to closing #70973 — earlier authorship should win.

#64832 caught something mine does not. It also routes the store call:

store = getattr(runner, "async_session_store", None)
entry = await store.get_or_create_session(source)

I checked, and that is correct: SessionStore.get_or_create_session → _get_or_create_session_impl, whose own docstring says it performs "SQLite SELECTs, routing-index rewrite + os.fsync, recovery DB queries". My PR awaits only the DB read, so that blocking call stays on the gateway event loop. runner.async_session_store exists on main (gateway/run.py:3196), so #64832's approach is available and strictly more complete on that axis. I'd treat that as the reference fix for the store half.

The gap none of the three close — why this recurred after #63712 was closed implemented_on_main.

That close cited the AST guard at tests/gateway/test_async_session_db.py:315 as preventing exactly this class. The guard is real, but it only scans:

_GATEWAY_FILES = ("gateway/run.py", "gateway/slash_commands.py")

The un-awaited call here lives in hermes_cli/context_switch_guard.py. The gateway passes runner._session_db into a helper in another module, so the facade escapes the scanned set entirely and the guard reports clean while the bug ships. That is why #63712 could be closed as fixed in good faith and this still reached users.

None of #63799, #64832, or #70973 touch test_async_session_db.py — all three fix the call site, none fix the detector. Whichever lands, the guard will keep missing the next helper that receives _session_db as a parameter.

Two options, happy to do either (or neither, if this is better as a maintainer call):

1. Extend _GATEWAY_FILES to include hermes_cli/context_switch_guard.py — one line, catches this file, still misses the general case.
2. Broaden the visitor to flag un-awaited calls on any parameter that receives an AsyncSessionDB/AsyncSessionStore at a gateway call site — catches the class, more work, some false-positive tuning.

I'd suggest (2) as a separate PR against whichever fix lands, so the detector stops being narrower than the invariant it advertises. Happy to write it.

For reference, my repro is in #70966 with the exact production failure (TypeError: object of type 'coroutine' has no len() at context_switch_guard.py:41, swallowed by the callers' debug-level except so the preflight-compression warning is silently dead). That symptom detail may be useful to whichever PR proceeds, since it shows the failure is not just a RuntimeWarning but a fully disabled feature.

[tyoon10]

Pushed 7a728dd, acting on both points from the review thread.

1. Store read now goes through the async facade (credit @Manison502 / #64832).

I verified the concern is real: SessionStore.get_or_create_session → _get_or_create_session_impl, whose own docstring says it performs "SQLite SELECTs, routing-index rewrite + os.fsync, recovery DB queries". My original patch fixed only the DB read, so that blocking I/O stayed on the event loop. Now read via runner.async_session_store.

I also dropped the sync-DB fallback I had added. Checking the actual callers, the only production ones are the two gateway sites and the runner always wires AsyncSessionDB (gateway/run.py:3585), so the branch was speculative — and it defeated static analysis, which turned out to matter (below).

2. Widened the guard that should have caught this in the first place.

test_no_raw_session_db_calls_on_gateway_loop scanned only:

_GATEWAY_FILES = ("gateway/run.py", "gateway/slash_commands.py")

The gateway passes its runner into helpers that live outside gateway/, so the facade crosses a module boundary the scanner never opens. That is precisely how the un-awaited call in hermes_cli/context_switch_guard.py shipped after #63712 was closed implemented_on_main citing this guard.

The encouraging part: the visitor's alias tracking was already correct. Running it directly against the unpatched file on a clean origin/main worktree:

alias_calls found in context_switch_guard.py: [('get_messages_as_conversation', 190)]
>>> visitor logic ALREADY detects it: True
>>> only the file list is wrong     : True

So this is a scope fix, not new detection logic.

Changes:

• _GATEWAY_FILES now also covers hermes_cli/context_switch_guard.py and plugins/teams_pipeline/runtime.py — the two non-gateway modules the gateway currently hands self/_self to.
• test_gateway_files_covers_every_runner_receiving_helper — resolves runner-passing call sites from the gateway AST back to their source modules and fails if any is outside the scan set. A hardcoded list rots; this one can't fall behind the code silently.
• test_raw_call_guard_detects_cross_module_alias_calls — pins the alias detection itself, so a future refactor can't quietly drop it.

I deliberately did not widen the scan to every module referencing _session_db. I measured that first: ~130 hits across 9 files (tui_gateway/server.py alone has 69), nearly all legitimate sync-SessionDB use in CLI/agent paths. "Modules that receive the runner" is the precise criterion — it is exactly 2 today.

Verification. With the extended scan set applied to the unfixed context_switch_guard.py, the guard now fails on the original call:

FAILED test_no_raw_session_db_calls_on_gateway_loop
  hermes_cli/context_switch_guard.py:190 <alias>.get_messages_as_conversation( (binds _session_db)

That is the call that reached users while the old scope reported clean. 42 passed across test_async_session_db.py, test_model_switch_preflight_warning.py, test_context_switch_guard.py, test_model_command_async_offload.py, test_model_switch_persistence.py.

One design note worth flagging: while writing this I first used a _maybe_await(...) shim and then an isinstance branch — both work at runtime, but both defeat the guard, which only recognises the literal await <alias>.<method>(...) shape. The guard was right and I was wrong, so the call sites are now spelled plainly. Worth knowing if anyone reaches for a similar helper: keeping these calls statically verifiable is the point.

On duplication — I still think earlier authorship should win. #63799 and #64832 predate mine and I said so above; happy for this to be closed in favour of either. If a maintainer prefers one of those, the guard-scope fix here is independent of which call-site fix lands and I'd be glad to split it into its own PR against whichever does. It is the part that stops this class of bug recurring.

[teknium1]

Thanks for identifying a real gateway regression. Current main confirms the premise: hermes_cli/context_switch_guard.py:185-190 calls the async facades without await, while hermes_state.py:8561-8568 makes each AsyncSessionDB method call awaitable. Both live gateway paths are currently synchronous calls at gateway/slash_commands.py:1843-1850 and :2148-2155.

Problems

• Commit 7a728ddceac703bbc430ad9dd621db98c5f3fc54 expands tests/gateway/test_async_session_db.py with source-text AST scans of gateway modules. AGENTS.md:1382-1435 explicitly bans source-reading tests because they assert implementation shape rather than behavior.

Suggested changes

• Retain the real AsyncSessionDB regression test, but remove the added source-scanning meta-guard. If both gateway handlers need direct coverage, exercise them through injected collaborators or an integration path rather than parsing source files.

This is an automated hermes-sweeper review.

[tyoon10]

Thanks — both points were correct and are now addressed in 9a177eb (rebased onto current main).

1. Source-scanning tests removed.

You're right, and I should have caught this myself: AGENTS.md:1382-1435 bans source-reading tests outright, and the two meta-guards in 7a728dd did exactly that. They asserted the shape of the source, would fail on a correct refactor, and could pass while the implementation was subtly broken. Both are gone.

2. I also reverted the _GATEWAY_FILES scope edit — for a reason your review surfaced indirectly.

While rebasing I checked whether the widened scan set still bought anything, and it doesn't: test_no_raw_session_db_calls_on_gateway_loop no longer exists on main. It was deleted in 39975613b ("test: prune wave 2 … 28,106 → 19,757 test functions"). That was the guard the wider list was meant to fix.

The only remaining consumer of _GATEWAY_FILES is test_sync_db_escape_confined_to_off_loop_sites, which counts ._db sync escapes — a different check that neither added file participates in (0 escapes each, verified). So widening the list fixed nothing and would just be unjustified diff. tests/gateway/test_async_session_db.py is restored byte-for-byte to main's version.

Worth flagging for the record: that prune removed the guard which #63712 was closed implemented_on_main on the strength of. So the un-awaited-facade class currently has no static detector at all. Not something I'm proposing to fix here — and given the AGENTS.md rule, an AST scan is the wrong shape for it anyway. A behavioural equivalent would need real call-path execution. Happy to open a separate issue if that's useful.

What remains is production fix + behavioural coverage only, 3 files:

• hermes_cli/context_switch_guard.py — await both facades (DB read and get_or_create_session, the latter per fix(gateway): await model-switch session history #64832)
• gateway/slash_commands.py — await at both call sites
• tests/gateway/test_model_switch_preflight_warning.py — executes the real helper against a real on-disk SessionDB through the AsyncSessionDB / AsyncSessionStore facades: warning fires end-to-end, store read goes through the async facade, threshold behaviour, and DB-failure degradation

10 passed across test_async_session_db.py, test_model_switch_preflight_warning.py, and test_context_switch_guard.py.

Standing offer unchanged: #63799 and #64832 predate this PR on the same bug, and I'm happy for this to be closed in favour of either.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #63799: both use the same synchronous AsyncSessionDB unwrap in the model-switch warning helper; this patch differs only in regression-test placement. #64832 remains a distinct async alternative.

[alt-glitch]

This was generated by AI during triage.

Related: #63799 and this PR use competing sync-unwrapping repairs for the model-switch warning; #64832 uses an await-based mechanism. The current-head mechanism difference means this should not retain a duplicate disposition.

[tyoon10]

Rebased onto current main — and the approach changed, because main moved underneath this PR in a way that makes the original fix wrong.

What changed upstream. a0b29343b (follow-up to #74155 by @Drexuxux) made both /model call sites dispatch enrich_model_switch_warnings_for_gateway through asyncio.to_thread, because merge_preflight_compression_warning runs the blocking resolve_display_context_length provider probe. That was the right fix for the loop-block.

It does not fix this bug. The premise still reproduces on current main: hermes_cli/context_switch_guard.py reads db.get_messages_as_conversation(...) straight through the AsyncSessionDB facade, whose __getattr__ returns an awaitable for every method call (hermes_state.py). messages is still a coroutine, _estimate_tokens still raises TypeError: object of type 'coroutine' has no len(), and both call sites still swallow it at debug level. The warning is still dead on every gateway /model switch.

Why the original fix is now the wrong shape. Making the helper a coroutine would force reverting a0b29343b's to_thread dispatch and would fail its two offload tests. Two correct fixes collided.

The resolution. Since the helper now runs on a worker thread, unwrap to the synchronous handle instead of awaiting:

sync_db = getattr(db, "_db", db)
messages = sync_db.get_messages_as_conversation(entry.session_id)

That is the established pattern in this repo — gateway/run.py:4658, :5538, :16260, :16328, :18814. The helper stays synchronous, so a0b29343b's dispatch and its offload tests are untouched.

On the store read. The earlier follow-up commit here also awaited store.get_or_create_session. That is no longer needed and has been dropped: runner.session_store is the plain sync SessionStore (the async facade is a separate attribute, async_session_store), and the whole helper is already off-loop. Blocking I/O in it no longer touches the event loop.

Net effect on the diff. This PR is now 26 lines of fix plus the behavioural test file, down from the previous version. The guard-scope edit to tests/gateway/test_async_session_db.py is gone entirely — git dropped that commit during rebase as already upstream.

Verification.

• RED→GREEN: reverting only the two-line unwrap makes 3 of 4 tests fail with the exact TypeError, plus the coroutine ... was never awaited RuntimeWarning. Restoring it passes 4/4.
• a0b29343b's offload tests (tests/gateway/test_model_command_context_offload.py): 2/2 pass.
• tests/gateway/test_async_session_db.py: 4/4 pass, file untouched.
• Full tests/gateway/: 4699 passed, 5 failed — all 5 fail identically on unmodified origin/main (Discord send and session-store-prune, pre-existing pollution unrelated to this path).

[GottZ]

This was generated by AI during triage.

Summary

Five PRs address or reference this issue complex. #63799 and #70973 synchronously unwrap AsyncSessionDB inside the now-offloaded helper, #64832 and #64956 use the older async-helper approach, and #63806 neither implements its advertised await guard nor touches the warning-read cause.

Related pull requests

• fix(gateway): unwrap AsyncSessionDB before sync read in model-switch warning #63799 best fix — (+68/-3) — best fix for AsyncSessionDB methods silently dropped when called without await (e.g. gateway /model switch) → lost writes + RuntimeWarning #63712 by the recorded verdict and a current-path fix for Gateway model-switch guard does not await AsyncSessionDB conversation load #64780/Gateway /model switch silently drops the preflight-compression warning (un-awaited AsyncSessionDB call -> TypeError swallowed at debug level) #70966: the diff unwraps _db so the worker-thread helper receives messages rather than a coroutine. Despite the keep_open review on fix(gateway): unwrap AsyncSessionDB before sync read in model-switch warning #63799, its event-loop objection predates the asyncio.to_thread caller contract; the current mechanism duplicates fix(gateway): await AsyncSessionDB in preflight-compression warning #70973 with weaker behavioral coverage, while the original AsyncSessionDB methods silently dropped when called without await (e.g. gateway /model switch) → lost writes + RuntimeWarning #63712 write defect is already fixed by 0a997aabbc90ca65483ca2cb2347f658cfcd9211 at gateway/slash_commands.py:1627 and :1872, per the issue's automated close verdict.
• fix(agent): add _MustAwait wrapper to AsyncSessionDB to prevent silent drops (#63712) #63806 [closed] related — (+7/-680) — unrelated: the diff does not add _MustAwait or modify the affected write or warning-read paths, and instead reverts substantial unrelated state functionality. It remains relevant as a closed historical attempt for AsyncSessionDB methods silently dropped when called without await (e.g. gateway /model switch) → lost writes + RuntimeWarning #63712, whose write defect is already implemented on main by 0a997aabbc90ca65483ca2cb2347f658cfcd9211, as documented by the maintainer-bot verdict.
• fix(gateway): await model-switch session history #64832 best fix — (+67/-10) — best existing fix by the recorded verdict, but author action is required: it awaits both session facades and adds a focused real-facade regression, directly addressing the dropped history read. Its favorable keep_open review confirms those boundaries, but the diff must be rebased around current main's whole-helper asyncio.to_thread offload so the blocking provider probe remains off-loop.
• fix(gateway): await AsyncSessionDB.get_messages_as_conversation in model-switch guard (#64780) #64956 [closed] duplicate — (+85/-23) — partial fix, closed in favor of fix(gateway): await model-switch session history #64832: it awaits the database read but leaves the session-store lookup synchronous, lacks focused regression coverage, and carries unrelated stacked changes. Although its keep_open review found the core defect salvageable, that contributor review explicitly required both async facades and isolation, which fix(gateway): await model-switch session history #64832 provides.
• fix(gateway): await AsyncSessionDB in preflight-compression warning #70973 best fix — (+208/-3) — best fix for Gateway model-switch guard does not await AsyncSessionDB conversation load #64780/Gateway /model switch silently drops the preflight-compression warning (un-awaited AsyncSessionDB call -> TypeError swallowed at debug level) #70966 by the recorded verdict: the current diff unwraps AsyncSessionDB inside the already offloaded helper and behaviorally tests the warning threshold, small-session case, and DB-failure degradation. This remains consistent with the keep_open review on fix(gateway): await AsyncSessionDB in preflight-compression warning #70973; its source-scanning objection was addressed by removing those tests, and the current worker-thread mechanism preserves the provider-probe offload.

Duplicates

#63799 and current-head #70973 implement the same worker-thread synchronous unwrap, with #70973 providing stronger behavioral coverage. Closed #64956 is the incomplete, stacked counterpart of #64832; #63806 is not a fix for the warning-read issue.

Suggested consolidation

Keep #70973 open with a salvage path: retain its current-main-compatible worker-thread unwrap and real-DB behavioral tests. Author action on #64832: rebase onto main or split out an implementation that preserves the whole-helper offload; meanwhile close #63799 as a duplicate of #70973 despite its older keep_open review because the reviewed event-loop concern was superseded by the caller offload and its current diff now duplicates #70973, retain #64956 as closed in favor of #64832, and leave #63806 closed as already moot for #63712.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I64780(["issue #64780 (open)"])
    I70966(["issue #70966 (open)"])
    subgraph Dup63799 ["PRs duplicating each other"]
        P63799["PR #63799 (open)"]
        P64832["PR #64832 (open)"]
        P64956["PR #64956 (closed)"]
        P70973["PR #70973 (open)"]
    end
    P70973 -->|best fix| I64780
    P70973 -->|best fix| I70966
    class I64780 open
    class I70966 open
    class P63799 open
    class P64832 open
    class P64956 closed
    class P70973 open
    class P63799 best
    class P64832 best
    class P64832 best
    class P70973 best
    class P70973 best
    class P70973 target
    click I64780 "https://github.com/NousResearch/hermes-agent/issues/64780"
    click I70966 "https://github.com/NousResearch/hermes-agent/issues/70966"
    click P63799 "https://github.com/NousResearch/hermes-agent/pull/63799"
    click P64832 "https://github.com/NousResearch/hermes-agent/pull/64832"
    click P64956 "https://github.com/NousResearch/hermes-agent/pull/64956"
    click P70973 "https://github.com/NousResearch/hermes-agent/pull/70973"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 5 pull requests and 3 issues in this complex. Each diff was read against this issue; Assessment working set: 65 kB of PR diffs, 18 kB of issue/PR text, 18 kB of discussion (16 comments), 17 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.