fix(goals): preserve continuation handoff state
- commit
- d395d302defa4ad153ff52097c4a8aad36b53855
- author
- brice · · 2026-08-14 06:33 UTC
- parent
- e05ba05
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +141 -31
M packages/agent/cli.go +39 -9
| @@ -693,8 +693,19 @@ func copyExtensionStates(states map[string]json.RawMessage) map[string]json.RawM | |||
| 693 | 693 | return out | |
| 694 | 694 | } | |
| 695 | 695 | ||
| 696 | + | var ( | |
| 697 | + | errGoalActiveReplacement = errors.New("active goal replacement") | |
| 698 | + | errGoalMissionMissing = errors.New("goal mission missing") | |
| 699 | + | errGoalMissionMismatch = errors.New("goal mission mismatch") | |
| 700 | + | errGoalStorageUnavailable = errors.New("goal storage unavailable") | |
| 701 | + | errGoalStateWrite = errors.New("goal state write") | |
| 702 | + | ) | |
| 703 | + | ||
| 696 | 704 | func persistToolResultState(mgr *extensions.Manager, sess *core.Session, result core.ToolResult, goalMaxTokenBudget ...*uint64) error { | |
| 697 | 705 | if sess == nil { | |
| 706 | + | if _, ok := tools.GoalUpdateFromResult(result); ok { | |
| 707 | + | return goalCommitError(errGoalStorageUnavailable) | |
| 708 | + | } | |
| 698 | 709 | return nil | |
| 699 | 710 | } | |
| 700 | 711 | if details, ok := result.Details.(extensions.ToolResultDetails); ok && details.Extension != "" && len(details.State) > 0 { | |
| @@ -706,17 +717,27 @@ func persistToolResultState(mgr *extensions.Manager, sess *core.Session, result | |||
| 706 | 717 | } | |
| 707 | 718 | } | |
| 708 | 719 | if err := persistGoalToolResult(sess, result, goalMaxTokenBudget...); err != nil { | |
| 709 | - | return &core.ToolResultCommitError{Message: safeGoalCommitMessage(err), Err: fmt.Errorf("goal state: %w", err)} | |
| 720 | + | return goalCommitError(err) | |
| 710 | 721 | } | |
| 711 | 722 | return nil | |
| 712 | 723 | } | |
| 713 | 724 | ||
| 725 | + | func goalCommitError(err error) error { | |
| 726 | + | return &core.ToolResultCommitError{Message: safeGoalCommitMessage(err), Err: fmt.Errorf("goal state: %w", err)} | |
| 727 | + | } | |
| 728 | + | ||
| 714 | 729 | func safeGoalCommitMessage(err error) string { | |
| 715 | - | switch err.Error() { | |
| 716 | - | case "cannot replace an active goal": | |
| 730 | + | switch { | |
| 731 | + | case errors.Is(err, errGoalActiveReplacement): | |
| 717 | 732 | return "goal transition rejected: the current goal is still active" | |
| 718 | - | case "goal update does not belong to the active mission": | |
| 733 | + | case errors.Is(err, errGoalMissionMissing): | |
| 734 | + | return "goal transition rejected: no active mission" | |
| 735 | + | case errors.Is(err, errGoalMissionMismatch): | |
| 719 | 736 | return "goal transition rejected: it does not belong to the active mission" | |
| 737 | + | case errors.Is(err, errGoalStorageUnavailable): | |
| 738 | + | return "goal state unavailable: session persistence is disabled" | |
| 739 | + | case errors.Is(err, errGoalStateWrite): | |
| 740 | + | return "goal state could not be saved" | |
| 720 | 741 | default: | |
| 721 | 742 | return "goal update could not be persisted" | |
| 722 | 743 | } | |
| @@ -732,17 +753,23 @@ func persistGoalToolResult(sess *core.Session, result core.ToolResult, goalMaxTo | |||
| 732 | 753 | } | |
| 733 | 754 | if update.Status == core.GoalActive { | |
| 734 | 755 | if sess.Meta.Goal != nil && sess.Meta.Goal.Status == core.GoalActive { | |
| 735 | - | return fmt.Errorf("cannot replace an active goal") | |
| 756 | + | return errGoalActiveReplacement | |
| 736 | 757 | } | |
| 737 | - | if sess.Meta.Mission != nil && update.MissionID != "" && update.MissionID != sess.Meta.Mission.ID { | |
| 738 | - | return fmt.Errorf("goal update does not belong to the active mission") | |
| 758 | + | if sess.Meta.Mission == nil { | |
| 759 | + | return errGoalMissionMissing | |
| 760 | + | } | |
| 761 | + | if update.MissionID == "" || update.MissionID != sess.Meta.Mission.ID { | |
| 762 | + | return errGoalMissionMismatch | |
| 739 | 763 | } | |
| 740 | 764 | goal := &core.SessionGoal{Objective: update.Objective, Status: core.GoalActive, Owner: core.GoalOwnerManager} | |
| 741 | 765 | if len(goalMaxTokenBudget) > 0 && goalMaxTokenBudget[0] != nil && *goalMaxTokenBudget[0] > 0 { | |
| 742 | 766 | budget := *goalMaxTokenBudget[0] | |
| 743 | 767 | goal.TokenBudget = &budget | |
| 744 | 768 | } | |
| 745 | - | return sess.UpdateGoal(goal) | |
| 769 | + | if err := sess.UpdateGoal(goal); err != nil { | |
| 770 | + | return fmt.Errorf("%w: %w", errGoalStateWrite, err) | |
| 771 | + | } | |
| 772 | + | return nil | |
| 746 | 773 | } | |
| 747 | 774 | if sess.Meta.Goal == nil || sess.Meta.Goal.Status != core.GoalActive { | |
| 748 | 775 | return nil | |
| @@ -750,7 +777,10 @@ func persistGoalToolResult(sess *core.Session, result core.ToolResult, goalMaxTo | |||
| 750 | 777 | goal := *sess.Meta.Goal | |
| 751 | 778 | goal.Status = update.Status | |
| 752 | 779 | goal.Reason = update.Reason | |
| 753 | - | return sess.UpdateGoal(&goal) | |
| 780 | + | if err := sess.UpdateGoal(&goal); err != nil { | |
| 781 | + | return fmt.Errorf("%w: %w", errGoalStateWrite, err) | |
| 782 | + | } | |
| 783 | + | return nil | |
| 754 | 784 | } | |
| 755 | 785 | ||
| 756 | 786 | func fanoutAgentEvent(mgr *extensions.Manager, ev core.AgentEvent) { | |
| @@ -693,8 +693,19 @@ func copyExtensionStates(states map[string]json.RawMessage) map[string]json.RawM | |||||
| 693 | return out | 693 | return out | ||
| 694 | } | 694 | } | ||
| 695 | 695 | ||||
| 696 | + | var ( | |||
| 697 | + | errGoalActiveReplacement = errors.New("active goal replacement") | |||
| 698 | + | errGoalMissionMissing = errors.New("goal mission missing") | |||
| 699 | + | errGoalMissionMismatch = errors.New("goal mission mismatch") | |||
| 700 | + | errGoalStorageUnavailable = errors.New("goal storage unavailable") | |||
| 701 | + | errGoalStateWrite = errors.New("goal state write") | |||
| 702 | + | ) | |||
| 703 | + | ||||
| 696 | func persistToolResultState(mgr *extensions.Manager, sess *core.Session, result core.ToolResult, goalMaxTokenBudget ...*uint64) error { | 704 | func persistToolResultState(mgr *extensions.Manager, sess *core.Session, result core.ToolResult, goalMaxTokenBudget ...*uint64) error { | ||
| 697 | if sess == nil { | 705 | if sess == nil { | ||
| 706 | + | if _, ok := tools.GoalUpdateFromResult(result); ok { | |||
| 707 | + | return goalCommitError(errGoalStorageUnavailable) | |||
| 708 | + | } | |||
| 698 | return nil | 709 | return nil | ||
| 699 | } | 710 | } | ||
| 700 | if details, ok := result.Details.(extensions.ToolResultDetails); ok && details.Extension != "" && len(details.State) > 0 { | 711 | if details, ok := result.Details.(extensions.ToolResultDetails); ok && details.Extension != "" && len(details.State) > 0 { | ||
| @@ -706,17 +717,27 @@ func persistToolResultState(mgr *extensions.Manager, sess *core.Session, result | |||||
| 706 | } | 717 | } | ||
| 707 | } | 718 | } | ||
| 708 | if err := persistGoalToolResult(sess, result, goalMaxTokenBudget...); err != nil { | 719 | if err := persistGoalToolResult(sess, result, goalMaxTokenBudget...); err != nil { | ||
| 709 | - | return &core.ToolResultCommitError{Message: safeGoalCommitMessage(err), Err: fmt.Errorf("goal state: %w", err)} | 720 | + | return goalCommitError(err) |
| 710 | } | 721 | } | ||
| 711 | return nil | 722 | return nil | ||
| 712 | } | 723 | } | ||
| 713 | 724 | ||||
| 725 | + | func goalCommitError(err error) error { | |||
| 726 | + | return &core.ToolResultCommitError{Message: safeGoalCommitMessage(err), Err: fmt.Errorf("goal state: %w", err)} | |||
| 727 | + | } | |||
| 728 | + | ||||
| 714 | func safeGoalCommitMessage(err error) string { | 729 | func safeGoalCommitMessage(err error) string { | ||
| 715 | - | switch err.Error() { | 730 | + | switch { |
| 716 | - | case "cannot replace an active goal": | 731 | + | case errors.Is(err, errGoalActiveReplacement): |
| 717 | return "goal transition rejected: the current goal is still active" | 732 | return "goal transition rejected: the current goal is still active" | ||
| 718 | - | case "goal update does not belong to the active mission": | 733 | + | case errors.Is(err, errGoalMissionMissing): |
| 734 | + | return "goal transition rejected: no active mission" | |||
| 735 | + | case errors.Is(err, errGoalMissionMismatch): | |||
| 719 | return "goal transition rejected: it does not belong to the active mission" | 736 | return "goal transition rejected: it does not belong to the active mission" | ||
| 737 | + | case errors.Is(err, errGoalStorageUnavailable): | |||
| 738 | + | return "goal state unavailable: session persistence is disabled" | |||
| 739 | + | case errors.Is(err, errGoalStateWrite): | |||
| 740 | + | return "goal state could not be saved" | |||
| 720 | default: | 741 | default: | ||
| 721 | return "goal update could not be persisted" | 742 | return "goal update could not be persisted" | ||
| 722 | } | 743 | } | ||
| @@ -732,17 +753,23 @@ func persistGoalToolResult(sess *core.Session, result core.ToolResult, goalMaxTo | |||||
| 732 | } | 753 | } | ||
| 733 | if update.Status == core.GoalActive { | 754 | if update.Status == core.GoalActive { | ||
| 734 | if sess.Meta.Goal != nil && sess.Meta.Goal.Status == core.GoalActive { | 755 | if sess.Meta.Goal != nil && sess.Meta.Goal.Status == core.GoalActive { | ||
| 735 | - | return fmt.Errorf("cannot replace an active goal") | 756 | + | return errGoalActiveReplacement |
| 736 | } | 757 | } | ||
| 737 | - | if sess.Meta.Mission != nil && update.MissionID != "" && update.MissionID != sess.Meta.Mission.ID { | 758 | + | if sess.Meta.Mission == nil { |
| 738 | - | return fmt.Errorf("goal update does not belong to the active mission") | 759 | + | return errGoalMissionMissing |
| 760 | + | } | |||
| 761 | + | if update.MissionID == "" || update.MissionID != sess.Meta.Mission.ID { | |||
| 762 | + | return errGoalMissionMismatch | |||
| 739 | } | 763 | } | ||
| 740 | goal := &core.SessionGoal{Objective: update.Objective, Status: core.GoalActive, Owner: core.GoalOwnerManager} | 764 | goal := &core.SessionGoal{Objective: update.Objective, Status: core.GoalActive, Owner: core.GoalOwnerManager} | ||
| 741 | if len(goalMaxTokenBudget) > 0 && goalMaxTokenBudget[0] != nil && *goalMaxTokenBudget[0] > 0 { | 765 | if len(goalMaxTokenBudget) > 0 && goalMaxTokenBudget[0] != nil && *goalMaxTokenBudget[0] > 0 { | ||
| 742 | budget := *goalMaxTokenBudget[0] | 766 | budget := *goalMaxTokenBudget[0] | ||
| 743 | goal.TokenBudget = &budget | 767 | goal.TokenBudget = &budget | ||
| 744 | } | 768 | } | ||
| 745 | - | return sess.UpdateGoal(goal) | 769 | + | if err := sess.UpdateGoal(goal); err != nil { |
| 770 | + | return fmt.Errorf("%w: %w", errGoalStateWrite, err) | |||
| 771 | + | } | |||
| 772 | + | return nil | |||
| 746 | } | 773 | } | ||
| 747 | if sess.Meta.Goal == nil || sess.Meta.Goal.Status != core.GoalActive { | 774 | if sess.Meta.Goal == nil || sess.Meta.Goal.Status != core.GoalActive { | ||
| 748 | return nil | 775 | return nil | ||
| @@ -750,7 +777,10 @@ func persistGoalToolResult(sess *core.Session, result core.ToolResult, goalMaxTo | |||||
| 750 | goal := *sess.Meta.Goal | 777 | goal := *sess.Meta.Goal | ||
| 751 | goal.Status = update.Status | 778 | goal.Status = update.Status | ||
| 752 | goal.Reason = update.Reason | 779 | goal.Reason = update.Reason | ||
| 753 | - | return sess.UpdateGoal(&goal) | 780 | + | if err := sess.UpdateGoal(&goal); err != nil { |
| 781 | + | return fmt.Errorf("%w: %w", errGoalStateWrite, err) | |||
| 782 | + | } | |||
| 783 | + | return nil | |||
| 754 | } | 784 | } | ||
| 755 | 785 | ||||
| 756 | func fanoutAgentEvent(mgr *extensions.Manager, ev core.AgentEvent) { | 786 | func fanoutAgentEvent(mgr *extensions.Manager, ev core.AgentEvent) { | ||
M packages/agent/cli_session_test.go +96 -16
| @@ -111,23 +111,97 @@ func TestToolResultPersistenceUsesBoundSession(t *testing.T) { | |||
| 111 | 111 | } | |
| 112 | 112 | } | |
| 113 | 113 | ||
| 114 | - | func TestPersistToolResultStateReturnsSafeGoalTransitionError(t *testing.T) { | |
| 115 | - | sess, err := core.NewSession(t.TempDir(), t.TempDir(), "provider", "model", "version") | |
| 116 | - | if err != nil { | |
| 117 | - | t.Fatal(err) | |
| 118 | - | } | |
| 119 | - | defer sess.Close() | |
| 120 | - | if err := sess.UpdateGoal(&core.SessionGoal{Objective: "current work", Status: core.GoalActive}); err != nil { | |
| 121 | - | t.Fatal(err) | |
| 114 | + | func TestPersistToolResultStateUsesSafeGoalPersistenceCategories(t *testing.T) { | |
| 115 | + | const privateGoal = "private goal text" | |
| 116 | + | const privateMission = "private-mission-id" | |
| 117 | + | const privateSessionContent = "private session content" | |
| 118 | + | ||
| 119 | + | newSession := func(t *testing.T) *core.Session { | |
| 120 | + | t.Helper() | |
| 121 | + | sess, err := core.NewSession(t.TempDir(), t.TempDir(), "provider", "model", "version") | |
| 122 | + | if err != nil { | |
| 123 | + | t.Fatal(err) | |
| 124 | + | } | |
| 125 | + | if err := sess.AppendMessage(provider.Message{Role: provider.RoleUser, Content: []provider.Content{provider.TextBlock{Text: privateSessionContent}}}); err != nil { | |
| 126 | + | t.Fatal(err) | |
| 127 | + | } | |
| 128 | + | t.Cleanup(func() { _ = sess.Close() }) | |
| 129 | + | return sess | |
| 122 | 130 | } | |
| 123 | 131 | ||
| 124 | - | err = persistToolResultState(nil, sess, core.ToolResult{Details: tools.GoalUpdate{Status: core.GoalActive, Objective: "unrelated work"}}) | |
| 125 | - | var safe *core.ToolResultCommitError | |
| 126 | - | if !errors.As(err, &safe) { | |
| 127 | - | t.Fatalf("persist error = %T %v, want safe tool result commit error", err, err) | |
| 132 | + | tests := []struct { | |
| 133 | + | name string | |
| 134 | + | setup func(*testing.T) *core.Session | |
| 135 | + | update tools.GoalUpdate | |
| 136 | + | message string | |
| 137 | + | }{ | |
| 138 | + | { | |
| 139 | + | name: "active goal", | |
| 140 | + | setup: func(t *testing.T) *core.Session { | |
| 141 | + | sess := newSession(t) | |
| 142 | + | if err := sess.UpdateGoal(&core.SessionGoal{Objective: privateGoal, Status: core.GoalActive}); err != nil { | |
| 143 | + | t.Fatal(err) | |
| 144 | + | } | |
| 145 | + | return sess | |
| 146 | + | }, | |
| 147 | + | update: tools.GoalUpdate{Status: core.GoalActive, Objective: privateGoal}, | |
| 148 | + | message: "goal transition rejected: the current goal is still active", | |
| 149 | + | }, | |
| 150 | + | { | |
| 151 | + | name: "missing mission", | |
| 152 | + | setup: newSession, | |
| 153 | + | update: tools.GoalUpdate{Status: core.GoalActive, Objective: privateGoal, MissionID: privateMission}, | |
| 154 | + | message: "goal transition rejected: no active mission", | |
| 155 | + | }, | |
| 156 | + | { | |
| 157 | + | name: "mismatched mission", | |
| 158 | + | setup: func(t *testing.T) *core.Session { | |
| 159 | + | sess := newSession(t) | |
| 160 | + | if err := sess.EnsureMission(privateGoal, core.MissionSourceUser); err != nil { | |
| 161 | + | t.Fatal(err) | |
| 162 | + | } | |
| 163 | + | return sess | |
| 164 | + | }, | |
| 165 | + | update: tools.GoalUpdate{Status: core.GoalActive, Objective: privateGoal, MissionID: privateMission}, | |
| 166 | + | message: "goal transition rejected: it does not belong to the active mission", | |
| 167 | + | }, | |
| 168 | + | { | |
| 169 | + | name: "storage unavailable", | |
| 170 | + | setup: func(*testing.T) *core.Session { return nil }, | |
| 171 | + | update: tools.GoalUpdate{Status: core.GoalActive, Objective: privateGoal, MissionID: privateMission}, | |
| 172 | + | message: "goal state unavailable: session persistence is disabled", | |
| 173 | + | }, | |
| 174 | + | { | |
| 175 | + | name: "write failure", | |
| 176 | + | setup: func(t *testing.T) *core.Session { | |
| 177 | + | sess := newSession(t) | |
| 178 | + | if err := sess.UpdateGoal(&core.SessionGoal{Objective: privateGoal, Status: core.GoalActive}); err != nil { | |
| 179 | + | t.Fatal(err) | |
| 180 | + | } | |
| 181 | + | if err := sess.Close(); err != nil { | |
| 182 | + | t.Fatal(err) | |
| 183 | + | } | |
| 184 | + | return sess | |
| 185 | + | }, | |
| 186 | + | update: tools.GoalUpdate{Status: core.GoalDone}, | |
| 187 | + | message: "goal state could not be saved", | |
| 188 | + | }, | |
| 128 | 189 | } | |
| 129 | - | if safe.Message != "goal transition rejected: the current goal is still active" { | |
| 130 | - | t.Fatalf("safe message = %q", safe.Message) | |
| 190 | + | for _, test := range tests { | |
| 191 | + | t.Run(test.name, func(t *testing.T) { | |
| 192 | + | sess := test.setup(t) | |
| 193 | + | err := persistToolResultState(nil, sess, core.ToolResult{Details: test.update}) | |
| 194 | + | var safe *core.ToolResultCommitError | |
| 195 | + | if !errors.As(err, &safe) { | |
| 196 | + | t.Fatalf("persist error = %T %v, want safe tool result commit error", err, err) | |
| 197 | + | } | |
| 198 | + | if safe.Message != test.message { | |
| 199 | + | t.Fatalf("safe message = %q, want %q", safe.Message, test.message) | |
| 200 | + | } | |
| 201 | + | if strings.Contains(safe.Message, privateGoal) || strings.Contains(safe.Message, privateMission) || strings.Contains(safe.Message, privateSessionContent) || (sess != nil && strings.Contains(safe.Message, sess.Path)) { | |
| 202 | + | t.Fatalf("safe message leaked private state: %q", safe.Message) | |
| 203 | + | } | |
| 204 | + | }) | |
| 131 | 205 | } | |
| 132 | 206 | } | |
| 133 | 207 | ||
| @@ -137,8 +211,11 @@ func TestPersistGoalToolResultCreatesActiveGoal(t *testing.T) { | |||
| 137 | 211 | t.Fatal(err) | |
| 138 | 212 | } | |
| 139 | 213 | defer sess.Close() | |
| 214 | + | if err := sess.EnsureMission("reproduce the reported failure", core.MissionSourceUser); err != nil { | |
| 215 | + | t.Fatal(err) | |
| 216 | + | } | |
| 140 | 217 | ||
| 141 | - | result := core.ToolResult{Details: tools.GoalUpdate{Status: core.GoalActive, Objective: "reproduce the reported failure"}} | |
| 218 | + | result := core.ToolResult{Details: tools.GoalUpdate{Status: core.GoalActive, Objective: "reproduce the reported failure", MissionID: sess.Meta.Mission.ID}} | |
| 142 | 219 | if err := persistGoalToolResult(sess, result); err != nil { | |
| 143 | 220 | t.Fatal(err) | |
| 144 | 221 | } | |
| @@ -153,9 +230,12 @@ func TestPersistGoalToolResultAppliesConfiguredBudget(t *testing.T) { | |||
| 153 | 230 | t.Fatal(err) | |
| 154 | 231 | } | |
| 155 | 232 | defer sess.Close() | |
| 233 | + | if err := sess.EnsureMission("reproduce the reported failure", core.MissionSourceUser); err != nil { | |
| 234 | + | t.Fatal(err) | |
| 235 | + | } | |
| 156 | 236 | ||
| 157 | 237 | budget := uint64(1_000_000) | |
| 158 | - | result := core.ToolResult{Details: tools.GoalUpdate{Status: core.GoalActive, Objective: "reproduce the reported failure"}} | |
| 238 | + | result := core.ToolResult{Details: tools.GoalUpdate{Status: core.GoalActive, Objective: "reproduce the reported failure", MissionID: sess.Meta.Mission.ID}} | |
| 159 | 239 | if err := persistGoalToolResult(sess, result, &budget); err != nil { | |
| 160 | 240 | t.Fatal(err) | |
| 161 | 241 | } | |
| @@ -111,23 +111,97 @@ func TestToolResultPersistenceUsesBoundSession(t *testing.T) { | |||||
| 111 | } | 111 | } | ||
| 112 | } | 112 | } | ||
| 113 | 113 | ||||
| 114 | - | func TestPersistToolResultStateReturnsSafeGoalTransitionError(t *testing.T) { | 114 | + | func TestPersistToolResultStateUsesSafeGoalPersistenceCategories(t *testing.T) { |
| 115 | - | sess, err := core.NewSession(t.TempDir(), t.TempDir(), "provider", "model", "version") | 115 | + | const privateGoal = "private goal text" |
| 116 | - | if err != nil { | 116 | + | const privateMission = "private-mission-id" |
| 117 | - | t.Fatal(err) | 117 | + | const privateSessionContent = "private session content" |
| 118 | - | } | 118 | + | |
| 119 | - | defer sess.Close() | 119 | + | newSession := func(t *testing.T) *core.Session { |
| 120 | - | if err := sess.UpdateGoal(&core.SessionGoal{Objective: "current work", Status: core.GoalActive}); err != nil { | 120 | + | t.Helper() |
| 121 | - | t.Fatal(err) | 121 | + | sess, err := core.NewSession(t.TempDir(), t.TempDir(), "provider", "model", "version") |
| 122 | + | if err != nil { | |||
| 123 | + | t.Fatal(err) | |||
| 124 | + | } | |||
| 125 | + | if err := sess.AppendMessage(provider.Message{Role: provider.RoleUser, Content: []provider.Content{provider.TextBlock{Text: privateSessionContent}}}); err != nil { | |||
| 126 | + | t.Fatal(err) | |||
| 127 | + | } | |||
| 128 | + | t.Cleanup(func() { _ = sess.Close() }) | |||
| 129 | + | return sess | |||
| 122 | } | 130 | } | ||
| 123 | 131 | ||||
| 124 | - | err = persistToolResultState(nil, sess, core.ToolResult{Details: tools.GoalUpdate{Status: core.GoalActive, Objective: "unrelated work"}}) | 132 | + | tests := []struct { |
| 125 | - | var safe *core.ToolResultCommitError | 133 | + | name string |
| 126 | - | if !errors.As(err, &safe) { | 134 | + | setup func(*testing.T) *core.Session |
| 127 | - | t.Fatalf("persist error = %T %v, want safe tool result commit error", err, err) | 135 | + | update tools.GoalUpdate |
| 136 | + | message string | |||
| 137 | + | }{ | |||
| 138 | + | { | |||
| 139 | + | name: "active goal", | |||
| 140 | + | setup: func(t *testing.T) *core.Session { | |||
| 141 | + | sess := newSession(t) | |||
| 142 | + | if err := sess.UpdateGoal(&core.SessionGoal{Objective: privateGoal, Status: core.GoalActive}); err != nil { | |||
| 143 | + | t.Fatal(err) | |||
| 144 | + | } | |||
| 145 | + | return sess | |||
| 146 | + | }, | |||
| 147 | + | update: tools.GoalUpdate{Status: core.GoalActive, Objective: privateGoal}, | |||
| 148 | + | message: "goal transition rejected: the current goal is still active", | |||
| 149 | + | }, | |||
| 150 | + | { | |||
| 151 | + | name: "missing mission", | |||
| 152 | + | setup: newSession, | |||
| 153 | + | update: tools.GoalUpdate{Status: core.GoalActive, Objective: privateGoal, MissionID: privateMission}, | |||
| 154 | + | message: "goal transition rejected: no active mission", | |||
| 155 | + | }, | |||
| 156 | + | { | |||
| 157 | + | name: "mismatched mission", | |||
| 158 | + | setup: func(t *testing.T) *core.Session { | |||
| 159 | + | sess := newSession(t) | |||
| 160 | + | if err := sess.EnsureMission(privateGoal, core.MissionSourceUser); err != nil { | |||
| 161 | + | t.Fatal(err) | |||
| 162 | + | } | |||
| 163 | + | return sess | |||
| 164 | + | }, | |||
| 165 | + | update: tools.GoalUpdate{Status: core.GoalActive, Objective: privateGoal, MissionID: privateMission}, | |||
| 166 | + | message: "goal transition rejected: it does not belong to the active mission", | |||
| 167 | + | }, | |||
| 168 | + | { | |||
| 169 | + | name: "storage unavailable", | |||
| 170 | + | setup: func(*testing.T) *core.Session { return nil }, | |||
| 171 | + | update: tools.GoalUpdate{Status: core.GoalActive, Objective: privateGoal, MissionID: privateMission}, | |||
| 172 | + | message: "goal state unavailable: session persistence is disabled", | |||
| 173 | + | }, | |||
| 174 | + | { | |||
| 175 | + | name: "write failure", | |||
| 176 | + | setup: func(t *testing.T) *core.Session { | |||
| 177 | + | sess := newSession(t) | |||
| 178 | + | if err := sess.UpdateGoal(&core.SessionGoal{Objective: privateGoal, Status: core.GoalActive}); err != nil { | |||
| 179 | + | t.Fatal(err) | |||
| 180 | + | } | |||
| 181 | + | if err := sess.Close(); err != nil { | |||
| 182 | + | t.Fatal(err) | |||
| 183 | + | } | |||
| 184 | + | return sess | |||
| 185 | + | }, | |||
| 186 | + | update: tools.GoalUpdate{Status: core.GoalDone}, | |||
| 187 | + | message: "goal state could not be saved", | |||
| 188 | + | }, | |||
| 128 | } | 189 | } | ||
| 129 | - | if safe.Message != "goal transition rejected: the current goal is still active" { | 190 | + | for _, test := range tests { |
| 130 | - | t.Fatalf("safe message = %q", safe.Message) | 191 | + | t.Run(test.name, func(t *testing.T) { |
| 192 | + | sess := test.setup(t) | |||
| 193 | + | err := persistToolResultState(nil, sess, core.ToolResult{Details: test.update}) | |||
| 194 | + | var safe *core.ToolResultCommitError | |||
| 195 | + | if !errors.As(err, &safe) { | |||
| 196 | + | t.Fatalf("persist error = %T %v, want safe tool result commit error", err, err) | |||
| 197 | + | } | |||
| 198 | + | if safe.Message != test.message { | |||
| 199 | + | t.Fatalf("safe message = %q, want %q", safe.Message, test.message) | |||
| 200 | + | } | |||
| 201 | + | if strings.Contains(safe.Message, privateGoal) || strings.Contains(safe.Message, privateMission) || strings.Contains(safe.Message, privateSessionContent) || (sess != nil && strings.Contains(safe.Message, sess.Path)) { | |||
| 202 | + | t.Fatalf("safe message leaked private state: %q", safe.Message) | |||
| 203 | + | } | |||
| 204 | + | }) | |||
| 131 | } | 205 | } | ||
| 132 | } | 206 | } | ||
| 133 | 207 | ||||
| @@ -137,8 +211,11 @@ func TestPersistGoalToolResultCreatesActiveGoal(t *testing.T) { | |||||
| 137 | t.Fatal(err) | 211 | t.Fatal(err) | ||
| 138 | } | 212 | } | ||
| 139 | defer sess.Close() | 213 | defer sess.Close() | ||
| 214 | + | if err := sess.EnsureMission("reproduce the reported failure", core.MissionSourceUser); err != nil { | |||
| 215 | + | t.Fatal(err) | |||
| 216 | + | } | |||
| 140 | 217 | ||||
| 141 | - | result := core.ToolResult{Details: tools.GoalUpdate{Status: core.GoalActive, Objective: "reproduce the reported failure"}} | 218 | + | result := core.ToolResult{Details: tools.GoalUpdate{Status: core.GoalActive, Objective: "reproduce the reported failure", MissionID: sess.Meta.Mission.ID}} |
| 142 | if err := persistGoalToolResult(sess, result); err != nil { | 219 | if err := persistGoalToolResult(sess, result); err != nil { | ||
| 143 | t.Fatal(err) | 220 | t.Fatal(err) | ||
| 144 | } | 221 | } | ||
| @@ -153,9 +230,12 @@ func TestPersistGoalToolResultAppliesConfiguredBudget(t *testing.T) { | |||||
| 153 | t.Fatal(err) | 230 | t.Fatal(err) | ||
| 154 | } | 231 | } | ||
| 155 | defer sess.Close() | 232 | defer sess.Close() | ||
| 233 | + | if err := sess.EnsureMission("reproduce the reported failure", core.MissionSourceUser); err != nil { | |||
| 234 | + | t.Fatal(err) | |||
| 235 | + | } | |||
| 156 | 236 | ||||
| 157 | budget := uint64(1_000_000) | 237 | budget := uint64(1_000_000) | ||
| 158 | - | result := core.ToolResult{Details: tools.GoalUpdate{Status: core.GoalActive, Objective: "reproduce the reported failure"}} | 238 | + | result := core.ToolResult{Details: tools.GoalUpdate{Status: core.GoalActive, Objective: "reproduce the reported failure", MissionID: sess.Meta.Mission.ID}} |
| 159 | if err := persistGoalToolResult(sess, result, &budget); err != nil { | 239 | if err := persistGoalToolResult(sess, result, &budget); err != nil { | ||
| 160 | t.Fatal(err) | 240 | t.Fatal(err) | ||
| 161 | } | 241 | } | ||
M packages/agent/modes/goal_runtime_test.go +2 -4
| @@ -79,10 +79,8 @@ func TestGoalContinuationDoesNotResendNoProgressMessagesIndefinitely(t *testing. | |||
| 79 | 79 | if got := current(); got == nil || got.Status != core.GoalStalled { | |
| 80 | 80 | t.Fatalf("goal after repeated no-progress responses = %#v", got) | |
| 81 | 81 | } | |
| 82 | - | select { | |
| 83 | - | case call := <-client.hits: | |
| 84 | - | t.Fatalf("unexpected continuation call %d after stalled goal", call) | |
| 85 | - | case <-time.After(100 * time.Millisecond): | |
| 82 | + | if got := client.count(); got != maxConsecutiveGoalNoProgressTurns { | |
| 83 | + | t.Fatalf("provider call count = %d, want %d", got, maxConsecutiveGoalNoProgressTurns) | |
| 86 | 84 | } | |
| 87 | 85 | } | |
| 88 | 86 | ||
| @@ -79,10 +79,8 @@ func TestGoalContinuationDoesNotResendNoProgressMessagesIndefinitely(t *testing. | |||||
| 79 | if got := current(); got == nil || got.Status != core.GoalStalled { | 79 | if got := current(); got == nil || got.Status != core.GoalStalled { | ||
| 80 | t.Fatalf("goal after repeated no-progress responses = %#v", got) | 80 | t.Fatalf("goal after repeated no-progress responses = %#v", got) | ||
| 81 | } | 81 | } | ||
| 82 | - | select { | 82 | + | if got := client.count(); got != maxConsecutiveGoalNoProgressTurns { |
| 83 | - | case call := <-client.hits: | 83 | + | t.Fatalf("provider call count = %d, want %d", got, maxConsecutiveGoalNoProgressTurns) |
| 84 | - | t.Fatalf("unexpected continuation call %d after stalled goal", call) | |||
| 85 | - | case <-time.After(100 * time.Millisecond): | |||
| 86 | } | 84 | } | ||
| 87 | } | 85 | } | ||
| 88 | 86 | ||||
M packages/agent/modes/interactive_turn.go +4 -2
| @@ -323,7 +323,7 @@ func (i *Interactive) startTurnRequest(parent context.Context, prompt string, im | |||
| 323 | 323 | if terminalGoalError { | |
| 324 | 324 | i.updateActiveGoal(core.GoalBlocked, "turn ended with an error") | |
| 325 | 325 | } | |
| 326 | - | i.finishGoalRun() | |
| 326 | + | continueGoal := i.finishGoalRun() | |
| 327 | 327 | i.mu.Lock() | |
| 328 | 328 | awaitingPre := i.awaitingStartupPre | |
| 329 | 329 | // A newer explicit prompt may have cleared the handoff while the | |
| @@ -381,7 +381,7 @@ func (i *Interactive) startTurnRequest(parent context.Context, prompt string, im | |||
| 381 | 381 | if recoverContextOverflow || shouldAutoCompact { | |
| 382 | 382 | i.resetStreamingStateLocked() | |
| 383 | 383 | } | |
| 384 | - | alertReason := mainAlertReason(ctx, err, lastTurnErr, lastStop, awaitingPre, hasNext || agentQueued > 0, offer, recoverContextOverflow, shouldAutoCompact) | |
| 384 | + | alertReason := mainAlertReason(ctx, err, lastTurnErr, lastStop, awaitingPre, hasNext || agentQueued > 0 || continueGoal, offer, recoverContextOverflow, shouldAutoCompact) | |
| 385 | 385 | i.busy = hasNext || continueQueued || continueStatusRescue || recoverContextOverflow || shouldAutoCompact | |
| 386 | 386 | i.mu.Unlock() | |
| 387 | 387 | if persistHandoff { | |
| @@ -417,6 +417,8 @@ func (i *Interactive) startTurnRequest(parent context.Context, prompt string, im | |||
| 417 | 417 | lastStop: lastStop, | |
| 418 | 418 | turnError: lastTurnErr, | |
| 419 | 419 | }) | |
| 420 | + | case continueGoal: | |
| 421 | + | i.requestGoalContinuationIfIdle(parent) | |
| 420 | 422 | } | |
| 421 | 423 | }() | |
| 422 | 424 | } | |
| @@ -323,7 +323,7 @@ func (i *Interactive) startTurnRequest(parent context.Context, prompt string, im | |||||
| 323 | if terminalGoalError { | 323 | if terminalGoalError { | ||
| 324 | i.updateActiveGoal(core.GoalBlocked, "turn ended with an error") | 324 | i.updateActiveGoal(core.GoalBlocked, "turn ended with an error") | ||
| 325 | } | 325 | } | ||
| 326 | - | i.finishGoalRun() | 326 | + | continueGoal := i.finishGoalRun() |
| 327 | i.mu.Lock() | 327 | i.mu.Lock() | ||
| 328 | awaitingPre := i.awaitingStartupPre | 328 | awaitingPre := i.awaitingStartupPre | ||
| 329 | // A newer explicit prompt may have cleared the handoff while the | 329 | // A newer explicit prompt may have cleared the handoff while the | ||
| @@ -381,7 +381,7 @@ func (i *Interactive) startTurnRequest(parent context.Context, prompt string, im | |||||
| 381 | if recoverContextOverflow || shouldAutoCompact { | 381 | if recoverContextOverflow || shouldAutoCompact { | ||
| 382 | i.resetStreamingStateLocked() | 382 | i.resetStreamingStateLocked() | ||
| 383 | } | 383 | } | ||
| 384 | - | alertReason := mainAlertReason(ctx, err, lastTurnErr, lastStop, awaitingPre, hasNext || agentQueued > 0, offer, recoverContextOverflow, shouldAutoCompact) | 384 | + | alertReason := mainAlertReason(ctx, err, lastTurnErr, lastStop, awaitingPre, hasNext || agentQueued > 0 || continueGoal, offer, recoverContextOverflow, shouldAutoCompact) |
| 385 | i.busy = hasNext || continueQueued || continueStatusRescue || recoverContextOverflow || shouldAutoCompact | 385 | i.busy = hasNext || continueQueued || continueStatusRescue || recoverContextOverflow || shouldAutoCompact | ||
| 386 | i.mu.Unlock() | 386 | i.mu.Unlock() | ||
| 387 | if persistHandoff { | 387 | if persistHandoff { | ||
| @@ -417,6 +417,8 @@ func (i *Interactive) startTurnRequest(parent context.Context, prompt string, im | |||||
| 417 | lastStop: lastStop, | 417 | lastStop: lastStop, | ||
| 418 | turnError: lastTurnErr, | 418 | turnError: lastTurnErr, | ||
| 419 | }) | 419 | }) | ||
| 420 | + | case continueGoal: | |||
| 421 | + | i.requestGoalContinuationIfIdle(parent) | |||
| 420 | } | 422 | } | ||
| 421 | }() | 423 | }() | ||
| 422 | } | 424 | } | ||