Conversation
The multiplexed inbound handler runs the whole message inside _profile_runtime_scope, which installs the routed profile's HERMES_HOME override and its secret scope as contextvars -- its own docstring notes they reach the agent worker "via copy_context()". The automatic pre-turn hygiene compression handed _compress_context to a bare loop.run_in_executor(None, fn). A bare executor hop starts the worker with an EMPTY context, so inside it get_hermes_home() fell back to the default profile and get_secret read process-global os.environ -- which under multiplexing may hold a different profile's credentials. The compressor's aux-client provider resolution therefore ran unscoped: it either failed closed (automatic compression silently stopped working for multiplexed profiles) or resolved against the wrong profile's keys and config. gateway/slash_commands.py already routes the manual /compress through _run_in_executor_with_context for exactly this reason, and says so in a comment at the call site. The automatic path -- which runs unattended on every inbound message whose session crosses the compression threshold, and so fires far more often than the slash command -- never got the same treatment. Spawn the hop through copy_context().run. The executor stays the default one, so pool behaviour and the caller's progress-aware Future polling are unchanged. Extracted as _spawn_hygiene_compression rather than fixing the lambda in place: the call site sits ~660 lines into _handle_message_with_agent, and AGENTS.md asks for exactly this extraction so the contract can be exercised directly instead of asserted against the god-file's source text.
Collaborator
|
Resolved on main by #100950 (merge You identified and fixed this a month before it showed up in a user's debug bundle, @Drexuxux, and this PR was missed in the pre-merge duplicate sweep (my error: it surfaced on the post-merge sweep). You are the first submitter of this fix; noting that here for the record. Closing as implemented on main. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
The multiplexed inbound handler runs the whole message inside
_profile_runtime_scope, which installs the routed profile'sHERMES_HOMEoverride and its secret scope. Both are contextvars — the helper's own
docstring says so:
The automatic pre-turn hygiene compression handed
_compress_contextto a bareexecutor hop:
loop.run_in_executor(None, fn)starts the worker with an empty context.So inside that thread:
get_hermes_home()fell back to the default profile — the compressorresolved config, model and skills from the wrong home;
get_secretread process-globalos.environ, which under multiplexing mayhold a different profile's credentials (the first-writer-wins YAML→env
bridge). The compressor's aux-client provider resolution therefore ran
unscoped: it either failed closed (automatic compression silently stopped
working for multiplexed profiles) or resolved against another profile's keys.
This is a known failure mode — the other caller was already fixed
gateway/slash_commands.pyroutes the manual/compressthrough_run_in_executor_with_contextand says exactly why at the call site:The automatic path never got the same treatment — and it runs unattended on
every inbound message whose session crosses the compression threshold, so it
fires far more often than the slash command.
How it was found
Grepped the repo for its own declared invariants (
chokepoint,must go through,single source of truth,not a bare …) and then looked forcall sites that bypass them. The
/compresscomment names the rule; the hygienepath was the remaining violation.
Type of Change
Changes Made
gateway/run.py— the hygiene hop now runs throughcopy_context().run,so the worker inherits the caller's contextvars. The executor stays the
default one, so pool behaviour and the caller's progress-aware
Futurepolling (
.done()/.result()/.cancel()) are unchanged.gateway/run.py— extracted asGatewayRunner._spawn_hygiene_compressionrather than patching the lambda in place. The call site sits ~660 lines into
_handle_message_with_agent; AGENTS.md asks for exactly this extraction sothe contract can be exercised for real instead of asserted against the
god-file's source text.
tests/gateway/test_hygiene_compression_profile_scope.py— 5 tests.Single-profile gateways never enter
_profile_runtime_scope, so theirbehaviour is byte-identical.
How to Test
.envfiles holddifferent auxiliary-provider keys.
pre-turn hygiene compression fires.
get_hermes_home()to thedefault profile and read credentials from process-global
os.environ.After: it resolves both from the routed profile.
Test Results
New file
tests/gateway/test_hygiene_compression_profile_scope.py— 5 tests.They drive the real
_profile_runtime_scopeand the real spawn helper, witha recording stand-in agent that captures what the worker thread actually sees;
the contextvar loss is a property of the hop itself, so mocking it away would
test nothing:
test_worker_sees_the_profile_homeget_hermes_home()inside the worker is<root>/profiles/codertest_worker_sees_the_profile_secret_not_the_process_envget_secretreturns the profile.envvalue, not the conflictingos.environonetest_arguments_and_result_are_passed_through_unchangedapprox_tokens/commit_fenceand the return value are untouchedtest_returns_a_future_the_progress_wait_can_poll.done()/.result()/.cancel()interface the caller's inactivity wait depends on is preservedTestSingleProfileGatewayUnchangedRed-without-fix, with only the
copy_context()hop reverted:With the fix:
Regression sweep — whole
tests/gateway/directory, both sides on the samemain:The failing files are the same pre-existing set (feishu, runtime_footer, update,
discord, systemd, …) — none of them touch this code path. The one-test delta is
test_telegram_start_polling_timeout.py::test_initial_connect_succeeds_on_current_generation_progress,which I verified fails identically with and without this change: it is a
pre-existing
mainregression frome05eba26a, where a new_polling_conflict_recovery_generationattribute was added to__init__butthe suite's
_bare_adapter()helper (which builds the adapter via__new__)was not updated.