fix(chat): suppress reserved SILENT control turns at ingress - #7018
nesquena-hermes merged 2 commits into
Conversation
Cron agents use the exact final response `[SILENT]` as a delivery-suppression
sentinel. If a wake relay accidentally POSTs that sentinel to `/api/chat/start`
and 8701 restarts while the turn is pending, session repair materializes it as
a visible `{role: user, _recovered: true}` message. The pending value can then
be recovered again on later restarts, creating repeated `[SILENT]` turns.
Treat the exact normalized sentinel as a successful no-op at both server-side
turn entry points: the HTTP `/api/chat/start` handler and
`start_session_turn`. Both checks run before session lookup, runtime barriers,
or pending-state mutation. Matching is deliberately exact and case-sensitive,
so `[silent]`, prose containing `[SILENT]`, and ordinary user text are not
suppressed.
Add regression tests proving both paths return HTTP/status 200 without session
lookup, plus negative cases for non-exact text. The new tests fail 3/3 before
the fix. Targeted and neighbouring chat-start suites pass 21/21.
|
| Filename | Overview |
|---|---|
| api/routes.py | Adds exact, case-sensitive sentinel recognition and early successful suppression at both targeted turn-entry functions. |
| tests/test_silent_control_suppression.py | Verifies both suppression paths, whitespace normalization, response shapes, and representative negative cases. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Incoming chat-start message] --> B{Exact normalized SILENT sentinel?}
B -->|Yes| C[Return 200 suppressed]
C --> D[No session lookup or mutation]
B -->|No| E[Continue normal turn admission]
Reviews (2): Last reviewed commit: "Merge branch 'master' into fix/suppress-..." | Re-trigger Greptile
nesquena-hermes
left a comment
There was a problem hiding this comment.
Gate-clean. Suppresses the reserved [SILENT] cron delivery-suppression sentinel at both chat-ingress entry points (start_session_turn + _handle_chat_start) before session lookup/pending-state mutation, so an errant wake relay POSTing [SILENT] can't materialize a phantom recovered user turn on restart. Codex SAFE TO SHIP (verified: both guards pre-mutation, exact whitespace-normalized case-sensitive match so normal messages unaffected, both response conventions terminate without double-write, cron.scheduler.SILENT_MARKER='[SILENT]' confirms the literal). Local full suite 14540/0 + 9 focused ingress tests. Non-blocking follow-up: /api/goal (routes.py:22640) has the same pre-existing exposure — will file a follow-up for class-wide suppression (out of scope for this relay-path fix). Shipping to experimental.
|
Shipped in exp-v0.52.222. Thanks @allenliang2022 — clean, well-scoped fix. Verified via the full Codex gate (both guards suppress before session lookup / pending-state mutation, matching is exact + whitespace-normalized + case-sensitive so ordinary messages are unaffected, both response conventions terminate without double-write, and cron.scheduler.SILENT_MARKER='[SILENT]' confirms the literal) and the full test suite. Filed #7019 as a follow-up to extend the same guard to the /api/goal ingress path (a pre-existing sibling exposure the gate surfaced — out of scope for this relay-path fix). |
* fix(goal): suppress reserved SILENT sentinel at /api/goal ingress A wake relay POSTing the exact [SILENT] suppression sentinel to /api/goal lets it reach _start_chat_stream_for_session, which persists it as pending_user_message. If 8701 restarts while the turn is pending, session recovery materializes it as a visible _recovered user turn — the same phantom-recovered-turn exposure #7018 closed for chat ingress. Apply the shared _is_silent_control_message() guard immediately after /api/goal session-ID validation and before session lookup or goal-state mutation, returning the same 200 no-op. Matching stays exact and case-sensitive; ordinary kickoff text is unaffected. Add tests mirroring test_silent_control_suppression.py for the goal path (args and text fields, before-lookup suppression, exact-match semantics). Closes #7019 * fix(sessions): keep evicted subagent parents in the import window The sidebar nests a subagent row under its parent only when the parent row is present in the same payload. The visible-window limit was applied as a flat per-row recency slice, so a frozen orchestrator (which stops writing while its leaves keep streaming) lost the recency race against its own leaves and fell outside the window -- promoting those leaves to top-level sidebar rows. Re-add subagent parents that the oversampled candidate set already projected, after the slice. No extra queries, no change to CLI_VISIBLE_SESSION_LIMIT. webui ancestors are deliberately not imported because that sidebar bucket already owns them. Supersedes #7031. * fix(sessions): document and pin the parent-recovery bound (greptile review) Greptile flagged (P1) that a selected subagent child whose parent ranks below the limit * 8 oversample is still promoted to a top-level row. That is real and measured (the parent drops out at candidate #25 of 24), but it is the bound of the design, not a regression: the walk reuses rows the projection already fetched and never issues an extra query. Resolving arbitrarily old ancestors needs an unbounded per-row lookup on the hot sidebar path -- the approach rejected in #7031 -- so the bound is documented and pinned instead. - Docstring: state that `limit` bounds the recency slice, not the row count, so callers must iterate rather than assume len(rows) <= limit; state that recovery is bounded by the oversampled candidate set. - Inline comment: mark the bound at the exact lines Greptile flagged and point at `candidate_limit` as the knob if the window proves too tight. - Tests: cover the candidate-window exhaustion Greptile said was untested -- parent inside the oversample is recovered, parent beyond it stays unresolved -- plus the over-limit return contract and a parent-cycle guard. No behaviour change; 7 tests pass. * fix(share): add _hadAppearance guard to prevent fabricated appearance choice Summary: The inline appearance bootstrap in static/share.html writes hermes-theme and hermes-skin to localStorage unconditionally on every page load, even when the browser had no prior appearance state. This fabricates an explicit user choice on first access via a shared link, making the server-side SETTINGS_DEFAULTS unreachable for deployments that customise the default theme or skin. Root Cause: share.html:9 — the boot IIFE resolves a theme+skin and calls localStorage.setItem() without guarding on whether the user had previously chosen an appearance. The same bug was fixed in index.html by PR #6808 (commit tomtong2015) but share.html was left unchanged. Change: 1. Added _hadAppearance guard before the two localStorage.setItem() calls: var _hadAppearance = localStorage.getItem('hermes-theme') !== null || localStorage.getItem('hermes-skin') !== null; if (_hadAppearance) { setItem('hermes-theme', t); setItem('hermes-skin', s); } 2. The first-paint DOM mutations (classList.add('dark'), dataset.skin) remain outside the guard — only persistence is protected. 3. Synced the skin allowlist with index.html: added neon-soft and neon-paint (zeus and verdigris were already present). Verification: - test_6808_appearance_bootstrap_no_fabricated_choice.py: 11/11 passed covering fresh-browser (no writes), pre-paint fallback, explicit state normalisation, and legacy migration survival. Closes #7030 * fix(tests): pin rebuild budget in issue2513 custom-provider catalog test test (3.13, 4) failed on this PR with: assert "@Custom:alpha-proxy:alpha/remote" in alpha_ids E AssertionError: assert '@Custom:alpha-proxy:alpha/remote' in {'alpha/sticky'} WARNING api.config:config.py:8355 live provider-catalog rebuild exceeded 4.0s budget - serving fallback, refreshing catalog out-of-band Pre-existing wall-clock flake, not a regression from this PR: this branch touches only api/agent_sessions.py and tests/test_subagent_parent_in_import_ window.py, and both api/config.py and this test file are byte-identical to origin/master. The same shard passed on 3.11 and 3.12. The test never pinned _LIVE_REBUILD_BUDGET_SECONDS, so it raced the global 4s budget in get_available_models(). On a starved runner the cold rebuild overruns, the degraded fallback catalog is served, and the monkeypatched- urlopen model alpha/remote is dropped - leaving only the config-declared sticky model, exactly as CI observed. Force the synchronous (unbounded) rebuild path, matching the existing precedent in tests/test_issue2540_models_endpoint_error.py:20-24. Verified: with the budget forced to 0.001s the unpatched test reproduces the CI assertion verbatim; patched it passes at 0.001s, 0, and default. * fix(#7013): preserve media deny coverage across platforms * fix(docker): keep repository agents out of runtime context * fix(docker): gate image runtime proof behind integration job * test(models): stabilize custom provider catalog regression * Release batch A: 6 low-risk gate-passed fixes (experimental) Batched contributor fixes, each individually Codex-gated during the overnight certifier cycles and re-verified clean-to-ship as a combined stage (Codex SAFE TO SHIP on the combined diff; full suite green except 8 pre-existing approval tests that fail identically on clean origin/master — CI green on same commit). - #7019 (@webtecnica) suppress reserved [SILENT] sentinel at /api/goal - #7031 (@carlotestor) keep evicted subagent parents in the sidebar import window - #7030 (@webtecnica) share.html first-visit appearance guard + skin-id sync - #6853 (@rodboev) exclude repo-root AGENTS.md from the Docker runtime image - #7013 (@webtecnica) test-only: platform-neutral test portability (playwright import guard + media tests served from allowed roots) - test-only (@carlotestor) pin custom-provider catalog rebuild budget (#7054) Co-authored-by: webtecnica <webtecnica@users.noreply.github.com> Co-authored-by: carlotestor <carlotestor@users.noreply.github.com> Co-authored-by: rodboev <rodboev@users.noreply.github.com> --------- Co-authored-by: webtecnica <webtecnica@gmail.com> Co-authored-by: carlotestor <carlotestor@users.noreply.github.com> Co-authored-by: Rod Boev <rod.boev@gmail.com> Co-authored-by: n <a@n> Co-authored-by: webtecnica <webtecnica@users.noreply.github.com> Co-authored-by: rodboev <rodboev@users.noreply.github.com>
…a#7018) Cron agents use the exact final response `[SILENT]` as a delivery-suppression sentinel. If a wake relay accidentally POSTs that sentinel to `/api/chat/start` and 8701 restarts while the turn is pending, session repair materializes it as a visible `{role: user, _recovered: true}` message. The pending value can then be recovered again on later restarts, creating repeated `[SILENT]` turns. Treat the exact normalized sentinel as a successful no-op at both server-side turn entry points: the HTTP `/api/chat/start` handler and `start_session_turn`. Both checks run before session lookup, runtime barriers, or pending-state mutation. Matching is deliberately exact and case-sensitive, so `[silent]`, prose containing `[SILENT]`, and ordinary user text are not suppressed. Add regression tests proving both paths return HTTP/status 200 without session lookup, plus negative cases for non-exact text. The new tests fail 3/3 before the fix. Targeted and neighbouring chat-start suites pass 21/21. Co-authored-by: allenliang2022 <allenliang2022@users.noreply.github.com> Co-authored-by: nesquena-hermes <nesquena+hermes@gmail.com>
…esquena#7018) (nesquena#7020) Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>
…a#7089) * fix(goal): suppress reserved SILENT sentinel at /api/goal ingress A wake relay POSTing the exact [SILENT] suppression sentinel to /api/goal lets it reach _start_chat_stream_for_session, which persists it as pending_user_message. If 8701 restarts while the turn is pending, session recovery materializes it as a visible _recovered user turn — the same phantom-recovered-turn exposure nesquena#7018 closed for chat ingress. Apply the shared _is_silent_control_message() guard immediately after /api/goal session-ID validation and before session lookup or goal-state mutation, returning the same 200 no-op. Matching stays exact and case-sensitive; ordinary kickoff text is unaffected. Add tests mirroring test_silent_control_suppression.py for the goal path (args and text fields, before-lookup suppression, exact-match semantics). Closes nesquena#7019 * fix(sessions): keep evicted subagent parents in the import window The sidebar nests a subagent row under its parent only when the parent row is present in the same payload. The visible-window limit was applied as a flat per-row recency slice, so a frozen orchestrator (which stops writing while its leaves keep streaming) lost the recency race against its own leaves and fell outside the window -- promoting those leaves to top-level sidebar rows. Re-add subagent parents that the oversampled candidate set already projected, after the slice. No extra queries, no change to CLI_VISIBLE_SESSION_LIMIT. webui ancestors are deliberately not imported because that sidebar bucket already owns them. Supersedes nesquena#7031. * fix(sessions): document and pin the parent-recovery bound (greptile review) Greptile flagged (P1) that a selected subagent child whose parent ranks below the limit * 8 oversample is still promoted to a top-level row. That is real and measured (the parent drops out at candidate nesquena#25 of 24), but it is the bound of the design, not a regression: the walk reuses rows the projection already fetched and never issues an extra query. Resolving arbitrarily old ancestors needs an unbounded per-row lookup on the hot sidebar path -- the approach rejected in nesquena#7031 -- so the bound is documented and pinned instead. - Docstring: state that `limit` bounds the recency slice, not the row count, so callers must iterate rather than assume len(rows) <= limit; state that recovery is bounded by the oversampled candidate set. - Inline comment: mark the bound at the exact lines Greptile flagged and point at `candidate_limit` as the knob if the window proves too tight. - Tests: cover the candidate-window exhaustion Greptile said was untested -- parent inside the oversample is recovered, parent beyond it stays unresolved -- plus the over-limit return contract and a parent-cycle guard. No behaviour change; 7 tests pass. * fix(share): add _hadAppearance guard to prevent fabricated appearance choice Summary: The inline appearance bootstrap in static/share.html writes hermes-theme and hermes-skin to localStorage unconditionally on every page load, even when the browser had no prior appearance state. This fabricates an explicit user choice on first access via a shared link, making the server-side SETTINGS_DEFAULTS unreachable for deployments that customise the default theme or skin. Root Cause: share.html:9 — the boot IIFE resolves a theme+skin and calls localStorage.setItem() without guarding on whether the user had previously chosen an appearance. The same bug was fixed in index.html by PR nesquena#6808 (commit tomtong2015) but share.html was left unchanged. Change: 1. Added _hadAppearance guard before the two localStorage.setItem() calls: var _hadAppearance = localStorage.getItem('hermes-theme') !== null || localStorage.getItem('hermes-skin') !== null; if (_hadAppearance) { setItem('hermes-theme', t); setItem('hermes-skin', s); } 2. The first-paint DOM mutations (classList.add('dark'), dataset.skin) remain outside the guard — only persistence is protected. 3. Synced the skin allowlist with index.html: added neon-soft and neon-paint (zeus and verdigris were already present). Verification: - test_6808_appearance_bootstrap_no_fabricated_choice.py: 11/11 passed covering fresh-browser (no writes), pre-paint fallback, explicit state normalisation, and legacy migration survival. Closes nesquena#7030 * fix(tests): pin rebuild budget in issue2513 custom-provider catalog test test (3.13, 4) failed on this PR with: assert "@Custom:alpha-proxy:alpha/remote" in alpha_ids E AssertionError: assert '@Custom:alpha-proxy:alpha/remote' in {'alpha/sticky'} WARNING api.config:config.py:8355 live provider-catalog rebuild exceeded 4.0s budget - serving fallback, refreshing catalog out-of-band Pre-existing wall-clock flake, not a regression from this PR: this branch touches only api/agent_sessions.py and tests/test_subagent_parent_in_import_ window.py, and both api/config.py and this test file are byte-identical to origin/master. The same shard passed on 3.11 and 3.12. The test never pinned _LIVE_REBUILD_BUDGET_SECONDS, so it raced the global 4s budget in get_available_models(). On a starved runner the cold rebuild overruns, the degraded fallback catalog is served, and the monkeypatched- urlopen model alpha/remote is dropped - leaving only the config-declared sticky model, exactly as CI observed. Force the synchronous (unbounded) rebuild path, matching the existing precedent in tests/test_issue2540_models_endpoint_error.py:20-24. Verified: with the budget forced to 0.001s the unpatched test reproduces the CI assertion verbatim; patched it passes at 0.001s, 0, and default. * fix(nesquena#7013): preserve media deny coverage across platforms * fix(docker): keep repository agents out of runtime context * fix(docker): gate image runtime proof behind integration job * test(models): stabilize custom provider catalog regression * Release batch A: 6 low-risk gate-passed fixes (experimental) Batched contributor fixes, each individually Codex-gated during the overnight certifier cycles and re-verified clean-to-ship as a combined stage (Codex SAFE TO SHIP on the combined diff; full suite green except 8 pre-existing approval tests that fail identically on clean origin/master — CI green on same commit). - nesquena#7019 (@webtecnica) suppress reserved [SILENT] sentinel at /api/goal - nesquena#7031 (@carlotestor) keep evicted subagent parents in the sidebar import window - nesquena#7030 (@webtecnica) share.html first-visit appearance guard + skin-id sync - nesquena#6853 (@rodboev) exclude repo-root AGENTS.md from the Docker runtime image - nesquena#7013 (@webtecnica) test-only: platform-neutral test portability (playwright import guard + media tests served from allowed roots) - test-only (@carlotestor) pin custom-provider catalog rebuild budget (nesquena#7054) Co-authored-by: webtecnica <webtecnica@users.noreply.github.com> Co-authored-by: carlotestor <carlotestor@users.noreply.github.com> Co-authored-by: rodboev <rodboev@users.noreply.github.com> --------- Co-authored-by: webtecnica <webtecnica@gmail.com> Co-authored-by: carlotestor <carlotestor@users.noreply.github.com> Co-authored-by: Rod Boev <rod.boev@gmail.com> Co-authored-by: n <a@n> Co-authored-by: webtecnica <webtecnica@users.noreply.github.com> Co-authored-by: rodboev <rodboev@users.noreply.github.com>
Problem
Cron agents use the exact final response
[SILENT]as a delivery-suppression sentinel. The scheduler correctly recognizes it and logsagent returned [SILENT] — skipping delivery.However, if an external wake relay accidentally POSTs that sentinel to
/api/chat/start, WebUI currently treats it as ordinary conversation content. If 8701 restarts while that turn is pending, session recovery materializes the pending value as:{"role": "user", "content": "[SILENT]", "_recovered": true}That creates a visible user turn. Because the value also lives in
pending_user_message, later restarts can recover it again, producing repeated[SILENT]user/assistant pairs even though the cron scheduler itself suppressed delivery correctly.I confirmed the provenance in a real affected sidecar: the leaked rows carry
_recovered: true, while the sidecar retainspending_user_message: "[SILENT]". The cron log for the same job saysskipping delivery, ruling out the scheduler's normal delivery path.Fix
Treat the exact normalized sentinel as a successful no-op at both server-side turn entry points:
/api/chat/start(_handle_chat_start)start_session_turnBoth checks run before session lookup, runtime barriers, or any pending/session mutation. They return status 200 with:
{"status": "suppressed", "reason": "silent_control_message"}Matching is deliberately exact and case-sensitive. Only
[SILENT](allowing surrounding whitespace) is suppressed;[silent], prose containing[SILENT], and ordinary user messages remain untouched.Verification
Added
tests/test_silent_control_suppression.pycovering:None.The new suite fails 3/3 before the fix. After the fix, the new tests plus neighboring chat-start/runtime-adapter and compression-recovery suites pass:
I also rebased the branch onto current
origin/masterafter initially discovering that my local worktree was based on an unrelated rolling branch; the final PR diff contains onlyapi/routes.pyand the new regression test.Scope
This prevents future sentinel turns at ingress. It intentionally does not rewrite already-persisted transcript history; existing recovered rows remain historical evidence rather than being destructively removed during an upgrade.