test(skills): cover additional rescan paths in skill_commands cache (salvage #19042) - #21181
Merged
Merged
Conversation
…14536) The rescan-on-platform-change fix landed in #18739 ships one regression test that exercises the HERMES_PLATFORM env-var path. Three other code paths in get_skill_commands / _resolve_skill_commands_platform have no direct coverage; this commit adds a regression test for each. - Gateway session context (HERMES_SESSION_PLATFORM via ContextVar): the resolver consults get_session_env after HERMES_PLATFORM, and the gateway sets that variable through set_session_vars (a ContextVar), not os.environ. The test uses set_session_vars / clear_session_vars to drive the actual gateway signal, and the disabled-skill stub reads the same value via get_session_env. A regression that swapped get_session_env for plain os.getenv would still pass an env-var-based test but break concurrent gateway sessions, which is the bug the ContextVar plumbing exists to prevent. - Returning to no-platform-scope (CLI / cron / RL rollouts after a gateway session): the cached telegram view must be dropped and the unfiltered scan repopulated when HERMES_PLATFORM is unset again. - Same-platform cache hit: consecutive calls under the same platform scope must NOT rescan. The rescan trigger is change in scope, not "always re-resolve" — a gateway serving many consecutive telegram requests should pay the scan cost once, not per request. The third test wraps scan_skill_commands with a spy after the cache is primed, so the assertion is on call_count == 0 across three subsequent get_skill_commands() calls. All 39 tests in tests/agent/test_skill_commands.py pass under scripts/run_tests.sh.
Contributor
🔎 Lint report:
|
alt-glitch
added
type/test
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
3 participants
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.
Closes #19042 via salvage.
Summary
Three regression tests on top of the rescan-on-platform-change fix from #18739, covering code paths in
_resolve_skill_commands_platformandget_skill_commandsnot exercised by the existing test.Validation
scripts/run_tests.sh tests/agent/test_skill_commands.py→ 39 passed.Original author: @Tranquil-Flow.