Merge pull request #89 from bnema/fix/issue-88-shutdown-result-origin
fix(subagents): attribute shutdown cancellations
10 files · +286 -62
M docs/subagents.md +1 -1
| @@ -115,7 +115,7 @@ During an active delegated turn, interactive mode shows compact rows beside the | |||
| 115 | 115 | ||
| 116 | 116 | The worker and supervisor communicate over newline-delimited JSON. Current messages use version `1` envelopes with `version`, `message_id`, `agent_id`, `turn_id`, `timestamp`, and `payload`. Unknown event names and payload fields are retained. Only versioned JSONL envelopes are accepted on the worker protocol. | |
| 117 | 117 | ||
| 118 | - | A completed worker emits a `turn.result` event and writes `result.json`. The host then injects `[auto-subagents update]`; that completion update, rather than polling or inspecting state, is the signal for the primary to continue. The inline output is bounded; after that update, inspect the full session through the stable references: | |
| 118 | + | A completed worker emits a `turn.result` event and writes `result.json`. Intentional cancellation records `shutdown_origin` as `targeted`, `session`, `deadline`, or `process`; manager updates describe that cancellation instead of reporting the worker as generically failed with `context canceled`. The host then injects `[auto-subagents update]`; that completion update, rather than polling or inspecting state, is the signal for the primary to continue. The inline output is bounded; after that update, inspect the full session through the stable references: | |
| 119 | 119 | ||
| 120 | 120 | If a provider rejects a child request because its payload or context window is too large, the worker compacts its persisted transcript and continues the same request once. If compaction or that retry cannot fit, the result uses error code `context_limit` and directs the caller to narrow the task or reduce gathered context; it does not include the provider's raw request error. | |
| 121 | 121 | ||
| @@ -115,7 +115,7 @@ During an active delegated turn, interactive mode shows compact rows beside the | |||||
| 115 | 115 | ||||
| 116 | The worker and supervisor communicate over newline-delimited JSON. Current messages use version `1` envelopes with `version`, `message_id`, `agent_id`, `turn_id`, `timestamp`, and `payload`. Unknown event names and payload fields are retained. Only versioned JSONL envelopes are accepted on the worker protocol. | 116 | The worker and supervisor communicate over newline-delimited JSON. Current messages use version `1` envelopes with `version`, `message_id`, `agent_id`, `turn_id`, `timestamp`, and `payload`. Unknown event names and payload fields are retained. Only versioned JSONL envelopes are accepted on the worker protocol. | ||
| 117 | 117 | ||||
| 118 | - | A completed worker emits a `turn.result` event and writes `result.json`. The host then injects `[auto-subagents update]`; that completion update, rather than polling or inspecting state, is the signal for the primary to continue. The inline output is bounded; after that update, inspect the full session through the stable references: | 118 | + | A completed worker emits a `turn.result` event and writes `result.json`. Intentional cancellation records `shutdown_origin` as `targeted`, `session`, `deadline`, or `process`; manager updates describe that cancellation instead of reporting the worker as generically failed with `context canceled`. The host then injects `[auto-subagents update]`; that completion update, rather than polling or inspecting state, is the signal for the primary to continue. The inline output is bounded; after that update, inspect the full session through the stable references: |
| 119 | 119 | ||||
| 120 | If a provider rejects a child request because its payload or context window is too large, the worker compacts its persisted transcript and continues the same request once. If compaction or that retry cannot fit, the result uses error code `context_limit` and directs the caller to narrow the task or reduce gathered context; it does not include the provider's raw request error. | 120 | If a provider rejects a child request because its payload or context window is too large, the worker compacts its persisted transcript and continues the same request once. If compaction or that retry cannot fit, the result uses error code `context_limit` and directs the caller to narrow the task or reduce gathered context; it does not include the provider's raw request error. | ||
| 121 | 121 | ||||
M packages/agent/modes/auto_subagents_test.go +32 -2
| @@ -158,8 +158,38 @@ func TestTrackStoppedSubagentWorkerDeliversTerminalUpdate(t *testing.T) { | |||
| 158 | 158 | if !strings.Contains(update, "task: investigate the stuck worker") { | |
| 159 | 159 | t.Fatalf("stop update missing task: %q", update) | |
| 160 | 160 | } | |
| 161 | - | if !strings.Contains(update, "status: killed") { | |
| 162 | - | t.Fatalf("stop update missing killed status: %q", update) | |
| 161 | + | if !strings.Contains(update, "status: cancelled") { | |
| 162 | + | t.Fatalf("stop update missing cancellation status: %q", update) | |
| 163 | + | } | |
| 164 | + | if !strings.Contains(update, "subagent stopped by request") || strings.Contains(update, "context canceled") { | |
| 165 | + | t.Fatalf("stop update missing attributed cancellation: %q", update) | |
| 166 | + | } | |
| 167 | + | } | |
| 168 | + | ||
| 169 | + | func TestTrackDeadlineFailureUsesAttributedResultError(t *testing.T) { | |
| 170 | + | mgr := subagents.New(subagents.Config{ | |
| 171 | + | Root: t.TempDir(), | |
| 172 | + | RepoRoot: t.TempDir(), | |
| 173 | + | NewRunner: func(*subagents.Agent) subagents.Runner { | |
| 174 | + | return subagents.RunnerFunc(func(context.Context, subagents.Sink) error { | |
| 175 | + | return context.DeadlineExceeded | |
| 176 | + | }) | |
| 177 | + | }, | |
| 178 | + | }) | |
| 179 | + | a, err := mgr.Spawn(context.Background(), "run until deadline") | |
| 180 | + | if err != nil { | |
| 181 | + | t.Fatal(err) | |
| 182 | + | } | |
| 183 | + | ||
| 184 | + | iv := newQueuedAutoSubagentsInteractive() | |
| 185 | + | iv.TrackSubagentWorker(a, a.Task) | |
| 186 | + | update := waitForQueuedPrompt(t, iv) | |
| 187 | + | if !strings.Contains(update, "status: failed") { | |
| 188 | + | t.Fatalf("deadline update changed failed status: %q", update) | |
| 189 | + | } | |
| 190 | + | want := subagents.ShutdownResultError(subagents.ShutdownOriginDeadline).Message | |
| 191 | + | if !strings.Contains(update, want) { | |
| 192 | + | t.Fatalf("deadline update missing attributed result error: %q", update) | |
| 163 | 193 | } | |
| 164 | 194 | } | |
| 165 | 195 | ||
| @@ -158,8 +158,38 @@ func TestTrackStoppedSubagentWorkerDeliversTerminalUpdate(t *testing.T) { | |||||
| 158 | if !strings.Contains(update, "task: investigate the stuck worker") { | 158 | if !strings.Contains(update, "task: investigate the stuck worker") { | ||
| 159 | t.Fatalf("stop update missing task: %q", update) | 159 | t.Fatalf("stop update missing task: %q", update) | ||
| 160 | } | 160 | } | ||
| 161 | - | if !strings.Contains(update, "status: killed") { | 161 | + | if !strings.Contains(update, "status: cancelled") { |
| 162 | - | t.Fatalf("stop update missing killed status: %q", update) | 162 | + | t.Fatalf("stop update missing cancellation status: %q", update) |
| 163 | + | } | |||
| 164 | + | if !strings.Contains(update, "subagent stopped by request") || strings.Contains(update, "context canceled") { | |||
| 165 | + | t.Fatalf("stop update missing attributed cancellation: %q", update) | |||
| 166 | + | } | |||
| 167 | + | } | |||
| 168 | + | ||||
| 169 | + | func TestTrackDeadlineFailureUsesAttributedResultError(t *testing.T) { | |||
| 170 | + | mgr := subagents.New(subagents.Config{ | |||
| 171 | + | Root: t.TempDir(), | |||
| 172 | + | RepoRoot: t.TempDir(), | |||
| 173 | + | NewRunner: func(*subagents.Agent) subagents.Runner { | |||
| 174 | + | return subagents.RunnerFunc(func(context.Context, subagents.Sink) error { | |||
| 175 | + | return context.DeadlineExceeded | |||
| 176 | + | }) | |||
| 177 | + | }, | |||
| 178 | + | }) | |||
| 179 | + | a, err := mgr.Spawn(context.Background(), "run until deadline") | |||
| 180 | + | if err != nil { | |||
| 181 | + | t.Fatal(err) | |||
| 182 | + | } | |||
| 183 | + | ||||
| 184 | + | iv := newQueuedAutoSubagentsInteractive() | |||
| 185 | + | iv.TrackSubagentWorker(a, a.Task) | |||
| 186 | + | update := waitForQueuedPrompt(t, iv) | |||
| 187 | + | if !strings.Contains(update, "status: failed") { | |||
| 188 | + | t.Fatalf("deadline update changed failed status: %q", update) | |||
| 189 | + | } | |||
| 190 | + | want := subagents.ShutdownResultError(subagents.ShutdownOriginDeadline).Message | |||
| 191 | + | if !strings.Contains(update, want) { | |||
| 192 | + | t.Fatalf("deadline update missing attributed result error: %q", update) | |||
| 163 | } | 193 | } | ||
| 164 | } | 194 | } | ||
| 165 | 195 | ||||
M packages/agent/modes/interactive.go +12 -1
| @@ -8821,6 +8821,15 @@ func (i *Interactive) flushSupervisorSummary(batch []*subagentWatchEntry) { | |||
| 8821 | 8821 | if status == "" { | |
| 8822 | 8822 | status = string(snap.Status) | |
| 8823 | 8823 | } | |
| 8824 | + | resultErr := "" | |
| 8825 | + | if snap.Result != nil && snap.Result.ShutdownOrigin != "" { | |
| 8826 | + | if snap.Result.Status == subagents.ResultCanceled { | |
| 8827 | + | status = "cancelled" | |
| 8828 | + | } | |
| 8829 | + | if snap.Result.Error != nil { | |
| 8830 | + | resultErr = snap.Result.Error.Message | |
| 8831 | + | } | |
| 8832 | + | } | |
| 8824 | 8833 | task := e.task | |
| 8825 | 8834 | if task == "" || !e.followUp { | |
| 8826 | 8835 | task = snap.Task | |
| @@ -8830,7 +8839,9 @@ func (i *Interactive) flushSupervisorSummary(batch []*subagentWatchEntry) { | |||
| 8830 | 8839 | } | |
| 8831 | 8840 | fmt.Fprintf(&sb, "%d. agent %s - status: %s\n", idx+1, snap.ID, status) | |
| 8832 | 8841 | fmt.Fprintf(&sb, " task: %s\n", truncateForSummary(task, 240)) | |
| 8833 | - | if snap.Err != "" { | |
| 8842 | + | if resultErr != "" { | |
| 8843 | + | fmt.Fprintf(&sb, " cancellation: %s\n", truncateForSummary(resultErr, 240)) | |
| 8844 | + | } else if snap.Err != "" { | |
| 8834 | 8845 | fmt.Fprintf(&sb, " error: %s\n", truncateForSummary(snap.Err, 240)) | |
| 8835 | 8846 | } else if e.err != "" { | |
| 8836 | 8847 | fmt.Fprintf(&sb, " turn error: %s\n", truncateForSummary(e.err, 240)) | |
| @@ -8821,6 +8821,15 @@ func (i *Interactive) flushSupervisorSummary(batch []*subagentWatchEntry) { | |||||
| 8821 | if status == "" { | 8821 | if status == "" { | ||
| 8822 | status = string(snap.Status) | 8822 | status = string(snap.Status) | ||
| 8823 | } | 8823 | } | ||
| 8824 | + | resultErr := "" | |||
| 8825 | + | if snap.Result != nil && snap.Result.ShutdownOrigin != "" { | |||
| 8826 | + | if snap.Result.Status == subagents.ResultCanceled { | |||
| 8827 | + | status = "cancelled" | |||
| 8828 | + | } | |||
| 8829 | + | if snap.Result.Error != nil { | |||
| 8830 | + | resultErr = snap.Result.Error.Message | |||
| 8831 | + | } | |||
| 8832 | + | } | |||
| 8824 | task := e.task | 8833 | task := e.task | ||
| 8825 | if task == "" || !e.followUp { | 8834 | if task == "" || !e.followUp { | ||
| 8826 | task = snap.Task | 8835 | task = snap.Task | ||
| @@ -8830,7 +8839,9 @@ func (i *Interactive) flushSupervisorSummary(batch []*subagentWatchEntry) { | |||||
| 8830 | } | 8839 | } | ||
| 8831 | fmt.Fprintf(&sb, "%d. agent %s - status: %s\n", idx+1, snap.ID, status) | 8840 | fmt.Fprintf(&sb, "%d. agent %s - status: %s\n", idx+1, snap.ID, status) | ||
| 8832 | fmt.Fprintf(&sb, " task: %s\n", truncateForSummary(task, 240)) | 8841 | fmt.Fprintf(&sb, " task: %s\n", truncateForSummary(task, 240)) | ||
| 8833 | - | if snap.Err != "" { | 8842 | + | if resultErr != "" { |
| 8843 | + | fmt.Fprintf(&sb, " cancellation: %s\n", truncateForSummary(resultErr, 240)) | |||
| 8844 | + | } else if snap.Err != "" { | |||
| 8834 | fmt.Fprintf(&sb, " error: %s\n", truncateForSummary(snap.Err, 240)) | 8845 | fmt.Fprintf(&sb, " error: %s\n", truncateForSummary(snap.Err, 240)) | ||
| 8835 | } else if e.err != "" { | 8846 | } else if e.err != "" { | ||
| 8836 | fmt.Fprintf(&sb, " turn error: %s\n", truncateForSummary(e.err, 240)) | 8847 | fmt.Fprintf(&sb, " turn error: %s\n", truncateForSummary(e.err, 240)) | ||
M packages/agent/subagent_worker.go +31 -9
| @@ -354,21 +354,33 @@ func runSubagentWorkerMode(ctx context.Context, args Args, version string) (runE | |||
| 354 | 354 | resultStatus = "failed" | |
| 355 | 355 | } | |
| 356 | 356 | } | |
| 357 | + | mu.Lock() | |
| 358 | + | resultShutdownOrigin := shutdownOrigin.Sanitized() | |
| 359 | + | mu.Unlock() | |
| 360 | + | switch { | |
| 361 | + | case errors.Is(err, context.DeadlineExceeded): | |
| 362 | + | resultShutdownOrigin = subagents.ShutdownOriginDeadline | |
| 363 | + | case !errors.Is(err, context.Canceled): | |
| 364 | + | resultShutdownOrigin = "" | |
| 365 | + | } | |
| 357 | 366 | resultPayload := map[string]any{ | |
| 358 | 367 | "status": resultStatus, | |
| 359 | 368 | "turn_id": turnID, | |
| 360 | 369 | "summary": boundedResultSummary(output), | |
| 361 | 370 | "output": boundedResultOutput(output, maxOutputBytes, maxOutputLines), | |
| 362 | - | "error": resultErrorPayload(err), | |
| 371 | + | "error": resultErrorPayload(err, resultShutdownOrigin), | |
| 372 | + | } | |
| 373 | + | if resultShutdownOrigin != "" { | |
| 374 | + | resultPayload["shutdown_origin"] = resultShutdownOrigin | |
| 363 | 375 | } | |
| 364 | 376 | em.emit("turn.result", resultPayload) | |
| 365 | 377 | if err != nil { | |
| 366 | - | em.emit("turn.failed", map[string]any{"turn_id": turnID, "error": resultErrorPayload(err)}) | |
| 378 | + | em.emit("turn.failed", map[string]any{"turn_id": turnID, "error": resultErrorPayload(err, resultShutdownOrigin)}) | |
| 367 | 379 | } | |
| 368 | 380 | em.emit("turn_end", map[string]any{ | |
| 369 | 381 | "step": step, | |
| 370 | 382 | "turn_id": turnID, | |
| 371 | - | "error": turnErrorMessage(err), | |
| 383 | + | "error": turnErrorMessage(err, resultShutdownOrigin), | |
| 372 | 384 | }) | |
| 373 | 385 | ||
| 374 | 386 | cancel() | |
| @@ -413,11 +425,11 @@ func runSubagentWorkerMode(ctx context.Context, args Args, version string) (runE | |||
| 413 | 425 | } | |
| 414 | 426 | ||
| 415 | 427 | requestShutdown := func(origin subagents.ShutdownOrigin) { | |
| 416 | - | mu.Lock() | |
| 417 | - | closing = true | |
| 418 | - | mu.Unlock() | |
| 419 | 428 | shutdownOnce.Do(func() { | |
| 429 | + | mu.Lock() | |
| 430 | + | closing = true | |
| 420 | 431 | shutdownOrigin = origin.Sanitized() | |
| 432 | + | mu.Unlock() | |
| 421 | 433 | close(shutdown) | |
| 422 | 434 | }) | |
| 423 | 435 | } | |
| @@ -725,23 +737,33 @@ func truncateOutputUTF8(value string, maxBytes int) string { | |||
| 725 | 737 | return string(data) | |
| 726 | 738 | } | |
| 727 | 739 | ||
| 728 | - | func resultErrorPayload(err error) map[string]any { | |
| 740 | + | func resultErrorPayload(err error, shutdownOrigin subagents.ShutdownOrigin) map[string]any { | |
| 729 | 741 | if err == nil { | |
| 730 | 742 | return nil | |
| 731 | 743 | } | |
| 732 | 744 | if provider.IsContextOverflowError(err) { | |
| 733 | 745 | return map[string]any{"code": "context_limit", "message": subagentContextLimitMessage} | |
| 734 | 746 | } | |
| 735 | - | if errors.Is(err, context.DeadlineExceeded) { | |
| 747 | + | if errors.Is(err, context.DeadlineExceeded) || shutdownOrigin == subagents.ShutdownOriginDeadline { | |
| 736 | 748 | return map[string]any{"code": "deadline_exceeded", "message": "subagent turn deadline exceeded; partial output is preserved in the result and history"} | |
| 737 | 749 | } | |
| 750 | + | if errors.Is(err, context.Canceled) { | |
| 751 | + | if resultErr := subagents.ShutdownResultError(shutdownOrigin); resultErr != nil { | |
| 752 | + | return map[string]any{"code": resultErr.Code, "message": resultErr.Message} | |
| 753 | + | } | |
| 754 | + | } | |
| 738 | 755 | return map[string]any{"code": "turn_failed", "message": truncateForLog(err.Error(), 500)} | |
| 739 | 756 | } | |
| 740 | 757 | ||
| 741 | - | func turnErrorMessage(err error) string { | |
| 758 | + | func turnErrorMessage(err error, shutdownOrigin subagents.ShutdownOrigin) string { | |
| 742 | 759 | if provider.IsContextOverflowError(err) { | |
| 743 | 760 | return subagentContextLimitMessage | |
| 744 | 761 | } | |
| 762 | + | if err != nil { | |
| 763 | + | if resultErr := subagents.ShutdownResultError(shutdownOrigin); resultErr != nil { | |
| 764 | + | return resultErr.Message | |
| 765 | + | } | |
| 766 | + | } | |
| 745 | 767 | return errString(err) | |
| 746 | 768 | } | |
| 747 | 769 | ||
| @@ -354,21 +354,33 @@ func runSubagentWorkerMode(ctx context.Context, args Args, version string) (runE | |||||
| 354 | resultStatus = "failed" | 354 | resultStatus = "failed" | ||
| 355 | } | 355 | } | ||
| 356 | } | 356 | } | ||
| 357 | + | mu.Lock() | |||
| 358 | + | resultShutdownOrigin := shutdownOrigin.Sanitized() | |||
| 359 | + | mu.Unlock() | |||
| 360 | + | switch { | |||
| 361 | + | case errors.Is(err, context.DeadlineExceeded): | |||
| 362 | + | resultShutdownOrigin = subagents.ShutdownOriginDeadline | |||
| 363 | + | case !errors.Is(err, context.Canceled): | |||
| 364 | + | resultShutdownOrigin = "" | |||
| 365 | + | } | |||
| 357 | resultPayload := map[string]any{ | 366 | resultPayload := map[string]any{ | ||
| 358 | "status": resultStatus, | 367 | "status": resultStatus, | ||
| 359 | "turn_id": turnID, | 368 | "turn_id": turnID, | ||
| 360 | "summary": boundedResultSummary(output), | 369 | "summary": boundedResultSummary(output), | ||
| 361 | "output": boundedResultOutput(output, maxOutputBytes, maxOutputLines), | 370 | "output": boundedResultOutput(output, maxOutputBytes, maxOutputLines), | ||
| 362 | - | "error": resultErrorPayload(err), | 371 | + | "error": resultErrorPayload(err, resultShutdownOrigin), |
| 372 | + | } | |||
| 373 | + | if resultShutdownOrigin != "" { | |||
| 374 | + | resultPayload["shutdown_origin"] = resultShutdownOrigin | |||
| 363 | } | 375 | } | ||
| 364 | em.emit("turn.result", resultPayload) | 376 | em.emit("turn.result", resultPayload) | ||
| 365 | if err != nil { | 377 | if err != nil { | ||
| 366 | - | em.emit("turn.failed", map[string]any{"turn_id": turnID, "error": resultErrorPayload(err)}) | 378 | + | em.emit("turn.failed", map[string]any{"turn_id": turnID, "error": resultErrorPayload(err, resultShutdownOrigin)}) |
| 367 | } | 379 | } | ||
| 368 | em.emit("turn_end", map[string]any{ | 380 | em.emit("turn_end", map[string]any{ | ||
| 369 | "step": step, | 381 | "step": step, | ||
| 370 | "turn_id": turnID, | 382 | "turn_id": turnID, | ||
| 371 | - | "error": turnErrorMessage(err), | 383 | + | "error": turnErrorMessage(err, resultShutdownOrigin), |
| 372 | }) | 384 | }) | ||
| 373 | 385 | ||||
| 374 | cancel() | 386 | cancel() | ||
| @@ -413,11 +425,11 @@ func runSubagentWorkerMode(ctx context.Context, args Args, version string) (runE | |||||
| 413 | } | 425 | } | ||
| 414 | 426 | ||||
| 415 | requestShutdown := func(origin subagents.ShutdownOrigin) { | 427 | requestShutdown := func(origin subagents.ShutdownOrigin) { | ||
| 416 | - | mu.Lock() | |||
| 417 | - | closing = true | |||
| 418 | - | mu.Unlock() | |||
| 419 | shutdownOnce.Do(func() { | 428 | shutdownOnce.Do(func() { | ||
| 429 | + | mu.Lock() | |||
| 430 | + | closing = true | |||
| 420 | shutdownOrigin = origin.Sanitized() | 431 | shutdownOrigin = origin.Sanitized() | ||
| 432 | + | mu.Unlock() | |||
| 421 | close(shutdown) | 433 | close(shutdown) | ||
| 422 | }) | 434 | }) | ||
| 423 | } | 435 | } | ||
| @@ -725,23 +737,33 @@ func truncateOutputUTF8(value string, maxBytes int) string { | |||||
| 725 | return string(data) | 737 | return string(data) | ||
| 726 | } | 738 | } | ||
| 727 | 739 | ||||
| 728 | - | func resultErrorPayload(err error) map[string]any { | 740 | + | func resultErrorPayload(err error, shutdownOrigin subagents.ShutdownOrigin) map[string]any { |
| 729 | if err == nil { | 741 | if err == nil { | ||
| 730 | return nil | 742 | return nil | ||
| 731 | } | 743 | } | ||
| 732 | if provider.IsContextOverflowError(err) { | 744 | if provider.IsContextOverflowError(err) { | ||
| 733 | return map[string]any{"code": "context_limit", "message": subagentContextLimitMessage} | 745 | return map[string]any{"code": "context_limit", "message": subagentContextLimitMessage} | ||
| 734 | } | 746 | } | ||
| 735 | - | if errors.Is(err, context.DeadlineExceeded) { | 747 | + | if errors.Is(err, context.DeadlineExceeded) || shutdownOrigin == subagents.ShutdownOriginDeadline { |
| 736 | return map[string]any{"code": "deadline_exceeded", "message": "subagent turn deadline exceeded; partial output is preserved in the result and history"} | 748 | return map[string]any{"code": "deadline_exceeded", "message": "subagent turn deadline exceeded; partial output is preserved in the result and history"} | ||
| 737 | } | 749 | } | ||
| 750 | + | if errors.Is(err, context.Canceled) { | |||
| 751 | + | if resultErr := subagents.ShutdownResultError(shutdownOrigin); resultErr != nil { | |||
| 752 | + | return map[string]any{"code": resultErr.Code, "message": resultErr.Message} | |||
| 753 | + | } | |||
| 754 | + | } | |||
| 738 | return map[string]any{"code": "turn_failed", "message": truncateForLog(err.Error(), 500)} | 755 | return map[string]any{"code": "turn_failed", "message": truncateForLog(err.Error(), 500)} | ||
| 739 | } | 756 | } | ||
| 740 | 757 | ||||
| 741 | - | func turnErrorMessage(err error) string { | 758 | + | func turnErrorMessage(err error, shutdownOrigin subagents.ShutdownOrigin) string { |
| 742 | if provider.IsContextOverflowError(err) { | 759 | if provider.IsContextOverflowError(err) { | ||
| 743 | return subagentContextLimitMessage | 760 | return subagentContextLimitMessage | ||
| 744 | } | 761 | } | ||
| 762 | + | if err != nil { | |||
| 763 | + | if resultErr := subagents.ShutdownResultError(shutdownOrigin); resultErr != nil { | |||
| 764 | + | return resultErr.Message | |||
| 765 | + | } | |||
| 766 | + | } | |||
| 745 | return errString(err) | 767 | return errString(err) | ||
| 746 | } | 768 | } | ||
| 747 | 769 | ||||
M packages/agent/subagent_worker_test.go +10 -3
| @@ -53,12 +53,19 @@ func TestSubagentTurnContextResetsDeadlineAndHonorsShutdown(t *testing.T) { | |||
| 53 | 53 | } | |
| 54 | 54 | ||
| 55 | 55 | func TestResultErrorPayloadClassifiesTurnDeadline(t *testing.T) { | |
| 56 | - | payload := resultErrorPayload(context.DeadlineExceeded) | |
| 56 | + | payload := resultErrorPayload(context.DeadlineExceeded, "") | |
| 57 | 57 | if payload["code"] != "deadline_exceeded" { | |
| 58 | 58 | t.Fatalf("deadline error code = %v, want deadline_exceeded", payload["code"]) | |
| 59 | 59 | } | |
| 60 | 60 | } | |
| 61 | 61 | ||
| 62 | + | func TestResultErrorPayloadAttributesShutdownCancellation(t *testing.T) { | |
| 63 | + | payload := resultErrorPayload(context.Canceled, subagents.ShutdownOriginSession) | |
| 64 | + | if payload["code"] != "shutdown" || payload["message"] != "subagent stopped during session shutdown" { | |
| 65 | + | t.Fatalf("session shutdown error = %#v", payload) | |
| 66 | + | } | |
| 67 | + | } | |
| 68 | + | ||
| 62 | 69 | func TestShutdownExitPayloadIncludesOnlyKnownOrigin(t *testing.T) { | |
| 63 | 70 | payload := shutdownExitPayload(subagents.ShutdownOriginSession) | |
| 64 | 71 | if payload["reason"] != "shutdown" || payload["origin"] != subagents.ShutdownOriginSession { | |
| @@ -257,7 +264,7 @@ func TestPromptWithContextRecoveryPropagatesCompactionFailure(t *testing.T) { | |||
| 257 | 264 | if err == nil || !strings.Contains(err.Error(), "compact transcript after initial overflow") { | |
| 258 | 265 | t.Fatalf("promptWithContextRecovery error = %v, want compaction failure", err) | |
| 259 | 266 | } | |
| 260 | - | payload := resultErrorPayload(err) | |
| 267 | + | payload := resultErrorPayload(err, "") | |
| 261 | 268 | if payload["code"] != "turn_failed" { | |
| 262 | 269 | t.Fatalf("result error payload = %#v, want turn_failed", payload) | |
| 263 | 270 | } | |
| @@ -736,7 +743,7 @@ func workerRequestUserTextCount(req provider.Request, want string) int { | |||
| 736 | 743 | } | |
| 737 | 744 | ||
| 738 | 745 | func TestResultErrorPayloadSanitizesContextOverflow(t *testing.T) { | |
| 739 | - | payload := resultErrorPayload(errors.New("provider error: input exceeds the context window; echoed task text")) | |
| 746 | + | payload := resultErrorPayload(errors.New("provider error: input exceeds the context window; echoed task text"), "") | |
| 740 | 747 | if payload["code"] != "context_limit" { | |
| 741 | 748 | t.Fatalf("error code = %q, want context_limit", payload["code"]) | |
| 742 | 749 | } | |
| @@ -53,12 +53,19 @@ func TestSubagentTurnContextResetsDeadlineAndHonorsShutdown(t *testing.T) { | |||||
| 53 | } | 53 | } | ||
| 54 | 54 | ||||
| 55 | func TestResultErrorPayloadClassifiesTurnDeadline(t *testing.T) { | 55 | func TestResultErrorPayloadClassifiesTurnDeadline(t *testing.T) { | ||
| 56 | - | payload := resultErrorPayload(context.DeadlineExceeded) | 56 | + | payload := resultErrorPayload(context.DeadlineExceeded, "") |
| 57 | if payload["code"] != "deadline_exceeded" { | 57 | if payload["code"] != "deadline_exceeded" { | ||
| 58 | t.Fatalf("deadline error code = %v, want deadline_exceeded", payload["code"]) | 58 | t.Fatalf("deadline error code = %v, want deadline_exceeded", payload["code"]) | ||
| 59 | } | 59 | } | ||
| 60 | } | 60 | } | ||
| 61 | 61 | ||||
| 62 | + | func TestResultErrorPayloadAttributesShutdownCancellation(t *testing.T) { | |||
| 63 | + | payload := resultErrorPayload(context.Canceled, subagents.ShutdownOriginSession) | |||
| 64 | + | if payload["code"] != "shutdown" || payload["message"] != "subagent stopped during session shutdown" { | |||
| 65 | + | t.Fatalf("session shutdown error = %#v", payload) | |||
| 66 | + | } | |||
| 67 | + | } | |||
| 68 | + | ||||
| 62 | func TestShutdownExitPayloadIncludesOnlyKnownOrigin(t *testing.T) { | 69 | func TestShutdownExitPayloadIncludesOnlyKnownOrigin(t *testing.T) { | ||
| 63 | payload := shutdownExitPayload(subagents.ShutdownOriginSession) | 70 | payload := shutdownExitPayload(subagents.ShutdownOriginSession) | ||
| 64 | if payload["reason"] != "shutdown" || payload["origin"] != subagents.ShutdownOriginSession { | 71 | if payload["reason"] != "shutdown" || payload["origin"] != subagents.ShutdownOriginSession { | ||
| @@ -257,7 +264,7 @@ func TestPromptWithContextRecoveryPropagatesCompactionFailure(t *testing.T) { | |||||
| 257 | if err == nil || !strings.Contains(err.Error(), "compact transcript after initial overflow") { | 264 | if err == nil || !strings.Contains(err.Error(), "compact transcript after initial overflow") { | ||
| 258 | t.Fatalf("promptWithContextRecovery error = %v, want compaction failure", err) | 265 | t.Fatalf("promptWithContextRecovery error = %v, want compaction failure", err) | ||
| 259 | } | 266 | } | ||
| 260 | - | payload := resultErrorPayload(err) | 267 | + | payload := resultErrorPayload(err, "") |
| 261 | if payload["code"] != "turn_failed" { | 268 | if payload["code"] != "turn_failed" { | ||
| 262 | t.Fatalf("result error payload = %#v, want turn_failed", payload) | 269 | t.Fatalf("result error payload = %#v, want turn_failed", payload) | ||
| 263 | } | 270 | } | ||
| @@ -736,7 +743,7 @@ func workerRequestUserTextCount(req provider.Request, want string) int { | |||||
| 736 | } | 743 | } | ||
| 737 | 744 | ||||
| 738 | func TestResultErrorPayloadSanitizesContextOverflow(t *testing.T) { | 745 | func TestResultErrorPayloadSanitizesContextOverflow(t *testing.T) { | ||
| 739 | - | payload := resultErrorPayload(errors.New("provider error: input exceeds the context window; echoed task text")) | 746 | + | payload := resultErrorPayload(errors.New("provider error: input exceeds the context window; echoed task text"), "") |
| 740 | if payload["code"] != "context_limit" { | 747 | if payload["code"] != "context_limit" { | ||
| 741 | t.Fatalf("error code = %q, want context_limit", payload["code"]) | 748 | t.Fatalf("error code = %q, want context_limit", payload["code"]) | ||
| 742 | } | 749 | } | ||
M packages/agent/subagents/agent.go +25 -0
| @@ -152,6 +152,7 @@ type Agent struct { | |||
| 152 | 152 | streamingAssistantLines int | |
| 153 | 153 | finished time.Time | |
| 154 | 154 | lastErr error | |
| 155 | + | shutdownOrigin ShutdownOrigin | |
| 155 | 156 | transcriptLoaded bool | |
| 156 | 157 | legacyEventState bool | |
| 157 | 158 | transcriptMu sync.Mutex | |
| @@ -410,6 +411,30 @@ func (a *Agent) setResult(result *TurnResult) { | |||
| 410 | 411 | a.lifecycleMu.Unlock() | |
| 411 | 412 | } | |
| 412 | 413 | ||
| 414 | + | func (a *Agent) shutdownOriginValue() ShutdownOrigin { | |
| 415 | + | a.mu.Lock() | |
| 416 | + | defer a.mu.Unlock() | |
| 417 | + | return a.shutdownOrigin | |
| 418 | + | } | |
| 419 | + | ||
| 420 | + | // prepareShutdown publishes the shutdown origin in the same critical section | |
| 421 | + | // as the killed status. Finalization can therefore observe either the previous | |
| 422 | + | // terminal state or a fully attributed cancellation, never an intermediate one. | |
| 423 | + | func (a *Agent) prepareShutdown(origin ShutdownOrigin) (Status, bool) { | |
| 424 | + | a.mu.Lock() | |
| 425 | + | defer a.mu.Unlock() | |
| 426 | + | status := a.status | |
| 427 | + | if status == StatusDone || status == StatusFailed || status == StatusKilled { | |
| 428 | + | return status, false | |
| 429 | + | } | |
| 430 | + | a.shutdownOrigin = origin.Sanitized() | |
| 431 | + | if status != StatusDetached { | |
| 432 | + | a.status = StatusKilled | |
| 433 | + | a.activity = "canceling" | |
| 434 | + | } | |
| 435 | + | return status, true | |
| 436 | + | } | |
| 437 | + | ||
| 413 | 438 | // Transcript returns a copy of the running transcript. Detached agents | |
| 414 | 439 | // hydrate their durable event history the first time it is requested. | |
| 415 | 440 | func (a *Agent) Transcript() []string { | |
| @@ -152,6 +152,7 @@ type Agent struct { | |||||
| 152 | streamingAssistantLines int | 152 | streamingAssistantLines int | ||
| 153 | finished time.Time | 153 | finished time.Time | ||
| 154 | lastErr error | 154 | lastErr error | ||
| 155 | + | shutdownOrigin ShutdownOrigin | |||
| 155 | transcriptLoaded bool | 156 | transcriptLoaded bool | ||
| 156 | legacyEventState bool | 157 | legacyEventState bool | ||
| 157 | transcriptMu sync.Mutex | 158 | transcriptMu sync.Mutex | ||
| @@ -410,6 +411,30 @@ func (a *Agent) setResult(result *TurnResult) { | |||||
| 410 | a.lifecycleMu.Unlock() | 411 | a.lifecycleMu.Unlock() | ||
| 411 | } | 412 | } | ||
| 412 | 413 | ||||
| 414 | + | func (a *Agent) shutdownOriginValue() ShutdownOrigin { | |||
| 415 | + | a.mu.Lock() | |||
| 416 | + | defer a.mu.Unlock() | |||
| 417 | + | return a.shutdownOrigin | |||
| 418 | + | } | |||
| 419 | + | ||||
| 420 | + | // prepareShutdown publishes the shutdown origin in the same critical section | |||
| 421 | + | // as the killed status. Finalization can therefore observe either the previous | |||
| 422 | + | // terminal state or a fully attributed cancellation, never an intermediate one. | |||
| 423 | + | func (a *Agent) prepareShutdown(origin ShutdownOrigin) (Status, bool) { | |||
| 424 | + | a.mu.Lock() | |||
| 425 | + | defer a.mu.Unlock() | |||
| 426 | + | status := a.status | |||
| 427 | + | if status == StatusDone || status == StatusFailed || status == StatusKilled { | |||
| 428 | + | return status, false | |||
| 429 | + | } | |||
| 430 | + | a.shutdownOrigin = origin.Sanitized() | |||
| 431 | + | if status != StatusDetached { | |||
| 432 | + | a.status = StatusKilled | |||
| 433 | + | a.activity = "canceling" | |||
| 434 | + | } | |||
| 435 | + | return status, true | |||
| 436 | + | } | |||
| 437 | + | ||||
| 413 | // Transcript returns a copy of the running transcript. Detached agents | 438 | // Transcript returns a copy of the running transcript. Detached agents | ||
| 414 | // hydrate their durable event history the first time it is requested. | 439 | // hydrate their durable event history the first time it is requested. | ||
| 415 | func (a *Agent) Transcript() []string { | 440 | func (a *Agent) Transcript() []string { | ||
M packages/agent/subagents/result.go +48 -24
| @@ -37,22 +37,40 @@ type ResultError struct { | |||
| 37 | 37 | Message string `json:"message,omitempty"` | |
| 38 | 38 | } | |
| 39 | 39 | ||
| 40 | + | // ShutdownResultError returns the stable, user-facing error associated with | |
| 41 | + | // an intentional worker shutdown. Unknown origins are not attributed. | |
| 42 | + | func ShutdownResultError(origin ShutdownOrigin) *ResultError { | |
| 43 | + | switch origin.Sanitized() { | |
| 44 | + | case ShutdownOriginTargeted: | |
| 45 | + | return &ResultError{Code: "shutdown", Message: "subagent stopped by request"} | |
| 46 | + | case ShutdownOriginSession: | |
| 47 | + | return &ResultError{Code: "shutdown", Message: "subagent stopped during session shutdown"} | |
| 48 | + | case ShutdownOriginDeadline: | |
| 49 | + | return &ResultError{Code: "deadline_exceeded", Message: "subagent turn deadline exceeded; partial output is preserved in the result and history"} | |
| 50 | + | case ShutdownOriginProcess: | |
| 51 | + | return &ResultError{Code: "shutdown", Message: "subagent stopped during process shutdown"} | |
| 52 | + | default: | |
| 53 | + | return nil | |
| 54 | + | } | |
| 55 | + | } | |
| 56 | + | ||
| 40 | 57 | // TurnResult is the protocol-level result for a delegated turn. Output is a | |
| 41 | 58 | // bounded inline preview; the full session/history remains available through | |
| 42 | 59 | // HistoryRef and the complete JSON result through ResultRef. | |
| 43 | 60 | type TurnResult struct { | |
| 44 | - | Version int `json:"version"` | |
| 45 | - | AgentID string `json:"agent_id,omitempty"` | |
| 46 | - | TurnID string `json:"turn_id,omitempty"` | |
| 47 | - | Status TurnStatus `json:"status"` | |
| 48 | - | Summary string `json:"summary,omitempty"` | |
| 49 | - | Output string `json:"output,omitempty"` | |
| 50 | - | Structured json.RawMessage `json:"structured,omitempty"` | |
| 51 | - | Artifacts []ArtifactRef `json:"artifacts,omitempty"` | |
| 52 | - | ChangedFiles []string `json:"changed_files,omitempty"` | |
| 53 | - | Usage map[string]any `json:"usage,omitempty"` | |
| 54 | - | Error *ResultError `json:"error,omitempty"` | |
| 55 | - | CreatedAt time.Time `json:"created_at"` | |
| 61 | + | Version int `json:"version"` | |
| 62 | + | AgentID string `json:"agent_id,omitempty"` | |
| 63 | + | TurnID string `json:"turn_id,omitempty"` | |
| 64 | + | Status TurnStatus `json:"status"` | |
| 65 | + | Summary string `json:"summary,omitempty"` | |
| 66 | + | Output string `json:"output,omitempty"` | |
| 67 | + | Structured json.RawMessage `json:"structured,omitempty"` | |
| 68 | + | Artifacts []ArtifactRef `json:"artifacts,omitempty"` | |
| 69 | + | ChangedFiles []string `json:"changed_files,omitempty"` | |
| 70 | + | Usage map[string]any `json:"usage,omitempty"` | |
| 71 | + | Error *ResultError `json:"error,omitempty"` | |
| 72 | + | ShutdownOrigin ShutdownOrigin `json:"shutdown_origin,omitempty"` | |
| 73 | + | CreatedAt time.Time `json:"created_at"` | |
| 56 | 74 | } | |
| 57 | 75 | ||
| 58 | 76 | func (r *TurnResult) Validate(maxBytes, maxLines int) error { | |
| @@ -71,6 +89,9 @@ func (r *TurnResult) Validate(maxBytes, maxLines int) error { | |||
| 71 | 89 | if r.AgentID == "" { | |
| 72 | 90 | return errors.New("subagents: result missing agent id") | |
| 73 | 91 | } | |
| 92 | + | if r.ShutdownOrigin != "" && r.ShutdownOrigin.Sanitized() == "" { | |
| 93 | + | return errors.New("subagents: result has invalid shutdown origin") | |
| 94 | + | } | |
| 74 | 95 | if r.TurnID == "" { | |
| 75 | 96 | return errors.New("subagents: result missing turn id") | |
| 76 | 97 | } | |
| @@ -443,6 +464,10 @@ func (f *Supervisor) ensureResult(a *Agent, status Status, runErr error) { | |||
| 443 | 464 | return | |
| 444 | 465 | } | |
| 445 | 466 | attempt := a.AttemptValue() | |
| 467 | + | shutdownOrigin := a.shutdownOriginValue() | |
| 468 | + | if errors.Is(runErr, context.DeadlineExceeded) { | |
| 469 | + | shutdownOrigin = ShutdownOriginDeadline | |
| 470 | + | } | |
| 446 | 471 | a.mu.Lock() | |
| 447 | 472 | partialOutput := a.lastAssistant | |
| 448 | 473 | a.mu.Unlock() | |
| @@ -515,6 +540,15 @@ func (f *Supervisor) ensureResult(a *Agent, status Status, runErr error) { | |||
| 515 | 540 | if result.CreatedAt.IsZero() { | |
| 516 | 541 | result.CreatedAt = time.Now().UTC() | |
| 517 | 542 | } | |
| 543 | + | if shutdownOrigin != "" { | |
| 544 | + | result.ShutdownOrigin = shutdownOrigin | |
| 545 | + | result.Error = ShutdownResultError(shutdownOrigin) | |
| 546 | + | if shutdownOrigin == ShutdownOriginDeadline { | |
| 547 | + | result.Status = ResultFailed | |
| 548 | + | } else { | |
| 549 | + | result.Status = ResultCanceled | |
| 550 | + | } | |
| 551 | + | } | |
| 518 | 552 | if runErr != nil { | |
| 519 | 553 | if status == StatusKilled || errors.Is(runErr, context.Canceled) { | |
| 520 | 554 | result.Status = ResultCanceled | |
| @@ -522,18 +556,8 @@ func (f *Supervisor) ensureResult(a *Agent, status Status, runErr error) { | |||
| 522 | 556 | result.Status = ResultFailed | |
| 523 | 557 | } | |
| 524 | 558 | if errors.Is(runErr, context.DeadlineExceeded) { | |
| 525 | - | // The parent deadline is authoritative for terminal categorization, so | |
| 526 | - | // it intentionally overrides any child result error, including | |
| 527 | - | // context_limit. The worker was first asked to stop cleanly, and any | |
| 528 | - | // streamed output remains in this result and HistoryRef(agentID). | |
| 529 | - | message := "subagent deadline exceeded; inspect history for received output" | |
| 530 | - | if result.Output != "" { | |
| 531 | - | message = "subagent deadline exceeded; partial output is preserved in the result and history" | |
| 532 | - | } | |
| 533 | - | result.Error = &ResultError{ | |
| 534 | - | Code: "deadline_exceeded", | |
| 535 | - | Message: message, | |
| 536 | - | } | |
| 559 | + | result.Status = ResultFailed | |
| 560 | + | result.ShutdownOrigin = ShutdownOriginDeadline | |
| 537 | 561 | } else if result.Error == nil { | |
| 538 | 562 | result.Error = &ResultError{Code: "runner_failed", Message: truncate(runErr.Error(), 500)} | |
| 539 | 563 | } | |
| @@ -37,22 +37,40 @@ type ResultError struct { | |||||
| 37 | Message string `json:"message,omitempty"` | 37 | Message string `json:"message,omitempty"` | ||
| 38 | } | 38 | } | ||
| 39 | 39 | ||||
| 40 | + | // ShutdownResultError returns the stable, user-facing error associated with | |||
| 41 | + | // an intentional worker shutdown. Unknown origins are not attributed. | |||
| 42 | + | func ShutdownResultError(origin ShutdownOrigin) *ResultError { | |||
| 43 | + | switch origin.Sanitized() { | |||
| 44 | + | case ShutdownOriginTargeted: | |||
| 45 | + | return &ResultError{Code: "shutdown", Message: "subagent stopped by request"} | |||
| 46 | + | case ShutdownOriginSession: | |||
| 47 | + | return &ResultError{Code: "shutdown", Message: "subagent stopped during session shutdown"} | |||
| 48 | + | case ShutdownOriginDeadline: | |||
| 49 | + | return &ResultError{Code: "deadline_exceeded", Message: "subagent turn deadline exceeded; partial output is preserved in the result and history"} | |||
| 50 | + | case ShutdownOriginProcess: | |||
| 51 | + | return &ResultError{Code: "shutdown", Message: "subagent stopped during process shutdown"} | |||
| 52 | + | default: | |||
| 53 | + | return nil | |||
| 54 | + | } | |||
| 55 | + | } | |||
| 56 | + | ||||
| 40 | // TurnResult is the protocol-level result for a delegated turn. Output is a | 57 | // TurnResult is the protocol-level result for a delegated turn. Output is a | ||
| 41 | // bounded inline preview; the full session/history remains available through | 58 | // bounded inline preview; the full session/history remains available through | ||
| 42 | // HistoryRef and the complete JSON result through ResultRef. | 59 | // HistoryRef and the complete JSON result through ResultRef. | ||
| 43 | type TurnResult struct { | 60 | type TurnResult struct { | ||
| 44 | - | Version int `json:"version"` | 61 | + | Version int `json:"version"` |
| 45 | - | AgentID string `json:"agent_id,omitempty"` | 62 | + | AgentID string `json:"agent_id,omitempty"` |
| 46 | - | TurnID string `json:"turn_id,omitempty"` | 63 | + | TurnID string `json:"turn_id,omitempty"` |
| 47 | - | Status TurnStatus `json:"status"` | 64 | + | Status TurnStatus `json:"status"` |
| 48 | - | Summary string `json:"summary,omitempty"` | 65 | + | Summary string `json:"summary,omitempty"` |
| 49 | - | Output string `json:"output,omitempty"` | 66 | + | Output string `json:"output,omitempty"` |
| 50 | - | Structured json.RawMessage `json:"structured,omitempty"` | 67 | + | Structured json.RawMessage `json:"structured,omitempty"` |
| 51 | - | Artifacts []ArtifactRef `json:"artifacts,omitempty"` | 68 | + | Artifacts []ArtifactRef `json:"artifacts,omitempty"` |
| 52 | - | ChangedFiles []string `json:"changed_files,omitempty"` | 69 | + | ChangedFiles []string `json:"changed_files,omitempty"` |
| 53 | - | Usage map[string]any `json:"usage,omitempty"` | 70 | + | Usage map[string]any `json:"usage,omitempty"` |
| 54 | - | Error *ResultError `json:"error,omitempty"` | 71 | + | Error *ResultError `json:"error,omitempty"` |
| 55 | - | CreatedAt time.Time `json:"created_at"` | 72 | + | ShutdownOrigin ShutdownOrigin `json:"shutdown_origin,omitempty"` |
| 73 | + | CreatedAt time.Time `json:"created_at"` | |||
| 56 | } | 74 | } | ||
| 57 | 75 | ||||
| 58 | func (r *TurnResult) Validate(maxBytes, maxLines int) error { | 76 | func (r *TurnResult) Validate(maxBytes, maxLines int) error { | ||
| @@ -71,6 +89,9 @@ func (r *TurnResult) Validate(maxBytes, maxLines int) error { | |||||
| 71 | if r.AgentID == "" { | 89 | if r.AgentID == "" { | ||
| 72 | return errors.New("subagents: result missing agent id") | 90 | return errors.New("subagents: result missing agent id") | ||
| 73 | } | 91 | } | ||
| 92 | + | if r.ShutdownOrigin != "" && r.ShutdownOrigin.Sanitized() == "" { | |||
| 93 | + | return errors.New("subagents: result has invalid shutdown origin") | |||
| 94 | + | } | |||
| 74 | if r.TurnID == "" { | 95 | if r.TurnID == "" { | ||
| 75 | return errors.New("subagents: result missing turn id") | 96 | return errors.New("subagents: result missing turn id") | ||
| 76 | } | 97 | } | ||
| @@ -443,6 +464,10 @@ func (f *Supervisor) ensureResult(a *Agent, status Status, runErr error) { | |||||
| 443 | return | 464 | return | ||
| 444 | } | 465 | } | ||
| 445 | attempt := a.AttemptValue() | 466 | attempt := a.AttemptValue() | ||
| 467 | + | shutdownOrigin := a.shutdownOriginValue() | |||
| 468 | + | if errors.Is(runErr, context.DeadlineExceeded) { | |||
| 469 | + | shutdownOrigin = ShutdownOriginDeadline | |||
| 470 | + | } | |||
| 446 | a.mu.Lock() | 471 | a.mu.Lock() | ||
| 447 | partialOutput := a.lastAssistant | 472 | partialOutput := a.lastAssistant | ||
| 448 | a.mu.Unlock() | 473 | a.mu.Unlock() | ||
| @@ -515,6 +540,15 @@ func (f *Supervisor) ensureResult(a *Agent, status Status, runErr error) { | |||||
| 515 | if result.CreatedAt.IsZero() { | 540 | if result.CreatedAt.IsZero() { | ||
| 516 | result.CreatedAt = time.Now().UTC() | 541 | result.CreatedAt = time.Now().UTC() | ||
| 517 | } | 542 | } | ||
| 543 | + | if shutdownOrigin != "" { | |||
| 544 | + | result.ShutdownOrigin = shutdownOrigin | |||
| 545 | + | result.Error = ShutdownResultError(shutdownOrigin) | |||
| 546 | + | if shutdownOrigin == ShutdownOriginDeadline { | |||
| 547 | + | result.Status = ResultFailed | |||
| 548 | + | } else { | |||
| 549 | + | result.Status = ResultCanceled | |||
| 550 | + | } | |||
| 551 | + | } | |||
| 518 | if runErr != nil { | 552 | if runErr != nil { | ||
| 519 | if status == StatusKilled || errors.Is(runErr, context.Canceled) { | 553 | if status == StatusKilled || errors.Is(runErr, context.Canceled) { | ||
| 520 | result.Status = ResultCanceled | 554 | result.Status = ResultCanceled | ||
| @@ -522,18 +556,8 @@ func (f *Supervisor) ensureResult(a *Agent, status Status, runErr error) { | |||||
| 522 | result.Status = ResultFailed | 556 | result.Status = ResultFailed | ||
| 523 | } | 557 | } | ||
| 524 | if errors.Is(runErr, context.DeadlineExceeded) { | 558 | if errors.Is(runErr, context.DeadlineExceeded) { | ||
| 525 | - | // The parent deadline is authoritative for terminal categorization, so | 559 | + | result.Status = ResultFailed |
| 526 | - | // it intentionally overrides any child result error, including | 560 | + | result.ShutdownOrigin = ShutdownOriginDeadline |
| 527 | - | // context_limit. The worker was first asked to stop cleanly, and any | |||
| 528 | - | // streamed output remains in this result and HistoryRef(agentID). | |||
| 529 | - | message := "subagent deadline exceeded; inspect history for received output" | |||
| 530 | - | if result.Output != "" { | |||
| 531 | - | message = "subagent deadline exceeded; partial output is preserved in the result and history" | |||
| 532 | - | } | |||
| 533 | - | result.Error = &ResultError{ | |||
| 534 | - | Code: "deadline_exceeded", | |||
| 535 | - | Message: message, | |||
| 536 | - | } | |||
| 537 | } else if result.Error == nil { | 561 | } else if result.Error == nil { | ||
| 538 | result.Error = &ResultError{Code: "runner_failed", Message: truncate(runErr.Error(), 500)} | 562 | result.Error = &ResultError{Code: "runner_failed", Message: truncate(runErr.Error(), 500)} | ||
| 539 | } | 563 | } | ||
M packages/agent/subagents/result_test.go +44 -14
| @@ -33,6 +33,20 @@ func TestTurnResultValidationAndBoundedOutput(t *testing.T) { | |||
| 33 | 33 | } | |
| 34 | 34 | } | |
| 35 | 35 | ||
| 36 | + | func TestTurnResultValidatesShutdownOrigin(t *testing.T) { | |
| 37 | + | result := &TurnResult{ | |
| 38 | + | Version: ProtocolVersion, AgentID: "a", TurnID: "t", Status: ResultCanceled, | |
| 39 | + | ShutdownOrigin: ShutdownOriginSession, | |
| 40 | + | } | |
| 41 | + | if err := result.Validate(0, 0); err != nil { | |
| 42 | + | t.Fatalf("known shutdown origin was rejected: %v", err) | |
| 43 | + | } | |
| 44 | + | result.ShutdownOrigin = ShutdownOrigin("private detail") | |
| 45 | + | if err := result.Validate(0, 0); err == nil { | |
| 46 | + | t.Fatal("unknown shutdown origin was accepted") | |
| 47 | + | } | |
| 48 | + | } | |
| 49 | + | ||
| 36 | 50 | func TestTurnResultBoundedHonorsLineAndUTF8Limits(t *testing.T) { | |
| 37 | 51 | result := &TurnResult{ | |
| 38 | 52 | Version: ProtocolVersion, AgentID: "a", TurnID: "t", Status: ResultSucceeded, | |
| @@ -108,11 +122,14 @@ func TestDecodeTurnResultEventValidatesIdentityAndStatus(t *testing.T) { | |||
| 108 | 122 | if _, err := decodeTurnResultEvent(mismatched, "agent-1", 100, 10); err == nil { | |
| 109 | 123 | t.Fatal("mismatched result agent was accepted") | |
| 110 | 124 | } | |
| 111 | - | valid := NewEvent(EventTurnResult, map[string]any{"status": string(ResultSucceeded)}) | |
| 125 | + | valid := NewEvent(EventTurnResult, map[string]any{ | |
| 126 | + | "status": string(ResultCanceled), | |
| 127 | + | "shutdown_origin": ShutdownOriginSession, | |
| 128 | + | }) | |
| 112 | 129 | valid.AgentID = "agent-1" | |
| 113 | 130 | valid.TurnID = "turn-1" | |
| 114 | 131 | result, err := decodeTurnResultEvent(valid, "agent-1", 100, 10) | |
| 115 | - | if err != nil || result.AgentID != "agent-1" || result.TurnID != "turn-1" { | |
| 132 | + | if err != nil || result.AgentID != "agent-1" || result.TurnID != "turn-1" || result.ShutdownOrigin != ShutdownOriginSession { | |
| 116 | 133 | t.Fatalf("valid result = %#v, err=%v", result, err) | |
| 117 | 134 | } | |
| 118 | 135 | } | |
| @@ -269,19 +286,22 @@ func checkCaptureFailureResult(t *testing.T, agent *Agent, stateDir string) { | |||
| 269 | 286 | ||
| 270 | 287 | func TestEnsureResultReconcilesChildStatusWithRunnerError(t *testing.T) { | |
| 271 | 288 | tests := []struct { | |
| 272 | - | name string | |
| 273 | - | status Status | |
| 274 | - | runErr error | |
| 275 | - | childCode string | |
| 276 | - | wantState TurnStatus | |
| 277 | - | wantCode string | |
| 278 | - | wantOutput string | |
| 289 | + | name string | |
| 290 | + | status Status | |
| 291 | + | runErr error | |
| 292 | + | childCode string | |
| 293 | + | shutdownOrigin ShutdownOrigin | |
| 294 | + | wantShutdownOrigin ShutdownOrigin | |
| 295 | + | wantState TurnStatus | |
| 296 | + | wantCode string | |
| 297 | + | wantOutput string | |
| 279 | 298 | }{ | |
| 280 | 299 | {name: "runner failure", status: StatusFailed, runErr: errors.New("runner failed"), wantState: ResultFailed, wantCode: "runner_failed"}, | |
| 281 | 300 | {name: "runner cancellation", status: StatusFailed, runErr: context.Canceled, wantState: ResultCanceled, wantCode: "runner_failed"}, | |
| 282 | - | {name: "deadline preserves recovery guidance", status: StatusFailed, runErr: context.DeadlineExceeded, wantState: ResultFailed, wantCode: "deadline_exceeded", wantOutput: "partial answer"}, | |
| 283 | - | {name: "deadline overrides child error", status: StatusFailed, runErr: context.DeadlineExceeded, childCode: "context_limit", wantState: ResultFailed, wantCode: "deadline_exceeded", wantOutput: "partial answer"}, | |
| 301 | + | {name: "deadline preserves recovery guidance", status: StatusFailed, runErr: context.DeadlineExceeded, wantShutdownOrigin: ShutdownOriginDeadline, wantState: ResultFailed, wantCode: "deadline_exceeded", wantOutput: "partial answer"}, | |
| 302 | + | {name: "deadline overrides child error", status: StatusFailed, runErr: context.DeadlineExceeded, childCode: "context_limit", wantShutdownOrigin: ShutdownOriginDeadline, wantState: ResultFailed, wantCode: "deadline_exceeded", wantOutput: "partial answer"}, | |
| 284 | 303 | {name: "killed", status: StatusKilled, runErr: context.Canceled, wantState: ResultCanceled, wantCode: "runner_failed"}, | |
| 304 | + | {name: "session shutdown", status: StatusKilled, runErr: context.Canceled, shutdownOrigin: ShutdownOriginSession, wantShutdownOrigin: ShutdownOriginSession, wantState: ResultCanceled, wantCode: "shutdown"}, | |
| 285 | 305 | } | |
| 286 | 306 | for _, tt := range tests { | |
| 287 | 307 | t.Run(tt.name, func(t *testing.T) { | |
| @@ -293,9 +313,10 @@ func TestEnsureResultReconcilesChildStatusWithRunnerError(t *testing.T) { | |||
| 293 | 313 | childError = &ResultError{Code: tt.childCode, Message: "child error"} | |
| 294 | 314 | } | |
| 295 | 315 | agent := &Agent{ | |
| 296 | - | ID: "agent-1", | |
| 297 | - | stateDir: stateDir, | |
| 298 | - | lastAssistant: tt.wantOutput, | |
| 316 | + | ID: "agent-1", | |
| 317 | + | stateDir: stateDir, | |
| 318 | + | lastAssistant: tt.wantOutput, | |
| 319 | + | shutdownOrigin: tt.shutdownOrigin, | |
| 299 | 320 | result: &TurnResult{ | |
| 300 | 321 | AgentID: "agent-1", TurnID: "turn-1", Status: childStatus, Error: childError, | |
| 301 | 322 | }, | |
| @@ -328,6 +349,15 @@ func TestEnsureResultReconcilesChildStatusWithRunnerError(t *testing.T) { | |||
| 328 | 349 | if durable.Error == nil || durable.Error.Code != tt.wantCode { | |
| 329 | 350 | t.Fatalf("durable result error = %#v, want code %q", durable.Error, tt.wantCode) | |
| 330 | 351 | } | |
| 352 | + | if durable.ShutdownOrigin != tt.wantShutdownOrigin { | |
| 353 | + | t.Fatalf("durable shutdown origin = %q, want %q", durable.ShutdownOrigin, tt.wantShutdownOrigin) | |
| 354 | + | } | |
| 355 | + | if tt.wantShutdownOrigin != "" { | |
| 356 | + | wantErr := ShutdownResultError(tt.wantShutdownOrigin) | |
| 357 | + | if *durable.Error != *wantErr { | |
| 358 | + | t.Fatalf("durable result error = %#v, want canonical %#v", durable.Error, wantErr) | |
| 359 | + | } | |
| 360 | + | } | |
| 331 | 361 | }) | |
| 332 | 362 | } | |
| 333 | 363 | } | |
| @@ -33,6 +33,20 @@ func TestTurnResultValidationAndBoundedOutput(t *testing.T) { | |||||
| 33 | } | 33 | } | ||
| 34 | } | 34 | } | ||
| 35 | 35 | ||||
| 36 | + | func TestTurnResultValidatesShutdownOrigin(t *testing.T) { | |||
| 37 | + | result := &TurnResult{ | |||
| 38 | + | Version: ProtocolVersion, AgentID: "a", TurnID: "t", Status: ResultCanceled, | |||
| 39 | + | ShutdownOrigin: ShutdownOriginSession, | |||
| 40 | + | } | |||
| 41 | + | if err := result.Validate(0, 0); err != nil { | |||
| 42 | + | t.Fatalf("known shutdown origin was rejected: %v", err) | |||
| 43 | + | } | |||
| 44 | + | result.ShutdownOrigin = ShutdownOrigin("private detail") | |||
| 45 | + | if err := result.Validate(0, 0); err == nil { | |||
| 46 | + | t.Fatal("unknown shutdown origin was accepted") | |||
| 47 | + | } | |||
| 48 | + | } | |||
| 49 | + | ||||
| 36 | func TestTurnResultBoundedHonorsLineAndUTF8Limits(t *testing.T) { | 50 | func TestTurnResultBoundedHonorsLineAndUTF8Limits(t *testing.T) { | ||
| 37 | result := &TurnResult{ | 51 | result := &TurnResult{ | ||
| 38 | Version: ProtocolVersion, AgentID: "a", TurnID: "t", Status: ResultSucceeded, | 52 | Version: ProtocolVersion, AgentID: "a", TurnID: "t", Status: ResultSucceeded, | ||
| @@ -108,11 +122,14 @@ func TestDecodeTurnResultEventValidatesIdentityAndStatus(t *testing.T) { | |||||
| 108 | if _, err := decodeTurnResultEvent(mismatched, "agent-1", 100, 10); err == nil { | 122 | if _, err := decodeTurnResultEvent(mismatched, "agent-1", 100, 10); err == nil { | ||
| 109 | t.Fatal("mismatched result agent was accepted") | 123 | t.Fatal("mismatched result agent was accepted") | ||
| 110 | } | 124 | } | ||
| 111 | - | valid := NewEvent(EventTurnResult, map[string]any{"status": string(ResultSucceeded)}) | 125 | + | valid := NewEvent(EventTurnResult, map[string]any{ |
| 126 | + | "status": string(ResultCanceled), | |||
| 127 | + | "shutdown_origin": ShutdownOriginSession, | |||
| 128 | + | }) | |||
| 112 | valid.AgentID = "agent-1" | 129 | valid.AgentID = "agent-1" | ||
| 113 | valid.TurnID = "turn-1" | 130 | valid.TurnID = "turn-1" | ||
| 114 | result, err := decodeTurnResultEvent(valid, "agent-1", 100, 10) | 131 | result, err := decodeTurnResultEvent(valid, "agent-1", 100, 10) | ||
| 115 | - | if err != nil || result.AgentID != "agent-1" || result.TurnID != "turn-1" { | 132 | + | if err != nil || result.AgentID != "agent-1" || result.TurnID != "turn-1" || result.ShutdownOrigin != ShutdownOriginSession { |
| 116 | t.Fatalf("valid result = %#v, err=%v", result, err) | 133 | t.Fatalf("valid result = %#v, err=%v", result, err) | ||
| 117 | } | 134 | } | ||
| 118 | } | 135 | } | ||
| @@ -269,19 +286,22 @@ func checkCaptureFailureResult(t *testing.T, agent *Agent, stateDir string) { | |||||
| 269 | 286 | ||||
| 270 | func TestEnsureResultReconcilesChildStatusWithRunnerError(t *testing.T) { | 287 | func TestEnsureResultReconcilesChildStatusWithRunnerError(t *testing.T) { | ||
| 271 | tests := []struct { | 288 | tests := []struct { | ||
| 272 | - | name string | 289 | + | name string |
| 273 | - | status Status | 290 | + | status Status |
| 274 | - | runErr error | 291 | + | runErr error |
| 275 | - | childCode string | 292 | + | childCode string |
| 276 | - | wantState TurnStatus | 293 | + | shutdownOrigin ShutdownOrigin |
| 277 | - | wantCode string | 294 | + | wantShutdownOrigin ShutdownOrigin |
| 278 | - | wantOutput string | 295 | + | wantState TurnStatus |
| 296 | + | wantCode string | |||
| 297 | + | wantOutput string | |||
| 279 | }{ | 298 | }{ | ||
| 280 | {name: "runner failure", status: StatusFailed, runErr: errors.New("runner failed"), wantState: ResultFailed, wantCode: "runner_failed"}, | 299 | {name: "runner failure", status: StatusFailed, runErr: errors.New("runner failed"), wantState: ResultFailed, wantCode: "runner_failed"}, | ||
| 281 | {name: "runner cancellation", status: StatusFailed, runErr: context.Canceled, wantState: ResultCanceled, wantCode: "runner_failed"}, | 300 | {name: "runner cancellation", status: StatusFailed, runErr: context.Canceled, wantState: ResultCanceled, wantCode: "runner_failed"}, | ||
| 282 | - | {name: "deadline preserves recovery guidance", status: StatusFailed, runErr: context.DeadlineExceeded, wantState: ResultFailed, wantCode: "deadline_exceeded", wantOutput: "partial answer"}, | 301 | + | {name: "deadline preserves recovery guidance", status: StatusFailed, runErr: context.DeadlineExceeded, wantShutdownOrigin: ShutdownOriginDeadline, wantState: ResultFailed, wantCode: "deadline_exceeded", wantOutput: "partial answer"}, |
| 283 | - | {name: "deadline overrides child error", status: StatusFailed, runErr: context.DeadlineExceeded, childCode: "context_limit", wantState: ResultFailed, wantCode: "deadline_exceeded", wantOutput: "partial answer"}, | 302 | + | {name: "deadline overrides child error", status: StatusFailed, runErr: context.DeadlineExceeded, childCode: "context_limit", wantShutdownOrigin: ShutdownOriginDeadline, wantState: ResultFailed, wantCode: "deadline_exceeded", wantOutput: "partial answer"}, |
| 284 | {name: "killed", status: StatusKilled, runErr: context.Canceled, wantState: ResultCanceled, wantCode: "runner_failed"}, | 303 | {name: "killed", status: StatusKilled, runErr: context.Canceled, wantState: ResultCanceled, wantCode: "runner_failed"}, | ||
| 304 | + | {name: "session shutdown", status: StatusKilled, runErr: context.Canceled, shutdownOrigin: ShutdownOriginSession, wantShutdownOrigin: ShutdownOriginSession, wantState: ResultCanceled, wantCode: "shutdown"}, | |||
| 285 | } | 305 | } | ||
| 286 | for _, tt := range tests { | 306 | for _, tt := range tests { | ||
| 287 | t.Run(tt.name, func(t *testing.T) { | 307 | t.Run(tt.name, func(t *testing.T) { | ||
| @@ -293,9 +313,10 @@ func TestEnsureResultReconcilesChildStatusWithRunnerError(t *testing.T) { | |||||
| 293 | childError = &ResultError{Code: tt.childCode, Message: "child error"} | 313 | childError = &ResultError{Code: tt.childCode, Message: "child error"} | ||
| 294 | } | 314 | } | ||
| 295 | agent := &Agent{ | 315 | agent := &Agent{ | ||
| 296 | - | ID: "agent-1", | 316 | + | ID: "agent-1", |
| 297 | - | stateDir: stateDir, | 317 | + | stateDir: stateDir, |
| 298 | - | lastAssistant: tt.wantOutput, | 318 | + | lastAssistant: tt.wantOutput, |
| 319 | + | shutdownOrigin: tt.shutdownOrigin, | |||
| 299 | result: &TurnResult{ | 320 | result: &TurnResult{ | ||
| 300 | AgentID: "agent-1", TurnID: "turn-1", Status: childStatus, Error: childError, | 321 | AgentID: "agent-1", TurnID: "turn-1", Status: childStatus, Error: childError, | ||
| 301 | }, | 322 | }, | ||
| @@ -328,6 +349,15 @@ func TestEnsureResultReconcilesChildStatusWithRunnerError(t *testing.T) { | |||||
| 328 | if durable.Error == nil || durable.Error.Code != tt.wantCode { | 349 | if durable.Error == nil || durable.Error.Code != tt.wantCode { | ||
| 329 | t.Fatalf("durable result error = %#v, want code %q", durable.Error, tt.wantCode) | 350 | t.Fatalf("durable result error = %#v, want code %q", durable.Error, tt.wantCode) | ||
| 330 | } | 351 | } | ||
| 352 | + | if durable.ShutdownOrigin != tt.wantShutdownOrigin { | |||
| 353 | + | t.Fatalf("durable shutdown origin = %q, want %q", durable.ShutdownOrigin, tt.wantShutdownOrigin) | |||
| 354 | + | } | |||
| 355 | + | if tt.wantShutdownOrigin != "" { | |||
| 356 | + | wantErr := ShutdownResultError(tt.wantShutdownOrigin) | |||
| 357 | + | if *durable.Error != *wantErr { | |||
| 358 | + | t.Fatalf("durable result error = %#v, want canonical %#v", durable.Error, wantErr) | |||
| 359 | + | } | |||
| 360 | + | } | |||
| 331 | }) | 361 | }) | ||
| 332 | } | 362 | } | ||
| 333 | } | 363 | } | ||
M packages/agent/subagents/supervisor.go +2 -8
| @@ -800,10 +800,8 @@ func (f *Supervisor) stop(ctx context.Context, id string, origin ShutdownOrigin) | |||
| 800 | 800 | // the pending cleanup below could then finalize an agent that was already | |
| 801 | 801 | // admitted to the runner. | |
| 802 | 802 | f.mu.Lock() | |
| 803 | - | a.mu.Lock() | |
| 804 | - | status := a.status | |
| 805 | - | if status == StatusDone || status == StatusFailed || status == StatusKilled { | |
| 806 | - | a.mu.Unlock() | |
| 803 | + | status, shutdownStarted := a.prepareShutdown(origin) | |
| 804 | + | if !shutdownStarted { | |
| 807 | 805 | f.mu.Unlock() | |
| 808 | 806 | f.operationMu.Unlock() | |
| 809 | 807 | return nil | |
| @@ -812,15 +810,12 @@ func (f *Supervisor) stop(ctx context.Context, id string, origin ShutdownOrigin) | |||
| 812 | 810 | path := a.InboxPath | |
| 813 | 811 | inbox := a.inbox | |
| 814 | 812 | pid := a.ProcessPIDValue() | |
| 815 | - | a.mu.Unlock() | |
| 816 | 813 | f.mu.Unlock() | |
| 817 | 814 | // The detached worker may take the full grace period to exit. Do not | |
| 818 | 815 | // serialize unrelated supervisor operations for that entire wait. | |
| 819 | 816 | f.operationMu.Unlock() | |
| 820 | 817 | return f.stopDetached(ctx, a, path, inbox, pid, origin) | |
| 821 | 818 | } | |
| 822 | - | a.status = StatusKilled | |
| 823 | - | a.activity = "canceling" | |
| 824 | 819 | atomic.CompareAndSwapInt32(&a.launchState, 0, 2) | |
| 825 | 820 | if status == StatusPending { | |
| 826 | 821 | for i, queued := range f.queue { | |
| @@ -830,7 +825,6 @@ func (f *Supervisor) stop(ctx context.Context, id string, origin ShutdownOrigin) | |||
| 830 | 825 | } | |
| 831 | 826 | } | |
| 832 | 827 | } | |
| 833 | - | a.mu.Unlock() | |
| 834 | 828 | f.mu.Unlock() | |
| 835 | 829 | f.operationMu.Unlock() | |
| 836 | 830 | a.setTurnState(TurnCanceling, a.CurrentTurnID()) | |
| @@ -800,10 +800,8 @@ func (f *Supervisor) stop(ctx context.Context, id string, origin ShutdownOrigin) | |||||
| 800 | // the pending cleanup below could then finalize an agent that was already | 800 | // the pending cleanup below could then finalize an agent that was already | ||
| 801 | // admitted to the runner. | 801 | // admitted to the runner. | ||
| 802 | f.mu.Lock() | 802 | f.mu.Lock() | ||
| 803 | - | a.mu.Lock() | 803 | + | status, shutdownStarted := a.prepareShutdown(origin) |
| 804 | - | status := a.status | 804 | + | if !shutdownStarted { |
| 805 | - | if status == StatusDone || status == StatusFailed || status == StatusKilled { | |||
| 806 | - | a.mu.Unlock() | |||
| 807 | f.mu.Unlock() | 805 | f.mu.Unlock() | ||
| 808 | f.operationMu.Unlock() | 806 | f.operationMu.Unlock() | ||
| 809 | return nil | 807 | return nil | ||
| @@ -812,15 +810,12 @@ func (f *Supervisor) stop(ctx context.Context, id string, origin ShutdownOrigin) | |||||
| 812 | path := a.InboxPath | 810 | path := a.InboxPath | ||
| 813 | inbox := a.inbox | 811 | inbox := a.inbox | ||
| 814 | pid := a.ProcessPIDValue() | 812 | pid := a.ProcessPIDValue() | ||
| 815 | - | a.mu.Unlock() | |||
| 816 | f.mu.Unlock() | 813 | f.mu.Unlock() | ||
| 817 | // The detached worker may take the full grace period to exit. Do not | 814 | // The detached worker may take the full grace period to exit. Do not | ||
| 818 | // serialize unrelated supervisor operations for that entire wait. | 815 | // serialize unrelated supervisor operations for that entire wait. | ||
| 819 | f.operationMu.Unlock() | 816 | f.operationMu.Unlock() | ||
| 820 | return f.stopDetached(ctx, a, path, inbox, pid, origin) | 817 | return f.stopDetached(ctx, a, path, inbox, pid, origin) | ||
| 821 | } | 818 | } | ||
| 822 | - | a.status = StatusKilled | |||
| 823 | - | a.activity = "canceling" | |||
| 824 | atomic.CompareAndSwapInt32(&a.launchState, 0, 2) | 819 | atomic.CompareAndSwapInt32(&a.launchState, 0, 2) | ||
| 825 | if status == StatusPending { | 820 | if status == StatusPending { | ||
| 826 | for i, queued := range f.queue { | 821 | for i, queued := range f.queue { | ||
| @@ -830,7 +825,6 @@ func (f *Supervisor) stop(ctx context.Context, id string, origin ShutdownOrigin) | |||||
| 830 | } | 825 | } | ||
| 831 | } | 826 | } | ||
| 832 | } | 827 | } | ||
| 833 | - | a.mu.Unlock() | |||
| 834 | f.mu.Unlock() | 828 | f.mu.Unlock() | ||
| 835 | f.operationMu.Unlock() | 829 | f.operationMu.Unlock() | ||
| 836 | a.setTurnState(TurnCanceling, a.CurrentTurnID()) | 830 | a.setTurnState(TurnCanceling, a.CurrentTurnID()) | ||
M packages/agent/subagents/supervisor_test.go +81 -0
| @@ -190,6 +190,87 @@ func TestStopCancelsRunningAgent(t *testing.T) { | |||
| 190 | 190 | } | |
| 191 | 191 | } | |
| 192 | 192 | ||
| 193 | + | func TestStopPublishesShutdownOriginBeforeConcurrentFinalization(t *testing.T) { | |
| 194 | + | started := make(chan struct{}) | |
| 195 | + | finish := make(chan struct{}) | |
| 196 | + | finalizing := make(chan struct{}) | |
| 197 | + | f := newTestSupervisor(t, func(a *Agent) Runner { | |
| 198 | + | return RunnerFunc(func(context.Context, Sink) error { | |
| 199 | + | close(started) | |
| 200 | + | <-finish | |
| 201 | + | close(finalizing) | |
| 202 | + | return nil | |
| 203 | + | }) | |
| 204 | + | }) | |
| 205 | + | a, err := f.Spawn(context.Background(), "finish while stopping") | |
| 206 | + | if err != nil { | |
| 207 | + | t.Fatal(err) | |
| 208 | + | } | |
| 209 | + | <-started | |
| 210 | + | ||
| 211 | + | // Hold lifecycle finalization after Runner.Run returns. Shutdown must publish | |
| 212 | + | // its origin before exposing StatusKilled, without relying on finalization | |
| 213 | + | // ordering to fill the durable result correctly. | |
| 214 | + | a.lifecycleMu.Lock() | |
| 215 | + | lifecycleLocked := true | |
| 216 | + | finishClosed := false | |
| 217 | + | defer func() { | |
| 218 | + | if !finishClosed { | |
| 219 | + | close(finish) | |
| 220 | + | } | |
| 221 | + | if lifecycleLocked { | |
| 222 | + | a.lifecycleMu.Unlock() | |
| 223 | + | } | |
| 224 | + | }() | |
| 225 | + | stopDone := make(chan error, 1) | |
| 226 | + | go func() { stopDone <- f.stop(context.Background(), a.ID, ShutdownOriginSession) }() | |
| 227 | + | ||
| 228 | + | deadline := time.Now().Add(time.Second) | |
| 229 | + | for a.Status() != StatusKilled { | |
| 230 | + | if time.Now().After(deadline) { | |
| 231 | + | t.Fatal("shutdown did not publish killed status") | |
| 232 | + | } | |
| 233 | + | runtime.Gosched() | |
| 234 | + | } | |
| 235 | + | a.mu.Lock() | |
| 236 | + | origin := a.shutdownOrigin | |
| 237 | + | a.mu.Unlock() | |
| 238 | + | if origin != ShutdownOriginSession { | |
| 239 | + | t.Fatalf("shutdown origin = %q when killed became observable, want %q", origin, ShutdownOriginSession) | |
| 240 | + | } | |
| 241 | + | ||
| 242 | + | close(finish) | |
| 243 | + | finishClosed = true | |
| 244 | + | <-finalizing | |
| 245 | + | deadline = time.Now().Add(time.Second) | |
| 246 | + | for { | |
| 247 | + | a.mu.Lock() | |
| 248 | + | finalizationStarted := !a.finished.IsZero() | |
| 249 | + | a.mu.Unlock() | |
| 250 | + | if finalizationStarted { | |
| 251 | + | break | |
| 252 | + | } | |
| 253 | + | if time.Now().After(deadline) { | |
| 254 | + | t.Fatal("runner finalization did not start") | |
| 255 | + | } | |
| 256 | + | runtime.Gosched() | |
| 257 | + | } | |
| 258 | + | a.lifecycleMu.Unlock() | |
| 259 | + | lifecycleLocked = false | |
| 260 | + | if err := <-stopDone; err != nil { | |
| 261 | + | t.Fatal(err) | |
| 262 | + | } | |
| 263 | + | a.Wait() | |
| 264 | + | ||
| 265 | + | result := a.Result() | |
| 266 | + | if result == nil || result.Status != ResultCanceled || result.ShutdownOrigin != ShutdownOriginSession { | |
| 267 | + | t.Fatalf("final result = %#v, want attributed session cancellation", result) | |
| 268 | + | } | |
| 269 | + | if result.Error == nil || result.Error.Code != "shutdown" { | |
| 270 | + | t.Fatalf("final result error = %#v, want shutdown", result.Error) | |
| 271 | + | } | |
| 272 | + | } | |
| 273 | + | ||
| 193 | 274 | func TestStopByRootSessionLeavesOtherSessionsRunning(t *testing.T) { | |
| 194 | 275 | started := make(chan string, 2) | |
| 195 | 276 | stopped := make(chan string, 2) | |
| @@ -190,6 +190,87 @@ func TestStopCancelsRunningAgent(t *testing.T) { | |||||
| 190 | } | 190 | } | ||
| 191 | } | 191 | } | ||
| 192 | 192 | ||||
| 193 | + | func TestStopPublishesShutdownOriginBeforeConcurrentFinalization(t *testing.T) { | |||
| 194 | + | started := make(chan struct{}) | |||
| 195 | + | finish := make(chan struct{}) | |||
| 196 | + | finalizing := make(chan struct{}) | |||
| 197 | + | f := newTestSupervisor(t, func(a *Agent) Runner { | |||
| 198 | + | return RunnerFunc(func(context.Context, Sink) error { | |||
| 199 | + | close(started) | |||
| 200 | + | <-finish | |||
| 201 | + | close(finalizing) | |||
| 202 | + | return nil | |||
| 203 | + | }) | |||
| 204 | + | }) | |||
| 205 | + | a, err := f.Spawn(context.Background(), "finish while stopping") | |||
| 206 | + | if err != nil { | |||
| 207 | + | t.Fatal(err) | |||
| 208 | + | } | |||
| 209 | + | <-started | |||
| 210 | + | ||||
| 211 | + | // Hold lifecycle finalization after Runner.Run returns. Shutdown must publish | |||
| 212 | + | // its origin before exposing StatusKilled, without relying on finalization | |||
| 213 | + | // ordering to fill the durable result correctly. | |||
| 214 | + | a.lifecycleMu.Lock() | |||
| 215 | + | lifecycleLocked := true | |||
| 216 | + | finishClosed := false | |||
| 217 | + | defer func() { | |||
| 218 | + | if !finishClosed { | |||
| 219 | + | close(finish) | |||
| 220 | + | } | |||
| 221 | + | if lifecycleLocked { | |||
| 222 | + | a.lifecycleMu.Unlock() | |||
| 223 | + | } | |||
| 224 | + | }() | |||
| 225 | + | stopDone := make(chan error, 1) | |||
| 226 | + | go func() { stopDone <- f.stop(context.Background(), a.ID, ShutdownOriginSession) }() | |||
| 227 | + | ||||
| 228 | + | deadline := time.Now().Add(time.Second) | |||
| 229 | + | for a.Status() != StatusKilled { | |||
| 230 | + | if time.Now().After(deadline) { | |||
| 231 | + | t.Fatal("shutdown did not publish killed status") | |||
| 232 | + | } | |||
| 233 | + | runtime.Gosched() | |||
| 234 | + | } | |||
| 235 | + | a.mu.Lock() | |||
| 236 | + | origin := a.shutdownOrigin | |||
| 237 | + | a.mu.Unlock() | |||
| 238 | + | if origin != ShutdownOriginSession { | |||
| 239 | + | t.Fatalf("shutdown origin = %q when killed became observable, want %q", origin, ShutdownOriginSession) | |||
| 240 | + | } | |||
| 241 | + | ||||
| 242 | + | close(finish) | |||
| 243 | + | finishClosed = true | |||
| 244 | + | <-finalizing | |||
| 245 | + | deadline = time.Now().Add(time.Second) | |||
| 246 | + | for { | |||
| 247 | + | a.mu.Lock() | |||
| 248 | + | finalizationStarted := !a.finished.IsZero() | |||
| 249 | + | a.mu.Unlock() | |||
| 250 | + | if finalizationStarted { | |||
| 251 | + | break | |||
| 252 | + | } | |||
| 253 | + | if time.Now().After(deadline) { | |||
| 254 | + | t.Fatal("runner finalization did not start") | |||
| 255 | + | } | |||
| 256 | + | runtime.Gosched() | |||
| 257 | + | } | |||
| 258 | + | a.lifecycleMu.Unlock() | |||
| 259 | + | lifecycleLocked = false | |||
| 260 | + | if err := <-stopDone; err != nil { | |||
| 261 | + | t.Fatal(err) | |||
| 262 | + | } | |||
| 263 | + | a.Wait() | |||
| 264 | + | ||||
| 265 | + | result := a.Result() | |||
| 266 | + | if result == nil || result.Status != ResultCanceled || result.ShutdownOrigin != ShutdownOriginSession { | |||
| 267 | + | t.Fatalf("final result = %#v, want attributed session cancellation", result) | |||
| 268 | + | } | |||
| 269 | + | if result.Error == nil || result.Error.Code != "shutdown" { | |||
| 270 | + | t.Fatalf("final result error = %#v, want shutdown", result.Error) | |||
| 271 | + | } | |||
| 272 | + | } | |||
| 273 | + | ||||
| 193 | func TestStopByRootSessionLeavesOtherSessionsRunning(t *testing.T) { | 274 | func TestStopByRootSessionLeavesOtherSessionsRunning(t *testing.T) { | ||
| 194 | started := make(chan string, 2) | 275 | started := make(chan string, 2) | ||
| 195 | stopped := make(chan string, 2) | 276 | stopped := make(chan string, 2) | ||