fix(guard): switching to Build no longer hijacks the session back to Goal#429
Merged
Merged
Conversation
…Goal (#428) When a user started /goal, interrupted, switched to Build, and prompted it, the guard would — after Build's response finished — automatically switch the main agent back to and start running review subagents. Root cause: the idle handler's "is this still a goal session?" check OR'd in two terms that ignored the explicit deactivation set by chat.params/chat.message: - Boolean(state.contract) — a stale contract re-activated the session; and - goalSessionActiveForAgent("goal", state) — which is TAUTOLOGICAL: it returns true unconditionally for the primary "goal" agent, so once a session had ever been a goal it was treated as one forever. The reviewer batch (pinned to PRIMARY_AGENT in launchReviewBatch) then switched the user's main agent back to Goal — the reported "automatically switching to Build" symptom. Fix: - resolveIdleSession now requires state.active for a session to be eligible for programmatic review / auto-continue. A stale contract or other persisted goal state can no longer re-activate a session the user explicitly paused. Switching back to goal (or running /goal) re-activates it and reviews resume with all state intact. - launchReviewBatch now refuses to run on a state.active===false session (defense-in-depth so a future regression in the eligibility check can never switch the user's main agent back to Goal). Adds 3 regression tests (the exact reported flow + a re-activation parity test + the launchReviewBatch guard). Updates the README troubleshooting note and adds a CHANGELOG entry. All 676 tests pass (673 baseline + 3 new); no new lint diagnostics.
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
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.
Closes #428
Root cause
When a user switches a goal session off the
goalagent (to Build/Plan, or via an action that cycles the agent),chat.params/chat.messagefaithfully setstate.active = false. But the idle handler's "is this still a Goal session?" check ignored that deactivation:Boolean(state.contract)— the contract persists from the prior goal, so a stale contract re-activated the session.goalSessionActiveForAgent("goal", state)is tautological —goalSessionActiveForAgentreturnstrueunconditionally when its first argument is the primary"goal"agent (seeagents.js), so once a session had ever been a goal, the idle path treated it as a goal forever, no matter how many times the user switched away.The result: on the Build turn's idle,
canProgrammaticReviewevaluated totrue, the guard launched a programmatic review cycle, andlaunchReviewBatch— which pins the batch toPRIMARY_AGENT— switched the user's main agent back togoal, exactly the symptom in the bug report.The fix
Two changes, both small and surgical:
1. Honor explicit deactivation in the idle path (root cause)
plugins/goal-guard/guard.js—resolveIdleSession()now requiresstate.activefor a session to be eligible for programmatic review or auto-continue:A stale contract (or any other persisted goal state) can no longer re-activate a session the user explicitly paused. Switching back to the
goalagent (or running/goal) re-activates it as before, and reviews then resume with all of the contract, evidence, and review-cycle state intact.2. Defense-in-depth in
launchReviewBatchplugins/goal-guard/review-runner.js—launchReviewBatch()now refuses to run on astate.active === falsesession, surfacing as aSessionBusyErrorso the outer retry loop defers cleanly. This guarantees that even a future regression in the idle-path eligibility check can never reach the call that pins the batch toPRIMARY_AGENTand switches the user's main agent back togoal.stateis the live store record, so a fresh agent switch is reflected here even if it happened after the caller's eligibility check.Why this is the smallest correct change
goalSessionexpression were either always-true (the tautology) or ignored deactivation (the contract). Removing them is strictly more correct —state.activewas already the source of truth everywhere else (auto-continue, thestate.dirty && state.activeandstate.active && !completionAllowedlog branches, sidebar scoping, etc.).goal-implementer,goal-explorer, …) is preserved: those agents keepstate.active = trueviagoalSessionActiveForAgentinchat.params/chat.message, so programmatic reviews still run after a worker handoff — covered by the existingprogrammatic review runs when a goal worker subagent was the last agent on the sessiontest.goalis unchanged and explicitly tested.Regression coverage
Three new tests, all named
REGRESSION [issue #428]:a goal session switched to Build is NOT re-activated on idle (no reviews, no agent switch)tests/plugin.test.mjs/goal→ switch to Build → prompt Build → idle → 0 reviewer subtasks launched, noagent: "goal"prompt sent, goal state preserved.companion: switching back to Goal re-activates and programmatic review resumestests/plugin.test.mjsa deactivated session never launches a reviewer batch (defense-in-depth in launchReviewBatch)tests/programmatic-review.test.mjsVerification
npm test— 676/676 pass (673 baseline + 3 new). Includes the existingprogrammatic review runs when a goal worker subagent was the last agent on the sessionparity test, which still passes.npm run lint— no new diagnostics (64 warnings / 60 infos are pre-existing onmain, verified viagit stash).node scripts/validate-opencode-config.mjs— OpenCode Goal Mode package validation passed.Files changed
Docs
goalon its own.## Unreleased→### Fix: switching to Build no longer hijacks the session back to Goal (issue #428)entry following the established format (root cause + fix narrative for release readers).Closes #428.