fix(gateway): consolidate live-config CI baseline repair (delete stale shadow handlers) - #139
Conversation
…NCE 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
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.
|
Updated: the 3rd failure (TestUniversalMediaEgress x3) is now fixed in this PR per Eric's Approach A — egress design authoritative. Root cause: the egress redesign (NousResearch#36060) intends code/config/log extensions to deliver only via the validated pass (validate_media_delivery_path), but the code still folded MEDIA_TAG_EXTRA_EXTS into the UNCONDITIONAL extract grammar, contradicting its own design comment. Fixed by pointing _MEDIA_TAG_EXT_ALTERNATION at MEDIA_DELIVERY_EXTS only. The send_file expanded-ext test used nonexistent /tmp paths (which correctly no longer extract under the validated design); updated its fixture to create a real file in a safe root. All 15 originally-failing baseline tests now pass. 426 passed across the affected modules; send_file 85/85; egress 13/13; ruff clean. Note: 5 OTHER tests (SSRF, url_safety, cross_profile_guard) are red on clean live-config too — a separate pre-existing baseline breakage outside this PR's scope, untouched here. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/exiao/hermes-agent/blob/260ac7de9143095b6c6eb87fe2081bf52eae8074/gateway/run.py#L15524
After this deletion activates the mixin handler, /fast gates on _resolve_gateway_model(user_config), which is only the global model. Actual gateway turns resolve a session /model override first in _resolve_session_agent_runtime, so a session using a supported override while the default is unsupported is incorrectly told /fast is unavailable; the inverse accepts and saves a fast override that the turn later silently drops. Resolve the effective model for session_key before applying this gate.
ℹ️ 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".
|
Addressed Codex P2 in 6cd8f8a: |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6cd8f8ad42
ℹ️ 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".
|
Addressed the latest Codex findings in a5a7909: cache eviction again releases dead-loop client transports, while validated code/config/log MEDIA tags inside protected spans now pass the safe validated delivery path. Focused regression suites: 235 passed. |
1 participant
Summary
Consolidates the live-config CI baseline repair. Supersedes #136, #137, #138 — three overlapping partial repairs spawned by three different babysitters, each unblocking its own PR without checking a repair was already in flight. All three edit
gateway/run.py, all three are red, and #136/#138 delete the same handlers (merging two would conflict).origin/live-confighad 15 failing tests across 4 modules, inherited red by every open PR through the shared test slices. This PR fixes 12 of 15 via the correct root cause. The remaining 3 are a genuine product-decision contradiction (below) left for a human call.Root cause + fix
1. Stale shadow handlers (11 tests:
test_fast_command×3,test_choice_picker×8)The June slash-command extraction moved
_handle_reasoning_command/_handle_fast_commandintoGatewaySlashCommandsMixin(gateway/slash_commands.py) with the new interactive choice-picker logic. But the OLD copies were left behind ingateway/run.pyinside theGatewayRunnerclass body.GatewayRunner(... GatewaySlashCommandsMixin)inherits the mixin, but a method defined directly on the subclass overrides an inherited one — so the stale run.py copies shadowed the mixin versions, making the choice-picker unreachable and/fast --globalunrecognized.Fix: delete the two stale defs from
gateway/run.py(166 lines). The picker-enabled mixin versions activate. (#137's approach — re-implementing session logic INTO the stale shadow — was wrong; the shadow must be deleted.)2. KANBAN_GUIDANCE ceiling (1 test)
Upstream
b5bd0ef38added legitimate attachment guidance (5749 chars vs the<5500cap). The test's docstring says the ceiling guards unbounded growth "with a little headroom." Raised to6500.Verification
test_fast_command,test_choice_picker,test_kanban_tools,test_send_file_tool.test_send_file_tool's 57 expanded-extension tests still pass — fix: restore live-config CI contracts #138 broke these by removingMEDIA_TAG_EXTRA_EXTSfrom the alternation; this PR does not touch that path.ruff checkclean.Deferred — needs a human decision (NOT in this PR)
3 tests in
test_platform_base.py::TestUniversalMediaEgressremain red on base. They contradict the passing send_file tests:MEDIA:/tmp/test_file.py(nonexistent) must extract.MEDIA:/nonexistent/script.py(nonexistent) must NOT extract.Both are nonexistent unknown-extension
.pypaths with no distinguishing property, so no single behavior satisfies both. The newer egress commit (2fd36b17c, Jul 16) shipped a "validated pass" design but never updated the older send_file fixture; both landed via the 886-commit upstream merge. Resolving means picking the egress design as authoritative AND fixing the send_file test to create its files — a security-path + test-override decision.