fix(goal): resume blocked goal at before_agent_start to close stale-flag race - #2
Merged
Conversation
…lag race The host emits before_agent_start BEFORE the final provider admission check, which can reject the run so no agent_start follows. The previous design set a sticky nextAgentStartWasUserTriggered flag in before_agent_start and consumed it in agent_start, so a rejected run left the flag set. The next run that bypasses before_agent_start (e.g. a goal-continuation / queued custom-message turn that starts the agent directly via agent.prompt) consumed the stale flag and wrongly resumed a blocked goal. Move the blocked->active resume into before_agent_start itself (it fires only for real user prompts) and remove the flag entirely. agent_start becomes pure accounting init: it reads the goal, sees status active (if just resumed), and begins accounting for the resumed turn. A continuation-style agent_start now has no resume path of its own and cannot trigger a stale resume. A rejected user prompt now resumes the goal at before_agent_start (the user's expressed intent to resume); no accounting begins until agent_start, preserving the invariant that the first resumed turn is accounted. This is the cleaner of the two options the fix considered; option (b)'s generation-token binding is not implementable against this host's available extension events (BeforeAgentStartEvent carries no generation token, and no extension event fires between a rejected before_agent_start and a later continuation agent_start). Tests: - regression: a before_agent_start with no following agent_start must resume the blocked goal to active (fails on the original flag-based code, which left it blocked waiting for agent_start). - a continuation-style agent_start with no preceding before_agent_start must not resume a blocked goal on its own. - existing happy path (before_agent_start -> agent_start -> resume + first resumed turn accounted) still passes.
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.
Defect
src/goal/lifecycle.tssetnextAgentStartWasUserTriggered = truein thebefore_agent_starthandler and consumed it only inagent_start. In the host (agent-session.ts),before_agent_startis emitted before the final provider admission check (_enforceFinalProviderAdmission), which can reject the run — so noagent_startfollows, the flag survives, and the next run that bypassesbefore_agent_start(e.g. a goal-continuation / queued custom-message turn that starts the agent directly viaagent.prompt) consumes the stale flag and wrongly resumes a blocked goal.Fix (option a)
Move the
blocked -> activeresume into thebefore_agent_starthandler itself (it fires only for real user prompts) and remove the flag entirely.agent_startbecomes pure accounting init: it reads the goal, seesstatus: active(if just resumed), and begins accounting for the resumed turn. A continuation-styleagent_startnow has no resume path of its own and cannot trigger a stale resume.A rejected user prompt now resumes the goal at
before_agent_start(the user's expressed intent to resume); no accounting begins untilagent_start, preserving the invariant that the first resumed turn is accounted.This is the cleaner of the two options: option (b)'s generation-token binding is not implementable against this host's available extension events —
BeforeAgentStartEventcarries no generation token, and no extension event fires between a rejectedbefore_agent_startand a later continuationagent_start, so a flag set inbefore_agent_startcannot be deterministically invalidated before the continuation consumes it. Option (a) eliminates the race mechanism entirely.Tests (TDD)
resumes a blocked goal at before_agent_start even when admission rejects the run): abefore_agent_startwith no followingagent_startmust resume the blocked goal toactive. Fails on the original flag-based code (which left itblockedwaiting foragent_start), passes with this fix.does not resume a blocked goal on a continuation-style agent_start with no preceding before_agent_start: a continuation-styleagent_startmust not resume a blocked goal on its own.before_agent_start -> agent_start -> resume + first resumed turn accounted) still passes.Evidence captured locally under
.omo/evidence/pi-todo-goal-upgrade/(task-f2a-red.txt,task-f2a-green.txt).Gates
npx vitest run— 87 passed (9 files)npm run check(tsgo + biome + no-excuse) — green (exit 0)Summary by cubic
Fixes a race where a stale user-trigger flag could wrongly resume a blocked goal on continuation-only turns by moving the resume logic to
before_agent_startand removing the flag.agent_startis now init-only, so rejected user runs no longer leak a resume into later turns.blocked -> activeinbefore_agent_start; removednextAgentStartWasUserTriggered.agent_startonly starts accounting when the goal is alreadyactive.agent_startno longer resumes.Written for commit 66224ec. Summary will update on new commits.