fix(gateway): implement session-scoped /fast + raise KANBAN_GUIDANCE ceiling - #137
fix(gateway): implement session-scoped /fast + raise KANBAN_GUIDANCE ceiling#137exiao wants to merge 2 commits into
Conversation
…ceiling Two pre-existing baseline test failures on live-config, unrelated to any open feature PR (verified on a clean live-config worktree): 1. test_fast_command.py (3 tests): the /fast session-scope feature was half-shipped. The tests expect /fast to be session-scoped by default with a --global flag (mirroring /reasoning), using the already-present _session_service_tier_overrides dict and _resolve/_set helpers. But _handle_fast_command still only wrote agent.service_tier to config.yaml globally and rejected --global as unknown. Rewrite it to match the shipped /reasoning pattern: bare /fast records a session override and leaves config.yaml untouched; --global persists to config.yaml and clears the session override. 2. test_kanban_guidance_prompt_size_bounded: upstream b5bd0ef added legitimate attachment guidance, growing KANBAN_GUIDANCE to 5749 chars past the test's <5500 cap. The test's docstring says the ceiling guards unbounded growth 'with a little headroom', so restore headroom by raising the cap to 6500 (still a real guard). 122 tests pass across both files; ruff clean. Patch note: ~/.hermes/plans/hermes-patches/baseline-flaky-tests.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0893678620
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Superseded by #139, which consolidates the live-config CI baseline repair. #139 fixes the same failing tests via the correct root cause (deleting the stale shadow slash-handlers in gateway/run.py that were overriding the picker-enabled mixin versions) and, unlike the partial repairs, does not break test_send_file_tool. See #139 for the full diagnosis and the one remaining product decision (the egress-vs-send_file contradiction). |
…e shadow handlers) (#139) * fix(gateway): delete stale shadow slash-handlers + raise KANBAN_GUIDANCE ceiling Consolidates the live-config CI baseline repair (supersedes #136, #137, #138). 15 tests were failing on live-config across 4 modules. This fixes 12 via the correct root cause; 3 egress tests are a separate product decision (see patch note). 1. Stale shadow handlers (11 tests: test_fast_command x3, test_choice_picker x8). The June slash-command extraction moved _handle_reasoning_command and _handle_fast_command into GatewaySlashCommandsMixin with the new choice-picker logic, but left the OLD copies in the GatewayRunner class body in run.py. A method defined on the subclass overrides an inherited mixin method, so the stale copies shadowed the picker-enabled versions, making the choice picker unreachable and /fast --global unrecognized. Delete the two stale defs (166 lines) so the mixin versions activate. #137's re-implement-into-the-shadow approach was wrong; the shadow must be deleted. 2. KANBAN_GUIDANCE size ceiling (1 test). Upstream b5bd0ef added legitimate attachment guidance (5749 chars vs the <5500 cap). Raise cap to 6500. 219 tests pass across the affected modules; send_file's 57 expanded-ext tests still pass (unlike #138 which broke them). ruff clean. Patch note: ~/.hermes/plans/hermes-patches/live-config-baseline-consolidated.md * fix(gateway): route MEDIA-tag code/config exts through validated egress Third baseline failure (test_platform_base.py::TestUniversalMediaEgress x3). The egress redesign (2fd36b1, NousResearch#36060) intends unknown/code extensions to deliver ONLY via the validated pass (validate_media_delivery_path: exists on disk, safe root, not denylisted) so a prompt-injection MEDIA:/etc/x.py stays visible instead of silently exfiltrating. But the code still folded MEDIA_TAG_EXTRA_EXTS (.py/.log/.toml/...) into _MEDIA_TAG_EXT_ALTERNATION, the UNCONDITIONAL extract grammar — contradicting its own design comment and the egress tests. Point _MEDIA_TAG_EXT_ALTERNATION at MEDIA_DELIVERY_EXTS only, so code/config/log tags route through the validated pass. MEDIA_TAG_EXTRA_EXTS is retained for the separate producer-tool detector in run.py (_TOOL_MEDIA_RE). test_send_file_tool's expanded-ext test used nonexistent /tmp paths, which under the validated design correctly no longer extract. Fixed the fixture to create a real file in a monkeypatched safe root, exercising the true intended delivery path for every extension. Verified: TestUniversalMediaEgress 13/13, test_send_file_tool 85/85, full affected set 426 passed. The 5 unrelated red tests (SSRF, url_safety, cross_profile_guard) are pre-existing on clean live-config and untouched here. Approach A per Eric: egress design authoritative. * fix(gateway): honor effective model for fast mode
Problem
origin/live-confighad 4 failing tests unrelated to any open feature PR. Confirmed pre-existing by running them on a cleanlive-configworktree with zero edits. They were dragging every open PR's CI red via the sharedPython testsslices.Fixes
1.
/fastsession-scope was half-shipped (3 tests)tests/gateway/test_fast_command.pywas updated to expect/fastto be session-scoped by default with a--globalflag (mirroring/reasoning), using the_session_service_tier_overridesdict and the_resolve_session_service_tier/_set_session_service_tier_overridehelpers — all of which already exist ingateway/run.py. But_handle_fast_commandwas never updated: it still only wroteagent.service_tierto config.yaml globally and rejected--globalas an unknown argument. The tests asserted behavior the handler didn't implement.Rewrote
_handle_fast_commandto match the shipped/reasoningpattern:--global(any position)._resolve_session_service_tier(session_key=...)./fast fast|normalrecords a session override and leaves config.yaml untouched./fast ... --globalwritesagent.service_tierto config.yaml AND clears the session override so the global default wins.No new env vars, no prompt-cache impact.
2.
KANBAN_GUIDANCEsize ceiling too tight (1 test)Upstream
b5bd0ef38("docs(kanban): port attachment guidance into KANBAN_GUIDANCE") added legitimate content, growing the block to 5749 chars against the test's< 5_500cap. The test's own docstring says the ceiling "guards against unbounded growth, not against any growth" and is sized "with a little headroom." Raised the cap to6_500to restore headroom (still a real unbounded-growth guard); lower bound unchanged.Verification
test_fast_command.py+test_kanban_tools.py: 122 passed.ruff checkclean on all changed files.Once merged, this un-reds the shared test slices for the other open PRs (#129, #131, #133).