fix(revived-run-tracker): gate background task notifications behind shouldManageSession - #1086
fix(revived-run-tracker): gate background task notifications behind shouldManageSession#1086mhenke wants to merge 1 commit into
Conversation
…houldManageSession - Adds shouldManageSession option to createRevivedRunTracker - Guards notifyParent with shouldManageSession check - Wires from src/index.ts using sessionMetadata.getAgent - Adds test: skips parent notification when shouldManageSession returns false Fixes alvinunreal#1079
Greptile SummaryThis PR gates revived-run terminal prompts on the parent session's recorded orchestrator mode to avoid restoring orchestration after a deliberate mode switch.
Confidence Score: 4/5The PR should not merge until the gate distinguishes a genuine mode switch from missing session metadata, which currently drops valid revived-task results. An idle orchestrator's bounded metadata entry can be evicted while its task runs, after which the new strict gate permanently suppresses the task's terminal notification; suppressed runs are also retained in tracker state. Files Needing Attention: src/hooks/task-session-manager/revived-run-tracker.ts, src/index.ts Important Files Changed
Reviews (1): Last reviewed commit: "fix(revived-run-tracker): gate backgroun..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if ( | ||
| options.shouldManageSession && | ||
| !options.shouldManageSession(run.parentSessionID) | ||
| ) | ||
| return; |
There was a problem hiding this comment.
When an idle orchestrator parent is evicted from the bounded session-metadata store while its task runs, shouldManageSession returns false and this new gate permanently drops the task's terminal completion or error notification. Missing metadata is recoverable rather than an authoritative mode switch, as sibling orchestrator gates restore stale mappings through registerSessionAsOrchestrator.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if ( | ||
| options.shouldManageSession && | ||
| !options.shouldManageSession(run.parentSessionID) | ||
| ) | ||
| return; |
There was a problem hiding this comment.
When this gate suppresses a terminal notification, it returns without discarding the completed run, retaining its RevivedRun entry until replacement or plugin disposal and causing unnecessary lifetime memory growth.
| if ( | |
| options.shouldManageSession && | |
| !options.shouldManageSession(run.parentSessionID) | |
| ) | |
| return; | |
| if ( | |
| options.shouldManageSession && | |
| !options.shouldManageSession(run.parentSessionID) | |
| ) { | |
| discardRun(run); | |
| return; | |
| } |
Fixes #1079
What problem are you solving?
Background task completion notifications inject
promptAsync({ agent: 'orchestrator' })regardless of the parent session's current mode. When a user switches to Plan or Build, the notification still fires and itsagent: 'orchestrator'flows throughchat.message→sessionMetadata.setAgent()→ flips the session metadata back to orchestrator, re-arming all orchestrator machinery (wake scheduler, tool gates, etc.).What does this change?
Adds
shouldManageSessiongate tocreateRevivedRunTracker. Guard innotifyParentchecks whether parent session is still in orchestrator mode before injecting the synthetic prompt. 3 lines in source + 1 test case.Why this approach?
shouldManageSessionis already the canonical predicate for orchestrator-mode gating, used 6+ times across the codebase (wake scheduler, task-session-manager, input-wait-tracker, cancel-task, task-revive, wait-for-user). DirectsessionMetadataaccess inside the tracker would couple it to a module it does not import. The optional callback preserves backward compatibility — callers that do not pass it get the old behavior.Related work
AI assistance
oh-my-opencode-slim.jsoncskeptic preset).Checklist
bun run check:ci,bun run typecheck, andbun testpassmasterbranch