Skip to content

🤖 fix: bind goal edits and complete_goal to the intended goal; heartbeat skips transient goal snapshots - #5636

Merged
ThomasK33 merged 2 commits into
mainfrom
fix/goals-5461-suspected
Oct 4, 2026
Merged

ThomasK33 merged 2 commits into
mainfrom
fix/goals-5461-suspected

Conversation

@ThomasK33

Copy link
Copy Markdown
Member

Summary

This PR fixes the three "suspected" items left in #5461. Each one is confirmed on main, and each has a regression test that failed before the fix.

  1. complete_goal without a goalId on an automatic goal turn now targets the goal the turn was dispatched for.
  2. Browser goal edits (sidebar, palette, /goal) no longer retry a conflict against the replacement goal.
  3. The idle-trigger heartbeat ignores transientGoalOnly activity snapshots.

Fixes #5461

Background

G4 of #5461 was fixed in #5536. The triage of the three remaining items is in the issue comments. All three are single-backend defects with small fixes.

Implementation

  1. complete_goal (item 1).
    • Until the stream-start listener's unawaited streaming=true metadata write lands (workspaceService.ts around :4381), WorkspaceGoalService.setGoal persists a user's goal replacement at once instead of queueing it.
    • complete_goal bound no goal id, so a continuation turn for goal A could complete A's replacement B.
    • GoalToolContext.goalId now carries the goal an automatic goal turn was dispatched for. The path is internal.goalId → streamWithHistory / prepareRolloverRequest → turnRequestBuilder (goalTurnGoalId). It is set only when goalTurnKind is set.
    • complete_goal uses it as expectedGoalId when the model passes none. A replaced goal now fails with goal_conflict.
    • User, delegated and heartbeat turns keep their current behavior because they are not bound to a goal.
  2. Goal edits (item 2).
    • The backend reports goal_conflict only when the goal id changed. setGoalWithConflictRetry re-read the goal and retried, so a Pause, budget edit, rename or completion clicked for goal A landed on its replacement B.
    • The helper is renamed setGoalForIntendedGoal. The sidebar and palette pass the goal they displayed (intendedGoalIdOf). A goal still pending persistence has no durable id yet, so for it the helper reads the current goal.
    • Slash commands read the current goal once.
    • A conflict now returns the existing "Goal changed in another window. Please try again." error.
  3. Heartbeat (item 3). HeartbeatService.handleActivityEvent now also returns on transientGoalOnly, as keepAwake.ts already does. These snapshots republish the last baseline, whose stale streaming:false pushed back the idle countdown.

Design choice (fail closed, smallest scope): on a conflict the user retries the edit. No edit is ever re-targeted.

Validation

These tests failed before the fix:

  • goal.test.ts "complete_goal without goalId on a goal turn refuses a goal replaced after dispatch": the tool completed the replacement goal ("Expected tool execution to fail").
  • setGoalForIntendedGoal.test.ts "returns a conflict without retrying against the replacement goal" ("Expected number of calls: 1, Received: 2"), and "targets the intended goal instead of the goal read at call time" (getGoal called 1 time, expected 0).
  • heartbeatService.test.ts "activity event ignores transientGoalOnly snapshots" ("Expected: 300000, Received: 1791140599644").

Plumbing tests:

  • agentSession.goalAutoPause.test.ts checks that the goal id reaches stream requests only for goal turns.
  • aiService.test.ts checks that it reaches the goal tool context only on goal turns.

chatCommands.test.ts pinned the old retry and now asserts a single attempt plus a restore result.

Visual evidence: none. No rendered UI changes. The only user-visible difference is that a goal edit racing a replacement shows the existing conflict error. The GoalTab stories take mocked handlers and do not exercise this path.

Risks

Low.

  • Goal editing paths: a user whose goal was replaced concurrently now has to retry the click.
  • Goal turns: a goal turn whose goal was replaced now fails complete_goal with a conflict instead of completing the replacement.
  • agentSession.ts gains one forwarded argument per request-builder call. There is no control-flow change.

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-04T19:30:31.937467Z 6d3136f PR opened
🔒 Security Review ✅ Completed 2026-10-04T19:34:00.793598Z 6d3136f PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ThomasK33

Copy link
Copy Markdown
Member Author

Readiness record (issue coordinator)

  • Head: 6d3136fb1aaa0d1e49e1c651ee0e83e45f8b5e57
  • Review assessments: 3 of 6. These were the normal and security reviews on PR open (both clean, 0 findings) and the final independent check.
  • Final independent check: READY. Non-blocking notes:
    1. Slash commands /goal pause|resume|complete read the current goal once, then write, and never retry a conflict. They show no specific goal, so a replacement between that read and the write still lands on the new goal. I am leaving this as designed and will record it on 🤖 workspace goals: decide goal auto-resume after terminal errors (G4) + 3 suspected issues #5461.
    2. set_goal is already refused on automatic goal turns (toolAvailability.ts), so a goal turn cannot replace its own goal before complete_goal.
  • CI: every required check passes on the head. Only the optional Pixel / Review is pending.
  • Local gate: make static-check passed, and 523 tests in the 8 touched test files passed.

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

@ThomasK33
ThomasK33 added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit c9123ad Oct 4, 2026
42 of 43 checks passed
@ThomasK33
ThomasK33 deleted the fix/goals-5461-suspected branch October 4, 2026 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🤖 workspace goals: decide goal auto-resume after terminal errors (G4) + 3 suspected issues

1 participant