fix(process): add supersession context to delayed notifications - #75719
fix(process): add supersession context to delayed notifications#75719cirwel wants to merge 1 commit into
Conversation
|
Thanks for the focused notification-boundary fix. The premise is present on current main: The branch is current: GitHub reports Automated hermes-sweeper review. |
|
Rebased onto current main (cc0af6b) — both touched files had moved upstream since the review. Clean rebase, no conflicts; the three affected test files pass post-rebase (94 tests), ruff clean. |
91db5f6 to
b3b9ed9
Compare
…ation hardening This week's completion-notification hardening series (b9e7bea, c0d2048, 84b4fb9, a96cd10, 7619564, 8dc9401) forced secret redaction, a spawning-session-boundary pre-flight, and title/compression filtering onto completion/async_delegation notifications. watch_match/watch_disabled/ watch_overflow_* events were left on the old, weaker behavior in three places — plus a fourth, related gap found while implementing the above: watch-event notifications weren't attributed to the subagent that owned the watched process. 1. Redaction: _format_gateway_process_notification (gateway/run.py) and the shared format_process_notification (tools/process_registry.py, also used by the TUI gateway) rendered watch_match's output/command from the producer-side, non-forced _redact_process_result pass only. A user with security.redact_secrets: false would get a raw secret sent straight to the chat platform if a watch pattern matched a line containing one. Both formatters now apply the same forced, unconditional redaction floor the completion path already has. 2. Session-boundary gate: _drain_watch_notifications called _inject_watch_notification directly, bypassing _classify_completion_target entirely. A watch_match/watch_disabled event from a process spawned in session A could still land in session B's chat after /new closed A. ProcessSession.parent_session_id (already stamped at spawn time) is now also carried on watch_match/watch_disabled events and checked before injection. Unstamped/global events (the cross-session overflow summaries) keep delivering unconditionally, matching completion's own legacy fallback. Watch events have no watcher to re-poll them later, so a "retry" (transient DB uncertainty) verdict fails open and delivers rather than losing the match outright. 3. Title/compression filters: title_generator._is_real_user_turn and context_compressor._is_synthetic_compression_user_turn didn't recognize any of the "[IMPORTANT: ...]"/"[ASYNC DELEGATION ...]" notification shapes — only the unrelated compaction/continuation/model-switch markers. Both now check message.get("display_kind") == "internal_notification" (the structural marker gateway/run.py stamps at persist time) as the primary signal, plus explicit text-prefix entries for the raw-string call paths that don't have a message dict to check. Async-delegation completions are excluded from the compression side of this: unlike watch/background-process bookkeeping, _format_async_delegation's own docstring says the block carries "the complete result summary" — genuine actionable content a real user turn would also carry — so treating it as synthetic would let compaction blank out a delegation's actual result (see test_completion_survives_compaction_verbatim_after_blank_echo, bc48241). They're still excluded from titling, where the boilerplate wrapper text would make a bad title regardless of the payload. 4. Subagent attribution: completion/async_delegation events resolve their task_id (via tools/delegate_tool.py's _active_subagents registry) into a "Started by subagent ... of delegation ... Task: ..." provenance line. watch_match already carried this; watch_disabled never did, in either formatter. Fixed by stamping task_id on the watch_disabled event dict and adding the same attribution lookup to both formatters — gateway/run.py's kept additive (not delegated to the shared formatter) to avoid silently swapping its _redact_gateway_user_facing_secrets guarantee for redact_terminal_output. Mutation-verified throughout, including the async-delegation exclusion against both the original wrong code and each half of the fix independently — all reproduce test_completion_survives_compaction_verbatim_after_blank_echo failing. Full neighbor sweep green (tests/gateway/, tests/agent/ compress/compaction suite, tests/tools/test_watch_patterns.py, tests/tools/test_process_registry.py, tests/tools/test_async_delegation.py). ruff clean. Adjacent open PRs checked, no semantic overlap: - NousResearch#75719 restructures the same gateway/run.py formatter for an unrelated concern (a "supersession context" note on delayed notifications) — textual proximity only. - NousResearch#61719 adds a different field (origin_ui_session_id) to the same watch_match/watch_disabled dict literals, for TUI/WebUI tab ownership — complementary, not overlapping. - NousResearch#73469 is an alternative architecture for the same-tick completion-coalescing race this week's series already solved differently; different function region.
Summary
Each notification now tells the agent to reconcile the result with the current conversation and that newer user instructions take precedence. Delivery is preserved; this does not silently suppress useful background results.
Why
Older background completions can arrive after the user has cancelled, redirected, or completed the underlying task. The existing notification text carries output but no authority boundary, so a delayed synthetic turn can resurrect superseded work.
This is complementary to:
Related behavior: #36184. This PR adds the shared reasoning boundary but does not claim to replace deterministic cancellation suppression.
Verification
HERMES_HOME=<temp> uv run --extra dev pytest ...: 110 passed, 1 existing Pythonforkpty()deprecation warninguv run --extra dev ruff check ...: passeduv run python -m compileall -q ...: passedgit diff --check: passed