diff --git a/docs/superpowers/plans/2026-04-27-background-task-retry-timeline.md b/docs/superpowers/plans/2026-04-27-background-task-retry-timeline.md new file mode 100644 index 000000000..4c8b5e25a --- /dev/null +++ b/docs/superpowers/plans/2026-04-27-background-task-retry-timeline.md @@ -0,0 +1,442 @@ +# Background Task Retry Timeline Implementation Plan + +> **For agentic workers:** REQUIRED: Use superpowers:subagent-driven-development (if subagents available) or superpowers:executing-plans to implement this plan. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add structured retry-attempt history to background tasks and surface a compact attempt timeline in parent chat while preserving separate retry child sessions. + +**Architecture:** Extend `BackgroundTask` with explicit `attempts[]` state and `currentAttemptID`, add small helper functions to keep task-level fields as a projection of the current attempt, and wire those helpers into background retry, session creation, and completion/error paths. Parent notifications remain the UI surface, but they are generated from structured attempt state instead of ad hoc retry text. + +**Tech Stack:** TypeScript, Bun test, OpenCode background task engine, parent chat notification flow + +--- + +## File Structure + +### Files to modify + +- `src/features/background-agent/types.ts` + - Extend `BackgroundTask` with `attempts[]` and `currentAttemptID` + - Add attempt type definition and any retry-observability support fields needed + +- `src/features/background-agent/manager.ts` + - Add/consume helper functions for attempt lifecycle + - Bind retry child session ids to exact attempts in `startTask()` + - Resolve lifecycle events through `sessionID -> attemptID` + - Generate final parent summary from `attempts[]` + +- `src/features/background-agent/fallback-retry-handler.ts` + - Create next attempt entry during retry scheduling + - Finalize failed attempt before queueing retry + - Preserve retry notification metadata without mutating historical attempts + +- `src/features/background-agent/background-task-notification-template.ts` + - Add compact attempt timeline rendering for parent-facing notifications + +- `src/tools/background-task/task-result-format.ts` + - Optional first-pass alignment if task results need to reference attempt-derived terminal state consistently + +### Files to test + +- `src/features/background-agent/manager.test.ts` +- `src/features/background-agent/fallback-retry-handler.test.ts` +- `src/tools/background-task/task-result-format.test.ts` + +### Files to inspect for patterns/reference only + +- `src/features/background-agent/session-idle-event-handler.ts` +- `src/features/background-agent/task-history.ts` +- `src/features/background-agent/session-status-classifier.ts` +- `docs/superpowers/specs/2026-04-27-background-task-retry-timeline-design.md` + +--- + +### Task 1: Define structured attempt state + +**Files:** +- Modify: `src/features/background-agent/types.ts` +- Test: `src/features/background-agent/manager.test.ts` + +- [ ] **Step 1: Add a focused failing test that expects attempt state on a new background task** + +Add a test in `src/features/background-agent/manager.test.ts` that launches a background task and expects: +- `attempts` to exist +- first attempt to have `attemptNumber: 1` +- `currentAttemptID` to point at that first attempt +- top-level task fields to still exist for compatibility + +- [ ] **Step 2: Run the new test to verify it fails for the expected reason** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts +``` + +Expected: the new assertion fails because `attempts[]` and `currentAttemptID` do not exist yet. + +- [ ] **Step 3: Add the attempt state types to `BackgroundTask`** + +Update `src/features/background-agent/types.ts` to add: +- `BackgroundTaskAttempt` type/interface with: + - `attemptID` + - `attemptNumber` + - `sessionID?` + - `providerID?` + - `modelID?` + - `variant?` + - `status` + - `error?` + - `startedAt?` + - `completedAt?` +- `attempts?: BackgroundTaskAttempt[]` +- `currentAttemptID?: string` + +- [ ] **Step 4: Initialize first attempt state when tasks are created** + +In `src/features/background-agent/manager.ts`, when `launch()` creates the initial `BackgroundTask`, initialize: +- one attempt entry in `pending` +- `currentAttemptID` referencing that entry +- top-level `model` copied into attempt model fields + +- [ ] **Step 5: Re-run the test to verify the new task has attempt state** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts +``` + +Expected: the new launch/creation test passes. + +--- + +### Task 2: Add attempt lifecycle helper functions + +**Files:** +- Modify: `src/features/background-agent/manager.ts` +- Test: `src/features/background-agent/manager.test.ts` + +- [ ] **Step 1: Add a failing test for exact attempt binding in `startTask()`** + +Add a test that simulates: +- a task with a pending retry attempt +- `startTask()` creating a child session +- the session being bound to the exact scheduled attempt, not merely "the latest pending attempt" + +The test should assert: +- `sessionID` lands on the correct attempt +- `currentAttemptID` remains correct +- top-level task `sessionID` mirrors that active attempt + +- [ ] **Step 2: Run the test to verify it fails before helpers exist** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts +``` + +Expected: binding assertions fail or require manual task mutation not yet implemented. + +- [ ] **Step 3: Implement helper functions inside `manager.ts`** + +Add small focused helpers, either in `manager.ts` or a dedicated sibling helper file if needed: +- `startAttempt(task, initialModel)` +- `bindAttemptSession(task, attemptID, sessionID, model)` +- `scheduleRetryAttempt(task, failedAttemptID, nextModel, error)` +- `finalizeAttempt(task, attemptID, terminalStatus, error?)` + +These helpers must enforce: +- only `currentAttemptID` is mutable +- finalized attempts are immutable +- binding by explicit `attemptID` + +- [ ] **Step 4: Add a `sessionID -> attemptID` mapping strategy** + +Implement one of: +- a map stored on the task +- or a lookup derived from attempts by session id + +The first implementation can be simple, but every lifecycle event must resolve the attempt through this mapping before mutating state. + +- [ ] **Step 5: Define an explicit queued work contract that carries `attemptID` into `startTask()`** + +Update the implementation plan so queued background work carries the scheduled `attemptID` explicitly. + +Concretely: +- extend the queue item / queued work shape to include `attemptID` +- ensure retry scheduling writes that `attemptID` at queue time +- ensure `startTask()` receives the exact `attemptID` and never infers “latest pending attempt” + +This is required to satisfy the approved spec’s exact-binding rule. + +- [ ] **Step 6: Re-run the manager tests** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts +``` + +Expected: new binding and helper tests pass. + +--- + +### Task 3: Record retries as new attempts instead of overwriting task state + +**Files:** +- Modify: `src/features/background-agent/fallback-retry-handler.ts` +- Test: `src/features/background-agent/fallback-retry-handler.test.ts` + +- [ ] **Step 1: Add a failing test for retry scheduling creating Attempt 2** + +Add a test that starts with a task already representing Attempt 1 and then runs `tryFallbackRetry()`. + +Expected behavior: +- Attempt 1 becomes terminal `error` +- Attempt 2 is created as `pending` +- `currentAttemptID` moves to Attempt 2 +- top-level `task.model` mirrors Attempt 2 model + +- [ ] **Step 2: Run the retry-handler test to verify it fails** + +Run: +```bash +bun test src/features/background-agent/fallback-retry-handler.test.ts +``` + +Expected: no structured attempt chain exists yet, so assertions fail. + +- [ ] **Step 3: Update retry scheduling to use attempt helpers** + +In `src/features/background-agent/fallback-retry-handler.ts`: +- finalize the current attempt before retry queueing +- create the next pending attempt +- preserve retry notification metadata on the task +- keep top-level compatibility fields aligned with the new active attempt + +- [ ] **Step 4: Re-run the retry-handler tests** + +Run: +```bash +bun test src/features/background-agent/fallback-retry-handler.test.ts +``` + +Expected: retry now produces a correct attempt chain. + +--- + +### Task 4: Route all session lifecycle mutations through attempt identity + +**Files:** +- Modify: `src/features/background-agent/manager.ts` +- Reference: `src/features/background-agent/session-idle-event-handler.ts` +- Test: `src/features/background-agent/manager.test.ts` + +- [ ] **Step 1: Add a failing stale-event regression test** + +Create a test that simulates: +- Attempt 1 fails and Attempt 2 becomes current +- a late event from Attempt 1’s old `sessionID` arrives + +Expected: +- Attempt 2 and top-level task projection do not change +- stale event is ignored for state mutation + +- [ ] **Step 2: Run the test to verify the stale-event case fails first** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts +``` + +Expected: stale-event mutation is not yet blocked. + +- [ ] **Step 3: Update lifecycle handlers to resolve `sessionID -> attemptID` first** + +Apply this rule in relevant background manager paths: +- `message.updated` +- `session.error` +- `session.status` +- completion/idle handling if they mutate attempt/task state + +Before mutating state: +1. resolve the `attemptID` from the incoming `sessionID` +2. verify it still matches `currentAttemptID` +3. otherwise ignore/log as stale + +- [ ] **Step 4: Re-run the manager tests** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts +``` + +Expected: stale-event regression passes. + +--- + +### Task 5: Render the attempt timeline in parent chat summaries + +**Files:** +- Modify: `src/features/background-agent/background-task-notification-template.ts` +- Test: `src/features/background-agent/manager.test.ts` + +- [ ] **Step 1: Add a failing notification-format test for multi-attempt tasks** + +Create a test that builds a completed/failed task with three attempts and expects parent-facing summary text containing: +- attempt number +- status +- model +- session id + +- [ ] **Step 2: Run the test to verify it fails first** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts +``` + +Expected: current notifications do not include a structured attempt timeline. + +- [ ] **Step 3: Update notification template to render compact attempt timeline** + +In `background-task-notification-template.ts`: +- keep the summary compact +- render one line per attempt +- include error text only for failed attempts where useful +- do not replace separate retry reminders; final summary is additive + +- [ ] **Step 4: Update manager-side aggregation so final summaries carry attempt history** + +`notifyParentSession()` currently batches through `completedTaskSummaries` in `manager.ts`, which only stores task-level summary data. + +Modify that aggregation path so the final per-task notification has access to the task’s structured `attempts[]` data at summary time. + +Allowed implementation directions: +- extend `BackgroundTaskNotificationTask` to include attempt timeline data +- or bypass the reduced aggregation shape for final parent summaries and pass the original task objects (or a richer projection) + +The key requirement is that the final parent summary must render the authoritative attempt timeline from structured state, not from task-level status alone. + +- [ ] **Step 5: Re-run notification tests** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts +``` + +Expected: parent-summary timeline is now shown from `attempts[]` state. + +--- + +### Task 6: Preserve retry observability messages from state + +**Files:** +- Modify: `src/features/background-agent/manager.ts` +- Test: `src/features/background-agent/manager.test.ts` + +- [ ] **Step 1: Add a failing test that retry-scheduled and retry-session-ready notifications are derived from attempt state** + +The test should verify: +- retry scheduled reminder still includes failed session id, failed model, error, next model +- retry session ready reminder includes new retry session id and attempt number + +- [ ] **Step 2: Run the test to verify current behavior is incomplete** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts +``` + +Expected: notifications are not yet driven by structured attempt state. + +- [ ] **Step 3: Refactor retry notifications to read from attempts** + +Make the existing retry observability path use `attempts[]` + `currentAttemptID` instead of ad hoc fields where practical. + +- [ ] **Step 4: Re-run manager tests** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts +``` + +Expected: retry observability remains correct after the attempt-state refactor. + +--- + +### Task 7: End-to-end regression sweep for background retry history + +**Files:** +- Test: `src/features/background-agent/manager.test.ts` +- Test: `src/features/background-agent/fallback-retry-handler.test.ts` +- Test: `src/tools/background-task/task-result-format.test.ts` + +- [ ] **Step 1: Add an end-to-end regression covering multiple retries followed by success** + +Test expectations: +- 3 attempts recorded +- first two failed with distinct models/session ids +- third completed successfully +- parent summary contains all three attempts in order + +- [ ] **Step 2: Run the focused regression suite** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts src/features/background-agent/fallback-retry-handler.test.ts src/tools/background-task/task-result-format.test.ts +``` + +Expected: all focused tests pass. + +- [ ] **Step 3: Run the broader fallback regression suite** + +Run: +```bash +bun test src/features/background-agent/manager.test.ts src/features/background-agent/fallback-retry-handler.test.ts src/features/background-agent/error-classifier.test.ts src/tools/background-task/task-result-format.test.ts src/tools/delegate-task/sync-session-poller.test.ts src/tools/delegate-task/sync-task.test.ts src/plugin/event.test.ts src/shared/model-error-classifier.test.ts +``` + +Expected: all tests pass. + +- [ ] **Step 4: Run typecheck and build** + +Run: +```bash +bun run typecheck +bun run build +``` + +Expected: both commands succeed with no errors. + +--- + +### Task 8: Final verification and handoff + +**Files:** +- Review: all modified files above + +- [ ] **Step 1: Manually verify task-level projection consistency** + +Check in code review that: +- active attempt and top-level fields always agree +- finalized attempts are not mutated later +- stale events are ignored + +- [ ] **Step 2: Confirm parent chat UX remains compact** + +Check that final attempt timeline is readable and not overly verbose. + +- [ ] **Step 3: Prepare implementation summary** + +Document: +- files changed +- new attempt-state invariants +- tests added/updated + +- [ ] **Step 4: Commit** + +```bash +git add src/features/background-agent/types.ts src/features/background-agent/manager.ts src/features/background-agent/fallback-retry-handler.ts src/features/background-agent/background-task-notification-template.ts src/features/background-agent/manager.test.ts src/features/background-agent/fallback-retry-handler.test.ts src/tools/background-task/task-result-format.test.ts docs/superpowers/specs/2026-04-27-background-task-retry-timeline-design.md docs/superpowers/plans/2026-04-27-background-task-retry-timeline.md +git commit -m "feat(background-task): add retry attempt timeline" +``` + +--- + +Plan complete and saved to `docs/superpowers/plans/2026-04-27-background-task-retry-timeline.md`. Ready to execute? diff --git a/docs/superpowers/specs/2026-04-27-background-task-retry-timeline-design.md b/docs/superpowers/specs/2026-04-27-background-task-retry-timeline-design.md new file mode 100644 index 000000000..e449801f1 --- /dev/null +++ b/docs/superpowers/specs/2026-04-27-background-task-retry-timeline-design.md @@ -0,0 +1,320 @@ +# Background Task Retry Timeline Design + +Date: 2026-04-27 +Status: Draft approved for spec review + +## Goal + +Make background task retries understandable from the parent chat UI. + +Today, retry attempts create separate child sessions, but the user mainly sees the first failed child session and has to infer whether a retry happened. The goal is to preserve separate retry child sessions while presenting an attempt timeline in the parent chat. + +## User Outcome + +For a background task that retries across models, the parent chat should show a compact attempt timeline such as: + +- Attempt 1 — failed — `openai/gpt-5.4-mini` — session `ses_aaa` +- Attempt 2 — failed — `anthropic/claude-haiku-4.5` — session `ses_bbb` +- Attempt 3 — completed — `google/gemini-2.5-flash-lite` — session `ses_ccc` + +The retry child sessions remain real, separate subagent sessions. The parent chat becomes the authoritative summary surface. + +## Scope + +### In scope + +- Add structured retry-attempt history to `BackgroundTask` +- Update background retry lifecycle to record one attempt per child session +- Surface the attempt timeline in parent chat notifications +- Include session ids and model ids for each attempt + +### Out of scope + +- Redesigning the full session list UI +- Building a timeline into `background_output` in the first iteration +- Migrating historical tasks created before this feature +- Merging retry child sessions into one synthetic session + +## Design Summary + +### 1. Background task state model + +Extend `BackgroundTask` with an `attempts` array. + +Also add: + +- `currentAttemptID?: string` + +Each attempt must have its own immutable identity so async events from superseded child sessions cannot mutate the wrong attempt. + +Each attempt should track: + +- `attemptID: string` +- `attemptNumber: number` +- `sessionID?: string` +- `providerID?: string` +- `modelID?: string` +- `variant?: string` +- `status: "pending" | "running" | "completed" | "error" | "cancelled" | "interrupt"` +- `error?: string` +- `startedAt?: Date` +- `completedAt?: Date` + +### Task-level invariants + +`BackgroundTask` keeps existing top-level fields (`status`, `sessionID`, `model`, `startedAt`, `completedAt`, `error`) for compatibility, but they must be treated as a **projection of the current/latest attempt**. + +Rules: + +- `currentAttemptID` points at the only attempt allowed to receive active lifecycle updates +- task-level `sessionID`, `model`, `status`, `startedAt`, `completedAt`, and `error` must mirror the current/latest attempt state +- historical attempts are read-only once terminalized + +This avoids two competing sources of truth. + +This turns retry history into structured task state instead of a series of inferred notifications. + +### 2. Attempt lifecycle + +#### Initial launch + +When a background task is first launched: + +- create Attempt 1 in `pending` +- populate model information from the initial task model +- once `startTask()` creates the first child session, fill in `sessionID`, `startedAt`, and mark `running` +- set `currentAttemptID` to Attempt 1 + +#### Retry scheduled + +When fallback retry is chosen: + +- finalize the current attempt as failed using the latest error and completion time +- create the next attempt as `pending` +- populate its next fallback model metadata before queueing +- update `currentAttemptID` to the new attempt + +The scheduler must pass the new `attemptID` forward to the later session-creation step. Binding must never target “the latest pending attempt” by inference. + +The previously active attempt becomes immutable at this point. + +#### Retry session ready + +When `startTask()` creates the retry child session: + +- bind the created child session to the exact scheduled `attemptID` +- assign the new `sessionID` +- set `startedAt` +- mark the attempt `running` + +Binding rule: + +- session creation must call something equivalent to `bindAttemptSession(attemptID, sessionID, ...)` +- binding succeeds only if that exact attempt is still the active pending/running attempt +- if the attempt is already superseded or terminal, the new session is aborted or ignored rather than rebound to another attempt + +#### Final completion or failure + +When the task finishes: + +- mark the current attempt `completed`, `error`, `cancelled`, or `interrupt` +- record `completedAt` + +### Pending sub-states + +Internally, a `pending` attempt can represent different operational conditions: + +1. queued behind concurrency +2. retry selected, new child session not yet created +3. session creation failed before a child session exists + +The first iteration may still render all three as `pending` in the parent chat timeline, but the implementation should distinguish them in state transitions and notification text so debugging remains clear. + +## Parent Chat Presentation + +### Balanced default + +The parent chat should show a balanced timeline by default: + +- one line per attempt +- model id +- outcome +- session id + +Example: + +```text +Background task attempts: +- Attempt 1 — ERROR — openai/gpt-5.4-mini — ses_aaa + Error: Forbidden: Selected provider is forbidden +- Attempt 2 — ERROR — anthropic/claude-haiku-4.5 — ses_bbb + Error: Too Many Requests +- Attempt 3 — COMPLETED — google/gemini-2.5-flash-lite — ses_ccc +``` + +### Parent notification rules + +The parent should receive three kinds of retry-related updates: + +1. **Retry scheduled** + - failed session id + - failed model + - failed error + - next model + +2. **Retry session ready** + - retry session id + - attempt number + - model + +3. **Final summary** + - compact attempt timeline for all attempts + +The final summary should be emitted for any terminal task outcome: + +- completed +- error +- cancelled +- interrupt + +The final summary is the user-facing source of truth. + +## Data Ownership + +`BackgroundTask` is the right owner for this state because: + +- retries mutate and requeue the same background task id +- child sessions are implementation details of that task lifecycle +- parent notifications already derive from background task state + +This avoids reconstructing attempt history from session logs or reminder text. + +## Mutation contract + +All attempt writes should go through a small set of helper functions owned by the background-task lifecycle. + +Suggested helpers: + +- `startAttempt(...)` +- `bindAttemptSession(...)` +- `scheduleRetry(...)` +- `finalizeAttempt(...)` + +Also maintain a lightweight `sessionID -> attemptID` lookup for active and historical child sessions associated with the task lifecycle. + +Rules: + +- only the attempt referenced by `currentAttemptID` may receive active updates +- once an attempt is finalized, later events from its child session are ignored +- retry scheduling must finalize the old attempt before creating the next one +- every lifecycle handler must first resolve an immutable attempt identity, either directly by `attemptID` or through `sessionID -> attemptID`, before mutating attempt or task-level state + +This is the key race-safety mechanism for async background retries. + +## Key Integration Points + +### Background retry path + +- `src/features/background-agent/fallback-retry-handler.ts` + - create the next attempt entry when retry is selected + - finalize the failed attempt before queueing + - record retry scheduling metadata without mutating historical attempts later + +### Session creation path + +- `src/features/background-agent/manager.ts` + - in `startTask()`, attach the created child session id to the exact scheduled `attemptID` + - emit the "retry session ready" reminder from attempt state + +### Completion and failure path + +- `src/features/background-agent/manager.ts` + - update the active attempt status when task completes or errors + - generate final parent summary from `attempts[]` + - ignore stale events that target older attempt session ids + - resolve every session lifecycle event through `sessionID -> attemptID` before applying updates + +### Background output + +Out of scope for the first iteration, but the same `attempts[]` state should make later extension straightforward. + +## Error Handling + +### Missing attempt session id + +If session creation fails before a retry session exists: + +- keep the attempt as `pending` until terminalized +- if the task fails permanently, mark that attempt `error` with no `sessionID` + +### Late events from superseded sessions + +If the old child session emits `session.error`, `message.updated`, `interrupt`, or other lifecycle events after a retry is already scheduled: + +- those events must not mutate the newly active attempt +- they may be logged for debugging +- they must be ignored for task state purposes unless they resolve to the currently active `attemptID` + +This means event handling must not rely on task-level `sessionID` alone. It must first map the incoming `sessionID` to the originating `attemptID`, then reject the mutation if that attempt is no longer current. + +### Retry with no visible child session yet + +This is expected between: + +- old failed child abort +- new child session creation + +The `Retry scheduled` notification should explain that the next attempt has been queued. The `Retry session ready` notification closes that observability gap. + +## Testing Strategy + +### Unit tests + +- attempt created for first launch +- attempt finalized on retry scheduling +- retry attempt receives the newly created child `sessionID` +- final summary renders all attempts in order +- final summary preserves separate statuses for failed and successful attempts + +### Regression tests + +- forbidden initial provider followed by successful fallback should produce two attempts +- multiple failed retries followed by success should show full attempt chain +- background task failure with no fallback available should still produce one terminal attempt + +## Risks + +### Risk: status drift between task and attempts + +Mitigation: + +- centralize attempt updates in helper functions +- avoid manual field-by-field writes scattered across retry and completion code +- keep task-level fields as a projection, not an independent state machine + +### Risk: duplicate retry attempt creation + +Mitigation: + +- create attempt entries only in the retry scheduling path +- use one active pending/running attempt at a time + +### Risk: stale child-session events corrupt the latest attempt + +Mitigation: + +- require `attemptID`/`currentAttemptID` +- require `sessionID -> attemptID` lookup for all child-session lifecycle events +- finalize attempts immutably +- ignore late events from superseded session ids + +### Risk: noisy parent chat + +Mitigation: + +- keep the final timeline compact +- use reminders only at retry boundaries and final completion + +## Recommendation + +Implement the attempt timeline as structured `BackgroundTask` state first, and derive parent chat summaries from that. This gives the cleanest UX while preserving separate retry child sessions and sets up future UI improvements without relying on fragile text parsing. diff --git a/src/create-managers.ts b/src/create-managers.ts index 9c0013fd4..602cc8502 100644 --- a/src/create-managers.ts +++ b/src/create-managers.ts @@ -58,6 +58,7 @@ export function createManagers(args: { deps.markServerRunningInProcessFn() } const tmuxSessionManager = new deps.TmuxSessionManagerClass(ctx, tmuxConfig) + const modelFallbackControllerAccessor = createModelFallbackControllerAccessor() deps.registerManagerForCleanupFn({ shutdown: async () => { @@ -110,6 +111,7 @@ export function createManagers(args: { }) }, enableParentSessionNotifications: backgroundNotificationHookEnabled, + modelFallbackControllerAccessor, }, ) @@ -122,8 +124,6 @@ export function createManagers(args: { pluginConfig, modelCacheState, }) - const modelFallbackControllerAccessor = createModelFallbackControllerAccessor() - return { tmuxSessionManager, backgroundManager, diff --git a/src/features/background-agent/attempt-lifecycle.ts b/src/features/background-agent/attempt-lifecycle.ts new file mode 100644 index 000000000..428e60b9c --- /dev/null +++ b/src/features/background-agent/attempt-lifecycle.ts @@ -0,0 +1,174 @@ +import type { DelegatedModelConfig } from "../../shared/model-resolution-types" +import type { BackgroundTask, BackgroundTaskAttempt, BackgroundTaskStatus } from "./types" + +type TerminalAttemptStatus = Extract + +function toAttemptModel(model: DelegatedModelConfig | undefined): Pick { + return { + providerID: model?.providerID, + modelID: model?.modelID, + variant: model?.variant, + } +} + +function toTaskModel(attempt: BackgroundTaskAttempt): DelegatedModelConfig | undefined { + if (!attempt.providerID || !attempt.modelID) { + return undefined + } + + return { + providerID: attempt.providerID, + modelID: attempt.modelID, + ...(attempt.variant ? { variant: attempt.variant } : {}), + } +} + +function getAttemptIndex(task: BackgroundTask, attemptID: string): number { + return task.attempts?.findIndex((attempt) => attempt.attemptID === attemptID) ?? -1 +} + +function getAttempt(task: BackgroundTask, attemptID: string): BackgroundTaskAttempt | undefined { + const index = getAttemptIndex(task, attemptID) + return index === -1 ? undefined : task.attempts?.[index] +} + +function isTerminalStatus(status: BackgroundTaskStatus): status is TerminalAttemptStatus { + return status === "completed" || status === "error" || status === "cancelled" || status === "interrupt" +} + +export function getCurrentAttempt(task: BackgroundTask): BackgroundTaskAttempt | undefined { + if (!task.currentAttemptID) { + return undefined + } + + return getAttempt(task, task.currentAttemptID) +} + +export function ensureCurrentAttempt( + task: BackgroundTask, + model: DelegatedModelConfig | undefined = task.model, +): BackgroundTaskAttempt { + const existingAttempt = getCurrentAttempt(task) + if (existingAttempt) { + return existingAttempt + } + + const attempt: BackgroundTaskAttempt = { + attemptID: `att_${crypto.randomUUID().slice(0, 8)}`, + attemptNumber: (task.attempts?.length ?? 0) + 1, + sessionID: task.sessionID, + ...toAttemptModel(model), + status: task.status, + error: task.error, + startedAt: task.startedAt, + completedAt: task.completedAt, + } + + task.attempts = [...(task.attempts ?? []), attempt] + task.currentAttemptID = attempt.attemptID + return attempt +} + +export function projectTaskFromCurrentAttempt(task: BackgroundTask): BackgroundTask { + const currentAttempt = getCurrentAttempt(task) + if (!currentAttempt) { + return task + } + + task.status = currentAttempt.status + task.sessionID = currentAttempt.sessionID + task.startedAt = currentAttempt.startedAt + task.completedAt = currentAttempt.completedAt + task.error = currentAttempt.error + task.model = toTaskModel(currentAttempt) + + return task +} + +export function startAttempt(task: BackgroundTask, model: DelegatedModelConfig | undefined): BackgroundTaskAttempt { + const attempt: BackgroundTaskAttempt = { + attemptID: `att_${crypto.randomUUID().slice(0, 8)}`, + attemptNumber: (task.attempts?.length ?? 0) + 1, + ...toAttemptModel(model), + status: "pending", + } + + task.attempts = [...(task.attempts ?? []), attempt] + task.currentAttemptID = attempt.attemptID + task.status = "pending" + task.sessionID = undefined + task.startedAt = undefined + task.completedAt = undefined + task.error = undefined + task.model = model + + return attempt +} + +export function bindAttemptSession( + task: BackgroundTask, + attemptID: string, + sessionID: string, + model: DelegatedModelConfig | undefined, +): BackgroundTaskAttempt | undefined { + ensureCurrentAttempt(task, model) + if (task.currentAttemptID !== attemptID) { + return undefined + } + + const attempt = getAttempt(task, attemptID) + if (!attempt || isTerminalStatus(attempt.status)) { + return undefined + } + + attempt.sessionID = sessionID + attempt.status = "running" + attempt.startedAt = new Date() + attempt.completedAt = undefined + attempt.error = undefined + attempt.providerID = model?.providerID ?? attempt.providerID + attempt.modelID = model?.modelID ?? attempt.modelID + attempt.variant = model?.variant ?? attempt.variant + + return getCurrentAttempt(projectTaskFromCurrentAttempt(task)) +} + +export function finalizeAttempt( + task: BackgroundTask, + attemptID: string, + status: TerminalAttemptStatus, + error?: string, +): BackgroundTaskAttempt | undefined { + const attempt = getAttempt(task, attemptID) + if (!attempt) { + return undefined + } + + attempt.status = status + attempt.completedAt = new Date() + attempt.error = error + + if (task.currentAttemptID === attemptID) { + projectTaskFromCurrentAttempt(task) + } + + return attempt +} + +export function scheduleRetryAttempt( + task: BackgroundTask, + failedAttemptID: string, + nextModel: DelegatedModelConfig, + error?: string, +): BackgroundTaskAttempt | undefined { + const failedAttempt = finalizeAttempt(task, failedAttemptID, "error", error) + if (!failedAttempt || task.currentAttemptID !== failedAttemptID) { + return undefined + } + + return startAttempt(task, nextModel) +} + +export function findAttemptBySession(task: BackgroundTask, sessionID: string): BackgroundTaskAttempt | undefined { + return task.attempts?.find((attempt) => attempt.sessionID === sessionID) +} diff --git a/src/features/background-agent/background-task-notification-template.test.ts b/src/features/background-agent/background-task-notification-template.test.ts index 37b416570..183975080 100644 --- a/src/features/background-agent/background-task-notification-template.test.ts +++ b/src/features/background-agent/background-task-notification-template.test.ts @@ -154,6 +154,76 @@ Use \`background_output(task_id="")\` to retrieve each result. }) }) + describe("#given a completed task with retry attempt history", () => { + test("#when building the final notification #then it renders the spec-aligned balanced attempt timeline", () => { + // given + const notification = buildBackgroundTaskNotificationText({ + task: { + id: "task-3", + description: "Fallback task", + status: "completed", + attempts: [ + { + attemptID: "att-1", + attemptNumber: 1, + sessionID: "ses-primary", + providerID: "genai-proxy-openai", + modelID: "gpt-5.4-mini", + status: "error", + error: "Forbidden: Selected provider is forbidden", + }, + { + attemptID: "att-2", + attemptNumber: 2, + sessionID: "ses-fallback", + providerID: "anthropic", + modelID: "claude-haiku-4.5", + status: "completed", + }, + ], + }, + duration: "10s", + statusText: "COMPLETED", + allComplete: true, + remainingCount: 0, + completedTasks: [ + { + id: "task-3", + description: "Fallback task", + status: "completed", + attempts: [ + { + attemptID: "att-1", + attemptNumber: 1, + sessionID: "ses-primary", + providerID: "genai-proxy-openai", + modelID: "gpt-5.4-mini", + status: "error", + error: "Forbidden: Selected provider is forbidden", + }, + { + attemptID: "att-2", + attemptNumber: 2, + sessionID: "ses-fallback", + providerID: "anthropic", + modelID: "claude-haiku-4.5", + status: "completed", + }, + ], + }, + ], + }) + + // then + expect(notification).toContain("[ALL BACKGROUND TASKS COMPLETE]") + expect(notification).toContain("- `task-3`: Fallback task") + expect(notification).toContain("Background task attempts:") + expect(notification).toContain(" - Attempt 1 — ERROR — genai-proxy-openai/gpt-5.4-mini — ses-primary") + expect(notification).toContain(" Error: Forbidden: Selected provider is forbidden") + expect(notification).toContain(" - Attempt 2 — COMPLETED — anthropic/claude-haiku-4.5 — ses-fallback") + }) + }) + describe("#given a single task notification with undefined description", () => { test("#when building the partial notification #then it uses task ID as fallback", () => { // given diff --git a/src/features/background-agent/background-task-notification-template.ts b/src/features/background-agent/background-task-notification-template.ts index ad6769fac..c3472c4a0 100644 --- a/src/features/background-agent/background-task-notification-template.ts +++ b/src/features/background-agent/background-task-notification-template.ts @@ -1,4 +1,4 @@ -import type { BackgroundTaskStatus } from "./types" +import type { BackgroundTaskAttempt, BackgroundTaskStatus } from "./types" export type BackgroundTaskNotificationStatus = "COMPLETED" | "CANCELLED" | "INTERRUPTED" | "ERROR" @@ -7,6 +7,55 @@ export interface BackgroundTaskNotificationTask { description: string status: BackgroundTaskStatus error?: string + attempts?: BackgroundTaskAttempt[] +} + +function formatAttemptModel(attempt: BackgroundTaskAttempt): string { + if (attempt.providerID && attempt.modelID) { + return `${attempt.providerID}/${attempt.modelID}` + } + + if (attempt.modelID) { + return attempt.modelID + } + + if (attempt.providerID) { + return attempt.providerID + } + + return "unknown-model" +} + +function formatAttemptTimeline(task: BackgroundTaskNotificationTask): string { + if (!task.attempts || task.attempts.length <= 1) { + return "" + } + + const lines = task.attempts + .map((attempt) => { + const attemptLines = [ + ` - Attempt ${attempt.attemptNumber} — ${attempt.status.toUpperCase()} — ${formatAttemptModel(attempt)} — ${attempt.sessionID ?? "unknown"}`, + ] + + if (attempt.status !== "completed" && attempt.error) { + attemptLines.push(` Error: ${attempt.error}`) + } + + return attemptLines.join("\n") + }) + .join("\n") + + return `Background task attempts:\n${lines}` +} + +function formatTaskSummaryLine(task: BackgroundTaskNotificationTask): string { + const baseLine = `- \`${task.id}\`: ${task.description || task.id}` + const statusSuffix = task.status === "completed" + ? "" + : ` [${task.status.toUpperCase()}]${task.error ? ` - ${task.error}` : ""}` + const timeline = formatAttemptTimeline(task) + + return `${baseLine}${statusSuffix}${timeline ? `\n${timeline}` : ""}` } export function buildBackgroundTaskNotificationText(input: { @@ -27,10 +76,10 @@ export function buildBackgroundTaskNotificationText(input: { const failedTasks = completedTasks.filter((t) => t.status !== "completed") const succeededText = succeededTasks.length > 0 - ? succeededTasks.map((t) => `- \`${t.id}\`: ${safeDescription(t)}`).join("\n") + ? succeededTasks.map((t) => formatTaskSummaryLine(t)).join("\n") : "" const failedText = failedTasks.length > 0 - ? failedTasks.map((t) => `- \`${t.id}\`: ${safeDescription(t)} [${t.status.toUpperCase()}]${t.error ? ` - ${t.error}` : ""}`).join("\n") + ? failedTasks.map((t) => formatTaskSummaryLine(t)).join("\n") : "" const hasFailures = failedTasks.length > 0 @@ -46,7 +95,7 @@ export function buildBackgroundTaskNotificationText(input: { body += `\n**Failed:**\n${failedText}\n` } if (!body) { - body = `- \`${task.id}\`: ${safeDescription(task)} [${task.status.toUpperCase()}]${task.error ? ` - ${task.error}` : ""}\n` + body = `${formatTaskSummaryLine(task)}\n` } return ` diff --git a/src/features/background-agent/constants.ts b/src/features/background-agent/constants.ts index 4129a2510..b622bae3c 100644 --- a/src/features/background-agent/constants.ts +++ b/src/features/background-agent/constants.ts @@ -45,6 +45,7 @@ export interface Todo { } export interface QueueItem { + attemptID: string task: BackgroundTask input: LaunchInput } diff --git a/src/features/background-agent/error-classifier.test.ts b/src/features/background-agent/error-classifier.test.ts index 1fe24e93d..156c6ef4c 100644 --- a/src/features/background-agent/error-classifier.test.ts +++ b/src/features/background-agent/error-classifier.test.ts @@ -251,24 +251,24 @@ describe("extractErrorMessage", () => { }) }) - describe("#given complex error with data wrapper", () => { - test("extracts from error.data.message", () => { - const error = { - data: { - message: "data message", - }, - } - expect(extractErrorMessage(error)).toBe("data message") - }) + describe("#given complex error with data wrapper", () => { + test("extracts from error.data.message", () => { + const error = { + data: { + message: "data message", + }, + } + expect(extractErrorMessage(error)).toBe("data message") + }) - test("prefers top over nested-level message", () => { - const error = { - message: "top level", - data: { message: "nested" }, - } - expect(extractErrorMessage(error)).toBe("top level") - }) - }) + test("prefers nested message over generic top-level message", () => { + const error = { + message: "Error", + data: { message: "Forbidden: Selected provider is forbidden" }, + } + expect(extractErrorMessage(error)).toBe("Forbidden: Selected provider is forbidden") + }) + }) describe("#given invalid inputs", () => { test("returns undefined for null", () => { diff --git a/src/features/background-agent/error-classifier.ts b/src/features/background-agent/error-classifier.ts index 5c7e90b46..523f61bc1 100644 --- a/src/features/background-agent/error-classifier.ts +++ b/src/features/background-agent/error-classifier.ts @@ -33,16 +33,15 @@ export function extractErrorName(error: unknown): string | undefined { export function extractErrorMessage(error: unknown): string | undefined { if (!error) return undefined if (typeof error === "string") return error - if (error instanceof Error) return error.message if (isRecord(error)) { const dataRaw = error["data"] const candidates: unknown[] = [ - error, dataRaw, - error["error"], isRecord(dataRaw) ? (dataRaw as Record)["error"] : undefined, + error["error"], error["cause"], + error, ] for (const candidate of candidates) { @@ -57,6 +56,8 @@ export function extractErrorMessage(error: unknown): string | undefined { } } + if (error instanceof Error) return error.message + try { return JSON.stringify(error) } catch { diff --git a/src/features/background-agent/fallback-retry-handler.test.ts b/src/features/background-agent/fallback-retry-handler.test.ts index 78e632bd2..6456f1d09 100644 --- a/src/features/background-agent/fallback-retry-handler.test.ts +++ b/src/features/background-agent/fallback-retry-handler.test.ts @@ -259,6 +259,57 @@ describe("tryFallbackRetry", () => { expect(queue![0].task).toBe(args.task) expect(args.processKey).toHaveBeenCalledWith(key) }) + + test("finalizes the failed attempt, creates a new pending attempt, and enqueues its explicit attemptID", async () => { + const args = createDefaultArgs({ + status: "running", + sessionID: "session-attempt-1", + startedAt: new Date("2026-04-27T00:00:00.000Z"), + attempts: [ + { + attemptID: "attempt-1", + attemptNumber: 1, + sessionID: "session-attempt-1", + providerID: "provider-a", + modelID: "original-model", + status: "running", + startedAt: new Date("2026-04-27T00:00:00.000Z"), + }, + ], + currentAttemptID: "attempt-1", + }) + + await tryFallbackRetry(args) + + expect(args.task.attempts).toHaveLength(2) + expect(args.task.attempts?.[0]).toMatchObject({ + attemptID: "attempt-1", + sessionID: "session-attempt-1", + status: "error", + error: "model overloaded", + }) + expect(args.task.attempts?.[0]?.completedAt).toBeInstanceOf(Date) + + const nextAttempt = args.task.attempts?.[1] + expect(nextAttempt).toBeDefined() + expect(nextAttempt?.attemptNumber).toBe(2) + expect(nextAttempt?.providerID).toBe("provider-a") + expect(nextAttempt?.modelID).toBe("fallback-model-1") + expect(nextAttempt?.status).toBe("pending") + + expect(args.task.currentAttemptID).toBe(nextAttempt?.attemptID) + expect(args.task.status).toBe("pending") + expect(args.task.model).toEqual({ + providerID: "provider-a", + modelID: "fallback-model-1", + variant: undefined, + }) + + const key = `${args.task.model!.providerID}/${args.task.model!.modelID}` + const queue = args.queuesByKey.get(key) + expect(queue).toBeDefined() + expect((queue?.[0] as QueueItem & { attemptID?: string })?.attemptID).toBe(nextAttempt?.attemptID) + }) }) describe("#given non-retryable error", () => { @@ -343,6 +394,25 @@ describe("tryFallbackRetry", () => { }) }) + describe("#given first fallback is a no-op for the current model", () => { + test("skips the no-op fallback and advances to the next distinct model", async () => { + const args = createDefaultArgs({ + model: { providerID: "provider-a", modelID: "fallback-model-1" }, + fallbackChain: [ + { model: "fallback-model-1", providers: ["provider-a"], variant: undefined }, + { model: "fallback-model-2", providers: ["provider-b"], variant: undefined }, + ], + }) + + const result = await tryFallbackRetry(args) + + expect(result).toBe(true) + expect(args.task.model?.providerID).toBe("provider-b") + expect(args.task.model?.modelID).toBe("fallback-model-2") + expect(args.task.attemptCount).toBe(2) + }) + }) + describe("#given disconnected fallback providers with connected preferred provider", () => { test("keeps fallback entry and selects connected preferred provider", async () => { ;(readProviderModelsCache as any).mockReturnValueOnce({ connected: ["provider-a"] }) diff --git a/src/features/background-agent/fallback-retry-handler.ts b/src/features/background-agent/fallback-retry-handler.ts index 58549cc98..52fb2ab5a 100644 --- a/src/features/background-agent/fallback-retry-handler.ts +++ b/src/features/background-agent/fallback-retry-handler.ts @@ -11,6 +11,11 @@ import { } from "../../shared/model-error-classifier" import { transformModelForProvider } from "../../shared/provider-model-id-transform" import { abortWithTimeout } from "./abort-with-timeout" +import { ensureCurrentAttempt, scheduleRetryAttempt } from "./attempt-lifecycle" + +function canonicalizeModelID(modelID: string): string { + return modelID.toLowerCase().replace(/\./g, "-") +} export async function tryFallbackRetry(args: { task: BackgroundTask @@ -21,8 +26,16 @@ export async function tryFallbackRetry(args: { idleDeferralTimers: Map> queuesByKey: Map processKey: (key: string) => void + onRetrying?: (details: { + task: BackgroundTask + source: string + previousSessionID?: string + failedModel?: string + failedError?: string + nextModel: string + }) => void }): Promise { - const { task, errorInfo, source, concurrencyManager, client, idleDeferralTimers, queuesByKey, processKey } = args + const { task, errorInfo, source, concurrencyManager, client, idleDeferralTimers, queuesByKey, processKey, onRetrying } = args const fallbackChain = task.fallbackChain const canRetry = shouldRetryError(errorInfo) && @@ -48,6 +61,7 @@ export async function tryFallbackRetry(args: { let selectedAttemptCount = attemptCount let nextFallback: FallbackEntry | undefined + let nextProviderID: string | undefined while (fallbackChain && selectedAttemptCount < fallbackChain.length) { const candidate = getNextFallback(fallbackChain, selectedAttemptCount) if (!candidate) break @@ -61,12 +75,31 @@ export async function tryFallbackRetry(args: { }) continue } + const candidateProviderID = selectFallbackProvider( + candidate.providers, + task.model?.providerID, + ) + const candidateModelID = transformModelForProvider(candidateProviderID, candidate.model) + const isNoOpFallback = + !!task.model && + candidateProviderID.toLowerCase() === task.model.providerID.toLowerCase() && + canonicalizeModelID(candidateModelID) === canonicalizeModelID(task.model.modelID) + if (isNoOpFallback) { + log("[background-agent] Skipping no-op fallback:", { + taskId: task.id, + source, + model: candidate.model, + providers: candidate.providers, + }) + continue + } nextFallback = candidate + nextProviderID = candidateProviderID break } if (!nextFallback) return false - const providerID = selectFallbackProvider( + const providerID = nextProviderID ?? selectFallbackProvider( nextFallback.providers, task.model?.providerID, ) @@ -92,19 +125,39 @@ export async function tryFallbackRetry(args: { } const previousSessionID = task.sessionID + const previousModel = task.model - task.attemptCount = selectedAttemptCount const transformedModelId = transformModelForProvider(providerID, nextFallback.model) - task.model = { + const nextModel = { providerID, modelID: transformedModelId, variant: nextFallback.variant, } - task.status = "pending" - task.sessionID = undefined - task.startedAt = undefined + task.attemptCount = selectedAttemptCount + const failedAttemptID = ensureCurrentAttempt(task, previousModel).attemptID + const nextAttempt = failedAttemptID + ? scheduleRetryAttempt(task, failedAttemptID, nextModel, errorInfo.message) + : undefined + if (!nextAttempt) { + return false + } + task.queuedAt = new Date() - task.error = undefined + task.retryNotification = { + previousSessionID, + failedModel: previousModel ? `${previousModel.providerID}/${previousModel.modelID}` : undefined, + failedError: errorInfo.message, + nextModel: `${providerID}/${transformedModelId}`, + } + + onRetrying?.({ + task, + source, + previousSessionID, + failedModel: task.retryNotification.failedModel, + failedError: errorInfo.message, + nextModel: `${providerID}/${transformedModelId}`, + }) const key = task.model ? `${task.model.providerID}/${task.model.modelID}` : task.agent const queue = queuesByKey.get(key) ?? [] @@ -117,7 +170,7 @@ export async function tryFallbackRetry(args: { parentModel: task.parentModel, parentAgent: task.parentAgent, parentTools: task.parentTools, - model: task.model, + model: nextModel, fallbackChain: task.fallbackChain, category: task.category, isUnstableAgent: task.isUnstableAgent, @@ -127,7 +180,7 @@ export async function tryFallbackRetry(args: { await abortWithTimeout(client, previousSessionID).catch(() => {}) } - queue.push({ task, input: retryInput }) + queue.push({ task, input: retryInput, attemptID: nextAttempt.attemptID }) queuesByKey.set(key, queue) processKey(key) return true diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index 8c855ebcf..5c099d5f1 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -172,7 +172,7 @@ class MockBackgroundManager { } } -function createMockTask(overrides: Partial & { id: string; sessionID: string; parentSessionID: string }): BackgroundTask { +function createMockTask(overrides: Partial & { id: string; parentSessionID: string; sessionID?: string }): BackgroundTask { return { parentMessageID: "mock-message-id", description: "test task", @@ -195,6 +195,21 @@ function createBackgroundManager(): BackgroundManager { return new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) } +function createBackgroundManagerWithOptions(options: unknown): BackgroundManager { + const client = { + session: { + prompt: async () => ({}), + promptAsync: async () => ({}), + abort: async () => ({}), + }, + } + return new BackgroundManager( + { client, directory: tmpdir() } as unknown as PluginInput, + undefined, + options as ConstructorParameters[2], + ) +} + function getConcurrencyManager(manager: BackgroundManager): ConcurrencyManager { return (manager as unknown as { concurrencyManager: ConcurrencyManager }).concurrencyManager } @@ -271,6 +286,399 @@ function createToastRemoveTaskTracker(): { removeTaskCalls: string[]; resetToast } } +describe("BackgroundManager session.error fallback hydration", () => { + test("hydrates fallbackChain from session fallback state before retrying sync child-session errors", async () => { + //#given + const fallbackChain = [ + { model: "fallback-model-1", providers: ["provider-a"], variant: undefined }, + ] + const getSessionFallbackChain = mock((sessionID: string) => + sessionID === "child-session" ? fallbackChain : undefined, + ) + const manager = createBackgroundManagerWithOptions({ + modelFallbackControllerAccessor: { + getSessionFallbackChain, + }, + }) + const task = createMockTask({ + id: "task-sync-fallback", + sessionID: "child-session", + parentSessionID: "parent-session", + fallbackChain: undefined, + }) + let capturedFallbackChain: BackgroundTask["fallbackChain"] + ;(manager as unknown as { + tryFallbackRetry: (task: BackgroundTask, errorInfo: { name?: string; message?: string }, source: string) => Promise + }).tryFallbackRetry = async (retryTask) => { + capturedFallbackChain = retryTask.fallbackChain + return true + } + + //#when + await (manager as unknown as { + handleSessionErrorEvent: (args: { + task: BackgroundTask + errorInfo: { name?: string; message?: string } + errorName: string | undefined + errorMessage: string | undefined + }) => Promise + }).handleSessionErrorEvent({ + task, + errorInfo: { + name: "APIError", + message: "Forbidden: Selected provider is forbidden", + }, + errorName: "APIError", + errorMessage: "Forbidden: Selected provider is forbidden", + }) + + //#then + expect(getSessionFallbackChain).toHaveBeenCalledWith("child-session") + expect(task.fallbackChain).toEqual(fallbackChain) + expect(capturedFallbackChain).toEqual(fallbackChain) + }) +}) + +describe("BackgroundManager prompt rejection fallback routing", () => { + test("routes launch-time prompt rejections into tryFallbackRetry before marking interrupt", async () => { + //#given + const promptError = { + name: "APIError", + data: { message: "Forbidden: Selected provider is forbidden" }, + } + const client = { + session: { + get: async () => ({ data: { directory: tmpdir() } }), + create: async () => ({ data: { id: "ses_launch_retry" } }), + promptAsync: async () => { + throw promptError + }, + abort: async () => ({}), + }, + } + const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) + stubNotifyParentSession(manager) + ;(manager as unknown as { + reserveSubagentSpawn: () => Promise<{ + spawnContext: { rootSessionID: string; parentDepth: number; childDepth: number } + descendantCount: number + commit: () => number + rollback: () => void + }> + }).reserveSubagentSpawn = async () => ({ + spawnContext: { rootSessionID: "parent-session", parentDepth: 0, childDepth: 1 }, + descendantCount: 1, + commit: () => 1, + rollback: () => {}, + }) + const retried: Array<{ taskId: string; errorInfo: { name?: string; message?: string }; source: string }> = [] + ;(manager as unknown as { + tryFallbackRetry: (task: BackgroundTask, errorInfo: { name?: string; message?: string }, source: string) => Promise + }).tryFallbackRetry = async (task, errorInfo, source) => { + retried.push({ taskId: task.id, errorInfo, source }) + task.status = "pending" + task.error = undefined + return true + } + + //#when + const launchedTask = await manager.launch({ + description: "background retry test", + prompt: "say hi", + agent: "sisyphus-junior", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + model: { providerID: "genai-proxy-openai", modelID: "gpt-5.4-mini" }, + fallbackChain: [{ model: "claude-haiku-4-5", providers: ["anthropic"] }], + }) + await flushBackgroundNotifications() + + //#then + const storedTask = getTaskMap(manager).get(launchedTask.id) + expect(retried).toHaveLength(1) + expect(retried[0]?.source).toBe("promptAsync.launch") + expect(retried[0]?.errorInfo).toEqual({ + name: "APIError", + message: "Forbidden: Selected provider is forbidden", + }) + expect(storedTask?.status).toBe("pending") + }) + + test("routes resume-time prompt rejections into tryFallbackRetry before marking interrupt", async () => { + //#given + const promptError = { + name: "APIError", + data: { message: "Forbidden: Selected provider is forbidden" }, + } + const client = { + session: { + promptAsync: async () => { + throw promptError + }, + abort: async () => ({}), + }, + } + const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) + stubNotifyParentSession(manager) + const task: BackgroundTask = { + id: "bg_resume_retry", + sessionID: "ses_resume_retry", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + description: "resume retry test", + prompt: "say hi", + agent: "sisyphus-junior", + status: "completed", + startedAt: new Date(), + completedAt: new Date(), + model: { providerID: "genai-proxy-openai", modelID: "gpt-5.4-mini" }, + fallbackChain: [{ model: "claude-haiku-4-5", providers: ["anthropic"] }], + concurrencyGroup: "genai-proxy-openai/gpt-5.4-mini", + } + getTaskMap(manager).set(task.id, task) + const retried: Array<{ taskId: string; errorInfo: { name?: string; message?: string }; source: string }> = [] + ;(manager as unknown as { + tryFallbackRetry: (task: BackgroundTask, errorInfo: { name?: string; message?: string }, source: string) => Promise + }).tryFallbackRetry = async (retryTask, errorInfo, source) => { + retried.push({ taskId: retryTask.id, errorInfo, source }) + retryTask.status = "pending" + retryTask.error = undefined + return true + } + + //#when + await manager.resume({ + sessionId: "ses_resume_retry", + prompt: "continue", + parentSessionID: "parent-session", + parentMessageID: "parent-message-2", + }) + await flushBackgroundNotifications() + + //#then + const storedTask = getTaskMap(manager).get(task.id) + expect(retried).toHaveLength(1) + expect(retried[0]?.source).toBe("promptAsync.resume") + expect(retried[0]?.errorInfo).toEqual({ + name: "APIError", + message: "Forbidden: Selected provider is forbidden", + }) + expect(storedTask?.status).toBe("pending") + }) +}) + +describe("BackgroundManager retry observability", () => { + test("queues a parent-visible retry notification when fallback retry is scheduled", async () => { + //#given + const client = { + session: { + abort: async () => ({}), + }, + } + const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) + const task = createMockTask({ + id: "bg_retry_observable", + parentSessionID: "parent-session", + fallbackChain: [{ model: "claude-haiku-4-5", providers: ["anthropic"] }], + attemptCount: 0, + status: "running", + attempts: [ + { + attemptID: "att_retry_visibility", + attemptNumber: 1, + sessionID: "ses_retry_visibility", + providerID: "genai-proxy-openai", + modelID: "gpt-5.4-mini", + status: "running", + }, + ], + currentAttemptID: "att_retry_visibility", + }) + getTaskMap(manager).set(task.id, task) + const queuePendingNotification = mock(() => {}) + ;(manager as unknown as { + queuePendingNotification: (sessionID: string | undefined, notification: string) => void + }).queuePendingNotification = queuePendingNotification + + //#when + await (manager as unknown as { + tryFallbackRetry: (task: BackgroundTask, errorInfo: { name?: string; message?: string }, source: string) => Promise + }).tryFallbackRetry(task, { + name: "APIError", + message: "Forbidden: Selected provider is forbidden", + }, "promptAsync.launch") + + //#then + expect(queuePendingNotification).toHaveBeenCalledTimes(1) + const [sessionID, notification] = queuePendingNotification.mock.calls[0] + expect(sessionID).toBe("parent-session") + expect(notification).toContain("[BACKGROUND TASK RETRYING]") + expect(notification).toContain("ses_retry_visibility") + expect(notification).toContain("genai-proxy-openai/gpt-5.4-mini") + expect(notification).toContain("anthropic/claude-haiku-4.5") + }) + + test("queues a second parent-visible notification once the retry session ID is created", async () => { + //#given + const queuePendingNotification = mock(() => {}) + const client = { + session: { + get: async () => ({ data: { directory: tmpdir() } }), + create: async () => ({ data: { id: "ses_retry_created" } }), + promptAsync: async () => ({}), + }, + } + const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) + ;(manager as unknown as { + queuePendingNotification: (sessionID: string | undefined, notification: string) => void + }).queuePendingNotification = queuePendingNotification + const task = createMockTask({ + id: "bg_retry_ready", + parentSessionID: "parent-session", + status: "pending", + attemptCount: 1, + queuedAt: new Date(), + model: { providerID: "anthropic", modelID: "claude-haiku-4.5" }, + fallbackChain: [{ model: "claude-haiku-4-5", providers: ["anthropic"] }], + concurrencyGroup: "anthropic/claude-haiku-4.5", + retryNotification: { + nextModel: "anthropic/claude-haiku-4.5", + }, + attempts: [ + { + attemptID: "att_retry_failed", + attemptNumber: 1, + sessionID: "ses_retry_visibility", + providerID: "genai-proxy-openai", + modelID: "gpt-5.4-mini", + status: "error", + error: "Forbidden: Selected provider is forbidden", + }, + { + attemptID: "att_retry_ready", + attemptNumber: 2, + providerID: "anthropic", + modelID: "claude-haiku-4.5", + status: "pending", + }, + ], + currentAttemptID: "att_retry_ready", + }) + getTaskMap(manager).set(task.id, task) + const taskInput = { + description: task.description, + prompt: task.prompt, + agent: task.agent, + parentSessionID: task.parentSessionID, + parentMessageID: task.parentMessageID, + model: task.model, + fallbackChain: task.fallbackChain, + category: task.category, + } + type RetryReadyQueueItem = { + task: BackgroundTask + input: typeof taskInput + attemptID: string + } + const item: RetryReadyQueueItem = { + task, + input: taskInput, + attemptID: task.currentAttemptID ?? "att_retry_ready", + } + + //#when + await (manager as unknown as { + startTask: (queueItem: RetryReadyQueueItem) => Promise + }).startTask(item) + + //#then + const notifications = queuePendingNotification.mock.calls.map((call) => call[1]) + const retryReadyNotification = notifications.find((notification) => notification.includes("[BACKGROUND TASK RETRY SESSION READY]")) + const expectedRetryLink = `http://127.0.0.1:4096/${Buffer.from(tmpdir()).toString("base64url")}/session/ses_retry_created` + expect(retryReadyNotification).toBeDefined() + expect(retryReadyNotification).toContain("**Retry attempt:** 2") + expect(retryReadyNotification).toContain("ses_retry_created") + expect(retryReadyNotification).toContain(expectedRetryLink) + expect(retryReadyNotification).toContain("ses_retry_visibility") + expect(retryReadyNotification).toContain("genai-proxy-openai/gpt-5.4-mini") + expect(retryReadyNotification).toContain("Forbidden: Selected provider is forbidden") + }) + + test("builds retry-ready links from the parent session directory when it differs from the manager directory", async () => { + //#given + const queuePendingNotification = mock(() => {}) + const managerDirectory = "/manager/dir" + const parentDirectory = "/parent/dir" + const client = { + session: { + get: async () => ({ data: { directory: parentDirectory } }), + create: async () => ({ data: { id: "ses_retry_created_parent_dir" } }), + promptAsync: async () => ({}), + }, + } + const manager = new BackgroundManager({ client, directory: managerDirectory } as unknown as PluginInput) + ;(manager as unknown as { + queuePendingNotification: (sessionID: string | undefined, notification: string) => void + }).queuePendingNotification = queuePendingNotification + const task = createMockTask({ + id: "bg_retry_ready_parent_dir", + parentSessionID: "parent-session", + status: "pending", + attemptCount: 1, + queuedAt: new Date(), + model: { providerID: "anthropic", modelID: "claude-haiku-4.5" }, + retryNotification: { + nextModel: "anthropic/claude-haiku-4.5", + }, + attempts: [ + { + attemptID: "att_retry_failed_parent_dir", + attemptNumber: 1, + sessionID: "ses_retry_failed_parent_dir", + providerID: "genai-proxy-openai", + modelID: "gpt-5.4-mini", + status: "error", + error: "Forbidden: Selected provider is forbidden", + }, + { + attemptID: "att_retry_ready_parent_dir", + attemptNumber: 2, + providerID: "anthropic", + modelID: "claude-haiku-4.5", + status: "pending", + }, + ], + currentAttemptID: "att_retry_ready_parent_dir", + }) + getTaskMap(manager).set(task.id, task) + const taskInput = { + description: task.description, + prompt: task.prompt, + agent: task.agent, + parentSessionID: task.parentSessionID, + parentMessageID: task.parentMessageID, + model: task.model, + fallbackChain: task.fallbackChain, + category: task.category, + } + + //#when + await (manager as unknown as { + startTask: (queueItem: { task: BackgroundTask; input: typeof taskInput; attemptID: string }) => Promise + }).startTask({ task, input: taskInput, attemptID: "att_retry_ready_parent_dir" }) + + //#then + const retryReadyNotification = queuePendingNotification.mock.calls + .map((call) => call[1]) + .find((notification) => notification.includes("[BACKGROUND TASK RETRY SESSION READY]")) + const expectedRetryLink = `http://127.0.0.1:4096/${Buffer.from(parentDirectory).toString("base64url")}/session/ses_retry_created_parent_dir` + expect(retryReadyNotification).toBeDefined() + expect(retryReadyNotification).toContain(expectedRetryLink) + + manager.shutdown() + }) +}) + function getCleanupSignals(): Array { const signals: Array = ["SIGINT", "SIGTERM", "beforeExit", "exit"] if (process.platform === "win32") { @@ -293,8 +701,6 @@ describe("BackgroundManager.getAllDescendantTasks", () => { }) test("should return empty array when no tasks exist", () => { - // given - empty manager - // when const result = manager.getAllDescendantTasks("session-a") @@ -504,7 +910,7 @@ describe("BackgroundManager.notifyParentSession - release ordering", () => { }) test("should keep queue blocked if release is after prompt (demonstrates the bug)", async () => { - // given - same setup + // given const { ConcurrencyManager } = await import("./concurrency") const concurrencyManager = new ConcurrencyManager({ defaultConcurrency: 1 }) @@ -657,9 +1063,7 @@ describe("BackgroundManager.resume", () => { }) test("should throw error when task not found", () => { - // given - empty manager - - // when / #then + // when / then expect(() => manager.resume({ sessionId: "non-existent", prompt: "continue", @@ -813,7 +1217,7 @@ describe("LaunchInput.skillContent", () => { parentMessageID: "parent-msg", } - // when / #then - should compile without skillContent + // when / then expect(input.skillContent).toBeUndefined() }) @@ -896,7 +1300,7 @@ describe("BackgroundManager.notifyParentSession - dynamic message lookup", () => }) test("should use currentMessage model/agent when available", async () => { - // given - currentMessage has model and agent + // given const task: BackgroundTask = { id: "task-1", sessionID: "session-child", @@ -919,7 +1323,7 @@ describe("BackgroundManager.notifyParentSession - dynamic message lookup", () => // when const promptBody = buildNotificationPromptBody(task, currentMessage) - // then - uses currentMessage values, not task.parentModel/parentAgent + // then expect(promptBody.agent).toBe("sisyphus") expect(promptBody.model).toEqual({ providerID: "anthropic", modelID: "claude-opus-4.7" }) }) @@ -945,7 +1349,7 @@ describe("BackgroundManager.notifyParentSession - dynamic message lookup", () => // when const promptBody = buildNotificationPromptBody(task, currentMessage) - // then - falls back to task.parentAgent + // then expect(promptBody.agent).toBe("FallbackAgent") expect("model" in promptBody).toBe(false) }) @@ -974,7 +1378,7 @@ describe("BackgroundManager.notifyParentSession - dynamic message lookup", () => // when const promptBody = buildNotificationPromptBody(task, currentMessage) - // then - model not passed due to incomplete data + // then expect(promptBody.agent).toBe("sisyphus") expect("model" in promptBody).toBe(false) }) @@ -999,7 +1403,7 @@ describe("BackgroundManager.notifyParentSession - dynamic message lookup", () => // when const promptBody = buildNotificationPromptBody(task, null) - // then - falls back to task.parentAgent, no model + // then expect(promptBody.agent).toBe("sisyphus") expect("model" in promptBody).toBe(false) }) @@ -1839,7 +2243,7 @@ describe("BackgroundManager.resume model persistence", () => { parentMessageID: "msg-2", }) - // then - model should be passed in prompt body + // then expect(promptCalls).toHaveLength(1) expect(promptCalls[0].body.model).toEqual({ providerID: "anthropic", modelID: "claude-sonnet-4-20250514" }) expect(promptCalls[0].body.agent).toBe("explore") @@ -1924,7 +2328,7 @@ describe("BackgroundManager.resume model persistence", () => { parentMessageID: "msg-2", }) - // then - model should NOT be in prompt body + // then expect(promptCalls).toHaveLength(1) expect("model" in promptCalls[0].body).toBe(false) expect(promptCalls[0].body.agent).toBe("explore") @@ -2037,11 +2441,48 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { expect(task.sessionID).toBeUndefined() }) + test("should initialize attempt state for a newly launched task", async () => { + // given + const input = { + description: "Test task", + prompt: "Do something", + agent: "test-agent", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + model: { + providerID: "openai", + modelID: "gpt-5.4-mini", + variant: "medium", + }, + } + + // when + const task = await manager.launch(input) + + // then + expect(task.attempts).toHaveLength(1) + expect(task.currentAttemptID).toBe(task.attempts?.[0]?.attemptID) + expect(task.attempts?.[0]).toEqual({ + attemptID: task.currentAttemptID, + attemptNumber: 1, + providerID: "openai", + modelID: "gpt-5.4-mini", + variant: "medium", + status: "pending", + }) + + expect(task.status).toBe("pending") + expect(task.model).toEqual(input.model) + expect(task.queuedAt).toBeInstanceOf(Date) + expect(task.startedAt).toBeUndefined() + expect(task.sessionID).toBeUndefined() + }) + test("should return immediately even with concurrency limit", async () => { // given const config = { defaultConcurrency: 1 } - manager.shutdown() - manager = new BackgroundManager({ client: mockClient, directory: tmpdir() } as unknown as PluginInput, config) + manager.shutdown() + manager = new BackgroundManager({ client: mockClient, directory: tmpdir() } as unknown as PluginInput, config) const input = { description: "Test task", @@ -2058,7 +2499,7 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { const endTime = Date.now() // then - expect(endTime - startTime).toBeLessThan(100) // Should be instant + expect(endTime - startTime).toBeLessThan(100) expect(task1.status).toBe("pending") expect(task2.status).toBe("pending") }) @@ -2224,8 +2665,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { // when const task = await manager.launch(input) - - // Give processKey time to run await new Promise(resolve => setTimeout(resolve, 50)) // then @@ -2254,7 +2693,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { const task = await manager.launch(input) const queuedAt = task.queuedAt - // Wait for transition await new Promise(resolve => setTimeout(resolve, 50)) // then @@ -3027,8 +3465,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { const task1 = await manager.launch(input) const task2 = await manager.launch(input) - - // Wait for first task to start await new Promise(resolve => setTimeout(resolve, 50)) // when @@ -3056,8 +3492,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { } const task = await manager.launch(input) - - // Wait for task to start await new Promise(resolve => setTimeout(resolve, 50)) // when @@ -3086,8 +3520,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { const task1 = await manager.launch(input) const task2 = await manager.launch(input) const task3 = await manager.launch(input) - - // Wait for first task to start await new Promise(resolve => setTimeout(resolve, 100)) // when - cancel middle task @@ -3194,8 +3626,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { // when const task1 = await manager.launch(input1) const task2 = await manager.launch(input2) - - // Wait for both to start await new Promise(resolve => setTimeout(resolve, 50)) // then - both should be running despite limit of 1 (different keys) @@ -3223,8 +3653,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { // when const task1 = await manager.launch(input) const task2 = await manager.launch(input) - - // Wait for processing await new Promise(resolve => setTimeout(resolve, 50)) // then - same key should respect limit @@ -3262,8 +3690,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { // when const task1 = await manager.launch(input1) const task2 = await manager.launch(input2) - - // Wait for both to start await new Promise(resolve => setTimeout(resolve, 50)) // then - different models should run in parallel @@ -3290,11 +3716,8 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { parentMessageID: "parent-message", } - // Launch two tasks (second will be pending) await manager.launch(input) const task2 = await manager.launch(input) - - // Wait for first to start await new Promise(resolve => setTimeout(resolve, 50)) // when @@ -3305,7 +3728,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { expect(pendingTask?.queuedAt).toBeInstanceOf(Date) expect(pendingTask?.startedAt).toBeUndefined() - // Verify TTL would use queuedAt (implementation detail check) const now = Date.now() const age = now - pendingTask!.queuedAt!.getTime() expect(age).toBeGreaterThanOrEqual(0) @@ -3327,8 +3749,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { // when const task = await manager.launch(input) - - // Wait for task to start await new Promise(resolve => setTimeout(resolve, 50)) // then @@ -3336,7 +3756,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { expect(runningTask?.status).toBe("running") expect(runningTask?.startedAt).toBeInstanceOf(Date) - // Verify TTL would use startedAt (implementation detail check) const now = Date.now() const age = now - runningTask!.startedAt!.getTime() expect(age).toBeGreaterThanOrEqual(0) @@ -3356,16 +3775,13 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { parentMessageID: "parent-message", } - // Launch task that will queue await manager.launch(input) const task2 = await manager.launch(input) const queuedAt = task2.queuedAt! - // Wait for first task to complete and second to start await new Promise(resolve => setTimeout(resolve, 50)) - // Simulate first task completion const tasks = Array.from(getTaskMap(manager).values()) const runningTask = tasks.find(t => t.status === "running" && t.id !== task2.id) if (runningTask?.concurrencyKey) { @@ -3373,7 +3789,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { getConcurrencyManager(manager).release(runningTask.concurrencyKey) } - // Wait for second task to start await new Promise(resolve => setTimeout(resolve, 100)) // then @@ -3408,17 +3823,15 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { const endTime = Date.now() // then - expect(endTime - startTime).toBeLessThan(200) // Should be very fast + expect(endTime - startTime).toBeLessThan(200) expect(tasks).toHaveLength(10) tasks.forEach(task => { expect(task.status).toBe("pending") expect(task.id).toMatch(/^bg_/) }) - // Wait for processing await new Promise(resolve => setTimeout(resolve, 100)) - // Verify 5 running, 5 pending const updatedTasks = tasks.map(t => manager.getTask(t.id)) const runningCount = updatedTasks.filter(t => t?.status === "running").length const pendingCount = updatedTasks.filter(t => t?.status === "pending").length @@ -5545,3 +5958,314 @@ describe("BackgroundManager - tool permission spread order", () => { manager.shutdown() }) }) + +describe("BackgroundManager.launch - attempt state initialization", () => { + test("newly launched task has attempt state with attemptNumber 1 and currentAttemptID pointing at it", async () => { + //#given + const manager = createBackgroundManager() + ;(manager as unknown as { + reserveSubagentSpawn: () => Promise<{ + spawnContext: { rootSessionID: string; parentDepth: number; childDepth: number } + descendantCount: number + commit: () => number + rollback: () => void + }> + }).reserveSubagentSpawn = async () => ({ + spawnContext: { rootSessionID: "parent-session", parentDepth: 0, childDepth: 1 }, + descendantCount: 1, + commit: () => 1, + rollback: () => {}, + }) + + //#when + const task = await manager.launch({ + description: "attempt state test", + prompt: "do something", + agent: "explore", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + model: { providerID: "anthropic", modelID: "claude-haiku-4.5" }, + }) + + //#then + const stored = getTaskMap(manager).get(task.id) + + expect(stored?.attempts).toBeDefined() + expect(stored?.attempts).toHaveLength(1) + + const firstAttempt = stored?.attempts?.[0] + expect(firstAttempt?.attemptNumber).toBe(1) + expect(firstAttempt?.status).toBe("pending") + expect(firstAttempt?.providerID).toBe("anthropic") + expect(firstAttempt?.modelID).toBe("claude-haiku-4.5") + + expect(stored?.currentAttemptID).toBeDefined() + expect(stored?.currentAttemptID).toBe(firstAttempt?.attemptID) + + expect(stored?.status).toBeDefined() + expect(stored?.model).toBeDefined() + expect(stored?.parentSessionID).toBe("parent-session") + + manager.shutdown() + }) +}) + +describe("BackgroundManager attempt lifecycle bindings", () => { + test("startTask binds the created session to the queued attempt ID and mirrors task projection", async () => { + //#given + const client = { + session: { + get: async () => ({ data: { directory: "/test/dir" } }), + create: async () => ({ data: { id: "session-attempt-2" } }), + promptAsync: async () => ({}), + abort: async () => ({}), + }, + } + const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) + const task: BackgroundTask = { + id: "task-attempt-binding", + status: "pending", + queuedAt: new Date(), + description: "retry binding task", + prompt: "continue", + agent: "sisyphus-junior", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + model: { providerID: "anthropic", modelID: "claude-haiku-4.5", variant: "max" }, + attempts: [ + { + attemptID: "attempt-1", + attemptNumber: 1, + sessionID: "session-attempt-1", + providerID: "openai", + modelID: "gpt-5.4-mini", + status: "error", + error: "first attempt failed", + startedAt: new Date("2026-04-27T00:00:00.000Z"), + completedAt: new Date("2026-04-27T00:00:05.000Z"), + }, + { + attemptID: "attempt-2", + attemptNumber: 2, + providerID: "anthropic", + modelID: "claude-haiku-4.5", + variant: "max", + status: "pending", + }, + ], + currentAttemptID: "attempt-2", + attemptCount: 1, + } + const input: import("./types").LaunchInput = { + description: task.description, + prompt: task.prompt, + agent: task.agent, + parentSessionID: task.parentSessionID, + parentMessageID: task.parentMessageID, + model: task.model, + } + + //#when + await (manager as unknown as { + startTask: (item: { task: BackgroundTask; input: import("./types").LaunchInput; attemptID: string }) => Promise + }).startTask({ task, input, attemptID: "attempt-2" }) + + //#then + const activeAttempt = task.attempts?.find((attempt) => attempt.attemptID === "attempt-2") + expect(activeAttempt).toBeDefined() + expect(activeAttempt?.sessionID).toBe("session-attempt-2") + expect(activeAttempt?.status).toBe("running") + expect(activeAttempt?.startedAt).toBeInstanceOf(Date) + expect(task.currentAttemptID).toBe("attempt-2") + expect(task.sessionID).toBe("session-attempt-2") + expect(task.status).toBe("running") + expect(task.attempts?.[0]).toMatchObject({ + attemptID: "attempt-1", + sessionID: "session-attempt-1", + status: "error", + error: "first attempt failed", + }) + + manager.shutdown() + }) + + test("historical attempt session IDs resolve to the task while stale session.error events leave the current attempt unchanged", async () => { + //#given + const manager = createBackgroundManager() + const task: BackgroundTask = { + id: "task-stale-session-event", + status: "running", + queuedAt: new Date("2026-04-27T00:00:00.000Z"), + startedAt: new Date("2026-04-27T00:00:10.000Z"), + sessionID: "session-attempt-2", + description: "ignore stale retry events", + prompt: "continue", + agent: "explore", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + model: { providerID: "anthropic", modelID: "claude-haiku-4.5" }, + attempts: [ + { + attemptID: "attempt-1", + attemptNumber: 1, + sessionID: "session-attempt-1", + providerID: "openai", + modelID: "gpt-5.4-mini", + status: "error", + error: "first attempt failed", + startedAt: new Date("2026-04-27T00:00:00.000Z"), + completedAt: new Date("2026-04-27T00:00:05.000Z"), + }, + { + attemptID: "attempt-2", + attemptNumber: 2, + sessionID: "session-attempt-2", + providerID: "anthropic", + modelID: "claude-haiku-4.5", + status: "running", + startedAt: new Date("2026-04-27T00:00:10.000Z"), + }, + ], + currentAttemptID: "attempt-2", + } + getTaskMap(manager).set(task.id, task) + + //#when + const resolvedTask = manager.findBySession("session-attempt-1") + manager.handleEvent({ + type: "session.error", + properties: { + sessionID: "session-attempt-1", + error: { name: "UnknownError", message: "late event from old session" }, + }, + }) + await flushBackgroundNotifications() + + //#then + expect(resolvedTask?.id).toBe(task.id) + expect(task.currentAttemptID).toBe("attempt-2") + expect(task.sessionID).toBe("session-attempt-2") + expect(task.status).toBe("running") + expect(task.error).toBeUndefined() + expect(task.attempts?.[0]).toMatchObject({ + attemptID: "attempt-1", + status: "error", + error: "first attempt failed", + }) + expect(task.attempts?.[1]).toMatchObject({ + attemptID: "attempt-2", + sessionID: "session-attempt-2", + status: "running", + }) + + manager.shutdown() + }) + + test("late launch prompt errors from a historical attempt do not interrupt the current retry attempt", async () => { + //#given + let rejectPrompt: ((error: unknown) => void) | undefined + const abortCalls: string[] = [] + const client = { + session: { + get: async () => ({ data: { directory: "/test/dir" } }), + create: async () => ({ data: { id: "session-attempt-1" } }), + promptAsync: async () => new Promise((_, reject) => { + rejectPrompt = reject + }), + abort: async ({ path }: { path: { id: string } }) => { + abortCalls.push(path.id) + return {} + }, + }, + } + const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) + stubNotifyParentSession(manager) + ;(manager as unknown as { + tryFallbackRetry: (task: BackgroundTask, errorInfo: { name?: string; message?: string }, source: string) => Promise + }).tryFallbackRetry = async () => false + const task: BackgroundTask = { + id: "task-stale-prompt-error", + status: "pending", + queuedAt: new Date("2026-04-27T00:00:00.000Z"), + description: "ignore stale prompt errors", + prompt: "continue", + agent: "sisyphus-junior", + parentSessionID: "parent-session", + parentMessageID: "parent-message", + model: { providerID: "openai", modelID: "gpt-5.4-mini" }, + attempts: [ + { + attemptID: "attempt-1", + attemptNumber: 1, + providerID: "openai", + modelID: "gpt-5.4-mini", + status: "pending", + }, + ], + currentAttemptID: "attempt-1", + } + getTaskMap(manager).set(task.id, task) + const input: import("./types").LaunchInput = { + description: task.description, + prompt: task.prompt, + agent: task.agent, + parentSessionID: task.parentSessionID, + parentMessageID: task.parentMessageID, + model: task.model, + } + + await (manager as unknown as { + startTask: (item: { task: BackgroundTask; input: import("./types").LaunchInput; attemptID: string }) => Promise + }).startTask({ task, input, attemptID: "attempt-1" }) + + task.attempts = [ + { + attemptID: "attempt-1", + attemptNumber: 1, + sessionID: "session-attempt-1", + providerID: "openai", + modelID: "gpt-5.4-mini", + status: "error", + error: "first attempt failed", + startedAt: new Date("2026-04-27T00:00:00.000Z"), + completedAt: new Date("2026-04-27T00:00:05.000Z"), + }, + { + attemptID: "attempt-2", + attemptNumber: 2, + sessionID: "session-attempt-2", + providerID: "anthropic", + modelID: "claude-haiku-4.5", + status: "running", + startedAt: new Date("2026-04-27T00:00:10.000Z"), + }, + ] + task.currentAttemptID = "attempt-2" + task.sessionID = "session-attempt-2" + task.status = "running" + task.error = undefined + + //#when + rejectPrompt?.({ name: "APIError", data: { message: "Forbidden: Selected provider is forbidden" } }) + await flushBackgroundNotifications() + + //#then + expect(task.currentAttemptID).toBe("attempt-2") + expect(task.sessionID).toBe("session-attempt-2") + expect(task.status).toBe("running") + expect(task.error).toBeUndefined() + expect(task.attempts?.[0]).toMatchObject({ + attemptID: "attempt-1", + status: "error", + error: "first attempt failed", + }) + expect(task.attempts?.[1]).toMatchObject({ + attemptID: "attempt-2", + status: "running", + sessionID: "session-attempt-2", + }) + expect(abortCalls).toEqual([]) + + manager.shutdown() + }) +}) diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index cb563d0aa..2df90e917 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -1,8 +1,10 @@ import type { PluginInput } from "@opencode-ai/plugin" +import type { ModelFallbackControllerAccessor } from "../../hooks/model-fallback" import { isAgentNotFoundError, FALLBACK_AGENT, buildFallbackBody } from "./spawner" import type { BackgroundTask, + BackgroundTaskAttempt, LaunchInput, ResumeInput, } from "./types" @@ -30,6 +32,7 @@ import { POLLING_INTERVAL_MS, TASK_CLEANUP_DELAY_MS, TASK_TTL_MS, + type QueueItem, } from "./constants" import { subagentSessions } from "../claude-code-session-state" @@ -47,6 +50,14 @@ import { isRecord, } from "./error-classifier" import { tryFallbackRetry } from "./fallback-retry-handler" +import { + bindAttemptSession, + ensureCurrentAttempt, + findAttemptBySession, + finalizeAttempt, + getCurrentAttempt, + startAttempt, +} from "./attempt-lifecycle" import { registerManagerForCleanup, unregisterManagerForCleanup } from "./process-cleanup" import { findNearestMessageExcludingCompaction, @@ -79,7 +90,6 @@ import { type OpencodeClient = PluginInput["client"] - interface MessagePartInfo { id?: string sessionID?: string @@ -119,9 +129,38 @@ interface Todo { id: string } -interface QueueItem { - task: BackgroundTask - input: LaunchInput +function formatAttemptModelSummary(attempt: Pick | undefined): string | undefined { + if (!attempt?.providerID || !attempt.modelID) { + return undefined + } + + return `${attempt.providerID}/${attempt.modelID}` +} + +function getPreviousAttempt(task: BackgroundTask, attemptID: string | undefined): BackgroundTaskAttempt | undefined { + if (!attemptID || !task.attempts || task.attempts.length === 0) { + return undefined + } + + const attemptIndex = task.attempts.findIndex((attempt) => attempt.attemptID === attemptID) + if (attemptIndex <= 0) { + return undefined + } + + return task.attempts[attemptIndex - 1] +} + +function cloneAttempts(task: BackgroundTask): BackgroundTaskAttempt[] | undefined { + if (!task.attempts) { + return undefined + } + + return task.attempts.map((attempt) => ({ ...attempt })) +} + +function buildLocalSessionUrl(directory: string, sessionID: string): string { + const encodedDirectory = Buffer.from(directory).toString("base64url") + return `http://127.0.0.1:4096/${encodedDirectory}/session/${sessionID}` } export interface SubagentSessionCreatedEvent { @@ -164,6 +203,7 @@ export class BackgroundManager { private rootDescendantCounts: Map private preStartDescendantReservations: Set private enableParentSessionNotifications: boolean + private modelFallbackControllerAccessor?: ModelFallbackControllerAccessor readonly taskHistory = new TaskHistory() private cachedCircuitBreakerSettings?: CircuitBreakerSettings @@ -175,6 +215,7 @@ export class BackgroundManager { onSubagentSessionCreated?: OnSubagentSessionCreated onShutdown?: () => void | Promise enableParentSessionNotifications?: boolean + modelFallbackControllerAccessor?: ModelFallbackControllerAccessor } ) { this.tasks = new Map() @@ -192,6 +233,7 @@ export class BackgroundManager { this.rootDescendantCounts = new Map() this.preStartDescendantReservations = new Set() this.enableParentSessionNotifications = options?.enableParentSessionNotifications ?? true + this.modelFallbackControllerAccessor = options?.modelFallbackControllerAccessor this.registerProcessCleanup() } @@ -370,6 +412,7 @@ export class BackgroundManager { attemptCount: 0, category: input.category, } + const firstAttempt = startAttempt(task, input.model) this.addTask(task) this.taskHistory.record(input.parentSessionID, { id: task.id, agent: input.agent, description: input.description, status: "pending", category: input.category }) @@ -384,7 +427,7 @@ export class BackgroundManager { // Add to queue const key = this.getConcurrencyKeyFromInput(input) const queue = this.queuesByKey.get(key) ?? [] - queue.push({ task, input }) + queue.push({ task, input, attemptID: firstAttempt.attemptID }) this.queuesByKey.set(key, queue) log("[background-agent] Task queued:", { taskId: task.id, key, queueLength: queue.length }) @@ -445,9 +488,13 @@ export class BackgroundManager { // Mark task as error so the parent polling loop detects the failure // instead of leaving it in a zombie "running" state with no prompt sent - item.task.status = "error" - item.task.error = error instanceof Error ? error.message : String(error) - item.task.completedAt = new Date() + if (item.task.currentAttemptID) { + finalizeAttempt(item.task, item.task.currentAttemptID, "error", error instanceof Error ? error.message : String(error)) + } else { + item.task.status = "error" + item.task.error = error instanceof Error ? error.message : String(error) + item.task.completedAt = new Date() + } if (item.task.concurrencyKey) { this.concurrencyManager.release(item.task.concurrencyKey) @@ -476,6 +523,7 @@ export class BackgroundManager { private async startTask(item: QueueItem): Promise { const { task, input } = item + const attemptID = item.attemptID ?? ensureCurrentAttempt(task, input.model).attemptID log("[background-agent] Starting task:", { taskId: task.id, @@ -558,9 +606,17 @@ export class BackgroundManager { return } - task.status = "running" - task.startedAt = new Date() - task.sessionID = sessionID + const boundAttempt = bindAttemptSession(task, attemptID, sessionID, input.model) + if (!boundAttempt) { + await this.abortSessionWithLogging(sessionID, "stale attempt binding cleanup") + subagentSessions.delete(sessionID) + if (task.rootSessionID) { + this.unregisterRootDescendant(task.rootSessionID) + } + this.concurrencyManager.release(concurrencyKey) + return + } + task.progress = { toolCalls: 0, lastUpdate: new Date(), @@ -568,6 +624,39 @@ export class BackgroundManager { task.concurrencyKey = concurrencyKey task.concurrencyGroup = concurrencyKey + if (task.retryNotification) { + const attemptNumber = boundAttempt.attemptNumber + const retrySessionUrl = buildLocalSessionUrl(parentDirectory, sessionID) + const previousAttempt = getPreviousAttempt(task, boundAttempt.attemptID) + const failedSessionID = previousAttempt?.sessionID ?? task.retryNotification.previousSessionID + const failedSessionLine = failedSessionID + ? `\n- Failed session: \`${failedSessionID}\`` + : "" + const failedModel = formatAttemptModelSummary(previousAttempt) ?? task.retryNotification.failedModel + const failedModelLine = failedModel + ? `\n- Failed model: \`${failedModel}\`` + : "" + const failedError = previousAttempt?.error ?? task.retryNotification.failedError + const failedErrorLine = failedError + ? `\n- Error: ${failedError}` + : "" + const retryModel = formatAttemptModelSummary(boundAttempt) ?? task.retryNotification.nextModel + this.queuePendingNotification( + task.parentSessionID, + ` +[BACKGROUND TASK RETRY SESSION READY] +**ID:** \`${task.id}\` +**Description:** ${task.description} +**Retry attempt:** ${attemptNumber} +**Retry session:** \`${sessionID}\` +**Retry link:** ${retrySessionUrl}${failedSessionLine}${failedModelLine}${failedErrorLine}${retryModel ? `\n- Model: \`${retryModel}\`` : ""} + +The fallback retry session is now created and can be inspected directly. +` + ) + task.retryNotification = undefined + } + this.taskHistory.record(input.parentSessionID, { id: task.id, sessionID, agent: input.agent, description: input.description, status: "running", category: input.category, startedAt: task.startedAt }) this.startPolling() @@ -645,16 +734,36 @@ export class BackgroundManager { } log("[background-agent] promptAsync error:", error) - const existingTask = this.findBySession(sessionID) + const resolvedTask = this.resolveTaskAttemptBySession(sessionID) + const existingTask = resolvedTask?.task + if (resolvedTask && !resolvedTask.isCurrent) { + log("[background-agent] Ignoring prompt error from stale attempt session", { + sessionID, + currentAttemptID: resolvedTask.task.currentAttemptID, + attemptID: resolvedTask.attemptID, + }) + return + } if (existingTask) { - existingTask.status = "interrupt" - const errorMessage = error instanceof Error ? error.message : String(error) - if (errorMessage.includes("agent.name") || errorMessage.includes("undefined") || isAgentNotFoundError(error)) { - existingTask.error = `Agent "${input.agent}" not found. Make sure the agent is registered in your opencode.json or provided by a plugin.` - } else { - existingTask.error = errorMessage + const errorInfo = { + name: extractErrorName(error), + message: extractErrorMessage(error), + } + if (await this.tryFallbackRetry(existingTask, errorInfo, "promptAsync.launch")) { + return + } + + const errorMessage = errorInfo.message ?? (error instanceof Error ? error.message : String(error)) + const terminalError = errorMessage.includes("agent.name") || errorMessage.includes("undefined") || isAgentNotFoundError(error) + ? `Agent "${input.agent}" not found. Make sure the agent is registered in your opencode.json or provided by a plugin.` + : errorMessage + if (existingTask.currentAttemptID) { + finalizeAttempt(existingTask, existingTask.currentAttemptID, "interrupt", terminalError) + } else { + existingTask.status = "interrupt" + existingTask.error = terminalError + existingTask.completedAt = new Date() } - existingTask.completedAt = new Date() if (existingTask.rootSessionID) { this.unregisterRootDescendant(existingTask.rootSessionID) } @@ -723,10 +832,35 @@ export class BackgroundManager { if (task.sessionID === sessionID) { return task } + if (findAttemptBySession(task, sessionID)) { + return task + } } return undefined } + private resolveTaskAttemptBySession(sessionID: string): { task: BackgroundTask; attemptID?: string; isCurrent: boolean } | undefined { + const task = this.findBySession(sessionID) + if (!task) { + return undefined + } + + const attempt = findAttemptBySession(task, sessionID) + if (!attempt) { + return { + task, + attemptID: undefined, + isCurrent: task.sessionID === sessionID, + } + } + + return { + task, + attemptID: attempt.attemptID, + isCurrent: task.currentAttemptID === attempt.attemptID, + } + } + private getConcurrencyKeyFromInput(input: LaunchInput): string { if (input.model) { return `${input.model.providerID}/${input.model.modelID}` @@ -941,8 +1075,16 @@ export class BackgroundManager { }, }).catch(async (error) => { log("[background-agent] resume prompt error:", error) + const errorInfo = { + name: extractErrorName(error), + message: extractErrorMessage(error), + } + if (await this.tryFallbackRetry(existingTask, errorInfo, "promptAsync.resume")) { + return + } + existingTask.status = "interrupt" - const errorMessage = error instanceof Error ? error.message : String(error) + const errorMessage = errorInfo.message ?? (error instanceof Error ? error.message : String(error)) existingTask.error = errorMessage existingTask.completedAt = new Date() if (existingTask.rootSessionID) { @@ -1044,8 +1186,11 @@ export class BackgroundManager { if (role !== "assistant") return - const task = this.findBySession(sessionID) - if (!task || task.status !== "running") return + const resolved = this.resolveTaskAttemptBySession(sessionID) + if (!resolved?.isCurrent) return + + const { task } = resolved + if (task.status !== "running") return const assistantError = (info as Record)["error"] if (!assistantError) return @@ -1067,8 +1212,10 @@ export class BackgroundManager { const sessionID = partInfo?.sessionID if (!sessionID) return - const task = this.findBySession(sessionID) - if (!task) return + const resolved = this.resolveTaskAttemptBySession(sessionID) + if (!resolved?.isCurrent) return + + const { task } = resolved if (this.hasOutputSignalFromPart(partInfo)) { this.markSessionOutputObserved(sessionID) @@ -1107,10 +1254,10 @@ export class BackgroundManager { task.progress.toolCalls += 1 task.progress.lastTool = partInfo.tool - const circuitBreaker = this.cachedCircuitBreakerSettings ?? resolveCircuitBreakerSettings(this.config) - this.cachedCircuitBreakerSettings = circuitBreaker - if (partInfo.tool) { - task.progress.toolCallWindow = recordToolCall( + const circuitBreaker = this.cachedCircuitBreakerSettings ?? resolveCircuitBreakerSettings(this.config) + this.cachedCircuitBreakerSettings = circuitBreaker + if (partInfo.tool) { + task.progress.toolCallWindow = recordToolCall( task.progress.toolCallWindow, partInfo.tool, circuitBreaker, @@ -1171,7 +1318,10 @@ export class BackgroundManager { if (!props || typeof props !== "object") return handleSessionIdleBackgroundEvent({ properties: props as Record, - findBySession: (id) => this.findBySession(id), + findBySession: (id) => { + const resolved = this.resolveTaskAttemptBySession(id) + return resolved?.isCurrent ? resolved.task : undefined + }, idleDeferralTimers: this.idleDeferralTimers, validateSessionHasOutput: (id) => this.validateSessionHasOutput(id), checkSessionTodos: (id) => this.checkSessionTodos(id), @@ -1184,8 +1334,11 @@ export class BackgroundManager { const sessionID = typeof props?.sessionID === "string" ? props.sessionID : undefined if (!sessionID) return - const task = this.findBySession(sessionID) - if (!task || task.status !== "running") return + const resolved = this.resolveTaskAttemptBySession(sessionID) + if (!resolved?.isCurrent) return + + const { task } = resolved + if (task.status !== "running") return const errorObj = props?.error as { name?: string; message?: string } | undefined const errorName = errorObj?.name @@ -1214,9 +1367,9 @@ export class BackgroundManager { this.clearSessionTodoObservation(sessionID) const tasksToCancel = new Map() - const directTask = this.findBySession(sessionID) - if (directTask) { - tasksToCancel.set(directTask.id, directTask) + const directTask = this.resolveTaskAttemptBySession(sessionID) + if (directTask?.isCurrent) { + tasksToCancel.set(directTask.task.id, directTask.task) } for (const descendant of this.getAllDescendantTasks(sessionID)) { tasksToCancel.set(descendant.id, descendant) @@ -1271,8 +1424,11 @@ export class BackgroundManager { const status = props?.status as { type?: string; message?: string } | undefined if (!sessionID || status?.type !== "retry") return - const task = this.findBySession(sessionID) - if (!task || task.status !== "running") return + const resolved = this.resolveTaskAttemptBySession(sessionID) + if (!resolved?.isCurrent) return + + const { task } = resolved + if (task.status !== "running") return const errorMessage = typeof status.message === "string" ? status.message : undefined const errorInfo = { name: "SessionRetry", message: errorMessage } @@ -1293,6 +1449,13 @@ export class BackgroundManager { }): Promise { const { task, errorInfo, errorMessage, errorName } = args + if (!task.fallbackChain && task.sessionID) { + const sessionFallbackChain = this.modelFallbackControllerAccessor?.getSessionFallbackChain(task.sessionID) + if (sessionFallbackChain?.length) { + task.fallbackChain = sessionFallbackChain + } + } + // Agent-not-found errors are handled by the prompt catch block with agent fallback. // Do not also trigger model fallback retry — that would race with the agent retry. if (isAgentNotFoundError({ message: errorInfo.message } as Error)) { @@ -1320,9 +1483,13 @@ export class BackgroundManager { canRetry, }) - task.status = "error" - task.error = errorMsg - task.completedAt = new Date() + if (task.currentAttemptID) { + finalizeAttempt(task, task.currentAttemptID, "error", errorMsg) + } else { + task.status = "error" + task.error = errorMsg + task.completedAt = new Date() + } if (task.rootSessionID) { this.unregisterRootDescendant(task.rootSessionID) } @@ -1377,6 +1544,26 @@ export class BackgroundManager { idleDeferralTimers: this.idleDeferralTimers, queuesByKey: this.queuesByKey, processKey: (key: string) => this.processKey(key), + onRetrying: ({ task, source }) => { + const currentAttempt = getCurrentAttempt(task) + const previousAttempt = getPreviousAttempt(task, currentAttempt?.attemptID) + const sourceText = source ? ` via ${source}` : "" + const failedSessionLine = previousAttempt?.sessionID ? `\n- Failed session: \`${previousAttempt.sessionID}\`` : "" + const failedModel = formatAttemptModelSummary(previousAttempt) + const failedModelLine = failedModel ? `\n- Failed model: \`${failedModel}\`` : "" + const failedErrorLine = previousAttempt?.error ? `\n- Error: ${previousAttempt.error}` : "" + const nextModel = formatAttemptModelSummary(currentAttempt) + this.queuePendingNotification( + task.parentSessionID, + ` +[BACKGROUND TASK RETRYING] +**ID:** \`${task.id}\` +**Description:** ${task.description}${sourceText}${failedSessionLine}${failedModelLine}${failedErrorLine}${nextModel ? `\n- Next model: \`${nextModel}\`` : ""} + +The task was re-queued on a fallback model after a retryable failure. +` + ) + }, }) return result.then((retried) => { if (retried && previousSessionID) { @@ -1595,14 +1782,18 @@ export class BackgroundManager { } const wasRunning = task.status === "running" - task.status = "cancelled" - task.completedAt = new Date() + if (task.currentAttemptID) { + finalizeAttempt(task, task.currentAttemptID, "cancelled", reason) + } else { + task.status = "cancelled" + task.completedAt = new Date() + if (reason) { + task.error = reason + } + } if (wasRunning && task.rootSessionID) { this.unregisterRootDescendant(task.rootSessionID) } - if (reason) { - task.error = reason - } this.taskHistory.record(task.parentSessionID, { id: task.id, sessionID: task.sessionID, agent: task.agent, description: task.description, status: "cancelled", category: task.category, startedAt: task.startedAt, completedAt: task.completedAt }) if (task.concurrencyKey) { @@ -1688,7 +1879,6 @@ export class BackgroundManager { unregisterManagerForCleanup(this) } - /** * Get all running tasks (for compaction hook) */ @@ -1715,8 +1905,12 @@ export class BackgroundManager { } // Atomically mark as completed to prevent race conditions - task.status = "completed" - task.completedAt = new Date() + if (task.currentAttemptID) { + finalizeAttempt(task, task.currentAttemptID, "completed") + } else { + task.status = "completed" + task.completedAt = new Date() + } this.taskHistory.record(task.parentSessionID, { id: task.id, sessionID: task.sessionID, agent: task.agent, description: task.description, status: "completed", category: task.category, startedAt: task.startedAt, completedAt: task.completedAt }) if (task.rootSessionID) { @@ -1780,6 +1974,7 @@ export class BackgroundManager { description: task.description, status: task.status, error: task.error, + attempts: cloneAttempts(task), }) // Update pending tracking and check if all tasks complete @@ -1801,7 +1996,7 @@ export class BackgroundManager { } const completedTasks = allComplete - ? (this.completedTaskSummaries.get(task.parentSessionID) ?? [{ id: task.id, description: task.description, status: task.status, error: task.error }]) + ? (this.completedTaskSummaries.get(task.parentSessionID) ?? [{ id: task.id, description: task.description, status: task.status, error: task.error, attempts: cloneAttempts(task) }]) : [] if (allComplete) { @@ -2008,9 +2203,13 @@ export class BackgroundManager { } private async failCrashedTask(task: BackgroundTask, errorMessage: string): Promise { - task.status = "error" - task.error = errorMessage - task.completedAt = new Date() + if (task.currentAttemptID) { + finalizeAttempt(task, task.currentAttemptID, "error", errorMessage) + } else { + task.status = "error" + task.error = errorMessage + task.completedAt = new Date() + } if (task.rootSessionID) { this.unregisterRootDescendant(task.rootSessionID) } @@ -2049,98 +2248,98 @@ export class BackgroundManager { if (this.pollingInFlight) return this.pollingInFlight = true try { - this.pruneStaleTasksAndNotifications() + this.pruneStaleTasksAndNotifications() - const statusResult = await this.client.session.status() - const allStatuses = normalizeSDKResponse(statusResult, {} as Record) + const statusResult = await this.client.session.status() + const allStatuses = normalizeSDKResponse(statusResult, {} as Record) - await this.checkAndInterruptStaleTasks(allStatuses) + await this.checkAndInterruptStaleTasks(allStatuses) - for (const task of this.tasks.values()) { - if (task.status !== "running") continue - - const sessionID = task.sessionID - if (!sessionID) continue + for (const task of this.tasks.values()) { + if (task.status !== "running") continue + + const sessionID = task.sessionID + if (!sessionID) continue - try { - const sessionStatus = allStatuses[sessionID] - // Handle retry before checking running state - if (sessionStatus?.type === "retry") { - const retryMessage = typeof (sessionStatus as { message?: string }).message === "string" - ? (sessionStatus as { message?: string }).message - : undefined - const errorInfo = { name: "SessionRetry", message: retryMessage } - if (await this.tryFallbackRetry(task, errorInfo, "polling:session.status")) { - continue - } - } - - // Only skip completion when session status is actively running. - // Unknown or terminal statuses (like "interrupted") fall through to completion. - if (sessionStatus && isActiveSessionStatus(sessionStatus.type)) { - log("[background-agent] Session still running, relying on event-based progress:", { - taskId: task.id, - sessionID, - sessionStatus: sessionStatus.type, - toolCalls: task.progress?.toolCalls ?? 0, - }) - continue - } - - if (sessionStatus && isTerminalSessionStatus(sessionStatus.type)) { - await this.tryCompleteTask(task, `polling (terminal session status: ${sessionStatus.type})`) - continue - } - - if (sessionStatus && sessionStatus.type !== "idle") { - log("[background-agent] Unknown session status, treating as potentially idle:", { - taskId: task.id, - sessionID, - sessionStatus: sessionStatus.type, - }) - } - - // Session is idle or no longer in status response (completed/disappeared) - const sessionGoneFromStatus = !sessionStatus - const sessionGoneThresholdReached = sessionGoneFromStatus - && (task.consecutiveMissedPolls ?? 0) >= MIN_SESSION_GONE_POLLS - const completionSource = sessionStatus?.type === "idle" - ? "polling (idle status)" - : "polling (session gone from status)" - const hasValidOutput = await this.validateSessionHasOutput(sessionID) - if (!hasValidOutput) { - if (sessionGoneThresholdReached) { - const sessionExists = await this.verifySessionExists(sessionID) - if (!sessionExists) { - log("[background-agent] Session no longer exists (crashed), marking task as error:", task.id) - await this.failCrashedTask(task, "Subagent session no longer exists (process likely crashed). The session disappeared without producing any output.") + try { + const sessionStatus = allStatuses[sessionID] + // Handle retry before checking running state + if (sessionStatus?.type === "retry") { + const retryMessage = typeof (sessionStatus as { message?: string }).message === "string" + ? (sessionStatus as { message?: string }).message + : undefined + const errorInfo = { name: "SessionRetry", message: retryMessage } + if (await this.tryFallbackRetry(task, errorInfo, "polling:session.status")) { continue } - - task.consecutiveMissedPolls = 0 } - log("[background-agent] Polling idle/gone but no valid output yet, waiting:", task.id) - continue + + // Only skip completion when session status is actively running. + // Unknown or terminal statuses (like "interrupted") fall through to completion. + if (sessionStatus && isActiveSessionStatus(sessionStatus.type)) { + log("[background-agent] Session still running, relying on event-based progress:", { + taskId: task.id, + sessionID, + sessionStatus: sessionStatus.type, + toolCalls: task.progress?.toolCalls ?? 0, + }) + continue + } + + if (sessionStatus && isTerminalSessionStatus(sessionStatus.type)) { + await this.tryCompleteTask(task, `polling (terminal session status: ${sessionStatus.type})`) + continue + } + + if (sessionStatus && sessionStatus.type !== "idle") { + log("[background-agent] Unknown session status, treating as potentially idle:", { + taskId: task.id, + sessionID, + sessionStatus: sessionStatus.type, + }) + } + + // Session is idle or no longer in status response (completed/disappeared) + const sessionGoneFromStatus = !sessionStatus + const sessionGoneThresholdReached = sessionGoneFromStatus + && (task.consecutiveMissedPolls ?? 0) >= MIN_SESSION_GONE_POLLS + const completionSource = sessionStatus?.type === "idle" + ? "polling (idle status)" + : "polling (session gone from status)" + const hasValidOutput = await this.validateSessionHasOutput(sessionID) + if (!hasValidOutput) { + if (sessionGoneThresholdReached) { + const sessionExists = await this.verifySessionExists(sessionID) + if (!sessionExists) { + log("[background-agent] Session no longer exists (crashed), marking task as error:", task.id) + await this.failCrashedTask(task, "Subagent session no longer exists (process likely crashed). The session disappeared without producing any output.") + continue + } + + task.consecutiveMissedPolls = 0 + } + log("[background-agent] Polling idle/gone but no valid output yet, waiting:", task.id) + continue + } + + // Re-check status after async operation + if (task.status !== "running") continue + + const hasIncompleteTodos = await this.checkSessionTodos(sessionID) + if (hasIncompleteTodos) { + log("[background-agent] Task has incomplete todos via polling, waiting:", task.id) + continue + } + + await this.tryCompleteTask(task, completionSource) + } catch (error) { + log("[background-agent] Poll error for task:", { taskId: task.id, error }) } - - // Re-check status after async operation - if (task.status !== "running") continue - - const hasIncompleteTodos = await this.checkSessionTodos(sessionID) - if (hasIncompleteTodos) { - log("[background-agent] Task has incomplete todos via polling, waiting:", task.id) - continue - } - - await this.tryCompleteTask(task, completionSource) - } catch (error) { - log("[background-agent] Poll error for task:", { taskId: task.id, error }) } - } - if (!this.hasRunningTasks()) { - this.stopPolling() - } + if (!this.hasRunningTasks()) { + this.stopPolling() + } } finally { this.pollingInFlight = false } diff --git a/src/features/background-agent/spawner.test.ts b/src/features/background-agent/spawner.test.ts index e39712501..9be6ed1fc 100644 --- a/src/features/background-agent/spawner.test.ts +++ b/src/features/background-agent/spawner.test.ts @@ -378,7 +378,7 @@ describe("background-agent spawner fallback model promotion", () => { //#when await startTask( - { task, input }, + { task, input, attemptID: "att_test123" }, { client, directory: "/tmp/test", diff --git a/src/features/background-agent/types.ts b/src/features/background-agent/types.ts index 5edbe102d..030fdc014 100644 --- a/src/features/background-agent/types.ts +++ b/src/features/background-agent/types.ts @@ -26,6 +26,21 @@ export interface TaskProgress { lastMessageAt?: Date } +export type BackgroundTaskAttemptStatus = BackgroundTaskStatus + +export interface BackgroundTaskAttempt { + attemptID: string + attemptNumber: number + sessionID?: string + providerID?: string + modelID?: string + variant?: string + status: BackgroundTaskAttemptStatus + error?: string + startedAt?: Date + completedAt?: Date +} + export interface BackgroundTask { id: string sessionID?: string @@ -61,6 +76,18 @@ export interface BackgroundTask { isUnstableAgent?: boolean /** Category used for this task (e.g., 'quick', 'visual-engineering') */ category?: string + /** Pending retry notification details for the next spawned retry session */ + retryNotification?: { + previousSessionID?: string + failedModel?: string + failedError?: string + nextModel: string + } + + /** Structured attempt history for retry observability */ + attempts?: BackgroundTaskAttempt[] + /** ID of the currently active attempt */ + currentAttemptID?: string /** Last message count for stability detection */ lastMsgCount?: number diff --git a/src/hooks/model-fallback/controller-accessor.ts b/src/hooks/model-fallback/controller-accessor.ts index 281ae9931..89c30571c 100644 --- a/src/hooks/model-fallback/controller-accessor.ts +++ b/src/hooks/model-fallback/controller-accessor.ts @@ -4,6 +4,7 @@ import type { ModelFallbackStateController } from "./fallback-state-controller" export type ModelFallbackControllerAccessor = { register: (controller: ModelFallbackStateController) => void setSessionFallbackChain: (sessionID: string, fallbackChain: FallbackEntry[] | undefined) => void + getSessionFallbackChain: (sessionID: string) => FallbackEntry[] | undefined clearSessionFallbackChain: (sessionID: string) => void } @@ -18,6 +19,10 @@ export function createModelFallbackControllerAccessor(): ModelFallbackController controller?.setSessionFallbackChain(sessionID, fallbackChain) } + function getSessionFallbackChain(sessionID: string): FallbackEntry[] | undefined { + return controller?.getSessionFallbackChain(sessionID) + } + function clearSessionFallbackChain(sessionID: string): void { controller?.clearSessionFallbackChain(sessionID) } @@ -25,6 +30,7 @@ export function createModelFallbackControllerAccessor(): ModelFallbackController return { register, setSessionFallbackChain, + getSessionFallbackChain, clearSessionFallbackChain, } } diff --git a/src/hooks/model-fallback/fallback-state-controller.ts b/src/hooks/model-fallback/fallback-state-controller.ts index 4230bfb0a..9bc3d1102 100644 --- a/src/hooks/model-fallback/fallback-state-controller.ts +++ b/src/hooks/model-fallback/fallback-state-controller.ts @@ -15,6 +15,7 @@ type ModelFallbackStateLike = { export type ModelFallbackStateController = { lastToastKey: Map setSessionFallbackChain: (sessionID: string, fallbackChain: FallbackEntry[] | undefined) => void + getSessionFallbackChain: (sessionID: string) => FallbackEntry[] | undefined clearSessionFallbackChain: (sessionID: string) => void setPendingModelFallback: ( sessionID: string, @@ -38,13 +39,18 @@ export function createModelFallbackStateController(input: { function setSessionFallbackChain(sessionID: string, fallbackChain: FallbackEntry[] | undefined): void { if (!sessionID) return - sessionFallbackChains.set(sessionID, fallbackChain?.length ? fallbackChain : []) + sessionFallbackChains.set(sessionID, fallbackChain?.length ? [...fallbackChain] : []) } function clearSessionFallbackChain(sessionID: string): void { sessionFallbackChains.delete(sessionID) } + function getSessionFallbackChain(sessionID: string): FallbackEntry[] | undefined { + const fallbackChain = sessionFallbackChains.get(sessionID) + return fallbackChain ? [...fallbackChain] : undefined + } + function setPendingModelFallback( sessionID: string, agentName: string, @@ -56,7 +62,7 @@ export function createModelFallbackStateController(input: { const fallbackChain = sessionFallbackChains.get(sessionID) ?? requirements?.fallbackChain if (!fallbackChain?.length) { - log("[model-fallback] No fallback chain for agent: " + agentName + " (key: " + agentKey + ")") + log(`[model-fallback] No fallback chain for agent: ${agentName} (key: ${agentKey})`) return false } @@ -69,12 +75,12 @@ export function createModelFallbackStateController(input: { attemptCount: 0, pending: true, }) - log("[model-fallback] Set pending fallback for session: " + sessionID + ", agent: " + agentName) + log(`[model-fallback] Set pending fallback for session: ${sessionID}, agent: ${agentName}`) return true } if (existing.pending) { - log("[model-fallback] Pending fallback already armed for session: " + sessionID) + log(`[model-fallback] Pending fallback already armed for session: ${sessionID}`) return false } @@ -82,10 +88,10 @@ export function createModelFallbackStateController(input: { existing.modelID = currentModelID existing.pending = true if (existing.attemptCount >= existing.fallbackChain.length) { - log("[model-fallback] Fallback chain exhausted for session: " + sessionID) + log(`[model-fallback] Fallback chain exhausted for session: ${sessionID}`) return false } - log("[model-fallback] Re-armed pending fallback for session: " + sessionID) + log(`[model-fallback] Re-armed pending fallback for session: ${sessionID}`) return true } @@ -96,7 +102,7 @@ export function createModelFallbackStateController(input: { const fallback = getNextReachableFallback(sessionID, state) if (fallback) return fallback - log("[model-fallback] No more fallbacks for session: " + sessionID) + log(`[model-fallback] No more fallbacks for session: ${sessionID}`) pendingModelFallbacks.delete(sessionID) return null } @@ -123,6 +129,7 @@ export function createModelFallbackStateController(input: { return { lastToastKey, setSessionFallbackChain, + getSessionFallbackChain, clearSessionFallbackChain, setPendingModelFallback, getNextFallback, diff --git a/src/hooks/model-fallback/hook.test.ts b/src/hooks/model-fallback/hook.test.ts index de9e66fd7..b12eee1cd 100644 --- a/src/hooks/model-fallback/hook.test.ts +++ b/src/hooks/model-fallback/hook.test.ts @@ -66,6 +66,7 @@ async function importFreshModelFallbackHookModule() { const { clearPendingModelFallback, createModelFallbackHook, + getSessionFallbackChain, setSessionFallbackChain, setPendingModelFallback, } = await importFreshModelFallbackHookModule() @@ -85,7 +86,6 @@ describe("model fallback hook", () => { }) test("applies pending fallback on chat.message by overriding model", async () => { - //#given const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, @@ -110,13 +110,11 @@ describe("model fallback hook", () => { parts: [{ type: "text", text: "continue" }], } - //#when await hook["chat.message"]?.( { sessionID: "ses_model_fallback_main" }, output, ) - //#then expect(output.message["model"]).toEqual({ providerID: "anthropic", modelID: "claude-opus-4-7", @@ -124,7 +122,6 @@ describe("model fallback hook", () => { }) test("preserves fallback progression across repeated session.error retries", async () => { - //#given const hook = modelFallback as unknown as { "chat.message"?: ( input: { sessionID: string }, @@ -145,16 +142,13 @@ describe("model fallback hook", () => { parts: [{ type: "text", text: "continue" }], } - //#when - first retry is applied await hook["chat.message"]?.({ sessionID }, firstOutput) - //#then expect(firstOutput.message["model"]).toEqual({ providerID: "anthropic", modelID: "claude-opus-4-7", }) - //#when - second error re-arms fallback and should advance to next entry expect( setPendingModelFallback(modelFallback, sessionID, "Sisyphus - Ultraworker", "anthropic", "claude-opus-4-7"), ).toBe(true) @@ -167,7 +161,6 @@ describe("model fallback hook", () => { } await hook["chat.message"]?.({ sessionID }, secondOutput) - //#then - chain should progress to entry[1], not repeat entry[0] expect(secondOutput.message["model"]).toEqual({ providerID: "opencode-go", modelID: "kimi-k2.5", @@ -176,11 +169,9 @@ describe("model fallback hook", () => { }) test("does not re-arm fallback when one is already pending", () => { - //#given const sessionID = "ses_model_fallback_pending_guard" clearPendingModelFallback(modelFallback, sessionID) - //#when const firstSet = setPendingModelFallback( modelFallback, sessionID, @@ -196,14 +187,28 @@ describe("model fallback hook", () => { "claude-opus-4-7-thinking", ) - //#then expect(firstSet).toBe(true) expect(secondSet).toBe(false) clearPendingModelFallback(modelFallback, sessionID) }) + test("isolates stored fallback chains from caller mutations on set and get", () => { + const sessionID = "ses_model_fallback_defensive_copy" + const originalChain = [ + { providers: ["anthropic"], model: "claude-opus-4-7" }, + ] + + setSessionFallbackChain(modelFallback, sessionID, originalChain) + originalChain.push({ providers: ["google"], model: "gemini-2.5-pro" }) + const retrieved = getSessionFallbackChain(modelFallback, sessionID) + retrieved?.push({ providers: ["openai"], model: "gpt-5.4" }) + + expect(getSessionFallbackChain(modelFallback, sessionID)).toEqual([ + { providers: ["anthropic"], model: "claude-opus-4-7" }, + ]) + }) + test("skips no-op fallback entries that resolve to same provider/model", async () => { - //#given const sessionID = "ses_model_fallback_noop_skip" clearPendingModelFallback(modelFallback, sessionID) @@ -236,10 +241,8 @@ describe("model fallback hook", () => { parts: [{ type: "text", text: "continue" }], } - //#when await hook["chat.message"]?.({ sessionID }, output) - //#then expect(output.message["model"]).toEqual({ providerID: "opencode", modelID: "kimi-k2.5-free", @@ -248,7 +251,6 @@ describe("model fallback hook", () => { }) test("skips no-op fallback entries even when variant differs", async () => { - //#given const sessionID = "ses_model_fallback_noop_variant_skip" clearPendingModelFallback(modelFallback, sessionID) @@ -282,10 +284,8 @@ describe("model fallback hook", () => { parts: [{ type: "text", text: "continue" }], } - //#when await hook["chat.message"]?.({ sessionID }, output) - //#then expect(output.message["model"]).toEqual({ providerID: "quotio", modelID: "gpt-5.2", @@ -295,7 +295,6 @@ describe("model fallback hook", () => { }) test("uses connected preferred provider when fallback entry providers are disconnected", async () => { - //#given const sessionID = "ses_model_fallback_preferred_provider" clearPendingModelFallback(modelFallback, sessionID) readConnectedProvidersCacheMock.mockReturnValue(["provider-x"]) @@ -328,10 +327,8 @@ describe("model fallback hook", () => { parts: [{ type: "text", text: "continue" }], } - //#when await hook["chat.message"]?.({ sessionID }, output) - //#then expect(output.message["model"]).toEqual({ providerID: "provider-x", modelID: "fallback-model", @@ -340,12 +337,10 @@ describe("model fallback hook", () => { }) test("does not fall back to hardcoded agent chain when session explicitly stores no fallback chain [regression #2941]", () => { - //#given const sessionID = "ses_model_fallback_explicit_none" clearPendingModelFallback(modelFallback, sessionID) setSessionFallbackChain(modelFallback, sessionID, undefined) - //#when const set = setPendingModelFallback( modelFallback, sessionID, @@ -354,13 +349,11 @@ describe("model fallback hook", () => { "claude-sonnet-4-6", ) - //#then expect(set).toBe(false) clearPendingModelFallback(modelFallback, sessionID) }) test("shows toast when fallback is applied", async () => { - //#given const toastCalls: Array<{ title: string; message: string }> = [] const hook = createModelFallbackHook({ toast: async ({ title, message }) => { @@ -390,16 +383,13 @@ describe("model fallback hook", () => { parts: [{ type: "text", text: "continue" }], } - //#when await hook["chat.message"]?.({ sessionID: "ses_model_fallback_toast" }, output) - //#then expect(toastCalls.length).toBe(1) expect(toastCalls[0]?.title).toBe("Model fallback") }) test("transforms model names for github-copilot provider via fallback chain", async () => { - //#given const sessionID = "ses_model_fallback_ghcp" clearPendingModelFallback(modelFallback, sessionID) @@ -410,7 +400,6 @@ describe("model fallback hook", () => { ) => Promise } - // Set a custom fallback chain that routes through github-copilot setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["github-copilot"], model: "claude-sonnet-4-6" }, ]) @@ -431,10 +420,8 @@ describe("model fallback hook", () => { parts: [{ type: "text", text: "continue" }], } - //#when await hook["chat.message"]?.({ sessionID }, output) - //#then - model name should be transformed from hyphen to dot notation expect(output.message["model"]).toEqual({ providerID: "github-copilot", modelID: "claude-sonnet-4.6", @@ -444,7 +431,6 @@ describe("model fallback hook", () => { }) test("preserves canonical google preview model names via fallback chain", async () => { - //#given const sessionID = "ses_model_fallback_google" clearPendingModelFallback(modelFallback, sessionID) @@ -455,7 +441,6 @@ describe("model fallback hook", () => { ) => Promise } - // Set a custom fallback chain that routes through google setSessionFallbackChain(modelFallback, sessionID, [ { providers: ["google"], model: "gemini-3.1-pro-preview" }, ]) @@ -476,10 +461,8 @@ describe("model fallback hook", () => { parts: [{ type: "text", text: "continue" }], } - //#when await hook["chat.message"]?.({ sessionID }, output) - //#then: model name should remain gemini-3.1-pro-preview because no google transform exists for this ID expect(output.message["model"]).toEqual({ providerID: "google", modelID: "gemini-3.1-pro-preview", diff --git a/src/hooks/model-fallback/hook.ts b/src/hooks/model-fallback/hook.ts index fee130ed8..71c1e605d 100644 --- a/src/hooks/model-fallback/hook.ts +++ b/src/hooks/model-fallback/hook.ts @@ -33,6 +33,7 @@ type ModelFallbackControllerWithState = Pick< ModelFallbackStateController, | "lastToastKey" | "setSessionFallbackChain" + | "getSessionFallbackChain" | "clearSessionFallbackChain" | "setPendingModelFallback" | "getNextFallback" @@ -70,6 +71,13 @@ export function clearSessionFallbackChain( controller.clearSessionFallbackChain(sessionID) } +export function getSessionFallbackChain( + controller: Pick, + sessionID: string, +): FallbackEntry[] | undefined { + return controller.getSessionFallbackChain(sessionID) +} + /** * Sets a pending model fallback for a session. * Called when a model error is detected in session.error handler. @@ -152,6 +160,7 @@ export function createModelFallbackHook(args?: ModelFallbackHookArgs): ModelFall return { lastToastKey: controller.lastToastKey, setSessionFallbackChain: controller.setSessionFallbackChain, + getSessionFallbackChain: controller.getSessionFallbackChain, clearSessionFallbackChain: controller.clearSessionFallbackChain, setPendingModelFallback: controller.setPendingModelFallback, getNextFallback: controller.getNextFallback, diff --git a/src/plugin/event.test.ts b/src/plugin/event.test.ts index ea880c145..c347e2dac 100644 --- a/src/plugin/event.test.ts +++ b/src/plugin/event.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, afterEach, mock, spyOn } from "bun:test" -import { createEventHandler } from "./event" +import { createEventHandler, extractErrorMessage } from "./event" import { createChatMessageHandler } from "./chat-message" import * as openclawRuntimeDispatch from "../openclaw/runtime-dispatch" import { _resetForTesting, setMainSession } from "../features/claude-code-session-state" @@ -12,55 +12,56 @@ type EventHandlerArgs = Parameters[0] type EventHandlerInput = Parameters>[0] type ChatMessageHandlerArgs = Parameters[0] +function cast(value: unknown): T { + return value as T +} + function asEventHandlerInput(input: EventInput): EventHandlerInput { - return input as unknown as EventHandlerInput + return cast(input) } function asEventHandlerContext(ctx: unknown): EventHandlerArgs["ctx"] { - return ctx as unknown as EventHandlerArgs["ctx"] + return cast(ctx) } function asChatMessageHandlerContext(ctx: unknown): ChatMessageHandlerArgs["ctx"] { - return ctx as unknown as ChatMessageHandlerArgs["ctx"] + return cast(ctx) } function asPluginConfig(config: unknown): EventHandlerArgs["pluginConfig"] { - return config as unknown as EventHandlerArgs["pluginConfig"] + return cast(config) } function asChatPluginConfig(config: unknown): ChatMessageHandlerArgs["pluginConfig"] { - return config as unknown as ChatMessageHandlerArgs["pluginConfig"] + return cast(config) } function createEventHandlerManagers( overrides: Record = {}, ): EventHandlerArgs["managers"] { - return { - ...({} as EventHandlerArgs["managers"]), + return cast({ tmuxSessionManager: { onSessionCreated: async () => {}, onSessionDeleted: async () => {}, }, ...overrides, - } as unknown as EventHandlerArgs["managers"] + }) } function createEventHandlerHooks( - overrides: Record, + overrides: Record = {}, ): EventHandlerArgs["hooks"] { - return { - ...({} as EventHandlerArgs["hooks"]), - ...overrides, - } as unknown as EventHandlerArgs["hooks"] + return cast(overrides) } function createChatMessageHandlerHooks( - overrides: Record, + overrides: Record = {}, ): ChatMessageHandlerArgs["hooks"] { - return { - ...({} as ChatMessageHandlerArgs["hooks"]), - ...overrides, - } as unknown as ChatMessageHandlerArgs["hooks"] + return cast(overrides) +} + +async function wait(ms: number): Promise { + await new Promise((resolve) => setTimeout(resolve, ms)) } function createIdleTrackingEventHandler(dispatchCalls: EventInput[]): ReturnType { @@ -93,14 +94,23 @@ afterEach(() => { _resetForTesting() }) - describe("createEventHandler - idle deduplication", () => { - it("#given synthetic idle fires first #when real idle arrives within 500ms #then real idle dispatched", async () => { - //#given +describe("event error extraction", () => { + it("prefers nested APIError message over generic top-level message", async () => { + const error = { + name: "APIError", + message: "Error", + data: { message: "Forbidden: Selected provider is forbidden" }, + } + const result = extractErrorMessage(error) + expect(result).toBe("Forbidden: Selected provider is forbidden") + }) +}) + +describe("createEventHandler - idle deduplication", () => { + it("dispatches both idle events when the real idle arrives within 500ms", async () => { const dispatchCalls: EventInput[] = [] const eventHandler = createIdleTrackingEventHandler(dispatchCalls) const sessionId = "ses_test123" - - //#when await eventHandler(asEventHandlerInput({ event: { type: "session.status", @@ -118,8 +128,6 @@ afterEach(() => { }, }, })) - - //#then expect(dispatchCalls).toHaveLength(2) expect(dispatchCalls[0]?.event.type).toBe("session.idle") expect(dispatchCalls[1]?.event.type).toBe("session.idle") @@ -127,13 +135,10 @@ afterEach(() => { expect((dispatchCalls[1]?.event.properties as { sessionID?: string } | undefined)?.sessionID).toBe(sessionId) }) - it("#given real idle fires first #when synthetic arrives within 500ms #then synthetic dropped", async () => { - //#given + it("drops the synthetic idle when a real idle already arrived within 500ms", async () => { const dispatchCalls: EventInput[] = [] const eventHandler = createIdleTrackingEventHandler(dispatchCalls) const sessionId = "ses_test456" - - //#when await eventHandler(asEventHandlerInput({ event: { type: "session.idle", @@ -151,15 +156,12 @@ afterEach(() => { }, }, })) - - //#then expect(dispatchCalls).toHaveLength(1) expect(dispatchCalls[0]?.event.type).toBe("session.idle") expect((dispatchCalls[0]?.event.properties as { sessionID?: string } | undefined)?.sessionID).toBe(sessionId) }) - it("both maps pruned on every event", async () => { - //#given + it("prunes both maps on every event", async () => { const eventHandler = createEventHandler({ ctx: {} as any, pluginConfig: {} as any, @@ -196,7 +198,6 @@ afterEach(() => { } as any, }) - // Trigger some synthetic idles await eventHandler({ event: { type: "session.status", @@ -217,7 +218,6 @@ afterEach(() => { }, }) - // Trigger some real idles await eventHandler({ event: { type: "session.idle", @@ -235,19 +235,13 @@ afterEach(() => { }, }, }) + await wait(600) - //#when - wait for dedup window to expire (600ms > 500ms) - await new Promise((resolve) => setTimeout(resolve, 600)) - - // Trigger any event to trigger pruning await eventHandler({ event: { type: "message.updated", }, } as any) - - //#then - both maps should be pruned (no dedup should occur for new events) - // We verify by checking that a new idle event for same session is dispatched const dispatchCalls: EventInput[] = [] const eventHandlerWithMock = createEventHandler({ ctx: {} as any, @@ -302,8 +296,7 @@ afterEach(() => { expect(dispatchCalls[0].event.type).toBe("session.idle") }) - it("dedup only applies within window - outside window both dispatch", async () => { - //#given + it("dispatches both idle events once the dedup window expires", async () => { const dispatchCalls: EventInput[] = [] const eventHandler = createEventHandler({ ctx: {} as any, @@ -348,8 +341,6 @@ afterEach(() => { }) const sessionId = "ses_outside_window" - - //#when - synthetic idle first await eventHandler({ event: { type: "session.status", @@ -359,14 +350,8 @@ afterEach(() => { }, }, }) - - //#then - synthetic dispatched expect(dispatchCalls.length).toBe(1) - - //#when - wait for dedup window to expire (600ms > 500ms) - await new Promise((resolve) => setTimeout(resolve, 600)) - - //#when - real idle arrives outside window + await wait(600) await eventHandler({ event: { type: "session.idle", @@ -375,8 +360,6 @@ afterEach(() => { }, }, }) - - //#then - real idle dispatched (outside dedup window) expect(dispatchCalls.length).toBe(2) expect(dispatchCalls[0].event.type).toBe("session.idle") expect(dispatchCalls[1].event.type).toBe("session.idle") @@ -385,7 +368,6 @@ afterEach(() => { describe("createEventHandler - event forwarding", () => { it("forwards message activity events to tmux session manager", async () => { - //#given const forwardedEvents: EventInput[] = [] const eventHandler = createEventHandler({ ctx: asEventHandlerContext({}), @@ -417,22 +399,17 @@ describe("createEventHandler - event forwarding", () => { }), hooks: createEventHandlerHooks({}), }) - - //#when await eventHandler(asEventHandlerInput({ event: { type: "message.part.delta", properties: { sessionID: "ses_tmux_activity", field: "text", delta: "x" }, }, })) - - //#then expect(forwardedEvents.length).toBe(1) expect(forwardedEvents[0]?.event.type).toBe("message.part.delta") }) it("does not forward tmux activity events when tmux integration is disabled", async () => { - //#given const forwardedEvents: EventInput[] = [] const eventHandler = createEventHandler({ ctx: asEventHandlerContext({}), @@ -464,21 +441,16 @@ describe("createEventHandler - event forwarding", () => { }), hooks: createEventHandlerHooks({}), }) - - //#when await eventHandler(asEventHandlerInput({ event: { type: "message.part.delta", properties: { sessionID: "ses_tmux_disabled", field: "text", delta: "x" }, }, })) - - //#then expect(forwardedEvents).toHaveLength(0) }) it("does not forward session.created to tmux session manager when tmux integration is disabled", async () => { - //#given const createdSessions: string[] = [] const eventHandler = createEventHandler({ ctx: asEventHandlerContext({}), @@ -512,21 +484,16 @@ describe("createEventHandler - event forwarding", () => { }), hooks: createEventHandlerHooks({}), }) - - //#when await eventHandler(asEventHandlerInput({ event: { type: "session.created", properties: { info: { id: "ses_tmux_disabled", parentID: "ses_parent" } }, }, })) - - //#then expect(createdSessions).toHaveLength(0) }) it("dispatches OpenClaw after session.created for main sessions (no parentID)", async () => { - //#given const openClawSpy = spyOn(openclawRuntimeDispatch, "dispatchOpenClawEvent").mockResolvedValue(null) const eventHandler = createEventHandler({ ctx: asEventHandlerContext({ directory: "/tmp/project-created" }), @@ -555,16 +522,12 @@ describe("createEventHandler - event forwarding", () => { }), hooks: createEventHandlerHooks({}), }) - - //#when - main session created (no parentID) await eventHandler(asEventHandlerInput({ event: { type: "session.created", properties: { info: { id: "ses_openclaw_created" } }, }, })) - - //#then - OpenClaw dispatch called for main session const [call] = openClawSpy.mock.calls[0] ?? [] expect(call).toMatchObject({ rawEvent: "session.created", @@ -576,8 +539,7 @@ describe("createEventHandler - event forwarding", () => { }) }) - it("does NOT dispatch OpenClaw for subagent sessions (with parentID)", async () => { - //#given + it("does not dispatch OpenClaw for subagent sessions with a parentID", async () => { const openClawSpy = spyOn(openclawRuntimeDispatch, "dispatchOpenClawEvent").mockResolvedValue(null) const eventHandler = createEventHandler({ ctx: asEventHandlerContext({ directory: "/tmp/project-created" }), @@ -606,21 +568,16 @@ describe("createEventHandler - event forwarding", () => { }), hooks: createEventHandlerHooks({}), }) - - //#when - subagent session created (with parentID) await eventHandler(asEventHandlerInput({ event: { type: "session.created", properties: { info: { id: "ses_subagent", parentID: "ses_parent" } }, }, })) - - //#then - OpenClaw dispatch NOT called for subagent session (handled by specialized callbacks) expect(openClawSpy.mock.calls.length).toBe(0) }) it("forwards session.deleted to write-existing-file-guard hook", async () => { - //#given const forwardedEvents: EventInput[] = [] const disconnectedSessions: string[] = [] const deletedSessions: string[] = [] @@ -662,16 +619,12 @@ describe("createEventHandler - event forwarding", () => { } as never, }) const sessionID = "ses_forward_delete_event" - - //#when await eventHandler(asEventHandlerInput({ event: { type: "session.deleted", properties: { info: { id: sessionID } }, }, })) - - //#then expect(forwardedEvents.length).toBe(1) expect(forwardedEvents[0]?.event.type).toBe("session.deleted") expect(disconnectedSessions).toEqual([sessionID]) @@ -717,7 +670,6 @@ describe("createEventHandler - event forwarding", () => { }) it("clears stored prompt params on session.deleted", async () => { - //#given const eventHandler = createEventHandler({ ctx: {} as never, pluginConfig: {} as never, @@ -742,23 +694,18 @@ describe("createEventHandler - event forwarding", () => { topP: 0.7, options: { reasoningEffort: "high" }, }) - - //#when await eventHandler(asEventHandlerInput({ event: { type: "session.deleted", properties: { info: { id: sessionID } }, }, })) - - //#then expect(getSessionPromptParams(sessionID)).toBeUndefined() }) }) describe("createEventHandler - retry dedupe lifecycle", () => { it("re-handles same retry key after session recovers to idle status", async () => { - //#given const sessionID = "ses_retry_recovery_rearm" setMainSession(sessionID) const abortCalls: string[] = [] @@ -844,8 +791,6 @@ describe("createEventHandler - retry dedupe lifecycle", () => { }, }, })) - - //#when - first retry key is handled await eventHandler(asEventHandlerInput({ event: { type: "session.status", @@ -865,8 +810,6 @@ describe("createEventHandler - retry dedupe lifecycle", () => { }, firstOutput, ) - - //#when - session recovers to non-retry idle state await eventHandler(asEventHandlerInput({ event: { type: "session.status", @@ -876,8 +819,6 @@ describe("createEventHandler - retry dedupe lifecycle", () => { }, }, })) - - //#when - same retry key appears again after recovery await eventHandler(asEventHandlerInput({ event: { type: "session.status", @@ -887,8 +828,6 @@ describe("createEventHandler - retry dedupe lifecycle", () => { }, }, })) - - //#then expect(abortCalls).toEqual([sessionID, sessionID]) expect(promptCalls).toEqual([sessionID, sessionID]) }) @@ -896,7 +835,6 @@ describe("createEventHandler - retry dedupe lifecycle", () => { describe("createEventHandler - session recovery compaction", () => { it("triggers compaction before sending continue after session error recovery", async () => { - //#given const sessionID = "ses_recovery_compaction" setMainSession(sessionID) const callOrder: string[] = [] @@ -932,8 +870,6 @@ describe("createEventHandler - session recovery compaction", () => { stopContinuationGuard: { isStopped: () => false }, }), }) - - //#when await eventHandler(asEventHandlerInput({ event: { type: "session.error", @@ -944,13 +880,10 @@ describe("createEventHandler - session recovery compaction", () => { }, }, })) - - //#then - summarize (compaction) must be called before prompt (continue) expect(callOrder).toEqual(["summarize", "prompt"]) }) it("sends continue even if compaction fails", async () => { - //#given const sessionID = "ses_recovery_compaction_fail" setMainSession(sessionID) const callOrder: string[] = [] @@ -986,8 +919,6 @@ describe("createEventHandler - session recovery compaction", () => { stopContinuationGuard: { isStopped: () => false }, }), }) - - //#when await eventHandler(asEventHandlerInput({ event: { type: "session.error", @@ -998,13 +929,10 @@ describe("createEventHandler - session recovery compaction", () => { }, }, })) - - //#then - continue is still sent even when compaction fails expect(callOrder).toEqual(["summarize", "prompt"]) }) it("continues dispatching later event hooks when an earlier hook throws", async () => { - //#given const runtimeFallbackCalls: EventInput[] = [] const eventHandler = createEventHandler({ @@ -1037,8 +965,6 @@ describe("createEventHandler - session recovery compaction", () => { stopContinuationGuard: { isStopped: () => false }, }), }) - - //#when let thrownError: unknown try { await eventHandler(asEventHandlerInput({ @@ -1053,8 +979,6 @@ describe("createEventHandler - session recovery compaction", () => { } catch (error) { thrownError = error } - - //#then expect(thrownError).toBeUndefined() expect(runtimeFallbackCalls).toHaveLength(1) expect(runtimeFallbackCalls[0]?.event.type).toBe("session.error") diff --git a/src/plugin/event.ts b/src/plugin/event.ts index 3db715a29..686f55fae 100644 --- a/src/plugin/event.ts +++ b/src/plugin/event.ts @@ -65,18 +65,17 @@ function extractErrorName(error: unknown): string | undefined { return undefined; } -function extractErrorMessage(error: unknown): string { +export function extractErrorMessage(error: unknown): string { if (!error) return ""; if (typeof error === "string") return error; - if (error instanceof Error) return error.message; if (isRecord(error)) { const candidates: unknown[] = [ - error, error.data, - error.error, isRecord(error.data) ? error.data.error : undefined, + error.error, error.cause, + error, ]; for (const candidate of candidates) { @@ -86,6 +85,8 @@ function extractErrorMessage(error: unknown): string { } } + if (error instanceof Error) return error.message; + try { return JSON.stringify(error); } catch { diff --git a/src/shared/model-error-classifier.test.ts b/src/shared/model-error-classifier.test.ts index 35c75de0a..c4989d199 100644 --- a/src/shared/model-error-classifier.test.ts +++ b/src/shared/model-error-classifier.test.ts @@ -237,6 +237,17 @@ describe("model-error-classifier", () => { //#then expect(result).toBe(true) }) + + test("treats forbidden provider message as retryable", () => { + //#given + const error = { message: "Forbidden: Selected provider is forbidden" } + + //#when + const result = shouldRetryError(error) + + //#then + expect(result).toBe(true) + }) }) export {} diff --git a/src/shared/model-error-classifier.ts b/src/shared/model-error-classifier.ts index b20918d18..611a71aac 100644 --- a/src/shared/model-error-classifier.ts +++ b/src/shared/model-error-classifier.ts @@ -2,8 +2,8 @@ import type { FallbackEntry } from "./model-requirements" import { readConnectedProvidersCache } from "./connected-providers-cache" /** - * Error names that indicate a retryable model error (deadstop). - * These errors completely halt the action loop and should trigger fallback retry. + * Error names that indicate a retryable model error. + * These errors halt execution and should trigger fallback retry. */ const RETRYABLE_ERROR_NAMES = new Set([ "providermodelnotfounderror", @@ -72,6 +72,8 @@ const RETRYABLE_MESSAGE_PATTERNS = [ "504", "429", "529", + "403", + "forbidden", ] /** @@ -119,7 +121,7 @@ export interface ErrorInfo { /** * Determines if an error is a retryable model error. - * Returns true if the error is a known retryable type OR matches retryable message patterns. + * Returns true if it's a known retryable type OR matches retryable message patterns. */ export function isRetryableModelError(error: ErrorInfo): boolean { // If we have an error name, check against known lists @@ -154,7 +156,7 @@ export function isRetryableModelError(error: ErrorInfo): boolean { /** * Determines if an error should trigger a fallback retry. - * Returns true for deadstop errors that completely halt the action loop. + * Returns true for errors that halt execution. */ export function shouldRetryError(error: ErrorInfo): boolean { return isRetryableModelError(error) diff --git a/src/tools/background-task/clients.ts b/src/tools/background-task/clients.ts index b94977c37..b334cbf15 100644 --- a/src/tools/background-task/clients.ts +++ b/src/tools/background-task/clients.ts @@ -2,7 +2,7 @@ import type { BackgroundManager } from "../../features/background-agent" export type BackgroundOutputMessage = { id?: string - info?: { role?: string; time?: string | { created?: number }; agent?: string } + info?: { role?: string; time?: string | { created?: number }; agent?: string; error?: unknown } parts?: Array<{ type?: string text?: string diff --git a/src/tools/background-task/task-result-format.test.ts b/src/tools/background-task/task-result-format.test.ts new file mode 100644 index 000000000..aa34b94e2 --- /dev/null +++ b/src/tools/background-task/task-result-format.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, test } from "bun:test" + +import type { BackgroundTask } from "../../features/background-agent" +import type { BackgroundOutputClient } from "./clients" +import { formatTaskResult } from "./task-result-format" + +function createTask(overrides: Partial = {}): BackgroundTask { + return { + id: "task-1", + sessionID: "ses-1", + parentSessionID: "main-1", + parentMessageID: "msg-1", + description: "background task", + prompt: "do work", + agent: "test-agent", + status: "completed", + startedAt: new Date("2026-01-01T00:00:00.000Z"), + completedAt: new Date("2026-01-01T00:00:05.000Z"), + ...overrides, + } +} + +describe("formatTaskResult", () => { + test("returns assistant session errors instead of masking them as success text", async () => { + const task = createTask() + const client: BackgroundOutputClient = { + session: { + messages: async () => ({ + data: [ + { + info: { + role: "assistant", + time: { created: 1 }, + error: { data: { message: "Forbidden: Selected provider is forbidden" } }, + }, + parts: [], + }, + ], + }), + }, + } + + const output = await formatTaskResult(task, client) + + expect(output).toContain("Session error") + expect(output).toContain("Forbidden: Selected provider is forbidden") + }) +}) diff --git a/src/tools/background-task/task-result-format.ts b/src/tools/background-task/task-result-format.ts index 564eb31fe..c71469703 100644 --- a/src/tools/background-task/task-result-format.ts +++ b/src/tools/background-task/task-result-format.ts @@ -1,4 +1,5 @@ import type { BackgroundTask } from "../../features/background-agent" +import { extractErrorMessage } from "../../features/background-agent/error-classifier" import { consumeNewMessages } from "../../shared/session-cursor" import type { BackgroundOutputClient, BackgroundOutputMessagesResult } from "./clients" import { extractMessages, getErrorMessage } from "./session-messages" @@ -56,6 +57,23 @@ Session ID: ${task.sessionID} return timeA.localeCompare(timeB) }) + const sessionError = sortedMessages + .filter((message) => message.info?.role === "assistant" && message.info?.error) + .map((message) => extractErrorMessage(message.info?.error)) + .find((message): message is string => typeof message === "string" && message.length > 0) + if (sessionError) { + return `Task Result + +Task ID: ${task.id} +Description: ${task.description} +Duration: ${formatDuration(task.startedAt ?? new Date(), task.completedAt)} +Session ID: ${task.sessionID} + +--- + +Session error: ${sessionError}` + } + const newMessages = consumeNewMessages(task.sessionID, sortedMessages) if (newMessages.length === 0) { const duration = formatDuration(task.startedAt ?? new Date(), task.completedAt) diff --git a/src/tools/delegate-task/executor-types.ts b/src/tools/delegate-task/executor-types.ts index 8b430c9ce..7efc08370 100644 --- a/src/tools/delegate-task/executor-types.ts +++ b/src/tools/delegate-task/executor-types.ts @@ -31,6 +31,7 @@ export interface SessionMessage { role?: string time?: { created?: number } finish?: string + error?: unknown agent?: string model?: { providerID: string; modelID: string; variant?: string } modelID?: string diff --git a/src/tools/delegate-task/sync-session-poller.test.ts b/src/tools/delegate-task/sync-session-poller.test.ts index b8b2d85ff..004fa0cb8 100644 --- a/src/tools/delegate-task/sync-session-poller.test.ts +++ b/src/tools/delegate-task/sync-session-poller.test.ts @@ -28,9 +28,83 @@ describe("pollSyncSession", () => { }) describe("native finish-based completion", () => { + test("returns terminal session error when assistant message contains info.error", async () => { + // given: error in assistant message + const { pollSyncSession } = require("./sync-session-poller") + + const mockClient = { + session: { + messages: async () => ({ + data: [ + { info: { id: "msg_001", role: "user", time: { created: 1000 } } }, + { + info: { + id: "msg_002", + role: "assistant", + time: { created: 2000 }, + error: { data: { message: "Forbidden: Selected provider is forbidden" } }, + }, + parts: [], + }, + ], + }), + status: async () => ({ data: { "ses_test": { type: "idle" } } }), + }, + } + + // when: calling pollSyncSession + const result = await pollSyncSession(createMockCtx(), mockClient, { + sessionID: "ses_test", + agentToUse: "test-agent", + toastManager: null, + taskId: undefined, + }) + + // then: returns error message + expect(result).toBe("Forbidden: Selected provider is forbidden") + }) + + test("ignores stale prior-turn assistant errors after a new user turn starts", async () => { + // given: prior error exists but user sent new message + const { pollSyncSession } = require("./sync-session-poller") + + const mockClient = { + session: { + messages: async () => ({ + data: [ + { info: { id: "msg_001", role: "user", time: { created: 1000 } } }, + { + info: { + id: "msg_002", + role: "assistant", + time: { created: 2000 }, + error: { data: { message: "Forbidden: Selected provider is forbidden" } }, + }, + parts: [], + }, + { info: { id: "msg_003", role: "user", time: { created: 3000 } } }, + ], + }), + status: async () => ({ data: { "ses_test": { type: "idle" } } }), + abort: async () => ({}), + }, + } + + // when: calling with stale error + const result = await pollSyncSession(createMockCtx(), mockClient, { + sessionID: "ses_test", + agentToUse: "test-agent", + toastManager: null, + taskId: undefined, + anchorMessageCount: 2, + }, 50) + + // then: times out (ignores stale error) + expect(result).toContain("Poll timeout reached") + }) + test("detects completion when assistant message has terminal finish reason", async () => { - //#given - session messages with a terminal assistant finish ("end_turn") - // and the assistant id > user id (native opencode condition) + // given: terminal assistant finish with assistant id > user id const { pollSyncSession } = require("./sync-session-poller") const mockClient = { @@ -48,7 +122,7 @@ describe("pollSyncSession", () => { }, } - //#when + // when: calling pollSyncSession const result = await pollSyncSession(createMockCtx(), mockClient, { sessionID: "ses_test", agentToUse: "test-agent", @@ -56,12 +130,12 @@ describe("pollSyncSession", () => { taskId: undefined, }) - //#then - should return null (success, no error) + // then: returns null (success) expect(result).toBeNull() }) test("keeps polling when assistant finish is tool-calls (non-terminal)", async () => { - //#given - first poll returns tool-calls finish, second returns end_turn + // given: first poll returns tool-calls, second returns end_turn const { pollSyncSession } = require("./sync-session-poller") let callCount = 0 @@ -99,7 +173,7 @@ describe("pollSyncSession", () => { }, } - //#when + // when: calling pollSyncSession const result = await pollSyncSession(createMockCtx(), mockClient, { sessionID: "ses_test", agentToUse: "test-agent", @@ -107,13 +181,13 @@ describe("pollSyncSession", () => { taskId: undefined, }) - //#then + // then: returns null after polling continues expect(result).toBeNull() expect(callCount).toBeGreaterThan(2) }) test("keeps polling when finish is 'unknown' (non-terminal)", async () => { - //#given + // given: first poll returns unknown finish const { pollSyncSession } = require("./sync-session-poller") let callCount = 0 @@ -151,7 +225,7 @@ describe("pollSyncSession", () => { }, } - //#when + // when: calling pollSyncSession const result = await pollSyncSession(createMockCtx(), mockClient, { sessionID: "ses_test", agentToUse: "test-agent", @@ -159,13 +233,13 @@ describe("pollSyncSession", () => { taskId: undefined, }) - //#then + // then: returns null after polling continues expect(result).toBeNull() expect(callCount).toBeGreaterThan(1) }) test("keeps polling when finish is 'stop' but assistant still has tool-call parts", async () => { - //#given + // given: finish is stop but tool-call parts exist const { pollSyncSession } = require("./sync-session-poller") let callCount = 0 @@ -203,7 +277,7 @@ describe("pollSyncSession", () => { }, } - //#when + // when: calling pollSyncSession const result = await pollSyncSession(createMockCtx(), mockClient, { sessionID: "ses_test", agentToUse: "test-agent", @@ -211,13 +285,13 @@ describe("pollSyncSession", () => { taskId: undefined, }) - //#then + // then: returns null after polling continues expect(result).toBeNull() expect(callCount).toBeGreaterThan(1) }) test("does not complete when assistant id < user id (user sent after assistant)", async () => { - //#given - assistant finished but user message came after it (agent still processing) + // given: assistant finished but user message came after it const { pollSyncSession } = require("./sync-session-poller") let callCount = 0 @@ -256,7 +330,7 @@ describe("pollSyncSession", () => { }, } - //#when + // when: calling pollSyncSession const result = await pollSyncSession(createMockCtx(), mockClient, { sessionID: "ses_test", agentToUse: "test-agent", @@ -264,7 +338,7 @@ describe("pollSyncSession", () => { taskId: undefined, }) - //#then + // then: returns null after polling continues expect(result).toBeNull() expect(callCount).toBeGreaterThan(1) }) @@ -272,7 +346,7 @@ describe("pollSyncSession", () => { describe("abort handling", () => { test("#given session completed AND abort fires #then returns completion result not abort", async () => { - //#given + // given: session completes and abort fires const { pollSyncSession } = require("./sync-session-poller") const controller = new AbortController() controller.abort() @@ -300,7 +374,7 @@ describe("pollSyncSession", () => { }, } - //#when + // when: calling pollSyncSession const result = await pollSyncSession({ sessionID: "parent-session", messageID: "parent-message", @@ -314,14 +388,14 @@ describe("pollSyncSession", () => { anchorMessageCount: 1, }) - //#then + // then: returns null with no abort expect(result).toBeNull() expect(messageCallCount).toBe(1) expect(abortCount).toBe(0) }) test("returns abort message when signal is aborted", async () => { - //#given + // given: abort signal already aborted const { pollSyncSession } = require("./sync-session-poller") let abortCount = 0 const mockClient = { @@ -334,7 +408,7 @@ describe("pollSyncSession", () => { }, } - //#when + // when: calling pollSyncSession with aborted signal const result = await pollSyncSession(createMockCtx(true), mockClient, { sessionID: "ses_abort", agentToUse: "test-agent", @@ -342,7 +416,7 @@ describe("pollSyncSession", () => { taskId: "task_123", }) - //#then + // then: returns abort message expect(result).toContain("Task aborted") expect(result).toContain("ses_abort") expect(abortCount).toBe(1) @@ -351,7 +425,7 @@ describe("pollSyncSession", () => { describe("timeout handling", () => { test("returns error string on timeout", async () => { - //#given - never returns a terminal finish, but timeout is very short + // given: no terminal finish and short timeout const { pollSyncSession } = require("./sync-session-poller") __setTimingConfig({ @@ -376,7 +450,7 @@ describe("pollSyncSession", () => { }, } - //#when + // when: calling pollSyncSession const result = await pollSyncSession(createMockCtx(), mockClient, { sessionID: "ses_timeout", agentToUse: "test-agent", @@ -384,19 +458,19 @@ describe("pollSyncSession", () => { taskId: undefined, }, 0) - //#then - timeout returns error string + // then: returns timeout error expect(result).toBe("Poll timeout reached after 50ms for session ses_timeout") expect(abortCount).toBe(1) }) }) - describe("non-idle session status", () => { - test("skips message check when session is not idle", async () => { - //#given - const { pollSyncSession } = require("./sync-session-poller") + describe("non-idle session status", () => { + test("skips message check when session is not idle", async () => { + // given: session is running (not idle) + const { pollSyncSession } = require("./sync-session-poller") - let statusCallCount = 0 - let messageCallCount = 0 + let statusCallCount = 0 + let messageCallCount = 0 const mockClient = { session: { messages: async () => { @@ -421,54 +495,54 @@ describe("pollSyncSession", () => { }, } - //#when - const result = await pollSyncSession(createMockCtx(), mockClient, { - sessionID: "ses_busy", - agentToUse: "test-agent", - toastManager: null, - taskId: undefined, - }) + // when: calling pollSyncSession + const result = await pollSyncSession(createMockCtx(), mockClient, { + sessionID: "ses_busy", + agentToUse: "test-agent", + toastManager: null, + taskId: undefined, + }) - //#then - should have waited for idle before checking messages - expect(result).toBeNull() - expect(statusCallCount).toBeGreaterThanOrEqual(3) - }) - }) + // then: waits for idle before checking messages + expect(result).toBeNull() + expect(statusCallCount).toBeGreaterThanOrEqual(3) + }) + }) describe("isSessionComplete edge cases", () => { test("returns false when messages array is empty", () => { const { isSessionComplete } = require("./sync-session-poller") - //#given - empty messages array + // given: empty messages array const messages: any[] = [] - //#when + // when: calling isSessionComplete const result = isSessionComplete(messages) - //#then - should return false + // then: returns false expect(result).toBe(false) }) test("returns false when no assistant message exists", () => { const { isSessionComplete } = require("./sync-session-poller") - //#given - only user messages, no assistant + // given: only user messages, no assistant const messages = [ { info: { id: "msg_001", role: "user", time: { created: 1000 } } }, { info: { id: "msg_002", role: "user", time: { created: 2000 } } }, ] - //#when + // when: calling isSessionComplete const result = isSessionComplete(messages) - //#then - should return false + // then: returns false expect(result).toBe(false) }) test("returns false when only assistant message exists (no user)", () => { const { isSessionComplete } = require("./sync-session-poller") - //#given - only assistant message, no user message + // given: only assistant message, no user message const messages = [ { info: { id: "msg_001", role: "assistant", time: { created: 1000 }, finish: "end_turn" }, @@ -476,17 +550,17 @@ describe("pollSyncSession", () => { }, ] - //#when + // when: calling isSessionComplete const result = isSessionComplete(messages) - //#then - should return false (no user message to compare IDs) + // then: returns false (no user message to compare IDs) expect(result).toBe(false) }) test("returns false when assistant message has missing finish field", () => { const { isSessionComplete } = require("./sync-session-poller") - //#given - assistant message without finish field + // given: assistant message without finish field const messages = [ { info: { id: "msg_001", role: "user", time: { created: 1000 } } }, { @@ -495,17 +569,17 @@ describe("pollSyncSession", () => { }, ] - //#when + // when: calling isSessionComplete const result = isSessionComplete(messages) - //#then - should return false (missing finish) + // then: returns false (missing finish) expect(result).toBe(false) }) test("returns false when assistant message has missing info.id field", () => { const { isSessionComplete } = require("./sync-session-poller") - //#given - assistant message without id in info + // given: assistant message without id in info const messages = [ { info: { id: "msg_001", role: "user", time: { created: 1000 } } }, { @@ -514,17 +588,17 @@ describe("pollSyncSession", () => { }, ] - //#when + // when: calling isSessionComplete const result = isSessionComplete(messages) - //#then - should return false (missing assistant id) + // then: returns false (missing assistant id) expect(result).toBe(false) }) test("returns false when finish is stop but assistant has tool-call parts", () => { const { isSessionComplete } = require("./sync-session-poller") - //#given - provider marks stop even though tool execution is still pending + // given: provider marks stop even though tool execution is pending const messages = [ { info: { id: "msg_001", role: "user", time: { created: 1000 } } }, { @@ -533,17 +607,17 @@ describe("pollSyncSession", () => { }, ] - //#when + // when: calling isSessionComplete const result = isSessionComplete(messages) - //#then - should return false because tool execution is still pending + // then: returns false because tool execution is still pending expect(result).toBe(false) }) test("returns false when finish is end_turn but assistant has tool-call parts", () => { const { isSessionComplete } = require("./sync-session-poller") - //#given - assistant emitted a terminal finish but still contains pending tool calls + // given: assistant emitted terminal finish but contains pending tool calls const messages = [ { info: { id: "msg_001", role: "user", time: { created: 1000 } } }, { @@ -552,17 +626,17 @@ describe("pollSyncSession", () => { }, ] - //#when + // when: calling isSessionComplete const result = isSessionComplete(messages) - //#then - should return false because tool execution is still pending + // then: returns false because tool execution is still pending expect(result).toBe(false) }) test("returns false when user message has missing info.id field", () => { const { isSessionComplete } = require("./sync-session-poller") - //#given - user message without id in info + // given: user message without id in info const messages = [ { info: { role: "user", time: { created: 1000 } } }, { @@ -571,10 +645,10 @@ describe("pollSyncSession", () => { }, ] - //#when + // when: calling isSessionComplete const result = isSessionComplete(messages) - //#then - should return false (missing user id) + // then: returns false (missing user id) expect(result).toBe(false) }) }) diff --git a/src/tools/delegate-task/sync-session-poller.ts b/src/tools/delegate-task/sync-session-poller.ts index d9bc40d01..255cfce3d 100644 --- a/src/tools/delegate-task/sync-session-poller.ts +++ b/src/tools/delegate-task/sync-session-poller.ts @@ -3,6 +3,7 @@ import type { SessionMessage } from "./executor-types" import { getDefaultSyncPollTimeoutMs, getTimingConfig } from "./timing" import { log } from "../../shared/logger" import { normalizeSDKResponse } from "../../shared" +import { extractErrorMessage } from "../../features/background-agent/error-classifier" const NON_TERMINAL_FINISH_REASONS = new Set(["tool-calls", "unknown"]) const PENDING_TOOL_PART_TYPES = new Set(["tool", "tool_use", "tool-call"]) @@ -32,6 +33,20 @@ async function fetchSessionMessages( return Array.isArray(rawData) ? (rawData as SessionMessage[]) : [] } +function getTerminalSessionError(messages: SessionMessage[]): string | null { + const lastAssistant = [...messages].reverse().find((msg) => msg.info?.role === "assistant") + const lastUser = [...messages].reverse().find((msg) => msg.info?.role === "user") + if (lastUser?.info?.id && lastAssistant?.info?.id && lastAssistant.info.id <= lastUser.info.id) { + return null + } + if (!lastAssistant?.info || !("error" in lastAssistant.info)) { + return null + } + + const errorMessage = extractErrorMessage((lastAssistant.info as { error?: unknown }).error) + return errorMessage && errorMessage.length > 0 ? errorMessage : "Session error" +} + export function isSessionComplete(messages: SessionMessage[]): boolean { let lastUser: SessionMessage | undefined let lastAssistant: SessionMessage | undefined @@ -137,12 +152,18 @@ export async function pollSyncSession( continue } + const sessionError = getTerminalSessionError(messages) + if (sessionError) { + log("[task] Poll detected terminal session error", { sessionID: input.sessionID, sessionError }) + return sessionError + } + if (isSessionComplete(messages)) { log("[task] Poll complete - terminal finish detected", { sessionID: input.sessionID, pollCount }) break } - // 计数新出现的 assistant 轮次,用于熔断无限循环 + // Count new assistant turns to circuit-break infinite loops const lastAssistant = [...messages].reverse().find((m) => m.info?.role === "assistant") if (lastAssistant?.info?.id && lastAssistant.info.id !== lastSeenAssistantId) { lastSeenAssistantId = lastAssistant.info.id diff --git a/src/tools/delegate-task/sync-task-fallback.ts b/src/tools/delegate-task/sync-task-fallback.ts index 6ad64ef3d..fbbc24316 100644 --- a/src/tools/delegate-task/sync-task-fallback.ts +++ b/src/tools/delegate-task/sync-task-fallback.ts @@ -22,13 +22,14 @@ export async function retrySyncPromptWithFallbacks(input: { categoryModel: DelegatedModelConfig | undefined fallbackChain: FallbackEntry[] | undefined sendPrompt: (categoryModel: DelegatedModelConfig) => Promise -}): Promise<{ promptError: string | null; categoryModel: DelegatedModelConfig | undefined }> { +}): Promise<{ promptError: string | null; categoryModel: DelegatedModelConfig | undefined; fallbackState?: ModelFallbackState }> { const { sessionID, initialError, categoryModel, fallbackChain, sendPrompt } = input if (!categoryModel || !fallbackChain || fallbackChain.length === 0) { return { promptError: initialError, categoryModel, + fallbackState: undefined, } } @@ -48,6 +49,7 @@ export async function retrySyncPromptWithFallbacks(input: { return { promptError: finalError, categoryModel, + fallbackState, } } @@ -57,6 +59,7 @@ export async function retrySyncPromptWithFallbacks(input: { return { promptError: null, categoryModel: fallbackModel, + fallbackState, } } @@ -66,3 +69,12 @@ export async function retrySyncPromptWithFallbacks(input: { fallbackState.pending = true } } + +export function getNextSyncFallbackModel( + sessionID: string, + fallbackState: ModelFallbackState | undefined, +): DelegatedModelConfig | null { + if (!fallbackState) return null + const nextFallback = getNextReachableFallback(sessionID, fallbackState) + return nextFallback ? toDelegatedModelConfig(nextFallback) : null +} diff --git a/src/tools/delegate-task/sync-task.test.ts b/src/tools/delegate-task/sync-task.test.ts index a81fe4eb1..e032f11aa 100644 --- a/src/tools/delegate-task/sync-task.test.ts +++ b/src/tools/delegate-task/sync-task.test.ts @@ -15,7 +15,6 @@ describe("executeSyncTask - cleanup on error paths", () => { let resetToastManager: (() => void) | null = null beforeEach(() => { - //#given - configure fast timing for all tests const { __setTimingConfig } = require("./timing") __setTimingConfig({ POLL_INTERVAL_MS: 10, @@ -24,7 +23,6 @@ describe("executeSyncTask - cleanup on error paths", () => { MAX_POLL_TIME_MS: 100, }) - //#given - reset call tracking removeTaskCalls = [] addTaskCalls = [] deleteCalls = [] @@ -32,7 +30,6 @@ describe("executeSyncTask - cleanup on error paths", () => { clearRequireCache("./sync-task") - //#given - initialize real task toast manager (avoid global module mocks) const { initTaskToastManager, _resetTaskToastManagerForTesting } = require("../../features/task-toast-manager/manager") _resetTaskToastManagerForTesting() resetToastManager = _resetTaskToastManagerForTesting @@ -48,7 +45,6 @@ describe("executeSyncTask - cleanup on error paths", () => { removeTaskCalls.push(id) }) - //#given - mock subagentSessions const { subagentSessions } = require("../../features/claude-code-session-state") spyOn(subagentSessions, "add").mockImplementation((id: string) => { addCalls.push(id) @@ -60,7 +56,6 @@ describe("executeSyncTask - cleanup on error paths", () => { }) afterEach(() => { - //#given - reset timing after each test const { __resetTimingConfig } = require("./timing") __resetTimingConfig() @@ -426,11 +421,248 @@ describe("executeSyncTask - cleanup on error paths", () => { expect(deleteCalls[0]).toBe("ses_test_12345678") }) - test("depth regression: blocks spawn when reserveSubagentSpawn throws depth limit error", async () => { - // This is a smoke test guarding against regressions where the depth limit - // would be silently bypassed (e.g. via a fallback path that hardcodes - // childDepth: 1). + test("retries sync session on retryable runtime session error using next fallback model", async () => { + const mockClient = { + session: { + create: async () => ({ data: { id: "ignored" } }), + }, + } + const { executeSyncTask } = require("./sync-task") + const createdSessions: string[] = [] + const attemptedModels: Array<{ providerID: string; modelID: string; variant?: string } | undefined> = [] + const polledSessions: string[] = [] + + const deps = { + createSyncSession: async () => { + const sessionID = createdSessions.length === 0 ? "ses_first" : "ses_second" + createdSessions.push(sessionID) + return { ok: true as const, sessionID } + }, + sendSyncPrompt: async (_client: unknown, input: { categoryModel?: { providerID: string; modelID: string; variant?: string } }) => { + attemptedModels.push(input.categoryModel) + return null + }, + pollSyncSession: async (_ctx: unknown, _client: unknown, input: { sessionID: string }) => { + polledSessions.push(input.sessionID) + return input.sessionID === "ses_first" + ? "Forbidden: Selected provider is forbidden" + : null + }, + fetchSyncResult: async (_client: unknown, sessionID: string) => ({ ok: true as const, textContent: `Result from ${sessionID}` }), + } + + const metadataCalls: any[] = [] + const mockCtx = { + sessionID: "parent-session", + callID: "call-123", + metadata: (input: any) => { metadataCalls.push(input) }, + } + + const mockExecutorCtx = { + client: mockClient, + directory: "/tmp", + onSyncSessionCreated: null, + modelFallbackControllerAccessor: { + setSessionFallbackChain: () => {}, + clearSessionFallbackChain: () => {}, + }, + } + + const args = { + prompt: "test prompt", + description: "test task", + category: "quick", + load_skills: [], + run_in_background: false, + command: null, + } + + const initialModel = { + providerID: "genai-proxy-openai", + modelID: "gpt-5.4-mini", + variant: undefined, + } + const fallbackChain = [ + { providers: ["genai-proxy-openai"], model: "gpt-5.4-mini" }, + { providers: ["genai-proxy-aws"], model: "us.anthropic.claude-haiku-4-5-20251001-v1:0" }, + ] + + const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, { + sessionID: "parent-session", + }, "sisyphus-junior", initialModel, undefined, undefined, fallbackChain, deps) + + expect(createdSessions).toEqual(["ses_first", "ses_second"]) + expect(polledSessions).toEqual(["ses_first", "ses_second"]) + expect(attemptedModels).toEqual([ + { providerID: "genai-proxy-openai", modelID: "gpt-5.4-mini", variant: undefined }, + { providerID: "genai-proxy-aws", modelID: "us.anthropic.claude-haiku-4-5-20251001-v1:0", variant: undefined }, + ]) + expect(result).toContain("Result from ses_second") + expect(deleteCalls).toContain("ses_first") + + const finalMetadata = metadataCalls.at(-1) + expect(finalMetadata.metadata.sessionId).toBe("ses_second") + expect(finalMetadata.metadata.taskId).toBe("ses_second") + expect(finalMetadata.metadata.model).toEqual({ + providerID: "genai-proxy-aws", + modelID: "us.anthropic.claude-haiku-4-5-20251001-v1:0", + variant: undefined, + }) + }) + + test("replays sync session side effects for retry-created sessions", async () => { + const mockClient = { + session: { + create: async () => ({ data: { id: "ignored" } }), + }, + } + + const { executeSyncTask } = require("./sync-task") + const createdSessions: string[] = [] + const onSyncSessionCreated = mock(async (_event: unknown) => {}) + + const deps = { + createSyncSession: async () => { + const sessionID = createdSessions.length === 0 ? "ses_first" : "ses_second" + createdSessions.push(sessionID) + return { ok: true as const, sessionID } + }, + sendSyncPrompt: async () => null, + pollSyncSession: async (_ctx: unknown, _client: unknown, input: { sessionID: string }) => { + return input.sessionID === "ses_first" + ? "Forbidden: Selected provider is forbidden" + : null + }, + fetchSyncResult: async (_client: unknown, sessionID: string) => ({ ok: true as const, textContent: `Result from ${sessionID}` }), + } + + const metadataCalls: any[] = [] + const mockCtx = { + sessionID: "parent-session", + callID: "call-123", + metadata: (input: any) => { metadataCalls.push(input) }, + } + + const mockExecutorCtx = { + client: mockClient, + directory: "/tmp", + onSyncSessionCreated, + modelFallbackControllerAccessor: { + setSessionFallbackChain: () => {}, + clearSessionFallbackChain: () => {}, + }, + } + + const args = { + prompt: "test prompt", + description: "test task", + category: "quick", + load_skills: [], + run_in_background: false, + command: null, + } + + const initialModel = { + providerID: "genai-proxy-openai", + modelID: "gpt-5.4-mini", + variant: undefined, + } + const fallbackChain = [ + { providers: ["genai-proxy-openai"], model: "gpt-5.4-mini" }, + { providers: ["genai-proxy-aws"], model: "us.anthropic.claude-haiku-4-5-20251001-v1:0" }, + ] + + const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, { + sessionID: "parent-session", + }, "sisyphus-junior", initialModel, undefined, undefined, fallbackChain, deps) + + expect(result).toContain("Result from ses_second") + expect(onSyncSessionCreated.mock.calls.map((call: any[]) => call[0])).toEqual([ + { sessionID: "ses_first", parentID: "parent-session", title: "test task" }, + { sessionID: "ses_second", parentID: "parent-session", title: "test task" }, + ]) + expect(addTaskCalls.map((task) => task.sessionID)).toEqual(["ses_first", "ses_second"]) + expect(addTaskCalls.map((task) => task.id)).toEqual(["sync_ses_firs", "sync_ses_firs"]) + }) + + test("publishes latest retry session metadata when final retry still fails", async () => { + const mockClient = { + session: { + create: async () => ({ data: { id: "ignored" } }), + }, + } + + const { executeSyncTask } = require("./sync-task") + const createdSessions: string[] = [] + + const deps = { + createSyncSession: async () => { + const sessionID = createdSessions.length === 0 ? "ses_first" : "ses_second" + createdSessions.push(sessionID) + return { ok: true as const, sessionID } + }, + sendSyncPrompt: async () => null, + pollSyncSession: async (_ctx: unknown, _client: unknown, input: { sessionID: string }) => { + return input.sessionID === "ses_first" + ? "Forbidden: Selected provider is forbidden" + : "Final retry failed" + }, + fetchSyncResult: async () => ({ ok: true as const, textContent: "unused" }), + } + + const metadataCalls: any[] = [] + const mockCtx = { + sessionID: "parent-session", + callID: "call-123", + metadata: (input: any) => { metadataCalls.push(input) }, + } + + const mockExecutorCtx = { + client: mockClient, + directory: "/tmp", + onSyncSessionCreated: null, + modelFallbackControllerAccessor: { + setSessionFallbackChain: () => {}, + clearSessionFallbackChain: () => {}, + }, + } + + const args = { + prompt: "test prompt", + description: "test task", + category: "quick", + load_skills: [], + run_in_background: false, + command: null, + } + + const initialModel = { + providerID: "genai-proxy-openai", + modelID: "gpt-5.4-mini", + variant: undefined, + } + const fallbackChain = [ + { providers: ["genai-proxy-openai"], model: "gpt-5.4-mini" }, + { providers: ["genai-proxy-aws"], model: "us.anthropic.claude-haiku-4-5-20251001-v1:0" }, + ] + + const result = await executeSyncTask(args, mockCtx, mockExecutorCtx, { + sessionID: "parent-session", + }, "sisyphus-junior", initialModel, undefined, undefined, fallbackChain, deps) + + expect(result).toBe("Final retry failed") + const finalMetadata = metadataCalls.at(-1) + expect(finalMetadata.metadata.sessionId).toBe("ses_second") + expect(finalMetadata.metadata.taskId).toBe("ses_second") + expect(finalMetadata.metadata.model).toEqual({ + providerID: "genai-proxy-aws", + modelID: "us.anthropic.claude-haiku-4-5-20251001-v1:0", + variant: undefined, + }) + }) + + test("depth regression: blocks spawn when reserveSubagentSpawn throws depth limit error", async () => { const mockClient = { session: { create: async () => ({ data: { id: "ses_test_12345678" } }), @@ -484,17 +716,10 @@ describe("executeSyncTask - cleanup on error paths", () => { expect(result).toContain("child depth 4") expect(result).toContain("maxDepth=3") expect(reserveSubagentSpawn).toHaveBeenCalledWith("parent-session") - // critical: createSyncSession must NOT have been called -- if it was, - // the depth guard was bypassed. expect(addCalls.length).toBe(0) }) test("depth regression: does not silently fall back to childDepth: 1 when manager methods are present", async () => { - // Guards against the dangerous fallback path in sync-task.ts that - // hardcodes childDepth: 1 if reserveSubagentSpawn / assertCanSpawn are - // not functions. With a real manager present, the fallback must NOT be - // taken. - const mockClient = { session: { create: async () => ({ data: { id: "ses_test_12345678" } }), diff --git a/src/tools/delegate-task/sync-task.ts b/src/tools/delegate-task/sync-task.ts index b1a0d6f38..5601247b7 100644 --- a/src/tools/delegate-task/sync-task.ts +++ b/src/tools/delegate-task/sync-task.ts @@ -9,9 +9,11 @@ import { SessionCategoryRegistry } from "../../shared/session-category-registry" import { formatDuration } from "./time-formatter" import { formatDetailedError } from "./error-formatting" import { syncTaskDeps, type SyncTaskDeps } from "./sync-task-deps" -import { retrySyncPromptWithFallbacks } from "./sync-task-fallback" +import { getNextSyncFallbackModel, retrySyncPromptWithFallbacks } from "./sync-task-fallback" import { buildTaskMetadataBlock } from "../../features/tool-metadata-store/task-metadata-contract" import { resolveMetadataModel } from "./resolve-metadata-model" +import { shouldRetryError } from "../../shared/model-error-classifier" +import type { ModelFallbackState } from "../../hooks/model-fallback/hook" export async function executeSyncTask( args: DelegateTaskArgs, @@ -38,12 +40,7 @@ export async function executeSyncTask( spawnReservation = await manager.reserveSubagentSpawn(parentContext.sessionID) } - // Depth guard. We must NOT silently fall back to childDepth: 1 - // when the manager is unavailable or lacks the spawn methods, because that - // would let subagents recurse without bound. The only safe fallback is - // when the manager genuinely cannot enforce limits (legacy SDK), in which - // case we still record childDepth: 1 but log a warning so regressions are - // visible. + // Only default to childDepth: 1 for legacy managers that cannot enforce spawn depth. let spawnContext: { rootSessionID: string; parentDepth: number; childDepth: number } if (spawnReservation?.spawnContext) { spawnContext = spawnReservation.spawnContext @@ -77,29 +74,61 @@ export async function executeSyncTask( const sessionID = createSessionResult.sessionID spawnReservation?.commit() syncSessionID = sessionID - subagentSessions.add(sessionID) - syncSubagentSessions.add(sessionID) - setSessionAgent(sessionID, agentToUse) - executorCtx.modelFallbackControllerAccessor?.setSessionFallbackChain(sessionID, fallbackChain) - if (args.category) { - SessionCategoryRegistry.register(sessionID, args.category) - } + const registerSyncSession = async (newSessionID: string): Promise => { + syncSessionID = newSessionID + subagentSessions.add(newSessionID) + syncSubagentSessions.add(newSessionID) + setSessionAgent(newSessionID, agentToUse) + executorCtx.modelFallbackControllerAccessor?.setSessionFallbackChain(newSessionID, fallbackChain) - if (onSyncSessionCreated) { - log("[task] Invoking onSyncSessionCreated callback", { sessionID, parentID: parentContext.sessionID }) - try { - await onSyncSessionCreated({ - sessionID, - parentID: parentContext.sessionID, - title: args.description, - }) - } catch (error) { - log("[task] onSyncSessionCreated callback failed", { error: String(error) }) + if (args.category) { + SessionCategoryRegistry.register(newSessionID, args.category) + } + + if (onSyncSessionCreated) { + log("[task] Invoking onSyncSessionCreated callback", { sessionID: newSessionID, parentID: parentContext.sessionID }) + try { + await onSyncSessionCreated({ + sessionID: newSessionID, + parentID: parentContext.sessionID, + title: args.description, + }) + } catch (error) { + log("[task] onSyncSessionCreated callback failed", { error: String(error) }) + } + await new Promise(r => setTimeout(r, 200)) } - await new Promise(r => setTimeout(r, 200)) } + const publishSyncMetadata = async ( + currentSessionID: string, + currentModel: DelegatedModelConfig | undefined, + currentTaskId: string, + spawnDepth: number, + ): Promise => { + await publishToolMetadata(ctx, { + title: args.description, + metadata: { + prompt: args.prompt, + agent: agentToUse, + category: args.category, + ...(args.requested_subagent_type !== undefined ? { requested_subagent_type: args.requested_subagent_type } : {}), + load_skills: args.load_skills, + description: args.description, + run_in_background: args.run_in_background, + taskId: currentSessionID, + sessionId: currentSessionID, + sync: true, + spawnDepth, + command: args.command, + model: resolveMetadataModel(currentModel, parentContext.model), + }, + }) + } + + await registerSyncSession(sessionID) + taskId = `sync_${sessionID.slice(0, 8)}` const startTime = new Date() @@ -115,26 +144,7 @@ export async function executeSyncTask( modelInfo, }) } - - const syncTaskMeta = { - title: args.description, - metadata: { - prompt: args.prompt, - agent: agentToUse, - category: args.category, - ...(args.requested_subagent_type !== undefined ? { requested_subagent_type: args.requested_subagent_type } : {}), - load_skills: args.load_skills, - description: args.description, - run_in_background: args.run_in_background, - taskId: sessionID, - sessionId: sessionID, - sync: true, - spawnDepth: spawnContext.childDepth, - command: args.command, - model: resolveMetadataModel(categoryModel, parentContext.model), - }, - } - await publishToolMetadata(ctx, syncTaskMeta) + await publishSyncMetadata(sessionID, categoryModel, taskId, spawnContext.childDepth) const syncPromptInput = { sessionID, @@ -147,51 +157,109 @@ export async function executeSyncTask( } let effectiveCategoryModel = categoryModel - let promptError = await deps.sendSyncPrompt(client, { - ...syncPromptInput, - categoryModel: effectiveCategoryModel, - }) - if (promptError) { - const promptResult = await retrySyncPromptWithFallbacks({ - sessionID, - initialError: promptError, - categoryModel: effectiveCategoryModel, - fallbackChain, - sendPrompt: async (fallbackModel) => { - return deps.sendSyncPrompt(client, { - ...syncPromptInput, - categoryModel: fallbackModel, - }) - }, - }) + let fallbackState: ModelFallbackState | undefined = effectiveCategoryModel && fallbackChain?.length + ? { + providerID: effectiveCategoryModel.providerID, + modelID: effectiveCategoryModel.modelID, + fallbackChain, + attemptCount: 0, + pending: true, + } + : undefined + let activeSessionID = sessionID - promptError = promptResult.promptError - effectiveCategoryModel = promptResult.categoryModel - - if (promptError) { - return promptError - } + const cleanupRetrySession = (currentSessionID: string): void => { + subagentSessions.delete(currentSessionID) + syncSubagentSessions.delete(currentSessionID) + executorCtx.modelFallbackControllerAccessor?.clearSessionFallbackChain(currentSessionID) + SessionCategoryRegistry.remove(currentSessionID) } try { - const pollError = await deps.pollSyncSession(ctx, client, { - sessionID, - agentToUse, - toastManager, - taskId, - }, syncPollTimeoutMs) - if (pollError) { - return pollError - } + while (true) { + let promptError = await deps.sendSyncPrompt(client, { + ...syncPromptInput, + sessionID: activeSessionID, + categoryModel: effectiveCategoryModel, + }) + if (promptError) { + const promptResult = await retrySyncPromptWithFallbacks({ + sessionID: activeSessionID, + initialError: promptError, + categoryModel: effectiveCategoryModel, + fallbackChain, + sendPrompt: async (fallbackModel) => { + return deps.sendSyncPrompt(client, { + ...syncPromptInput, + sessionID: activeSessionID, + categoryModel: fallbackModel, + }) + }, + }) - const result = await deps.fetchSyncResult(client, sessionID) + promptError = promptResult.promptError + effectiveCategoryModel = promptResult.categoryModel + fallbackState = promptResult.fallbackState ?? fallbackState + + if (promptError) { + return promptError + } + } + + const pollError = await deps.pollSyncSession(ctx, client, { + sessionID: activeSessionID, + agentToUse, + toastManager, + taskId, + }, syncPollTimeoutMs) + if (pollError) { + const nextFallbackModel = shouldRetryError({ message: pollError }) + ? getNextSyncFallbackModel(activeSessionID, fallbackState) + : null + if (!nextFallbackModel) { + return pollError + } + + cleanupRetrySession(activeSessionID) + + const retrySessionResult = await deps.createSyncSession(client, { + parentSessionID: parentContext.sessionID, + agentToUse, + description: args.description, + defaultDirectory: directory, + }) + if (!retrySessionResult.ok) { + return retrySessionResult.error + } + + activeSessionID = retrySessionResult.sessionID + effectiveCategoryModel = nextFallbackModel + await registerSyncSession(activeSessionID) + if (toastManager && taskId) { + toastManager.addTask({ + id: taskId, + sessionID: activeSessionID, + description: args.description, + agent: agentToUse, + isBackground: false, + category: args.category, + skills: args.load_skills, + modelInfo, + }) + } + if (taskId) { + await publishSyncMetadata(activeSessionID, effectiveCategoryModel, taskId, spawnContext.childDepth) + } + continue + } + + const result = await deps.fetchSyncResult(client, activeSessionID) if (!result.ok) { return result.error } const duration = formatDuration(startTime) - // 检测模型路由是否与父 session 不同,给用户可见的提示 const actualModelStr = effectiveCategoryModel ? `${effectiveCategoryModel.providerID}/${effectiveCategoryModel.modelID}` : undefined @@ -205,6 +273,8 @@ export async function executeSyncTask( modelRoutingNote = `\nModel: ${actualModelStr}${args.category ? ` (category: ${args.category})` : ""}` } + await publishSyncMetadata(activeSessionID, effectiveCategoryModel, taskId!, spawnContext.childDepth) + return `Task completed in ${duration}. Agent: ${agentToUse}${args.category ? ` (category: ${args.category})` : ""}${modelRoutingNote} @@ -214,11 +284,12 @@ Agent: ${agentToUse}${args.category ? ` (category: ${args.category})` : ""}${mod ${result.textContent || "(No text output)"} ${buildTaskMetadataBlock({ - sessionId: sessionID, - taskId: sessionID, + sessionId: activeSessionID, + taskId: activeSessionID, agent: agentToUse, category: args.category, })}` + } } finally { if (toastManager && taskId !== undefined) { toastManager.removeTask(taskId)