fix(gateway,tools): gate background-process completions on spawning-session boundary - #85939
Merged
Merged
Conversation
… boundary Plain type=completion events built in _run_process_watcher carried only session_key (chat/thread routing) with no spawning-session stamp, so after /new (or a session switch) a completion notification from the OLD session was injected into the chat's NEW session. Main already solved this exact class for async delegations via the _classify_completion_target pre-flight (_USER_BOUNDARY_END_REASONS drop on user-closed sessions, deliver on idle-ends, follow the compression-tip chain), but the gate only ran for type=async_delegation events. Kernel salvage of #16455: - Stamp the spawning conversation's session-db id (HERMES_SESSION_ID via session-scoped env) on the ProcessSession and the pending_watchers entry at spawn time in tools/terminal_tool.py; persist it through the process registry checkpoint/restore so recovered watchers keep the stamp. - Thread the stamp into the completion_evt built by _run_process_watcher (watcher entry first, ProcessSession fallback for recovered watchers). - In _deliver_completion_notification, run the SAME pre-flight classifier for stamped type=completion events: terminal -> drop with a log (output stays available via process(action='log')), retry -> False so the watcher re-polls, deliver -> proceed. The policy has exactly one owner (_classify_completion_target); nothing is forked. Unstamped legacy events keep today's deliver-always behavior, and the async-delegation path is untouched. Based on the session-boundary approach from #16455 by @Tosko4 (original PR was over-scoped across adapters/slash-commands/cron; this lands the kernel only). Tests: completion from a /new-closed session is dropped; completion after an idle-end still delivers; unstamped legacy event delivers; retry verdict returns retryable False without adapter injection; async_delegation gate unchanged; stamp survives checkpoint recovery.
Contributor
૮ >ﻌ< ა ci reviewran on 0a24aa1 — fix(gateway): gate background-process completions on spawnin
|
alt-glitch
added
type/bug
14 tasks
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
2 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.
Summary
Background-process completions no longer land in the wrong conversation after
/new— the spawning session id is stamped on the watcher at spawn and delivery is gated through the existing session-boundary policy.Main already solved this class for async delegations (
_classify_completion_target+_USER_BOUNDARY_END_REASONS); plaintype=completionevents carried only chat routing and bypassed the gate. This is the kernel salvage of PR #16455 by @Tosko4 (the original threaded boundary locks through 20 files; adapters/slash-commands/cron are deliberately untouched here).Changes
tools/terminal_tool.py+tools/process_registry.py: stampparent_session_idon watcher metadata at spawn; persists through checkpoint recovery.gateway/run.py:_run_process_watcherincludes the stamp incompletion_evt;_deliver_completion_notificationruns the EXISTING_classify_completion_targetfor stamped completion events (terminal → drop with log, retry → re-poll, deliver → proceed). One policy owner — no forked predicate. Unstamped legacy events behave exactly as today.Validation
(The one red in the mirror
test_process_registry.pyrun —test_checkpoint_redacts_command_with_inline_secret— is proven pre-existing on pristine origin/main and unrelated.)Based on the session-boundary approach from #16455 by @Tosko4.
Infographic