From 4e3dc9160772c199fabe138d36022634aaf90d4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Mon, 31 Aug 2026 21:52:46 +0000 Subject: [PATCH] feat(agents): add explicit branch recovery for resumed sessions (#3216) --- .agents/skills/code-review/SKILL.md | 10 +- .agents/skills/pr/SKILL.md | 16 +- .github/pull_request_template.md | 6 +- .../agent/runtime/lifecycle/env_preparer.go | 7 + .../lifecycle/env_preparer_worktree.go | 4 +- .../env_preparer_worktree_env_test.go | 18 + .../agent/runtime/lifecycle/manager_launch.go | 17 +- .../internal/agent/runtime/lifecycle/types.go | 29 +- apps/backend/internal/backendapp/adapters.go | 2 + .../orchestrator/executor/executor.go | 15 +- .../orchestrator/executor/executor_resume.go | 181 +++++-- .../executor/executor_resume_test.go | 25 + .../orchestrator/handlers/handlers.go | 17 +- .../orchestrator/handlers/handlers_test.go | 21 + .../orchestrator/session_branch_recovery.go | 460 ++++++++++++++++++ .../session_branch_recovery_test.go | 199 ++++++++ .../internal/orchestrator/session_launch.go | 21 +- .../internal/orchestrator/task_operations.go | 49 +- .../orchestrator/task_operations_test.go | 211 +++++++- apps/backend/internal/worktree/errors.go | 51 ++ apps/backend/internal/worktree/errors_test.go | 21 + apps/backend/internal/worktree/manager_git.go | 43 ++ .../internal/worktree/manager_lifecycle.go | 76 ++- .../manager_recreate_recovery_test.go | 204 ++++++++ .../internal/worktree/manager_replacement.go | 215 ++++++++ .../worktree/manager_reuse_required_test.go | 134 +++++ .../backend/internal/worktree/manager_test.go | 3 + apps/backend/internal/worktree/worktree.go | 6 + .../quick-chat/quick-chat-session-view.tsx | 36 +- .../task/chat/chat-input-container.tsx | 8 + .../chat/messages/action-message-actions.tsx | 167 +++++++ .../messages/action-message-recovery.test.tsx | 87 ++++ .../chat/messages/action-message-recovery.tsx | 153 ++++++ .../task/chat/messages/action-message.tsx | 221 ++------- .../chat/messages/status-message.test.tsx | 30 ++ .../task/chat/messages/status-message.tsx | 48 +- .../task/chat/session-stopped-banner.test.tsx | 66 ++- .../task/chat/session-stopped-banner.tsx | 188 +++++-- .../task/ensure-session-error.test.tsx | 16 + .../components/task/ensure-session-error.tsx | 111 ++++- .../components/task/preview-session-tabs.tsx | 28 +- .../components/run-error-entry.test.tsx | 26 + .../simple/components/run-error-entry.tsx | 149 +++++- apps/web/components/task/task-page-inner.tsx | 11 +- .../e2e/helpers/session-resume-recovery.ts | 102 ++++ apps/web/e2e/pages/session-page.ts | 25 + .../mobile-session-resume-recovery.spec.ts | 140 ++++++ .../session/session-resume-recovery.spec.ts | 141 ++++++ .../session/use-session-recovery-actions.ts | 105 ++++ .../session/use-session-recovery-feedback.ts | 38 ++ .../session/use-session-resumption.test.ts | 74 ++- .../domains/session/use-session-resumption.ts | 95 +++- .../lib/services/session-launch-service.ts | 1 + .../lib/services/session-recovery-service.ts | 82 ++++ apps/web/lib/ws/client.test.ts | 36 +- apps/web/lib/ws/client.ts | 8 +- apps/web/lib/ws/request-error.ts | 33 ++ apps/web/src/locales/en/task.json | 11 + apps/web/src/locales/pseudo/task.json | 11 + apps/web/src/locales/pt-pt/task.json | 11 + apps/web/src/locales/zh-cn/task.json | 11 + apps/web/src/locales/zh-hk/task.json | 25 +- apps/web/src/locales/zh-tw/task.json | 21 +- ...31-explicit-new-branch-session-recovery.md | 74 +++ docs/decisions/INDEX.md | 1 + docs/plans/session-resume-recovery/plan.md | 191 ++++++++ .../task-01-create-replacement-worktrees.md | 158 ++++++ ...task-02-persist-branch-recovery-warning.md | 149 ++++++ .../task-03-show-recovery-errors.md | 158 ++++++ .../task-04-verify-recovery-flows.md | 160 ++++++ docs/specs/agents/README.md | 1 + .../agent-resume-runtime-recovery.md | 88 +++- .../agent-resume-runtime-recovery.md | 282 +++++++++++ 73 files changed, 5208 insertions(+), 430 deletions(-) create mode 100644 apps/backend/internal/orchestrator/session_branch_recovery.go create mode 100644 apps/backend/internal/orchestrator/session_branch_recovery_test.go create mode 100644 apps/backend/internal/worktree/manager_replacement.go create mode 100644 apps/web/components/task/chat/messages/action-message-actions.tsx create mode 100644 apps/web/components/task/chat/messages/action-message-recovery.tsx create mode 100644 apps/web/e2e/helpers/session-resume-recovery.ts create mode 100644 apps/web/e2e/tests/session/mobile-session-resume-recovery.spec.ts create mode 100644 apps/web/e2e/tests/session/session-resume-recovery.spec.ts create mode 100644 apps/web/hooks/domains/session/use-session-recovery-actions.ts create mode 100644 apps/web/hooks/domains/session/use-session-recovery-feedback.ts create mode 100644 apps/web/lib/services/session-recovery-service.ts create mode 100644 apps/web/lib/ws/request-error.ts create mode 100644 docs/decisions/2026-08-31-explicit-new-branch-session-recovery.md create mode 100644 docs/plans/session-resume-recovery/plan.md create mode 100644 docs/plans/session-resume-recovery/task-01-create-replacement-worktrees.md create mode 100644 docs/plans/session-resume-recovery/task-02-persist-branch-recovery-warning.md create mode 100644 docs/plans/session-resume-recovery/task-03-show-recovery-errors.md create mode 100644 docs/plans/session-resume-recovery/task-04-verify-recovery-flows.md create mode 100644 docs/specs/agents/system-design/agent-resume-runtime-recovery.md diff --git a/.agents/skills/code-review/SKILL.md b/.agents/skills/code-review/SKILL.md index ef2ac931c0..c0d5c43ad7 100644 --- a/.agents/skills/code-review/SKILL.md +++ b/.agents/skills/code-review/SKILL.md @@ -17,10 +17,12 @@ Start from intent and evidence: read the spec/task first when available, then ch ### Architecture discussion gate -For a large architectural change, require a linked issue with maintainer -discussion before the PR opens. If the issue or discussion is missing, report a -blocker. Prefer one logical change and a small diff because this limits risk and -maintainer burden. +For a large architectural change, verify the authenticated actor's repository +permission before the PR opens. Maintainers and collaborators with `push`, +`maintain`, or `admin` permission may proceed without a linked issue. For an +actor without write access, require a linked issue with maintainer discussion; +if the issue or discussion is missing, report a blocker. Prefer one logical +change and a small diff because this limits risk and maintainer burden. ## Available skills diff --git a/.agents/skills/pr/SKILL.md b/.agents/skills/pr/SKILL.md index bce06d2359..1243de7d74 100644 --- a/.agents/skills/pr/SKILL.md +++ b/.agents/skills/pr/SKILL.md @@ -71,11 +71,17 @@ explicitly requests task tracking. when the user explicitly requests separate PRs. Use `--draft` if requested, otherwise create as ready-for-review. - **Architecture and scope gate:** Before running any PR creation command, check - large changes for a linked issue with maintainer discussion. If it is missing, - stop and report the blocker. Do not open a PR to start the discussion. Prefer - one logical change and the smallest practical diff; split unrelated cleanup, - refactoring, and feature work into separate PRs. + **Architecture and scope gate:** Before running any PR creation command, verify + the authenticated actor's repository permission. On GitHub, query + `gh api repos/{owner}/{repo}/collaborators/{login}/permission`; `push`, + `maintain`, and `admin` permissions are write-authorized, so those actors may + open large or architectural PRs directly. On other hosts, use the equivalent + repository permission check. For an actor without write access, require a + linked issue with maintainer discussion before opening a large or architectural + PR. If that issue or discussion is missing, stop and report the blocker. Do not + open a PR to start the discussion. Prefer one logical change and the smallest + practical diff; split unrelated cleanup, refactoring, and feature work into + separate PRs. **PR title** must follow Conventional Commits format (see `/commit` for full rules). CI validates via `pr-title.yml` — the PR title becomes the squash-merge commit used for release notes. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 667b15a710..5a9dae6419 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -15,10 +15,10 @@ LANGUAGE Keep the surrounding explanation in English. ARCHITECTURE AND SCOPE - If the change is large or architectural, require a linked issue with maintainer discussion before opening the PR. + For contributors without repository write access, if the change is large or architectural, require a linked issue with maintainer discussion before opening the PR. Maintainers and collaborators with `push`, `maintain`, or `admin` permission may open the PR directly; verify the authenticated actor's repository permission before applying this gate. Large changes include new subsystems, public API or protocol changes, persistence changes, new execution boundaries, authentication or permission-model changes, and cross-cutting changes across subsystems. - If the issue or discussion is missing, stop and report the blocker. Do not open a PR to start the discussion. + If a non-write contributor is missing the issue or discussion, stop and report the blocker. Do not open a PR to start the discussion. Prefer one logical change and the smallest practical diff. Split unrelated cleanup, refactoring, and feature work. IMPORTANT CHANGES (optional) @@ -65,7 +65,7 @@ RULES ## Checklist -- [ ] If this is a large architectural change, I discussed the direction in a linked issue before opening this PR. +- [ ] If I do not have repository write access and this is a large architectural change, I discussed the direction in a linked issue before opening this PR. - [ ] This PR contains one logical change; unrelated work is split into separate PRs. - [ ] I have performed a self-review of my code. - [ ] I have manually tested my changes and they work as expected. diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go index 97d5b6b2ba..a72bef55d9 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer.go @@ -82,6 +82,9 @@ type RepoPrepareSpec struct { // environment. It forbids worktree creation/recreation and all repository // mutating setup paths. WorkspaceReuseRequired bool + // AllowBranchReplacement is set only for the explicit user recovery action. + // It permits a missing persisted branch to be replaced with a fresh branch. + AllowBranchReplacement bool WorktreeBranch string WorktreeBranchPrefix string WorktreeBranchTemplate string @@ -133,6 +136,9 @@ type EnvPrepareRequest struct { // in this request. RepoPrepareSpec carries the same value so multi-repo // callers can retain it while requests are split. WorkspaceReuseRequired bool + // AllowBranchReplacement permits the explicit new-branch recovery action to + // materialize a replacement while retaining the environment record. + AllowBranchReplacement bool WorktreeBranchPrefix string WorktreeBranchTemplate string @@ -185,6 +191,7 @@ func (r *EnvPrepareRequest) RepoSpecs() []RepoPrepareSpec { RemoteContribution: r.RemoteContribution, WorktreeID: r.WorktreeID, WorkspaceReuseRequired: r.WorkspaceReuseRequired, + AllowBranchReplacement: r.AllowBranchReplacement, WorktreeBranch: r.WorktreeBranch, WorktreeBranchPrefix: r.WorktreeBranchPrefix, WorktreeBranchTemplate: r.WorktreeBranchTemplate, diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go index efd67b7a5f..693af1cad0 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree.go @@ -306,7 +306,8 @@ func buildWorktreeCreateRequest(req *EnvPrepareRequest) worktree.CreateRequest { RefreshRepositoryWithState: req.RefreshRepositoryWithState, RemoteRefState: req.RemoteRefState, WorktreeID: req.WorktreeID, - ReuseRequired: req.WorkspaceReuseRequired, + ReuseRequired: req.WorkspaceReuseRequired && !req.AllowBranchReplacement, + AllowBranchReplacement: req.AllowBranchReplacement, TaskDirName: req.TaskDirName, RepoName: req.RepoName, BranchSlug: req.BranchSlug, @@ -532,6 +533,7 @@ func (p *WorktreePreparer) prepareOneRepo( subReq.ContributionDestination = spec.ContributionDestination subReq.WorktreeID = spec.WorktreeID subReq.WorkspaceReuseRequired = req.WorkspaceReuseRequired || spec.WorkspaceReuseRequired + subReq.AllowBranchReplacement = req.AllowBranchReplacement || spec.AllowBranchReplacement subReq.WorktreeBranchPrefix = spec.WorktreeBranchPrefix subReq.WorktreeBranchTemplate = spec.WorktreeBranchTemplate subReq.WorktreeBranchTicket = spec.WorktreeBranchTicket diff --git a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_env_test.go b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_env_test.go index 99ea7cc0b5..c7adf2af77 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_env_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/env_preparer_worktree_env_test.go @@ -23,3 +23,21 @@ func TestBuildWorktreeCreateRequestForwardsProfileEnv(t *testing.T) { t.Fatalf("CreateRequest.ScriptEnv = %#v, want profile env forwarded", got.ScriptEnv) } } + +func TestBuildWorktreeCreateRequestAllowsExplicitBranchReplacement(t *testing.T) { + req := &EnvPrepareRequest{ + WorkspaceReuseRequired: true, + AllowBranchReplacement: true, + RepositoryID: "repo-1", + RepositoryPath: "/repo", + } + + got := buildWorktreeCreateRequest(req) + + if !got.AllowBranchReplacement { + t.Fatal("CreateRequest.AllowBranchReplacement = false, want true") + } + if got.ReuseRequired { + t.Fatal("explicit branch replacement must not use attach-only ReuseRequired") + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go index f5fda53439..0cd92b8e62 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go @@ -602,6 +602,13 @@ func (m *Manager) launchResolveWorkspacePath(ctx context.Context, req *LaunchReq return } +func shouldPrepareEnvironment(req *LaunchRequest) bool { + if req == nil || req.ACPSessionID == "" { + return true + } + return req.UseWorktree && req.RepositoryPath != "" +} + // resolveScratchWorkspace creates and returns the scratch workspace path for a // repo-less task. Returns empty string when the path could not be created. func (m *Manager) resolveScratchWorkspace(ctx context.Context, req *LaunchRequest) string { @@ -1028,6 +1035,7 @@ func buildEnvPrepareRequest(req *LaunchRequest, workspacePath string, execName e TaskRepositoryID: req.TaskRepositoryID, UseWorktree: req.UseWorktree, WorkspaceReuseRequired: req.WorkspaceReuseRequired, + AllowBranchReplacement: req.AllowBranchReplacement, WorktreeID: req.WorktreeID, SetupScript: req.SetupScript, RepoSetupScript: repoSetupScript, @@ -1073,6 +1081,7 @@ func buildEnvPrepareRequest(req *LaunchRequest, workspacePath string, execName e PRNumber: r.PRNumber, RemoteContribution: r.RemoteContribution, WorktreeID: r.WorktreeID, + AllowBranchReplacement: req.AllowBranchReplacement || r.AllowBranchReplacement, WorktreeBranchPrefix: r.WorktreeBranchPrefix, WorktreeBranchTemplate: r.WorktreeBranchTemplate, WorktreeBranchTicket: r.WorktreeBranchTicket, @@ -1391,9 +1400,11 @@ func (m *Manager) launchInternal(ctx context.Context, req *LaunchRequest) (*Agen reqWithWorktree.EnvironmentFinalized = true // 4b. Run environment preparation (if preparer registered for this executor type). - // Skip on resume (ACPSessionID set) — workspace was already prepared during initial launch. + // Native ACP resume normally reuses the already-prepared workspace, but a + // worktree resume must re-enter preparation so a missing branch can be + // reported and an explicit replacement action can materialize it. var prepResult *EnvPrepareResult - if req.ACPSessionID == "" { + if shouldPrepareEnvironment(req) { prepResult = m.runEnvironmentPreparerWithProgress(ctx, &reqWithWorktree, workspacePath, progressRecorder.Callback(0)) } else { m.logger.Debug("skipping environment preparation for resumed session", @@ -1419,7 +1430,7 @@ func (m *Manager) launchInternal(ctx context.Context, req *LaunchRequest) (*Agen // 7. Build runtime request and create instance (agent not started yet) var runtimeProgress PrepareProgressCallback - if req.ACPSessionID == "" { + if shouldPrepareEnvironment(req) { runtimeProgress = progressRecorder.Callback(progressRecorder.Len()) } execReq, execInstance, rt, err := m.launchBuildExecutorRequest(ctx, executionID, &reqWithWorktree, agentConfig, profileInfo, mainRepoGitDir, worktreeID, worktreeBranch, runtimeProgress) diff --git a/apps/backend/internal/agent/runtime/lifecycle/types.go b/apps/backend/internal/agent/runtime/lifecycle/types.go index 12ce2783d8..c2c7fcf212 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/types.go +++ b/apps/backend/internal/agent/runtime/lifecycle/types.go @@ -765,17 +765,20 @@ func (ae *AgentExecution) EndSessionSpan() { // the top level. When LaunchRequest.Repositories is set, each entry produces // one prepared worktree under the shared TaskDirName. type RepoLaunchSpec struct { - TaskRepositoryID string - RepositoryID string - RepositoryPath string - RepositoryURL string // Clone URL for remote executors that need to clone - RepoName string // Repository name used as subdirectory inside TaskDirName - BaseBranch string - DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing - CheckoutBranch string - PRNumber int // GitHub PR number when CheckoutBranch is a PR head; enables refs/pull//head fetch for fork PRs. - RemoteContribution *models.RemoteContribution - WorktreeID string // Existing worktree ID to reuse (skip creation if set) + TaskRepositoryID string + RepositoryID string + RepositoryPath string + RepositoryURL string // Clone URL for remote executors that need to clone + RepoName string // Repository name used as subdirectory inside TaskDirName + BaseBranch string + DefaultBranch string // Repository's default_branch, used as fallback when BaseBranch is missing + CheckoutBranch string + PRNumber int // GitHub PR number when CheckoutBranch is a PR head; enables refs/pull//head fetch for fork PRs. + RemoteContribution *models.RemoteContribution + WorktreeID string // Existing worktree ID to reuse (skip creation if set) + // AllowBranchReplacement permits the explicit new-branch recovery action for + // this repository while retaining its environment record. + AllowBranchReplacement bool WorktreeBranchPrefix string WorktreeBranchTemplate string WorktreeBranchTicket string @@ -849,6 +852,9 @@ type LaunchRequest struct { TaskEnvironmentID string // Env this session belongs to (shared across sessions in same task) // WorkspaceReuseRequired selects attach-only environment preparation. WorkspaceReuseRequired bool + // AllowBranchReplacement is an explicit user-selected recovery permission. + // Normal resume leaves it false so a missing branch remains a visible error. + AllowBranchReplacement bool TaskTitle string // Human-readable task title for semantic worktree naming // AgentProfileID is the stable Office identity for routed Office launches. // For non-Office launches it is also the concrete execution profile. @@ -977,6 +983,7 @@ func (r *LaunchRequest) RepoSpecs() []RepoLaunchSpec { ComparisonTarget: r.ComparisonTarget, ContributionDestination: r.ContributionDestination, WorktreeID: r.WorktreeID, + AllowBranchReplacement: r.AllowBranchReplacement, WorktreeBranchPrefix: r.WorktreeBranchPrefix, WorktreeBranchTemplate: r.WorktreeBranchTemplate, WorktreeBranchTicket: r.WorktreeBranchTicket, diff --git a/apps/backend/internal/backendapp/adapters.go b/apps/backend/internal/backendapp/adapters.go index 4cc10cc268..80f3c56ba4 100644 --- a/apps/backend/internal/backendapp/adapters.go +++ b/apps/backend/internal/backendapp/adapters.go @@ -224,6 +224,7 @@ func buildLifecycleLaunchRequest( SessionID: req.SessionID, TaskEnvironmentID: req.TaskEnvironmentID, WorkspaceReuseRequired: req.WorkspaceReuseRequired, + AllowBranchReplacement: req.AllowBranchReplacement, TaskTitle: req.TaskTitle, AgentProfileID: officeProfileID, ExecutionProfileID: req.AgentProfileID, @@ -326,6 +327,7 @@ func lifecycleRepoLaunchSpecs(repos []executor.RepoSpec) []lifecycle.RepoLaunchS ContributionDestination: r.ContributionDestination, ComparisonTarget: r.ComparisonTarget, WorktreeID: r.WorktreeID, + AllowBranchReplacement: r.AllowBranchReplacement, WorktreeBranchPrefix: r.WorktreeBranchPrefix, WorktreeBranchTemplate: r.WorktreeBranchTemplate, WorktreeBranchTicket: r.WorktreeBranchTicket, diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index e2f7fb9659..f20d2c45c0 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -371,6 +371,9 @@ type LaunchAgentRequest struct { // WorkspaceReuseRequired selects attach-only preparation of an already-ready // task environment. It must never be inferred from a sibling execution ID. WorkspaceReuseRequired bool + // AllowBranchReplacement is granted only by the explicit new-branch recovery + // action. It permits lifecycle to replace a confirmed missing worktree branch. + AllowBranchReplacement bool TaskTitle string // Human-readable task title for semantic worktree naming AgentProfileID string TurnID string // Durable Kandev turn for the initial prompt, when present @@ -489,11 +492,13 @@ type RepoSpec struct { ContributionDestination *models.ContributionDestination ComparisonTarget *models.ComparisonTarget WorktreeID string - WorktreeBranchPrefix string - WorktreeBranchTemplate string - WorktreeBranchTicket string - PullBeforeWorktree bool - RemoteSyncHandled bool + // AllowBranchReplacement permits explicit branch replacement for this repo. + AllowBranchReplacement bool + WorktreeBranchPrefix string + WorktreeBranchTemplate string + WorktreeBranchTicket string + PullBeforeWorktree bool + RemoteSyncHandled bool // RefreshRepository is an optional provider-authenticated refresh deferred // until worktree materialization. A valid reusable worktree bypasses it. RefreshRepository func(context.Context) error diff --git a/apps/backend/internal/orchestrator/executor/executor_resume.go b/apps/backend/internal/orchestrator/executor/executor_resume.go index 45c5c6e9af..fc566f105c 100644 --- a/apps/backend/internal/orchestrator/executor/executor_resume.go +++ b/apps/backend/internal/orchestrator/executor/executor_resume.go @@ -738,9 +738,37 @@ func buildPrepareResultMetadata(result *lifecycle.EnvPrepareResult) map[string]i return lifecycle.SerializePrepareResult(result) } +// ResumeOptions controls explicit recovery behavior for a session resume. +// Branch replacement is intentionally opt-in; ordinary resume preserves the +// original worktree branch and reports when it is unrecoverable. +type ResumeOptions struct { + AllowBranchReplacement bool +} + // ResumeSession restarts an existing task session using its stored worktree. // When startAgent is false, only the executor runtime is started (agent process is not launched). func (e *Executor) ResumeSession(ctx context.Context, session *models.TaskSession, startAgent bool) (*TaskExecution, error) { + return e.resumeSession(ctx, session, startAgent, ResumeOptions{}) +} + +// ResumeSessionWithOptions restarts an existing task session with an explicit +// recovery permission. It keeps the same session and provider resume identity. +func (e *Executor) ResumeSessionWithOptions( + ctx context.Context, + session *models.TaskSession, + startAgent bool, + options ResumeOptions, +) (*TaskExecution, error) { + return e.resumeSession(ctx, session, startAgent, options) +} + +//nolint:cyclop,gocognit,funlen // Resume coordinates the established launch, rollback, and stale-execution recovery sequence. +func (e *Executor) resumeSession( + ctx context.Context, + session *models.TaskSession, + startAgent bool, + options ResumeOptions, +) (*TaskExecution, error) { if session != nil { resumeSnapshot := *session resumeSnapshot.Metadata = cloneMetadata(session.Metadata) @@ -778,8 +806,8 @@ func (e *Executor) ResumeSession(ctx context.Context, session *models.TaskSessio return nil } } - req, _, execCfg, existingEnv, _, err := e.buildResumeRequestAtCredentialBoundary( - ctx, task, session, startAgent, beforeCredentialLease, + req, _, execCfg, existingEnv, _, err := e.buildResumeRequestAtCredentialBoundaryWithOptions( + ctx, task, session, startAgent, beforeCredentialLease, options, ) if err != nil { if resumeStatePersisted { @@ -1085,7 +1113,7 @@ func (e *Executor) validateAndLockResume(ctx context.Context, session *models.Ta // repository details, worktree settings, and ACP resume token. // Returns the request, repository ID, executor config, existing ExecutorRunning record (may be nil), and error. func (e *Executor) buildResumeRequest(ctx context.Context, task *v1.Task, session *models.TaskSession, startAgent bool) (*LaunchAgentRequest, string, executorConfig, *models.TaskEnvironment, *models.ExecutorRunning, error) { - return e.buildResumeRequestAtCredentialBoundary(ctx, task, session, startAgent, nil) + return e.buildResumeRequestAtCredentialBoundaryWithOptions(ctx, task, session, startAgent, nil, ResumeOptions{}) } // buildResumeRequestAtCredentialBoundary prepares a resume request and invokes @@ -1099,23 +1127,72 @@ func (e *Executor) buildResumeRequestAtCredentialBoundary( startAgent bool, beforeCredentialLease func() error, ) (*LaunchAgentRequest, string, executorConfig, *models.TaskEnvironment, *models.ExecutorRunning, error) { + return e.buildResumeRequestAtCredentialBoundaryWithOptions( + ctx, task, session, startAgent, beforeCredentialLease, ResumeOptions{}, + ) +} + +func (e *Executor) buildResumeRequestAtCredentialBoundaryWithOptions( + ctx context.Context, + task *v1.Task, + session *models.TaskSession, + startAgent bool, + beforeCredentialLease func() error, + options ResumeOptions, +) (*LaunchAgentRequest, string, executorConfig, *models.TaskEnvironment, *models.ExecutorRunning, error) { + req, metadata := newResumeLaunchRequest(task, session, startAgent, options) + execConfig := e.applyExecutorConfigToResumeRequest(ctx, req, task, session, metadata) + repositoryID, existingEnv, allRepos, err := e.prepareResumeRepositorySettings( + ctx, task, session, req, + ) + if err != nil { + return nil, "", execConfig, existingEnv, nil, err + } + if err := e.applyResumeMCPSettings(ctx, task.ID, session, req); err != nil { + return nil, "", execConfig, existingEnv, nil, err + } + + existingRunning := e.applyRunningRecordToResumeRequest(ctx, req, task, session, startAgent) + if err := e.applyResumeWorkspaceFolders(ctx, task.ID, req); err != nil { + return nil, "", execConfig, existingEnv, existingRunning, err + } + if err := e.configureResumeGitHubCredentials( + ctx, req, session, allRepos, beforeCredentialLease, + ); err != nil { + return nil, "", execConfig, existingEnv, existingRunning, err + } + e.injectGitLabWorkspaceCredentials(ctx, req) + if err := e.resolveLaunchEnvironment(ctx, req, execConfig.ProfileEnvVars, allRepos); err != nil { + return nil, "", execConfig, existingEnv, existingRunning, err + } + + return req, repositoryID, execConfig, existingEnv, existingRunning, nil +} + +func newResumeLaunchRequest( + task *v1.Task, + session *models.TaskSession, + startAgent bool, + options ResumeOptions, +) (*LaunchAgentRequest, map[string]interface{}) { executionProfileID := session.ExecutionProfileID if executionProfileID == "" { executionProfileID = session.AgentProfileID } req := &LaunchAgentRequest{ - TaskID: task.ID, - WorkspaceID: task.WorkspaceID, - SessionID: session.ID, - TaskTitle: task.Title, - AgentProfileID: executionProfileID, - OfficeAgentProfileID: session.AgentProfileID, - StartAgent: startAgent, - TaskDescription: task.Description, - Priority: task.Priority, - IsEphemeral: task.IsEphemeral, - IsPassthrough: session.IsPassthrough, - TaskEnvironmentID: session.TaskEnvironmentID, + TaskID: task.ID, + WorkspaceID: task.WorkspaceID, + SessionID: session.ID, + TaskTitle: task.Title, + AgentProfileID: executionProfileID, + OfficeAgentProfileID: session.AgentProfileID, + StartAgent: startAgent, + TaskDescription: task.Description, + Priority: task.Priority, + IsEphemeral: task.IsEphemeral, + IsPassthrough: session.IsPassthrough, + TaskEnvironmentID: session.TaskEnvironmentID, + AllowBranchReplacement: options.AllowBranchReplacement, } metadata := map[string]interface{}{} @@ -1131,12 +1208,18 @@ func (e *Executor) buildResumeRequestAtCredentialBoundary( metadata["worktree_id"] = session.Worktrees[0].WorktreeID } req.WorktreeBranchTicket = worktree.TicketForBranchName(task.Identifier, metadata) + return req, metadata +} - execConfig := e.applyExecutorConfigToResumeRequest(ctx, req, task, session, metadata) - +func (e *Executor) prepareResumeRepositorySettings( + ctx context.Context, + task *v1.Task, + session *models.TaskSession, + req *LaunchAgentRequest, +) (string, *models.TaskEnvironment, []*repoInfo, error) { existingEnv, err := e.resolveResumeTaskEnvironment(ctx, task.ID, session) if err != nil { - return nil, "", execConfig, nil, nil, err + return "", nil, nil, err } if session.TaskEnvironmentID != "" { req.TaskEnvironmentID = session.TaskEnvironmentID @@ -1150,52 +1233,52 @@ func (e *Executor) buildResumeRequestAtCredentialBoundary( allRepos, err := e.resolveAllRepoInfoForSession(ctx, task.ID, session.ID) if err != nil { - return nil, "", execConfig, nil, nil, err + return "", nil, nil, err } req.McpProviders = deriveMCPProviders(allRepos) repositoryID, err := e.applyResumeRepoConfig(ctx, task, session, req, existingEnv, allRepos) if err != nil { - return nil, "", execConfig, nil, nil, err - } - if len(allRepos) > 0 { - req.PullBeforeWorktree = allRepos[0].PullBeforeWorktree - req.RemoteSyncHandled = allRepos[0].RemoteSyncHandled - req.RefreshRepository = allRepos[0].RefreshRepository - req.RefreshRepositoryWithState = allRepos[0].RefreshRepositoryWithState - req.RemoteRefState = allRepos[0].RemoteRefState + return "", nil, nil, err } + applyResumeRepositoryFlags(req, allRepos) if err := e.validateReuseEnvironmentInventory(ctx, req, existingEnv); err != nil { - return nil, "", execConfig, existingEnv, nil, err + return "", existingEnv, nil, err } e.reuseExistingEnvironment(ctx, req, existingEnv) + return repositoryID, existingEnv, allRepos, nil +} - req.McpMode, err = e.resolveTaskSessionMCPMode(ctx, task.ID, session, true) - if err != nil { - return nil, "", execConfig, nil, nil, err +func applyResumeRepositoryFlags(req *LaunchAgentRequest, repositories []*repoInfo) { + if len(repositories) == 0 { + return } - profileContext, err := e.resolveTaskSessionMCPProfile(ctx, task.ID, session, true) + first := repositories[0] + req.PullBeforeWorktree = first.PullBeforeWorktree + req.RemoteSyncHandled = first.RemoteSyncHandled + req.RefreshRepository = first.RefreshRepository + req.RefreshRepositoryWithState = first.RefreshRepositoryWithState + req.RemoteRefState = first.RemoteRefState +} + +func (e *Executor) applyResumeMCPSettings( + ctx context.Context, + taskID string, + session *models.TaskSession, + req *LaunchAgentRequest, +) error { + mode, err := e.resolveTaskSessionMCPMode(ctx, taskID, session, true) if err != nil { - return nil, "", execConfig, nil, nil, err + return err } + profileContext, err := e.resolveTaskSessionMCPProfile(ctx, taskID, session, true) + if err != nil { + return err + } + req.McpMode = mode profileContext.Providers = req.McpProviders req.McpProfile = &profileContext - - existingRunning := e.applyRunningRecordToResumeRequest(ctx, req, task, session, startAgent) - if err := e.applyResumeWorkspaceFolders(ctx, task.ID, req); err != nil { - return nil, "", execConfig, existingEnv, nil, err - } - if err := e.configureResumeGitHubCredentials( - ctx, req, session, allRepos, beforeCredentialLease, - ); err != nil { - return nil, "", execConfig, existingEnv, existingRunning, err - } - e.injectGitLabWorkspaceCredentials(ctx, req) - if err := e.resolveLaunchEnvironment(ctx, req, execConfig.ProfileEnvVars, allRepos); err != nil { - return nil, "", execConfig, existingEnv, existingRunning, err - } - - return req, repositoryID, execConfig, existingEnv, existingRunning, nil + return nil } func (e *Executor) applyResumeWorkspaceFolders( diff --git a/apps/backend/internal/orchestrator/executor/executor_resume_test.go b/apps/backend/internal/orchestrator/executor/executor_resume_test.go index a4c2ece3ad..acee6ea8c1 100644 --- a/apps/backend/internal/orchestrator/executor/executor_resume_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_resume_test.go @@ -106,6 +106,31 @@ func setupLiveResumeTestFixture(repo *mockRepository) { } } +func TestBuildResumeRequestWithOptions_ForwardsBranchReplacementPermission(t *testing.T) { + repo := newMockRepository() + setupLiveResumeTestFixture(repo) + exec := newTestExecutor(t, &mockAgentManager{}, repo) + + req, _, _, _, _, err := exec.buildResumeRequestAtCredentialBoundaryWithOptions( + context.Background(), repo.tasks["task-1"].ToAPI(), repo.sessions["sess-1"], true, nil, + ResumeOptions{AllowBranchReplacement: true}, + ) + if err != nil { + t.Fatalf("buildResumeRequestWithOptions returned error: %v", err) + } + if !req.AllowBranchReplacement { + t.Fatal("resume request lost explicit branch replacement permission") + } + + normalReq, _, _, _, _, err := exec.buildResumeRequest(context.Background(), repo.tasks["task-1"].ToAPI(), repo.sessions["sess-1"], true) + if err != nil { + t.Fatalf("normal buildResumeRequest returned error: %v", err) + } + if normalReq.AllowBranchReplacement { + t.Fatal("normal resume unexpectedly enabled branch replacement") + } +} + type resumeCredentialStateIssuer struct { repo *mockRepository observedState models.TaskSessionState diff --git a/apps/backend/internal/orchestrator/handlers/handlers.go b/apps/backend/internal/orchestrator/handlers/handlers.go index 16591db7cd..e697be94ba 100644 --- a/apps/backend/internal/orchestrator/handlers/handlers.go +++ b/apps/backend/internal/orchestrator/handlers/handlers.go @@ -245,7 +245,15 @@ func (h *Handlers) wsSetPlanMode(ctx context.Context, msg *ws.Message) (*ws.Mess type wsRecoverSessionRequest struct { TaskID string `json:"task_id"` SessionID string `json:"session_id"` - Action string `json:"action"` // "resume", "fresh_start", "runtime_retry", or "cancel_retry" + Action string `json:"action"` // "resume", "resume_new_branch", "fresh_start", "runtime_retry", or "cancel_retry" +} + +func branchRecoveryConflictResponse(msg *ws.Message, err error) (*ws.Message, error) { + var branchRecoveryErr *orchestrator.BranchRecoveryError + if !errors.As(err, &branchRecoveryErr) { + return nil, nil + } + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeConflict, err.Error(), branchRecoveryErr.Details()) } // wsRecoverSession recovers a session by id (resume or fresh start). @@ -268,12 +276,15 @@ func (h *Handlers) wsRecoverSession(ctx context.Context, msg *ws.Message) (*ws.M return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{"cancelled": cancelled}) } - if req.Action != "resume" && req.Action != "fresh_start" && req.Action != "runtime_retry" { - return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "action must be 'resume', 'fresh_start', 'runtime_retry', or 'cancel_retry'", nil) + if req.Action != "resume" && req.Action != "resume_new_branch" && req.Action != "fresh_start" && req.Action != "runtime_retry" { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "action must be 'resume', 'resume_new_branch', 'fresh_start', 'runtime_retry', or 'cancel_retry'", nil) } resp, err := h.service.RecoverSession(ctx, req.TaskID, req.SessionID, req.Action) if err != nil { + if recoveryResponse, responseErr := branchRecoveryConflictResponse(msg, err); recoveryResponse != nil || responseErr != nil { + return recoveryResponse, responseErr + } h.logger.Error("failed to recover session", zap.String("task_id", req.TaskID), zap.String("session_id", req.SessionID), diff --git a/apps/backend/internal/orchestrator/handlers/handlers_test.go b/apps/backend/internal/orchestrator/handlers/handlers_test.go index 8398d8cb92..6d63b4a23f 100644 --- a/apps/backend/internal/orchestrator/handlers/handlers_test.go +++ b/apps/backend/internal/orchestrator/handlers/handlers_test.go @@ -3,6 +3,7 @@ package handlers import ( "context" "encoding/json" + "errors" "testing" "github.com/kandev/kandev/internal/common/logger" @@ -38,6 +39,26 @@ func TestWsRecoverSessionCancelRetryReportsServiceResult(t *testing.T) { require.False(t, payload.Cancelled) } +func TestBranchRecoveryConflictResponsePreservesRecoveryDetails(t *testing.T) { + msg := createTestMessage(t, ws.ActionSessionRecover, map[string]interface{}{}) + err := &orchestrator.BranchRecoveryError{ + Cause: errors.New("branch is gone"), + SessionID: "session-1", + RepositoryID: "repo-1", + OriginalBranch: "feature/lost", + BaseBranch: "main", + } + + response, responseErr := branchRecoveryConflictResponse(msg, err) + require.NoError(t, responseErr) + require.NotNil(t, response) + payload := parseError(t, response) + require.Equal(t, ws.ErrorCodeConflict, payload.Code) + require.Equal(t, "feature/lost", payload.Details["original_branch"]) + require.Equal(t, "main", payload.Details["base_branch"]) + require.Equal(t, "resume_new_branch", payload.Details["recovery_action"]) +} + func TestWsEnsureSessionRequestParsesAutoStartOverride(t *testing.T) { tests := []struct { name string diff --git a/apps/backend/internal/orchestrator/session_branch_recovery.go b/apps/backend/internal/orchestrator/session_branch_recovery.go new file mode 100644 index 0000000000..3130f12544 --- /dev/null +++ b/apps/backend/internal/orchestrator/session_branch_recovery.go @@ -0,0 +1,460 @@ +package orchestrator + +import ( + "context" + "crypto/sha256" + "encoding/json" + "errors" + "fmt" + "strings" + "time" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/worktree" + v1 "github.com/kandev/kandev/pkg/api/v1" + "go.uber.org/zap" +) + +// BranchRecoveryError identifies a worktree branch that cannot be restored. +// It wraps the original error so callers can still use errors.Is/errors.As. +type BranchRecoveryError struct { + Cause error + SessionID string + RepositoryID string + OriginalBranch string + BaseBranch string +} + +func (e *BranchRecoveryError) Error() string { + if e == nil { + return "the worktree branch is no longer available" + } + branch := strings.TrimSpace(e.OriginalBranch) + if branch == "" { + return "the worktree branch is no longer available; continue on a new branch to keep the conversation history" + } + return fmt.Sprintf( + "the worktree branch %q is no longer available; continue on a new branch to keep the conversation history, but code changes from the lost branch cannot be recovered", + branch, + ) +} + +func (e *BranchRecoveryError) Unwrap() error { + if e == nil { + return nil + } + return e.Cause +} + +// Details returns the stable WebSocket recovery contract for this error. +func (e *BranchRecoveryError) Details() map[string]interface{} { + if e == nil { + return nil + } + return map[string]interface{}{ + "kind": "branch_unrecoverable", + "recovery_action": "resume_new_branch", + "original_branch": e.OriginalBranch, + "base_branch": e.BaseBranch, + "repository_id": e.RepositoryID, + "session_id": e.SessionID, + } +} + +type branchRecoveryRepoSnapshot struct { + RowID string + RepositoryID string + BranchSlug string + WorktreeID string + WorktreeBranch string + BaseBranch string + Position int +} + +type branchRecoverySnapshot struct { + TaskID string + SessionID string + EnvironmentID string + Repositories []branchRecoveryRepoSnapshot +} + +const ( + branchRecoveryWarningKeyPrefix = "branch_recreated_warning:" + branchRecoveryWarningClaimStaleAfter = 5 * time.Minute +) + +type branchRecoveryWarningClaim struct { + ClaimedAt time.Time `json:"claimed_at"` +} + +type branchRecoveryStore interface { + ListTaskRepositories(ctx context.Context, taskID string) ([]*models.TaskRepository, error) + ListTaskEnvironmentRepos(ctx context.Context, envID string) ([]*models.TaskEnvironmentRepo, error) +} + +func (s *Service) captureBranchRecoverySnapshot( + ctx context.Context, + taskID, sessionID string, +) (*branchRecoverySnapshot, error) { + store, env, err := s.loadBranchRecoverySnapshotContext(ctx, taskID, sessionID) + if err != nil { + return nil, err + } + snapshot := &branchRecoverySnapshot{ + TaskID: taskID, + SessionID: sessionID, + } + if env == nil { + return snapshot, nil + } + rows, err := s.branchRecoveryEnvironmentRepos(ctx, store, env) + if err != nil { + return nil, err + } + baseBranches, err := s.branchRecoveryBaseBranches(ctx, store, taskID) + if err != nil { + return nil, err + } + snapshot.EnvironmentID = env.ID + snapshot.Repositories = branchRecoveryRepoSnapshots(rows, baseBranches) + return snapshot, nil +} + +func (s *Service) loadBranchRecoverySnapshotContext( + ctx context.Context, + taskID, sessionID string, +) (branchRecoveryStore, *models.TaskEnvironment, error) { + if s.repo == nil { + return nil, nil, fmt.Errorf("repository is not configured") + } + store, ok := s.repo.(branchRecoveryStore) + if !ok { + return nil, nil, fmt.Errorf("repository does not support branch recovery snapshots") + } + session, err := s.repo.GetTaskSession(ctx, sessionID) + if err != nil { + return nil, nil, fmt.Errorf("load session for branch recovery: %w", err) + } + if session == nil { + return nil, nil, fmt.Errorf("session %q was not found", sessionID) + } + env, err := s.repo.GetTaskEnvironmentByTaskID(ctx, taskID) + if err != nil { + return nil, nil, fmt.Errorf("load task environment for branch recovery: %w", err) + } + if env == nil && session.TaskEnvironmentID != "" { + env, err = s.repo.GetTaskEnvironment(ctx, session.TaskEnvironmentID) + if err != nil { + return nil, nil, fmt.Errorf("load session environment for branch recovery: %w", err) + } + } + return store, env, nil +} + +func (s *Service) branchRecoveryEnvironmentRepos( + ctx context.Context, + store branchRecoveryStore, + env *models.TaskEnvironment, +) ([]*models.TaskEnvironmentRepo, error) { + if len(env.Repos) > 0 { + return env.Repos, nil + } + rows, err := store.ListTaskEnvironmentRepos(ctx, env.ID) + if err != nil { + return nil, fmt.Errorf("load environment repositories for branch recovery: %w", err) + } + return rows, nil +} + +func (s *Service) branchRecoveryBaseBranches( + ctx context.Context, + store branchRecoveryStore, + taskID string, +) (map[string]string, error) { + taskRepos, err := store.ListTaskRepositories(ctx, taskID) + if err != nil { + return nil, fmt.Errorf("load task repositories for branch recovery: %w", err) + } + baseBranches := make(map[string]string, len(taskRepos)) + for _, taskRepo := range taskRepos { + if taskRepo != nil && taskRepo.RepositoryID != "" { + baseBranches[taskRepo.RepositoryID] = taskRepo.BaseBranch + } + } + return baseBranches, nil +} + +func branchRecoveryRepoSnapshots( + rows []*models.TaskEnvironmentRepo, + baseBranches map[string]string, +) []branchRecoveryRepoSnapshot { + snapshots := make([]branchRecoveryRepoSnapshot, 0, len(rows)) + for _, row := range rows { + if row == nil || row.WorktreeID == "" || row.WorktreeBranch == "" { + continue + } + snapshots = append(snapshots, branchRecoveryRepoSnapshot{ + RowID: row.ID, + RepositoryID: row.RepositoryID, + BranchSlug: row.BranchSlug, + WorktreeID: row.WorktreeID, + WorktreeBranch: row.WorktreeBranch, + BaseBranch: baseBranches[row.RepositoryID], + Position: row.Position, + }) + } + return snapshots +} + +func (s *Service) branchRecoveryError(ctx context.Context, taskID, sessionID string, cause error) error { + if cause == nil || !errors.Is(cause, worktree.ErrBranchUnrecoverable) { + return cause + } + var existing *BranchRecoveryError + if errors.As(cause, &existing) { + return cause + } + + originalBranch := "" + var branchErr *worktree.BranchUnrecoverableError + if errors.As(cause, &branchErr) { + originalBranch = branchErr.BranchName() + } + snapshot, snapshotErr := s.captureBranchRecoverySnapshot(ctx, taskID, sessionID) + if snapshotErr != nil { + snapshot = nil + } + repositoryID, baseBranch := branchRecoveryContext(snapshot, originalBranch) + return &BranchRecoveryError{ + Cause: cause, + SessionID: sessionID, + RepositoryID: repositoryID, + OriginalBranch: originalBranch, + BaseBranch: baseBranch, + } +} + +func branchRecoveryContext(snapshot *branchRecoverySnapshot, originalBranch string) (string, string) { + if snapshot == nil { + return "", "" + } + for _, repo := range snapshot.Repositories { + if repo.WorktreeBranch == originalBranch { + return repo.RepositoryID, repo.BaseBranch + } + } + if len(snapshot.Repositories) == 1 { + return snapshot.Repositories[0].RepositoryID, snapshot.Repositories[0].BaseBranch + } + return "", "" +} + +func (s *Service) persistBranchRecoveryWarnings( + ctx context.Context, + taskID, sessionID string, + before *branchRecoverySnapshot, +) { + if before == nil || len(before.Repositories) == 0 || s.messageCreator == nil { + return + } + after, err := s.captureBranchRecoverySnapshot(ctx, taskID, sessionID) + if err != nil { + s.logger.Warn("failed to inspect branch recovery result", zapFieldsBranchRecovery(taskID, sessionID, err)...) + return + } + for _, previous := range before.Repositories { + current, found := findBranchRecoveryRepo(after, previous) + if !found || current.WorktreeBranch == "" || current.WorktreeBranch == previous.WorktreeBranch || previous.WorktreeBranch == "" { + continue + } + s.persistBranchRecoveryWarning(ctx, taskID, sessionID, previous, current) + } +} + +func findBranchRecoveryRepo(snapshot *branchRecoverySnapshot, previous branchRecoveryRepoSnapshot) (branchRecoveryRepoSnapshot, bool) { + if snapshot == nil { + return branchRecoveryRepoSnapshot{}, false + } + for _, current := range snapshot.Repositories { + if previous.RowID != "" && current.RowID == previous.RowID { + return current, true + } + if current.RepositoryID == previous.RepositoryID && current.BranchSlug == previous.BranchSlug && current.Position == previous.Position { + return current, true + } + } + return branchRecoveryRepoSnapshot{}, false +} + +func (s *Service) persistBranchRecoveryWarning( + ctx context.Context, + taskID, sessionID string, + previous, current branchRecoveryRepoSnapshot, +) { + decisionID := branchRecoveryDecisionID(taskID, sessionID, previous.RepositoryID, previous.WorktreeBranch, current.WorktreeBranch, current.BaseBranch) + key := branchRecoveryWarningKeyPrefix + decisionID + release, claimed := s.claimBranchRecoveryWarning(ctx, sessionID, key) + if !claimed { + return + } + metadata := map[string]interface{}{ + "variant": "warning", + "kind": "branch_recreated", + "original_branch": previous.WorktreeBranch, + "new_branch": current.WorktreeBranch, + "base_branch": current.BaseBranch, + "session_id": sessionID, + "repository_id": current.RepositoryID, + "decision_id": decisionID, + } + err := s.messageCreator.CreateSessionMessage( + context.WithoutCancel(ctx), + taskID, + "branch_recreated", + sessionID, + string(v1.MessageTypeStatus), + s.getActiveTurnID(sessionID), + metadata, + false, + ) + if err != nil { + release() + s.logger.Warn("failed to persist branch recovery warning", zapFieldsBranchRecovery(taskID, sessionID, err)...) + } +} + +func (s *Service) claimBranchRecoveryWarning(ctx context.Context, sessionID, key string) (func(), bool) { + if s.repo == nil { + return func() {}, false + } + claimCtx := context.WithoutCancel(ctx) + if claimer, ok := s.repo.(failedSessionMetadataClaimer); ok { + return s.claimBranchRecoveryWarningWithState(claimCtx, sessionID, key, claimer) + } + return s.claimBranchRecoveryWarningWithoutState(claimCtx, sessionID, key) +} + +func (s *Service) claimBranchRecoveryWarningWithState( + ctx context.Context, + sessionID, key string, + claimer failedSessionMetadataClaimer, +) (func(), bool) { + session, err := s.repo.GetTaskSession(ctx, sessionID) + if err != nil || session == nil { + return func() {}, false + } + claim := branchRecoveryWarningClaim{ClaimedAt: time.Now().UTC()} + claimed, err := claimer.SetSessionMetadataKeyIfAbsentIfState(ctx, sessionID, key, claim, session.State) + if err != nil { + return func() {}, false + } + if !claimed { + session, claimed = s.reclaimBranchRecoveryWarningClaim(ctx, sessionID, key, session.State) + if claimed { + claimed, err = claimer.SetSessionMetadataKeyIfAbsentIfState(ctx, sessionID, key, claim, session.State) + } + } + if err != nil || !claimed { + return func() {}, false + } + return func() { + s.releaseBranchRecoveryWarningClaim(ctx, sessionID, key, session.State) + }, true +} + +func (s *Service) claimBranchRecoveryWarningWithoutState(ctx context.Context, sessionID, key string) (func(), bool) { + claimed, err := s.repo.SetSessionMetadataKeyIfAbsent( + ctx, sessionID, key, branchRecoveryWarningClaim{ClaimedAt: time.Now().UTC()}, + ) + return func() {}, err == nil && claimed +} + +func (s *Service) reclaimBranchRecoveryWarningClaim( + ctx context.Context, + sessionID, key string, + expectedState models.TaskSessionState, +) (*models.TaskSession, bool) { + if !s.reclaimStaleBranchRecoveryWarningClaim(ctx, sessionID, key, expectedState) { + return nil, false + } + session, err := s.repo.GetTaskSession(ctx, sessionID) + return session, err == nil && session != nil +} + +func (s *Service) releaseBranchRecoveryWarningClaim(ctx context.Context, sessionID, key string, expectedState models.TaskSessionState) { + releaser, ok := s.repo.(failedSessionMetadataClaimReleaser) + if !ok { + return + } + _, _ = releaser.RemoveSessionMetadataKeyIfState(ctx, sessionID, key, expectedState) +} + +// reclaimStaleBranchRecoveryWarningClaim releases a timestamped claim left by +// a crashed process. A matching persisted warning wins over reclamation so a +// crash after message insertion cannot cause a duplicate warning. +func (s *Service) reclaimStaleBranchRecoveryWarningClaim( + ctx context.Context, + sessionID, key string, + expectedState models.TaskSessionState, +) bool { + releaser, ok := s.repo.(failedSessionMetadataClaimReleaser) + if !ok { + return false + } + session, err := s.repo.GetTaskSession(ctx, sessionID) + if err != nil || session == nil || session.State != expectedState || session.Metadata == nil { + return false + } + claim, ok := branchRecoveryWarningClaimFromMetadata(session.Metadata[key]) + if !ok || time.Since(claim.ClaimedAt) < branchRecoveryWarningClaimStaleAfter { + return false + } + decisionID := strings.TrimPrefix(key, branchRecoveryWarningKeyPrefix) + if decisionID == "" || decisionID == key { + return false + } + messages, err := s.repo.ListMessages(ctx, sessionID) + if err != nil || branchRecoveryWarningMessageExists(messages, decisionID) { + return false + } + removed, err := releaser.RemoveSessionMetadataKeyIfState(ctx, sessionID, key, session.State) + return err == nil && removed +} + +func branchRecoveryWarningClaimFromMetadata(value interface{}) (branchRecoveryWarningClaim, bool) { + data, err := json.Marshal(value) + if err != nil { + return branchRecoveryWarningClaim{}, false + } + var claim branchRecoveryWarningClaim + if err := json.Unmarshal(data, &claim); err != nil || claim.ClaimedAt.IsZero() { + return branchRecoveryWarningClaim{}, false + } + return claim, true +} + +func branchRecoveryWarningMessageExists(messages []*models.Message, decisionID string) bool { + for _, message := range messages { + if message == nil || message.Metadata == nil { + continue + } + if message.Metadata["kind"] != "branch_recreated" || message.Metadata["decision_id"] != decisionID { + continue + } + return true + } + return false +} + +func branchRecoveryDecisionID(parts ...string) string { + sum := sha256.Sum256([]byte(strings.Join(parts, "\x00"))) + return fmt.Sprintf("branch-recreated-%x", sum[:12]) +} + +func zapFieldsBranchRecovery(taskID, sessionID string, err error) []zap.Field { + return []zap.Field{ + zap.String("task_id", taskID), + zap.String("session_id", sessionID), + zap.Error(err), + } +} diff --git a/apps/backend/internal/orchestrator/session_branch_recovery_test.go b/apps/backend/internal/orchestrator/session_branch_recovery_test.go new file mode 100644 index 0000000000..caf1d3922c --- /dev/null +++ b/apps/backend/internal/orchestrator/session_branch_recovery_test.go @@ -0,0 +1,199 @@ +package orchestrator + +import ( + "context" + "errors" + "fmt" + "testing" + "time" + + "github.com/kandev/kandev/internal/task/models" + "github.com/kandev/kandev/internal/worktree" + v1 "github.com/kandev/kandev/pkg/api/v1" + "github.com/stretchr/testify/require" +) + +func TestBranchRecoveryErrorPreservesTypedCauseAndDetails(t *testing.T) { + cause := fmt.Errorf("restore worktree: %w", &worktree.BranchUnrecoverableError{Branch: "feature/lost"}) + service := &Service{} + + err := service.branchRecoveryError(context.Background(), "task-1", "session-1", cause) + + var recoveryErr *BranchRecoveryError + require.ErrorAs(t, err, &recoveryErr) + require.ErrorIs(t, err, worktree.ErrBranchUnrecoverable) + require.Equal(t, "session-1", recoveryErr.SessionID) + require.Equal(t, "feature/lost", recoveryErr.OriginalBranch) + require.Equal(t, map[string]interface{}{ + "kind": "branch_unrecoverable", + "recovery_action": "resume_new_branch", + "original_branch": "feature/lost", + "base_branch": "", + "repository_id": "", + "session_id": "session-1", + }, recoveryErr.Details()) + require.Contains(t, recoveryErr.Error(), "continue on a new branch") +} + +func TestPersistBranchRecoveryWarningsIsIdempotent(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "task-branch-recovery", "session-branch-recovery", "step1") + now := time.Now().UTC() + require.NoError(t, repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-branch-recovery", WorkspaceID: "ws1", Name: "backend", DefaultBranch: "main", + CreatedAt: now, UpdatedAt: now, + })) + require.NoError(t, repo.CreateTaskRepository(ctx, &models.TaskRepository{ + ID: "task-repo-branch-recovery", TaskID: "task-branch-recovery", RepositoryID: "repo-branch-recovery", + BaseBranch: "main", Position: 0, CreatedAt: now, UpdatedAt: now, + })) + require.NoError(t, repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{ + ID: "env-branch-recovery", TaskID: "task-branch-recovery", + ExecutorType: string(models.ExecutorTypeWorktree), WorkspacePath: t.TempDir(), Status: models.TaskEnvironmentStatusReady, + Repos: []*models.TaskEnvironmentRepo{{ + ID: "env-repo-branch-recovery", RepositoryID: "repo-branch-recovery", BranchSlug: "task-branch-recovery", + WorktreeID: "worktree-branch-recovery", WorktreeBranch: "feature/lost", Position: 0, + CreatedAt: now, UpdatedAt: now, + }}, + })) + session, err := repo.GetTaskSession(ctx, "session-branch-recovery") + require.NoError(t, err) + session.TaskEnvironmentID = "env-branch-recovery" + require.NoError(t, repo.UpdateTaskSession(ctx, session)) + + service := createTestService(repo, newMockStepGetter(), newMockTaskRepo()) + messages := &mockMessageCreator{} + service.messageCreator = messages + before, err := service.captureBranchRecoverySnapshot(ctx, "task-branch-recovery", "session-branch-recovery") + require.NoError(t, err) + + row, err := repo.ListTaskEnvironmentRepos(ctx, "env-branch-recovery") + require.NoError(t, err) + require.Len(t, row, 1) + row[0].WorktreeBranch = "feature/recreated" + require.NoError(t, repo.UpdateTaskEnvironmentRepo(ctx, row[0])) + + service.persistBranchRecoveryWarnings(ctx, "task-branch-recovery", "session-branch-recovery", before) + service.persistBranchRecoveryWarnings(ctx, "task-branch-recovery", "session-branch-recovery", before) + + require.Len(t, messages.sessionMessages, 1) + warning := messages.sessionMessages[0] + require.Equal(t, "branch_recreated", warning.content) + require.Equal(t, string(v1.MessageTypeStatus), warning.messageType) + require.Equal(t, "warning", warning.metadata["variant"]) + require.Equal(t, "branch_recreated", warning.metadata["kind"]) + require.Equal(t, "feature/lost", warning.metadata["original_branch"]) + require.Equal(t, "feature/recreated", warning.metadata["new_branch"]) + require.Equal(t, "main", warning.metadata["base_branch"]) + require.Equal(t, "session-branch-recovery", warning.metadata["session_id"]) + require.Equal(t, "repo-branch-recovery", warning.metadata["repository_id"]) +} + +func TestPersistBranchRecoveryWarningsReclaimsStaleClaim(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "task-stale-claim", "session-stale-claim", "step1") + now := time.Now().UTC() + require.NoError(t, repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-stale-claim", WorkspaceID: "ws1", Name: "backend", DefaultBranch: "main", + CreatedAt: now, UpdatedAt: now, + })) + require.NoError(t, repo.CreateTaskRepository(ctx, &models.TaskRepository{ + ID: "task-repo-stale-claim", TaskID: "task-stale-claim", RepositoryID: "repo-stale-claim", + BaseBranch: "main", Position: 0, CreatedAt: now, UpdatedAt: now, + })) + require.NoError(t, repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{ + ID: "env-stale-claim", TaskID: "task-stale-claim", + ExecutorType: string(models.ExecutorTypeWorktree), WorkspacePath: t.TempDir(), Status: models.TaskEnvironmentStatusReady, + Repos: []*models.TaskEnvironmentRepo{{ + ID: "env-repo-stale-claim", RepositoryID: "repo-stale-claim", BranchSlug: "task-stale-claim", + WorktreeID: "worktree-stale-claim", WorktreeBranch: "feature/lost", Position: 0, + CreatedAt: now, UpdatedAt: now, + }}, + })) + session, err := repo.GetTaskSession(ctx, "session-stale-claim") + require.NoError(t, err) + session.TaskEnvironmentID = "env-stale-claim" + require.NoError(t, repo.UpdateTaskSession(ctx, session)) + + decisionID := branchRecoveryDecisionID( + "task-stale-claim", "session-stale-claim", "repo-stale-claim", "feature/lost", "feature/recreated", "main", + ) + claimed, err := repo.SetSessionMetadataKeyIfAbsent( + ctx, + "session-stale-claim", + branchRecoveryWarningKeyPrefix+decisionID, + branchRecoveryWarningClaim{ClaimedAt: time.Now().Add(-(branchRecoveryWarningClaimStaleAfter + time.Minute))}, + ) + require.NoError(t, err) + require.True(t, claimed) + + service := createTestService(repo, newMockStepGetter(), newMockTaskRepo()) + messages := &mockMessageCreator{} + service.messageCreator = messages + before, err := service.captureBranchRecoverySnapshot(ctx, "task-stale-claim", "session-stale-claim") + require.NoError(t, err) + + row, err := repo.ListTaskEnvironmentRepos(ctx, "env-stale-claim") + require.NoError(t, err) + require.Len(t, row, 1) + row[0].WorktreeBranch = "feature/recreated" + require.NoError(t, repo.UpdateTaskEnvironmentRepo(ctx, row[0])) + + service.persistBranchRecoveryWarnings(ctx, "task-stale-claim", "session-stale-claim", before) + + require.Len(t, messages.sessionMessages, 1) + require.Equal(t, "branch_recreated", messages.sessionMessages[0].metadata["kind"]) +} + +func TestPersistBranchRecoveryWarningReleasesClaimWhenMessageWriteFails(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + seedSession(t, repo, "task-warning-write-failure", "session-warning-write-failure", "step1") + writeErr := errors.New("message write failed") + messages := &mockMessageCreator{sessionMessageErr: writeErr} + service := createTestService(repo, newMockStepGetter(), newMockTaskRepo()) + service.messageCreator = messages + previous := branchRecoveryRepoSnapshot{ + RepositoryID: "repo-warning-write-failure", + BranchSlug: "primary", + WorktreeBranch: "feature/lost", + BaseBranch: "main", + } + current := previous + current.WorktreeBranch = "feature/recreated" + + service.persistBranchRecoveryWarning( + ctx, + "task-warning-write-failure", + "session-warning-write-failure", + previous, + current, + ) + if len(messages.sessionMessages) != 0 { + t.Fatalf("failed message write was recorded as a success: %+v", messages.sessionMessages) + } + + messages.sessionMessageErr = nil + service.persistBranchRecoveryWarning( + ctx, + "task-warning-write-failure", + "session-warning-write-failure", + previous, + current, + ) + if len(messages.sessionMessages) != 1 { + t.Fatalf("warning messages = %d, want one retry after the initial write failure", len(messages.sessionMessages)) + } + if messages.sessionMessageAttempts != 2 { + t.Fatalf("message write attempts = %d, want one failed attempt and one retry", messages.sessionMessageAttempts) + } +} + +func TestBranchRecoveryErrorLeavesOrdinaryErrorsUntouched(t *testing.T) { + cause := errors.New("ordinary resume failure") + service := &Service{} + + require.Same(t, cause, service.branchRecoveryError(context.Background(), "task-1", "session-1", cause)) +} diff --git a/apps/backend/internal/orchestrator/session_launch.go b/apps/backend/internal/orchestrator/session_launch.go index 8e1d56a569..592b615f08 100644 --- a/apps/backend/internal/orchestrator/session_launch.go +++ b/apps/backend/internal/orchestrator/session_launch.go @@ -79,6 +79,9 @@ type LaunchSessionRequest struct { // block that survives first-turn canonicalization, so a WS // client must not be able to forge one and fabricate server authority. SpawnOrigin *SpawnOrigin `json:"-"` + // AllowBranchReplacement is set only by RecoverSession for the explicit + // resume_new_branch action. Clients cannot grant this permission directly. + AllowBranchReplacement bool `json:"-"` } // SpawnOrigin describes the agent session that spawned a new sibling session. @@ -317,7 +320,9 @@ func (s *Service) launchStartCreated(ctx context.Context, req *LaunchSessionRequ // launchResume resumes a stopped session. func (s *Service) launchResume(ctx context.Context, req *LaunchSessionRequest) (*LaunchSessionResponse, error) { - execution, err := s.ResumeTaskSession(ctx, req.TaskID, req.SessionID) + execution, err := s.ResumeTaskSessionWithOptions(ctx, req.TaskID, req.SessionID, executor.ResumeOptions{ + AllowBranchReplacement: req.AllowBranchReplacement, + }) if err != nil { return nil, err } @@ -376,7 +381,9 @@ func (s *Service) launchRestoreWorkspace(ctx context.Context, req *LaunchSession } // RecoverSession handles user-initiated recovery after an agent CLI failure. -// action is "resume" (retry with existing ACP session) or "fresh_start" (clear token, start fresh). +// action is "resume" (retry with existing ACP session), "resume_new_branch" +// (retry after replacing a confirmed missing branch), or "fresh_start" (clear +// token, start fresh). func (s *Service) RecoverSession(ctx context.Context, taskID, sessionID, action string) (*LaunchSessionResponse, error) { // Guard before the switch: "fresh_start" clears the resume token, so an // unauthorized call would mutate the session even if the launch failed. @@ -400,14 +407,18 @@ func (s *Service) RecoverSession(ctx context.Context, taskID, sessionID, action } case "resume": // no-op — relaunch with existing resume token + case "resume_new_branch": + // The launch carries the explicit permission. It is not persisted on the + // session and cannot be inferred from a previous failed attempt. default: return nil, fmt.Errorf("invalid recovery action: %s", action) } resp, err := s.LaunchSession(ctx, &LaunchSessionRequest{ - TaskID: taskID, - SessionID: sessionID, - Intent: IntentResume, + TaskID: taskID, + SessionID: sessionID, + Intent: IntentResume, + AllowBranchReplacement: action == "resume_new_branch", }) if err != nil { return nil, normalizeRecoverSessionError(err) diff --git a/apps/backend/internal/orchestrator/task_operations.go b/apps/backend/internal/orchestrator/task_operations.go index 53cff0308d..86f39f8c22 100644 --- a/apps/backend/internal/orchestrator/task_operations.go +++ b/apps/backend/internal/orchestrator/task_operations.go @@ -1915,6 +1915,18 @@ func (s *Service) expandPromptReferencesWithContext( // ResumeTaskSession restarts a specific task session using its stored worktree. func (s *Service) ResumeTaskSession(ctx context.Context, taskID, sessionID string) (*executor.TaskExecution, error) { + return s.ResumeTaskSessionWithOptions(ctx, taskID, sessionID, executor.ResumeOptions{}) +} + +// ResumeTaskSessionWithOptions restarts a task session with an explicit +// recovery permission. The default ResumeTaskSession path remains unchanged. +// +//nolint:cyclop,gocognit,funlen // Resume coordinates state validation, launch recovery, and ready-state persistence. +func (s *Service) ResumeTaskSessionWithOptions( + ctx context.Context, + taskID, sessionID string, + options executor.ResumeOptions, +) (*executor.TaskExecution, error) { s.logger.Debug("resuming task session", zap.String("task_id", taskID), zap.String("session_id", sessionID)) @@ -1937,6 +1949,7 @@ func (s *Service) ResumeTaskSession(ctx context.Context, taskID, sessionID strin return nil, fmt.Errorf("session is not resumable: no executor record") } s.noteMissingWorktreesBeforeResume(sessionID, session) + branchRecoveryBefore := s.captureBranchRecoveryBeforeResume(ctx, taskID, sessionID, options) // Completed sessions cannot be restarted — they require a new session. // Failed and cancelled sessions keep the resume token so the relaunched @@ -1980,13 +1993,19 @@ func (s *Service) ResumeTaskSession(ctx context.Context, taskID, sessionID strin // Use context.WithoutCancel to prevent WebSocket request timeout from canceling the resume. // Session resume can take time and shouldn't be tied to the WS request lifecycle. resumeCtx := context.WithoutCancel(ctx) - execution, err := s.executor.ResumeSession(resumeCtx, session, true) + persistBranchRecovery := func() { + if options.AllowBranchReplacement { + s.persistBranchRecoveryWarnings(resumeCtx, taskID, sessionID, branchRecoveryBefore) + } + } + execution, err := s.executor.ResumeSessionWithOptions(resumeCtx, session, true, options) var readySession *models.TaskSession if err != nil { // If the execution is already running (duplicate resume request), return it as success. if errors.Is(err, executor.ErrExecutionAlreadyRunning) { - execution, readySession, err = s.recoverAlreadyRunningResume(resumeCtx, taskID, sessionID) + execution, readySession, err = s.recoverAlreadyRunningResume(resumeCtx, taskID, sessionID, options) if err != nil && errors.Is(err, ErrAgentNotReadyForPrompt) { + persistBranchRecovery() return nil, err } } @@ -2001,6 +2020,7 @@ func (s *Service) ResumeTaskSession(ctx context.Context, taskID, sessionID strin if task, taskErr := s.repo.GetTask(resumeCtx, taskID); taskErr == nil && task != nil && task.ArchivedAt != nil { return nil, executor.ErrTaskArchived } + persistBranchRecovery() // Use resumeCtx (WithoutCancel) for the failure-recording writes too — // if the caller's ctx was already cancelled (e.g. WS client navigated // away), the SessionStateFailed and TaskStateFailed updates would @@ -2009,6 +2029,7 @@ func (s *Service) ResumeTaskSession(ctx context.Context, taskID, sessionID strin // ResumeSession launches the workspace directly rather than through // LaunchPreparedSession. Record this failure with the same state CAS, // persisted recovery claim, and archive-safe task CAS as early launch. + err = s.branchRecoveryError(resumeCtx, taskID, sessionID, err) return nil, s.handleSessionLaunchFailure( resumeCtx, taskID, sessionID, err, session, ) @@ -2017,10 +2038,12 @@ func (s *Service) ResumeTaskSession(ctx context.Context, taskID, sessionID strin if readySession == nil { readySession, err = s.waitForResumedSessionReady(resumeCtx, sessionID) if err != nil { + persistBranchRecovery() return nil, err } } execution.SessionState = v1.TaskSessionState(readySession.State) + persistBranchRecovery() // Backfill the initial user message when a prior failed launch never got // to recordInitialMessage. Without this, the resume can succeed and the @@ -2051,6 +2074,25 @@ func (s *Service) ResumeTaskSession(ctx context.Context, taskID, sessionID strin return execution, nil } +func (s *Service) captureBranchRecoveryBeforeResume( + ctx context.Context, + taskID, sessionID string, + options executor.ResumeOptions, +) *branchRecoverySnapshot { + if !options.AllowBranchReplacement { + return nil + } + snapshot, err := s.captureBranchRecoverySnapshot(ctx, taskID, sessionID) + if err != nil { + s.logger.Warn("failed to capture branch recovery state before resume", + zap.String("task_id", taskID), + zap.String("session_id", sessionID), + zap.Error(err)) + return nil + } + return snapshot +} + func (s *Service) waitForResumedSessionReady(ctx context.Context, sessionID string) (*models.TaskSession, error) { return s.waitForSessionAndAgentReady(ctx, sessionID, "after resume") } @@ -2059,6 +2101,7 @@ func (s *Service) recoverAlreadyRunningResume( resumeCtx context.Context, taskID string, sessionID string, + options executor.ResumeOptions, ) (*executor.TaskExecution, *models.TaskSession, error) { existing, ok := s.executor.GetExecutionBySession(sessionID) if !ok || existing == nil { @@ -2086,7 +2129,7 @@ func (s *Service) recoverAlreadyRunningResume( return nil, nil, fmt.Errorf("task session does not belong to task") } - execution, err := s.executor.ResumeSession(resumeCtx, session, true) + execution, err := s.executor.ResumeSessionWithOptions(resumeCtx, session, true, options) if err != nil { return nil, nil, err } diff --git a/apps/backend/internal/orchestrator/task_operations_test.go b/apps/backend/internal/orchestrator/task_operations_test.go index 2c3dbd108b..ae4ba623c1 100644 --- a/apps/backend/internal/orchestrator/task_operations_test.go +++ b/apps/backend/internal/orchestrator/task_operations_test.go @@ -4390,23 +4390,24 @@ func TestIssue1884_StepProfileSignalGateStaysInTaskMode(t *testing.T) { // mockMessageCreator implements MessageCreator for testing. // Only CreateUserMessage is tracked; all other methods are no-op stubs. type mockMessageCreator struct { - mu sync.Mutex - userMessages []mockUserMessage - sessionMessages []mockSessionMessage - sessionMessageDone chan struct{} - sessionMessageOnce sync.Once - sessionMessageErr error - agentMessages []mockAgentMessage - agentMessageWrites int - agentStreamWrites int - thinkingWrites int - toolCallWrites int - toolUpdateWrites int - userMessageErr error - permissionClaimFn func(context.Context, models.PermissionResolutionClaimRequest) (*models.PermissionResolutionClaimResult, error) - permissionFinishFn func(context.Context, models.PermissionResolutionFinalizeRequest) (*models.PermissionResolutionFinalizeResult, error) - permissionAuditFn func(context.Context, string, string, string, string) (*models.PermissionResolutionAudit, error) - permissionUpdateFn func(context.Context, string, string, string, string, models.PermissionStatus) error + mu sync.Mutex + userMessages []mockUserMessage + sessionMessages []mockSessionMessage + sessionMessageAttempts int + sessionMessageDone chan struct{} + sessionMessageOnce sync.Once + sessionMessageErr error + agentMessages []mockAgentMessage + agentMessageWrites int + agentStreamWrites int + thinkingWrites int + toolCallWrites int + toolUpdateWrites int + userMessageErr error + permissionClaimFn func(context.Context, models.PermissionResolutionClaimRequest) (*models.PermissionResolutionClaimResult, error) + permissionFinishFn func(context.Context, models.PermissionResolutionFinalizeRequest) (*models.PermissionResolutionFinalizeResult, error) + permissionAuditFn func(context.Context, string, string, string, string) (*models.PermissionResolutionAudit, error) + permissionUpdateFn func(context.Context, string, string, string, string, models.PermissionStatus) error } type mockUserMessage struct { @@ -4451,6 +4452,7 @@ func (m *mockMessageCreator) UpdateToolCallMessage(context.Context, string, stri func (m *mockMessageCreator) CreateSessionMessage(_ context.Context, taskID, content, sessionID, messageType, turnID string, metadata map[string]interface{}, requestsInput bool) error { m.mu.Lock() defer m.mu.Unlock() + m.sessionMessageAttempts++ if m.sessionMessageErr != nil { return m.sessionMessageErr } @@ -5438,6 +5440,109 @@ func TestResumeTaskSession_FailedKeepsResumeToken(t *testing.T) { } } +// TestRecoverSession_ResumeNewBranchPreservesSessionAndProviderIdentity proves +// the service-level recovery action carries the explicit replacement permission +// without turning it into a fresh session or provider conversation. Worktree +// materialization itself is covered by the lifecycle tests; this boundary test +// verifies the request that reaches that materializer. +func TestRecoverSession_ResumeNewBranchPreservesSessionAndProviderIdentity(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + taskRepo := newMockTaskRepo() + + var captured *executor.LaunchAgentRequest + startAgentProcessCalled := false + agentMgr := &sessionUpdatingAgentManager{ + mockAgentManager: &mockAgentManager{ + launchAgentFunc: func(_ context.Context, req *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) { + captured = req + return &executor.LaunchAgentResponse{ + AgentExecutionID: "exec-recovered", + Status: v1.AgentStatusStarting, + }, nil + }, + }, + repo: repo, + sessionID: "session-recover-new-branch", + taskID: "task-recover-new-branch", + onStartCalled: &startAgentProcessCalled, + } + svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr) + svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{}) + + seedTaskAndSession(t, repo, "task-recover-new-branch", "session-recover-new-branch", models.TaskSessionStateFailed) + now := time.Now().UTC() + require.NoError(t, repo.CreateExecutor(ctx, &models.Executor{ + ID: "executor-recover-new-branch", + Name: "Worktree", + Type: models.ExecutorTypeWorktree, + Status: models.ExecutorStatusActive, + Resumable: true, + })) + require.NoError(t, repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-recover-new-branch", + WorkspaceID: "ws1", + Name: "backend", + SourceType: "local", + LocalPath: t.TempDir(), + DefaultBranch: "main", + CreatedAt: now, + UpdatedAt: now, + })) + require.NoError(t, repo.CreateTaskRepository(ctx, &models.TaskRepository{ + ID: "task-repo-recover-new-branch", + TaskID: "task-recover-new-branch", + RepositoryID: "repo-recover-new-branch", + BaseBranch: "main", + Position: 0, + CreatedAt: now, + UpdatedAt: now, + })) + + session, err := repo.GetTaskSession(ctx, "session-recover-new-branch") + require.NoError(t, err) + session.AgentProfileID = "profile-recover-new-branch" + session.ExecutorID = "executor-recover-new-branch" + session.RepositoryID = "repo-recover-new-branch" + session.BaseBranch = "main" + require.NoError(t, repo.UpdateTaskSession(ctx, session)) + require.NoError(t, repo.UpsertExecutorRunning(ctx, &models.ExecutorRunning{ + ID: "running-recover-new-branch", + SessionID: "session-recover-new-branch", + TaskID: "task-recover-new-branch", + AgentExecutionID: "exec-before-recovery", + ResumeToken: "acp-session-recover-new-branch", + Resumable: true, + CreatedAt: now, + UpdatedAt: now, + })) + + response, err := svc.RecoverSession(ctx, "task-recover-new-branch", "session-recover-new-branch", "resume_new_branch") + require.NoError(t, err) + require.NotNil(t, response) + require.Equal(t, "task-recover-new-branch", response.TaskID) + require.Equal(t, "session-recover-new-branch", response.SessionID) + require.NotNil(t, captured) + require.Equal(t, "session-recover-new-branch", captured.SessionID) + require.Equal(t, "acp-session-recover-new-branch", captured.ACPSessionID) + require.True(t, captured.AllowBranchReplacement) + require.True(t, captured.UseWorktree) + require.Equal(t, "main", captured.Branch) + require.Equal(t, "main", captured.BaseBranch) + require.True(t, startAgentProcessCalled) + + reloadedSession, err := repo.GetTaskSession(ctx, "session-recover-new-branch") + require.NoError(t, err) + require.Equal(t, session.ID, reloadedSession.ID) + require.Equal(t, models.TaskSessionStateWaitingForInput, reloadedSession.State) + reloadedRunning, err := repo.GetExecutorRunningBySessionID(ctx, "session-recover-new-branch") + require.NoError(t, err) + require.Equal(t, "acp-session-recover-new-branch", reloadedRunning.ResumeToken) + sessions, err := repo.ListTaskSessions(ctx, "task-recover-new-branch") + require.NoError(t, err) + require.Len(t, sessions, 1) +} + // TestResumeTaskSession_ArchiveCancelledSessionResumesSuccessfully is the // end-to-end regression for "Can't resume this un-archived task": a session // cancelled by an archive (Service.ArchiveTask / cascade archive) must resume @@ -5662,6 +5767,78 @@ func TestResumeTaskSession_AlreadyFailedMissingRemoteRefCreatesNeutralRecoveryMe } } +func TestResumeTaskSession_PersistsBranchRecoveryWarningWhenPreparationFailsLater(t *testing.T) { + ctx := context.Background() + repo := setupTestRepo(t) + taskRepo := newMockTaskRepo() + seedTaskAndSession(t, repo, "task-partial-recovery", "session-partial-recovery", models.TaskSessionStateFailed) + now := time.Now().UTC() + if err := repo.CreateRepository(ctx, &models.Repository{ + ID: "repo-partial-recovery", WorkspaceID: "ws1", Name: "backend", DefaultBranch: "main", + CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("CreateRepository: %v", err) + } + if err := repo.CreateTaskRepository(ctx, &models.TaskRepository{ + ID: "task-repo-partial-recovery", TaskID: "task-partial-recovery", RepositoryID: "repo-partial-recovery", + BaseBranch: "main", Position: 0, CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatalf("CreateTaskRepository: %v", err) + } + if err := repo.CreateTaskEnvironment(ctx, &models.TaskEnvironment{ + ID: "env-partial-recovery", TaskID: "task-partial-recovery", + ExecutorType: string(models.ExecutorTypeWorktree), WorkspacePath: t.TempDir(), Status: models.TaskEnvironmentStatusReady, + Repos: []*models.TaskEnvironmentRepo{{ + ID: "env-repo-partial-recovery", RepositoryID: "repo-partial-recovery", BranchSlug: "backend", + WorktreeID: "worktree-partial-recovery", WorktreeBranch: "feature/lost", Position: 0, + CreatedAt: now, UpdatedAt: now, + }}, + }); err != nil { + t.Fatalf("CreateTaskEnvironment: %v", err) + } + session, err := repo.GetTaskSession(ctx, "session-partial-recovery") + if err != nil { + t.Fatalf("GetTaskSession: %v", err) + } + session.AgentProfileID = "profile-partial-recovery" + session.TaskEnvironmentID = "env-partial-recovery" + if err := repo.UpdateTaskSession(ctx, session); err != nil { + t.Fatalf("UpdateTaskSession: %v", err) + } + + launchErr := errors.New("second repository preparation failed") + agentMgr := &mockAgentManager{ + launchAgentFunc: func(context.Context, *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) { + rows, err := repo.ListTaskEnvironmentRepos(ctx, "env-partial-recovery") + if err != nil { + return nil, err + } + rows[0].WorktreeBranch = "feature/replaced" + if err := repo.UpdateTaskEnvironmentRepo(ctx, rows[0]); err != nil { + return nil, err + } + return nil, launchErr + }, + } + svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr) + svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{}) + messages := &mockMessageCreator{} + svc.messageCreator = messages + + _, err = svc.ResumeTaskSessionWithOptions(ctx, "task-partial-recovery", "session-partial-recovery", executor.ResumeOptions{ + AllowBranchReplacement: true, + }) + if !errors.Is(err, launchErr) { + t.Fatalf("ResumeTaskSessionWithOptions error = %v, want %v", err, launchErr) + } + if len(messages.sessionMessages) != 1 { + t.Fatalf("warning messages = %d, want one warning after partial preparation", len(messages.sessionMessages)) + } + if got := messages.sessionMessages[0].metadata["kind"]; got != "branch_recreated" { + t.Fatalf("warning kind = %v, want branch_recreated", got) + } +} + func TestResumeTaskSession_UnrelatedLaunchFailureDoesNotCreateRecoveryMessage(t *testing.T) { ctx := context.Background() repo := setupTestRepo(t) diff --git a/apps/backend/internal/worktree/errors.go b/apps/backend/internal/worktree/errors.go index b47319ab8a..1a4b2235c2 100644 --- a/apps/backend/internal/worktree/errors.go +++ b/apps/backend/internal/worktree/errors.go @@ -26,6 +26,11 @@ var ( // ErrInvalidBaseBranch is returned when the base branch does not exist. ErrInvalidBaseBranch = errors.New("base branch does not exist") + // ErrRemoteRefMissing is returned when Git confirms that the requested + // remote ref is unavailable. It is narrower than ErrInvalidBaseBranch so + // callers can distinguish a deleted branch from a failed fetch. + ErrRemoteRefMissing = errors.New("remote ref does not exist") + // ErrWorktreeCorrupted is returned when the worktree directory is corrupted or invalid. ErrWorktreeCorrupted = errors.New("worktree directory is corrupted") @@ -101,6 +106,51 @@ var ( ErrWorktreePathOwnedByAnotherTask = errors.New("worktree path is owned by another task") ) +// BranchUnrecoverableError identifies the branch that could not be restored. +// It wraps ErrBranchUnrecoverable so callers can preserve the existing +// sentinel check while presenting a branch-specific recovery action. +type BranchUnrecoverableError struct { + Branch string +} + +type confirmedRemoteRefMissingError struct { + detail string +} + +func (e *confirmedRemoteRefMissingError) Error() string { + if e == nil || e.detail == "" { + return ErrRemoteRefMissing.Error() + } + return fmt.Sprintf("%s: %s", ErrRemoteRefMissing, e.detail) +} + +func (e *confirmedRemoteRefMissingError) Unwrap() []error { + return []error{ErrRemoteRefMissing, ErrInvalidBaseBranch} +} + +func newConfirmedRemoteRefMissingError(detail string) error { + return &confirmedRemoteRefMissingError{detail: strings.TrimSpace(detail)} +} + +func (e *BranchUnrecoverableError) Error() string { + if e == nil || e.Branch == "" { + return ErrBranchUnrecoverable.Error() + } + return fmt.Sprintf("%s: %q", ErrBranchUnrecoverable, e.Branch) +} + +func (e *BranchUnrecoverableError) Unwrap() error { + return ErrBranchUnrecoverable +} + +// BranchName returns the branch that was requested by the persisted worktree. +func (e *BranchUnrecoverableError) BranchName() string { + if e == nil { + return "" + } + return e.Branch +} + // containsAuthFailure checks if git output indicates an authentication failure. func containsAuthFailure(lowerOutput string) bool { return strings.Contains(lowerOutput, "authentication failed") || @@ -138,6 +188,7 @@ func isRemoteRefMissingError(err error) bool { return strings.Contains(msg, "couldn't find remote ref") || strings.Contains(msg, "does not appear to be a git repository") || strings.Contains(msg, "no such remote") || + strings.Contains(msg, "no remote configured") || strings.Contains(msg, "cannot determine remote head") || strings.Contains(msg, "remote head refers to nonexistent ref") } diff --git a/apps/backend/internal/worktree/errors_test.go b/apps/backend/internal/worktree/errors_test.go index 21f17406e9..3398a1d459 100644 --- a/apps/backend/internal/worktree/errors_test.go +++ b/apps/backend/internal/worktree/errors_test.go @@ -14,6 +14,27 @@ func TestInvalidBaseBranchRemainsSentinelDetectable(t *testing.T) { } } +func TestIsRemoteRefMissingErrorRequiresConfirmedRemoteEvidence(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + {name: "missing ref", err: errors.New("fatal: couldn't find remote ref feature/lost"), want: true}, + {name: "no remote", err: errors.New("fatal: no remote configured"), want: true}, + {name: "authentication failure", err: errors.New("fatal: Authentication failed"), want: false}, + {name: "network failure", err: errors.New("fatal: unable to access remote: connection timed out"), want: false}, + {name: "generic invalid branch", err: fmt.Errorf("%w: feature/lost", ErrInvalidBaseBranch), want: false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := isRemoteRefMissingError(tt.err); got != tt.want { + t.Fatalf("isRemoteRefMissingError(%v) = %v, want %v", tt.err, got, tt.want) + } + }) + } +} + func TestClassifyGitError_BranchCheckedOut(t *testing.T) { output := "fatal: 'feature/pr-branch' is already checked out at '/tmp/worktree-123'" err := ClassifyGitError(output, fmt.Errorf("exit status 128")) diff --git a/apps/backend/internal/worktree/manager_git.go b/apps/backend/internal/worktree/manager_git.go index 4b938bed3f..5238293e9b 100644 --- a/apps/backend/internal/worktree/manager_git.go +++ b/apps/backend/internal/worktree/manager_git.go @@ -70,6 +70,49 @@ func (m *Manager) branchExists(ctx context.Context, repoPath, branch string) (bo return true, nil } +// remoteBranchExists performs an authoritative, bounded probe for a branch on +// origin. A successful `ls-remote --exit-code` with exit status 2 means the +// remote answered and did not advertise the requested ref. Any other failure +// remains an error because transport, authentication, and timeout failures do +// not prove that the branch was deleted. +func (m *Manager) remoteBranchExists(ctx context.Context, repoPath, branch string) (bool, error) { + branch = normalizeOriginBranchName(branch) + if branch == "" { + return false, fmt.Errorf("remote branch name is empty: %w", ErrGitCommandFailed) + } + output, err := m.runBoundedGitInspect( + ctx, + repoPath, + "ls-remote", + "--exit-code", + "--heads", + "origin", + "refs/heads/"+branch, + ) + if err == nil { + return true, nil + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 2 { + return false, nil + } + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return false, err + } + if containsAuthFailure(strings.ToLower(output)) { + return false, ErrAuthFailed + } + return false, ErrGitCommandFailed +} + +func normalizeOriginBranchName(branch string) string { + branch = strings.TrimSpace(branch) + for _, prefix := range []string{"refs/remotes/origin/", "refs/heads/", "origin/"} { + branch = strings.TrimPrefix(branch, prefix) + } + return branch +} + // runBoundedGitInspect runs a non-interactive local git inspection after // acquiring the lifecycle throttle. The timeout starts after admission so // queue wait does not consume the command's inspection budget. diff --git a/apps/backend/internal/worktree/manager_lifecycle.go b/apps/backend/internal/worktree/manager_lifecycle.go index 5b1945b483..22d31bd3e7 100644 --- a/apps/backend/internal/worktree/manager_lifecycle.go +++ b/apps/backend/internal/worktree/manager_lifecycle.go @@ -1,3 +1,5 @@ +//revive:disable:file-length-limit // Legacy lifecycle coordination remains in one file; replacement recovery lives in manager_replacement.go. + package worktree import ( @@ -293,11 +295,52 @@ func (m *Manager) reuseRequiredWorktree(ctx context.Context, req CreateRequest) defer func() { _ = handle.Close() }() } if !valid { + if err := m.classifyUnavailableReusableWorktree(ctx, req, wt); err != nil { + return nil, err + } return nil, ErrReuseWorktreeUnavailable } return wt, nil } +// classifyUnavailableReusableWorktree distinguishes a missing checkout from a +// branch that no longer exists in either the local repository or its +// remote-tracking refs. Attach-only reuse must not recreate a checkout, but a +// confirmed missing branch still needs the typed recovery signal used by the +// explicit new-branch action. +func (m *Manager) classifyUnavailableReusableWorktree( + ctx context.Context, + req CreateRequest, + wt *Worktree, +) error { + if wt == nil || wt.Branch == "" || !m.isGitRepo(req.RepositoryPath) { + return nil + } + localExists, err := m.branchExists(ctx, req.RepositoryPath, wt.Branch) + if err != nil { + return fmt.Errorf("%w: verify saved branch %q: %w", ErrReuseWorktreeUnavailable, wt.Branch, err) + } + if localExists { + return nil + } + branch := normalizeOriginBranchName(wt.Branch) + remoteExists, err := m.branchExists(ctx, req.RepositoryPath, "refs/remotes/origin/"+branch) + if err != nil { + return fmt.Errorf("%w: verify saved remote branch %q: %w", ErrReuseWorktreeUnavailable, wt.Branch, err) + } + if remoteExists { + return nil + } + remoteExists, err = m.remoteBranchExists(ctx, req.RepositoryPath, branch) + if err != nil { + return fmt.Errorf("%w: verify authoritative remote branch %q: %w", ErrReuseWorktreeUnavailable, wt.Branch, err) + } + if remoteExists { + return nil + } + return fmt.Errorf("%w: %w", ErrReuseWorktreeUnavailable, &BranchUnrecoverableError{Branch: wt.Branch}) +} + // tryReuseExisting looks for an existing worktree to reuse, recreating it if // the on-disk directory is missing. Returns (wt, true, err) when fully handled // (caller should return immediately). Returns (nil, false, nil) when no reuse @@ -729,7 +772,8 @@ func (m *Manager) createInTaskDir(ctx context.Context, req CreateRequest, baseRe } if selectedRef == "" { if req.PRNumber > 0 { - return nil, fmt.Errorf("%w: refreshed pull request head %d was not materialized", ErrBranchUnrecoverable, req.PRNumber) + err := &BranchUnrecoverableError{Branch: req.CheckoutBranch} + return nil, fmt.Errorf("%w: refreshed pull request head %d was not materialized", err, req.PRNumber) } branchName = req.CheckoutBranch checkoutMode.CheckoutBranch = "" @@ -1032,10 +1076,10 @@ func (m *Manager) fetchBranchToLocalWithPolicy( } if required { - if isRemoteBranchMissingError(outputStr) { + if isRemoteBranchMissingError(outputStr) || isRemoteRefMissingError(errors.New(outputStr)) { return nil, fmt.Errorf( "required refresh of checkout branch %q found no remote ref: %w", - branch, ErrInvalidBaseBranch, + branch, newConfirmedRemoteRefMissingError(outputStr), ) } reason := classifyGitFallbackReason(err, outputStr, fetchCtxErr) @@ -1056,7 +1100,17 @@ func (m *Manager) fetchBranchToLocalWithPolicy( return nil, fmt.Errorf("could not verify local branch %q after fetch failure (%s): %w", branch, strings.TrimSpace(outputStr), existsErr) } if !exists { - return nil, fmt.Errorf("%w: branch %q not found locally or on remote: %s", ErrInvalidBaseBranch, branch, outputStr) + if isRemoteRefMissingError(errors.New(outputStr)) { + return nil, fmt.Errorf( + "branch %q not found locally or on remote: %w", + branch, newConfirmedRemoteRefMissingError(outputStr), + ) + } + reason := classifyGitFallbackReason(err, outputStr, fetchCtxErr) + return nil, fmt.Errorf( + "could not fetch branch %q and no local branch is available (%s): %w", + branch, reason, syncFailureCause(reason, err, fetchCtxErr), + ) } reason := classifyGitFallbackReason(err, outputStr, fetchCtxErr) @@ -1887,7 +1941,11 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe return nil, prepareErr } if selectedRef == "" { - return nil, fmt.Errorf("%w: refreshed branch %q was not materialized", ErrBranchUnrecoverable, sourceBranch) + err := &BranchUnrecoverableError{Branch: sourceBranch} + if req.AllowBranchReplacement { + return m.replaceUnrecoverableWorktree(ctx, existing, req, err) + } + return nil, fmt.Errorf("%w: refreshed branch %q was not materialized", err, sourceBranch) } if selectedRef != existing.Branch { refreshedStartPoint = selectedRef @@ -1917,8 +1975,12 @@ func (m *Manager) recreate(ctx context.Context, existing *Worktree, req CreateRe // Only a confirmed-missing remote ref means the work is gone; // transient fetch failures (network, auth) keep their own error // so callers don't treat a reachable branch as unrecoverable. - if isRemoteRefMissingError(fetchErr) { - return nil, fmt.Errorf("%w: %q", ErrBranchUnrecoverable, existing.Branch) + if errors.Is(fetchErr, ErrRemoteRefMissing) || isRemoteRefMissingError(fetchErr) { + err := &BranchUnrecoverableError{Branch: existing.Branch} + if req.AllowBranchReplacement { + return m.replaceUnrecoverableWorktree(ctx, existing, req, err) + } + return nil, err } return nil, fetchErr } diff --git a/apps/backend/internal/worktree/manager_recreate_recovery_test.go b/apps/backend/internal/worktree/manager_recreate_recovery_test.go index d3d1125bac..a7d7892032 100644 --- a/apps/backend/internal/worktree/manager_recreate_recovery_test.go +++ b/apps/backend/internal/worktree/manager_recreate_recovery_test.go @@ -187,6 +187,210 @@ func TestRecreate_BranchGoneEverywhereReturnsUnrecoverable(t *testing.T) { if !errors.Is(err, ErrBranchUnrecoverable) { t.Fatalf("recreate() err = %v, want ErrBranchUnrecoverable", err) } + var branchErr interface{ BranchName() string } + if !errors.As(err, &branchErr) { + t.Fatalf("recreate() err = %v, want a typed branch recovery error", err) + } + if got := branchErr.BranchName(); got != existing.Branch { + t.Fatalf("branch recovery error names %q, want %q", got, existing.Branch) + } +} + +func TestRecreate_DoesNotReplaceAfterTransientFetchFailure(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + runGit(t, repoPath, "remote", "set-url", "origin", "https://127.0.0.1:1/unavailable.git") + archiveDeletesLocalBranch(t, repoPath, "feature/pr-branch") + + cfg := newTestConfig(t) + store := newMockStore() + mgr, err := NewManager(cfg, store, newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + existing := &Worktree{ + ID: "wt-transient-fetch", + SessionID: "session-transient-fetch", + TaskID: "task-transient-fetch", + TaskDirName: "task-transient-fetch", + TaskEnvironmentID: "env-transient-fetch", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + Path: filepath.Join(cfg.TasksBasePath, "task-transient-fetch", "repo-1"), + Branch: "feature/pr-branch", + Status: StatusDeleted, + } + store.worktrees[existing.ID] = existing + + _, err = mgr.recreate(context.Background(), existing, CreateRequest{ + SessionID: existing.SessionID, + TaskID: existing.TaskID, + TaskEnvironmentID: existing.TaskEnvironmentID, + RepositoryID: existing.RepositoryID, + RepositoryPath: repoPath, + TaskDirName: existing.TaskDirName, + RepoName: "repo-1", + BaseBranch: "main", + AllowBranchReplacement: true, + }) + if err == nil { + t.Fatal("recreate() succeeded after a transient fetch failure") + } + if errors.Is(err, ErrBranchUnrecoverable) { + t.Fatalf("recreate() misclassified transient fetch failure as branch loss: %v", err) + } + if errors.Is(err, ErrInvalidBaseBranch) { + t.Fatalf("recreate() misclassified transient fetch failure as invalid branch: %v", err) + } + if errors.Is(err, ErrRemoteRefMissing) { + t.Fatalf("recreate() reported confirmed remote ref loss for transient fetch failure: %v", err) + } +} + +func TestRecreate_AllowsExplicitReplacementWithFreshBranchAndPath(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + runGit(t, repoPath, "push", "origin", "--delete", "feature/pr-branch") + archiveDeletesLocalBranch(t, repoPath, "feature/pr-branch") + runGit(t, repoPath, "fetch", "--prune", "origin") + + cfg := newTestConfig(t) + store := newMockStore() + mgr, err := NewManager(cfg, store, newTestLogger()) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + existing := &Worktree{ + ID: "wt-replace", + SessionID: "session-replace", + TaskID: "task-replace", + TaskDirName: "task-replace", + TaskEnvironmentID: "env-replace", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + Path: filepath.Join(cfg.TasksBasePath, "task-replace", "repo-1-primary"), + Branch: "feature/pr-branch", + BranchSlug: "primary", + BaseBranch: "main", + Status: StatusDeleted, + } + store.worktrees[existing.ID] = existing + + wt, err := mgr.recreate(context.Background(), existing, CreateRequest{ + SessionID: existing.SessionID, + TaskID: existing.TaskID, + TaskEnvironmentID: existing.TaskEnvironmentID, + RepositoryID: existing.RepositoryID, + RepositoryPath: repoPath, + BaseBranch: "main", + TaskDirName: existing.TaskDirName, + RepoName: "repo-1", + BranchIdentitySlug: existing.BranchSlug, + PullBeforeWorktree: true, + AllowBranchReplacement: true, + }) + if err != nil { + t.Fatalf("recreate() explicit replacement: %v", err) + } + if wt.ID != existing.ID { + t.Fatalf("replacement worktree ID = %q, want stable ID %q", wt.ID, existing.ID) + } + if wt.Branch == existing.Branch { + t.Fatalf("replacement branch = %q, want a fresh branch", wt.Branch) + } + if wt.Path == existing.Path { + t.Fatalf("replacement path = %q, want a fresh path", wt.Path) + } + if wt.BranchSlug != existing.BranchSlug { + t.Fatalf("replacement branch slug = %q, want stable identity %q", wt.BranchSlug, existing.BranchSlug) + } + if wt.Status != StatusActive || wt.DeletedAt != nil { + t.Fatalf("replacement status = %q, deleted_at = %v, want active and nil", wt.Status, wt.DeletedAt) + } + if got := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "--abbrev-ref", "HEAD")); got != wt.Branch { + t.Fatalf("replacement HEAD branch = %q, want %q", got, wt.Branch) + } + mainSHA := strings.TrimSpace(runGit(t, repoPath, "rev-parse", "main")) + if got := strings.TrimSpace(runGit(t, wt.Path, "rev-parse", "HEAD")); got != mainSHA { + t.Fatalf("replacement HEAD = %q, want main SHA %q", got, mainSHA) + } + stored := store.worktrees[existing.ID] + if stored == nil || stored.Branch != wt.Branch || stored.Path != wt.Path { + t.Fatalf("stored replacement = %#v, want branch %q and path %q", stored, wt.Branch, wt.Path) + } +} + +type replacementUpdateErrorStore struct { + *mockStore + err error + attempted *Worktree +} + +func (s *replacementUpdateErrorStore) UpdateWorktree(_ context.Context, wt *Worktree) error { + copy := *wt + s.attempted = © + return s.err +} + +func TestRecreate_CleansReplacementWhenPersistenceFails(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + runGit(t, repoPath, "push", "origin", "--delete", "feature/pr-branch") + archiveDeletesLocalBranch(t, repoPath, "feature/pr-branch") + runGit(t, repoPath, "fetch", "--prune", "origin") + + cfg := newTestConfig(t) + storeErr := errors.New("replacement persistence failed") + store := &replacementUpdateErrorStore{mockStore: newMockStore(), err: storeErr} + existing := &Worktree{ + ID: "wt-replace-persist-failure", + SessionID: "session-replace-persist-failure", + TaskID: "task-replace-persist-failure", + TaskDirName: "task-replace-persist-failure", + TaskEnvironmentID: "env-replace-persist-failure", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + Path: filepath.Join(cfg.TasksBasePath, "task-replace-persist-failure", "repo-1-old"), + Branch: "feature/pr-branch", + BaseBranch: "main", + Status: StatusDeleted, + } + store.worktrees[existing.ID] = existing + mgr, err := NewManager(cfg, store, newTestLogger()) + if err != nil { + t.Fatalf("NewManager() error = %v", err) + } + + _, err = mgr.recreate(context.Background(), existing, CreateRequest{ + SessionID: existing.SessionID, + TaskID: existing.TaskID, + TaskEnvironmentID: existing.TaskEnvironmentID, + RepositoryID: existing.RepositoryID, + RepositoryPath: repoPath, + BaseBranch: existing.BaseBranch, + TaskDirName: existing.TaskDirName, + RepoName: "repo-1", + BranchIdentitySlug: "primary", + PullBeforeWorktree: true, + AllowBranchReplacement: true, + }) + if !errors.Is(err, storeErr) { + t.Fatalf("recreate() error = %v, want %v", err, storeErr) + } + if store.attempted == nil { + t.Fatal("replacement persistence was not attempted") + } + if _, statErr := os.Stat(store.attempted.Path); !errors.Is(statErr, os.ErrNotExist) { + t.Fatalf("replacement worktree path still exists after persistence failure: %q (%v)", store.attempted.Path, statErr) + } + branchExists, branchErr := mgr.branchExists(context.Background(), repoPath, "refs/heads/"+store.attempted.Branch) + if branchErr != nil { + t.Fatalf("verify replacement branch cleanup: %v", branchErr) + } + if branchExists { + t.Fatalf("replacement branch %q still exists after persistence failure", store.attempted.Branch) + } + stored := store.worktrees[existing.ID] + if stored == nil || stored.Path != existing.Path || stored.Branch != existing.Branch || stored.Status != StatusDeleted { + t.Fatalf("stored worktree = %#v, want the original deleted record %#v", stored, existing) + } } // TestBranchRecoveryStatus covers the three probe outcomes used by the diff --git a/apps/backend/internal/worktree/manager_replacement.go b/apps/backend/internal/worktree/manager_replacement.go new file mode 100644 index 0000000000..e5b35cb9fd --- /dev/null +++ b/apps/backend/internal/worktree/manager_replacement.go @@ -0,0 +1,215 @@ +package worktree + +import ( + "context" + "errors" + "fmt" + "os" + "strings" + "time" + + "go.uber.org/zap" +) + +const replacementBranchAttempts = 20 + +// replaceUnrecoverableWorktree creates a fresh branch and worktree while +// retaining the existing task-environment repository record. The caller holds +// the repository lock from recreate, so this helper must not acquire it again. +func (m *Manager) replaceUnrecoverableWorktree( + ctx context.Context, + existing *Worktree, + req CreateRequest, + branchErr *BranchUnrecoverableError, +) (*Worktree, error) { + if existing == nil { + return nil, branchErr + } + replacementReq, err := replacementRequest(existing, req) + if err != nil { + return nil, err + } + baseRef, fallbackWarning, fallbackDetail, err := m.resolveBaseRefWithFallback(ctx, &replacementReq) + if err != nil { + return nil, fmt.Errorf("resolve replacement base branch: %w", err) + } + branchName, worktreePath, err := m.replacementTarget(ctx, replacementReq) + if err != nil { + return nil, err + } + if _, err := m.gitAddWorktree(ctx, replacementReq.RepositoryPath, branchName, worktreePath, baseRef); err != nil { + return nil, fmt.Errorf("create replacement worktree for %q: %w", branchErr.BranchName(), err) + } + + replacement := replacementRecord(existing, replacementReq, branchName, worktreePath, fallbackWarning, fallbackDetail) + if err := m.persistReplacementWorktree(ctx, &replacement); err != nil { + if cleanupErr := m.cleanupFailedReplacement(ctx, &replacement); cleanupErr != nil { + return nil, errors.Join(err, fmt.Errorf("cleanup replacement after persistence failure: %w", cleanupErr)) + } + return nil, err + } + m.copyConfiguredFiles(ctx, replacementReq, &replacement) + m.runWorktreeSetupScript(ctx, &replacement, replacementReq.ScriptEnv) + m.logger.Info("replaced unrecoverable worktree branch", + zap.String("session_id", replacement.SessionID), + zap.String("task_id", replacement.TaskID), + zap.String("original_branch", branchErr.BranchName()), + zap.String("new_branch", replacement.Branch), + zap.String("path", replacement.Path)) + return &replacement, nil +} + +// cleanupFailedReplacement removes the physical checkout and the branch that +// gitAddWorktree created before the replacement record could be persisted. The +// original record remains authoritative when this compensation runs. +func (m *Manager) cleanupFailedReplacement(ctx context.Context, replacement *Worktree) error { + if replacement == nil { + return nil + } + cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), m.inspectTimeout) + defer cancel() + + var cleanupErrs []error + if err := m.removeWorktreeDir(cleanupCtx, replacement.Path, replacement.RepositoryPath); err != nil { + cleanupErrs = append(cleanupErrs, fmt.Errorf("remove replacement worktree: %w", err)) + } + if err := m.cleanupFailedReplacementBranch(cleanupCtx, replacement); err != nil { + cleanupErrs = append(cleanupErrs, err) + } + return errors.Join(cleanupErrs...) +} + +func (m *Manager) cleanupFailedReplacementBranch(ctx context.Context, replacement *Worktree) error { + branchRef := "refs/heads/" + replacement.Branch + exists, err := m.branchExists(ctx, replacement.RepositoryPath, branchRef) + if err != nil { + return fmt.Errorf("verify replacement branch: %w", err) + } + if !exists { + return nil + } + output, err := m.runBoundedGitInspect( + ctx, + replacement.RepositoryPath, + "rev-parse", + "--verify", + branchRef+"^{commit}", + ) + if err != nil { + return fmt.Errorf("resolve replacement branch: %w", err) + } + _, err = deleteBranchRefIfOwned(ctx, replacement.RepositoryPath, newBranchAddSnapshot{ + branchRef: branchRef, + branchOID: strings.TrimSpace(output), + }) + if err != nil { + return fmt.Errorf("delete replacement branch: %w", err) + } + return nil +} + +func replacementRequest(existing *Worktree, req CreateRequest) (CreateRequest, error) { + replacementReq := req + if replacementReq.BaseBranch == "" { + replacementReq.BaseBranch = existing.BaseBranch + } + if replacementReq.TaskDirName == "" { + replacementReq.TaskDirName = existing.TaskDirName + } + if replacementReq.TaskEnvironmentID == "" { + replacementReq.TaskEnvironmentID = existing.TaskEnvironmentID + } + if replacementReq.BaseBranch == "" || replacementReq.TaskDirName == "" || replacementReq.RepoName == "" { + return CreateRequest{}, fmt.Errorf("replace unrecoverable worktree: %w", ErrTaskDirRequired) + } + return replacementReq, nil +} + +func replacementRecord( + existing *Worktree, + req CreateRequest, + branchName, worktreePath, fallbackWarning, fallbackDetail string, +) Worktree { + identitySlug := existing.BranchSlug + if identitySlug == "" { + identitySlug = requestBranchIdentitySlug(req) + } + replacement := *existing + replacement.TaskDirName = req.TaskDirName + replacement.TaskEnvironmentID = req.TaskEnvironmentID + replacement.BranchSlug = identitySlug + replacement.RepositoryPath = req.RepositoryPath + replacement.Path = worktreePath + replacement.Branch = branchName + replacement.BaseBranch = req.BaseBranch + replacement.Status = StatusActive + replacement.DeletedAt = nil + replacement.FetchWarning = "" + replacement.FetchWarningDetail = "" + replacement.BaseBranchFallbackWarning = fallbackWarning + replacement.BaseBranchFallbackDetail = fallbackDetail + replacement.UpdatedAt = time.Now() + if replacement.SessionID == "" { + replacement.SessionID = req.SessionID + } + if replacement.TaskID == "" { + replacement.TaskID = req.TaskID + } + return replacement +} + +func (m *Manager) persistReplacementWorktree(ctx context.Context, replacement *Worktree) error { + if m.store != nil { + if err := m.store.UpdateWorktree(ctx, replacement); err != nil { + return fmt.Errorf("persist replacement worktree: %w", err) + } + } + if replacement.SessionID != "" { + m.mu.Lock() + m.worktrees[cacheKey(replacement.SessionID, replacement.RepositoryID, replacement.BranchSlug)] = replacement + m.mu.Unlock() + } + return nil +} + +func (m *Manager) replacementTarget(ctx context.Context, req CreateRequest) (string, string, error) { + _, branchBase := m.buildWorktreeNames(req) + for attempt := 0; attempt < replacementBranchAttempts; attempt++ { + branchName := replacementBranchName(branchBase, attempt) + exists, err := m.branchExists(ctx, req.RepositoryPath, branchName) + if err != nil { + return "", "", fmt.Errorf("check replacement branch %q: %w", branchName, err) + } + if exists { + continue + } + path, err := m.replacementPath(req, branchName) + if err != nil { + return "", "", err + } + if _, err := os.Lstat(path); errors.Is(err, os.ErrNotExist) { + return branchName, path, nil + } else if err != nil { + return "", "", fmt.Errorf("inspect replacement worktree path: %w", err) + } + } + return "", "", fmt.Errorf("unable to find a unique replacement branch after %d attempts: %w", replacementBranchAttempts, ErrBranchExists) +} + +func replacementBranchName(branchBase string, attempt int) string { + if attempt == 0 { + return branchBase + } + return branchBase + "-" + SmallSuffix(3) +} + +func (m *Manager) replacementPath(req CreateRequest, branchName string) (string, error) { + pathReq := req + pathReq.BranchSlug = SanitizeBranchSlug(branchName) + pathReq.BranchIdentitySlug = requestBranchIdentitySlug(req) + path, err := m.prepareTaskWorktreePath(pathReq) + if err != nil { + return "", fmt.Errorf("prepare replacement worktree path: %w", err) + } + return path, nil +} diff --git a/apps/backend/internal/worktree/manager_reuse_required_test.go b/apps/backend/internal/worktree/manager_reuse_required_test.go index 48223d522a..bdc5ca17e3 100644 --- a/apps/backend/internal/worktree/manager_reuse_required_test.go +++ b/apps/backend/internal/worktree/manager_reuse_required_test.go @@ -28,6 +28,140 @@ func TestCreate_ReuseRequiredRejectsMissingCanonicalWorktree(t *testing.T) { } } +func TestCreate_ReuseRequiredReportsUnrecoverableBranch(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + runGit(t, repoPath, "push", "origin", "--delete", "feature/pr-branch") + runGit(t, repoPath, "branch", "-D", "feature/pr-branch") + runGit(t, repoPath, "fetch", "--prune", "origin") + + store := newMockStore() + store.worktrees["canonical-worktree"] = &Worktree{ + ID: "canonical-worktree", + TaskID: "task-1", + TaskEnvironmentID: "environment-1", + RepositoryID: "repository-1", + Path: filepath.Join(t.TempDir(), "missing-worktree"), + Branch: "feature/pr-branch", + Status: StatusActive, + } + mgr, err := NewManager(newTestConfig(t), store, newTestLogger()) + if err != nil { + t.Fatalf("NewManager() error = %v", err) + } + + _, err = mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-1", + TaskEnvironmentID: "environment-1", + RepositoryID: "repository-1", + RepositoryPath: repoPath, + BaseBranch: "main", + WorktreeID: "canonical-worktree", + ReuseRequired: true, + }) + if !errors.Is(err, ErrBranchUnrecoverable) { + t.Fatalf("Create() error = %v, want ErrBranchUnrecoverable", err) + } + if !errors.Is(err, ErrReuseWorktreeUnavailable) { + t.Fatalf("Create() error = %v, want ErrReuseWorktreeUnavailable context", err) + } + var branchErr *BranchUnrecoverableError + if !errors.As(err, &branchErr) || branchErr.BranchName() != "feature/pr-branch" { + t.Fatalf("Create() error = %v, want branch-specific unrecoverable error", err) + } +} + +func TestCreate_ReuseRequiredDoesNotReportBranchLossWhenTrackingRefWasPruned(t *testing.T) { + repoPath := initGitRepoWithRemote(t) + runGit(t, repoPath, "update-ref", "-d", "refs/remotes/origin/feature/pr-branch") + runGit(t, repoPath, "branch", "-D", "feature/pr-branch") + + store := newMockStore() + store.worktrees["canonical-worktree"] = &Worktree{ + ID: "canonical-worktree", + TaskID: "task-1", + TaskEnvironmentID: "environment-1", + RepositoryID: "repository-1", + Path: filepath.Join(t.TempDir(), "missing-worktree"), + Branch: "feature/pr-branch", + Status: StatusActive, + } + mgr, err := NewManager(newTestConfig(t), store, newTestLogger()) + if err != nil { + t.Fatalf("NewManager() error = %v", err) + } + + _, err = mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-1", + TaskEnvironmentID: "environment-1", + RepositoryID: "repository-1", + RepositoryPath: repoPath, + BaseBranch: "main", + WorktreeID: "canonical-worktree", + ReuseRequired: true, + }) + if !errors.Is(err, ErrReuseWorktreeUnavailable) { + t.Fatalf("Create() error = %v, want ErrReuseWorktreeUnavailable", err) + } + if errors.Is(err, ErrBranchUnrecoverable) { + t.Fatalf("Create() misclassified a branch that still exists on origin as unrecoverable: %v", err) + } +} + +func TestCreate_ReuseRequiredDoesNotReportBranchLossWhenRemoteProbeFails(t *testing.T) { + scriptDir := writeFakeGitScript(t, ` +case "${1:-}" in + rev-parse) + exit 1 + ;; + ls-remote) + echo "fatal: Authentication failed" >&2 + exit 128 + ;; + *) + exit 0 + ;; +esac`) + t.Setenv("PATH", scriptDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + repoPath := t.TempDir() + if err := os.MkdirAll(filepath.Join(repoPath, ".git"), 0755); err != nil { + t.Fatalf("create fake repository: %v", err) + } + store := newMockStore() + store.worktrees["canonical-worktree"] = &Worktree{ + ID: "canonical-worktree", + TaskID: "task-1", + TaskEnvironmentID: "environment-1", + RepositoryID: "repository-1", + Path: filepath.Join(t.TempDir(), "missing-worktree"), + Branch: "feature/pr-branch", + Status: StatusActive, + } + mgr, err := NewManager(newTestConfig(t), store, newTestLogger()) + if err != nil { + t.Fatalf("NewManager() error = %v", err) + } + + _, err = mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-1", + TaskEnvironmentID: "environment-1", + RepositoryID: "repository-1", + RepositoryPath: repoPath, + BaseBranch: "main", + WorktreeID: "canonical-worktree", + ReuseRequired: true, + }) + if !errors.Is(err, ErrReuseWorktreeUnavailable) { + t.Fatalf("Create() error = %v, want ErrReuseWorktreeUnavailable", err) + } + if errors.Is(err, ErrBranchUnrecoverable) { + t.Fatalf("Create() misclassified an authentication failure as branch loss: %v", err) + } + if !errors.Is(err, ErrAuthFailed) { + t.Fatalf("Create() error = %v, want ErrAuthFailed from the remote probe", err) + } +} + // TestCreate_RecreatesPersistedWorktreeWhenTasksBaseWasRemoved verifies that // normal reuse treats a removed managed tasks root as a missing worktree. The // persisted task-root identity still identifies the original safe slot, so the diff --git a/apps/backend/internal/worktree/manager_test.go b/apps/backend/internal/worktree/manager_test.go index 3fb345b9af..3cc6dff985 100644 --- a/apps/backend/internal/worktree/manager_test.go +++ b/apps/backend/internal/worktree/manager_test.go @@ -1042,6 +1042,9 @@ esac if !errors.Is(err, ErrInvalidBaseBranch) { t.Fatalf("error = %v, want ErrInvalidBaseBranch", err) } + if !errors.Is(err, ErrRemoteRefMissing) { + t.Fatalf("error = %v, want confirmed remote-ref-missing sentinel", err) + } } func TestFetchBranchToLocal_FetchFailsNoBranch(t *testing.T) { diff --git a/apps/backend/internal/worktree/worktree.go b/apps/backend/internal/worktree/worktree.go index 47a6cb6c43..a3ede31ffb 100644 --- a/apps/backend/internal/worktree/worktree.go +++ b/apps/backend/internal/worktree/worktree.go @@ -160,6 +160,12 @@ type CreateRequest struct { // recreate a worktree in this mode. ReuseRequired bool + // AllowBranchReplacement explicitly permits recovery to create a new branch + // when the persisted worktree branch no longer exists. It is only set by the + // user-selected resume-new-branch action; ordinary resume keeps the original + // branch and returns ErrBranchUnrecoverable. + AllowBranchReplacement bool + // TaskTitle is the human-readable task title (optional). // If provided, it will be used to generate semantic worktree/branch names. // The title is sanitized and truncated to 20 characters. diff --git a/apps/web/components/quick-chat/quick-chat-session-view.tsx b/apps/web/components/quick-chat/quick-chat-session-view.tsx index 2a7f36022e..7c767f8b61 100644 --- a/apps/web/components/quick-chat/quick-chat-session-view.tsx +++ b/apps/web/components/quick-chat/quick-chat-session-view.tsx @@ -4,6 +4,7 @@ import { useAppStore } from "@/components/state-provider"; import { useEnsureTaskSession } from "@/hooks/use-ensure-task-session"; import { useSessionResumption } from "@/hooks/domains/session/use-session-resumption"; import { PassthroughTerminal } from "@/components/task/passthrough-terminal"; +import { SessionRecoveryFeedback } from "@/components/task/ensure-session-error"; import type { QuickChatSession } from "@/lib/state/slices/ui/types"; import { QuickChatContent } from "./quick-chat-content"; import { useTranslation } from "react-i18next"; @@ -32,23 +33,38 @@ export function QuickChatSessionView({ session, onInitialPromptSent }: QuickChat useEnsureTaskSession(session.sessionId); const taskSession = useAppStore((state) => state.taskSessions.items[session.sessionId] ?? null); const taskId = taskSession ? (session.taskId ?? taskSession.task_id ?? null) : null; - useSessionResumption(taskId, session.sessionId); + const resumption = useSessionResumption(taskId, session.sessionId); const isPassthrough = useIsQuickChatPassthrough(session.sessionId); + const recoveryFeedback = ( + void resumption.resumeSession()} + /> + ); if (isPassthrough) { return ( -
- +
+ {recoveryFeedback} +
+ +
); } const isConfig = session.kind === "config"; return ( - +
+ {recoveryFeedback} +
+ +
+
); } diff --git a/apps/web/components/task/chat/chat-input-container.tsx b/apps/web/components/task/chat/chat-input-container.tsx index 8f47f6f411..06058c81ab 100644 --- a/apps/web/components/task/chat/chat-input-container.tsx +++ b/apps/web/components/task/chat/chat-input-container.tsx @@ -9,6 +9,7 @@ import type { MCPAttachmentHistory } from "@/lib/state/slices/session-runtime/ty import type { EntityReference } from "@/lib/types/entity-reference"; import { useChatInputContainer } from "./use-chat-input-container"; import { SessionStoppedBanner } from "./session-stopped-banner"; +import { useSessionRecoveryActions } from "@/hooks/domains/session/use-session-recovery-actions"; import { ChatInputBody, type ChatInputContextAreaProps, @@ -279,6 +280,7 @@ function useChatPromptEnhancement({ } export const ChatInputContainer = forwardRef( + // eslint-disable-next-line complexity -- top-level component chooses the stopped or editor surface after shared hook setup. function ChatInputContainer(props, ref) { const { sessionId, taskId, taskTitle, taskDescription, isAgentBusy, isStarting, isSending } = props; @@ -312,6 +314,11 @@ export const ChatInputContainer = forwardRef ); diff --git a/apps/web/components/task/chat/messages/action-message-actions.tsx b/apps/web/components/task/chat/messages/action-message-actions.tsx new file mode 100644 index 0000000000..226def3533 --- /dev/null +++ b/apps/web/components/task/chat/messages/action-message-actions.tsx @@ -0,0 +1,167 @@ +"use client"; + +import { useCallback, useState, type ElementType, type ReactElement } from "react"; +import { + IconAlertTriangle, + IconArchive, + IconGitCommit, + IconPlayerPlay, + IconRefresh, + IconSparkles, + IconTrash, + IconX, +} from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import { cn } from "@/lib/utils"; +import { getWebSocketClient } from "@/lib/ws/connection"; +import { useAppStore, useAppStoreApi } from "@/components/state-provider"; +import { useArchiveAndSwitchTask } from "@/hooks/use-task-actions"; +import { useTaskRemoval } from "@/hooks/use-task-removal"; +import { deleteTask } from "@/lib/api/domains/kanban-api"; +import type { MessageAction } from "@/components/task/chat/types"; + +export const ACTION_ICON_MAP: Record = { + archive: IconArchive, + trash: IconTrash, + refresh: IconRefresh, + "player-play": IconPlayerPlay, + sparkles: IconSparkles, + "git-commit": IconGitCommit, + "alert-triangle": IconAlertTriangle, + x: IconX, +}; + +export function ActionButtons({ + actions, + taskId, + onRecoveryRequested, + compact = false, + labelOverride, +}: { + actions: MessageAction[]; + taskId?: string; + onRecoveryRequested?: () => void; + compact?: boolean; + labelOverride?: string; +}) { + return ( +
+ {actions.map((action, i) => ( + + ))} +
+ ); +} + +export function ActionButton({ + action, + messageTaskId, + onCompleted, + compact = false, + labelOverride, +}: { + action: MessageAction; + messageTaskId?: string; + onCompleted?: () => void; + compact?: boolean; + labelOverride?: string; +}): ReactElement | null { + const [state, setState] = useState<"idle" | "busy" | "done" | "error">("idle"); + const activeTaskId = useAppStore((s) => s.tasks.activeTaskId); + const taskId = messageTaskId || activeTaskId; + const store = useAppStoreApi(); + const archiveAndSwitch = useArchiveAndSwitchTask(); + const { removeTaskFromBoard } = useTaskRemoval({ store }); + + const execute = useCallback(async () => { + if (state === "busy") return; + setState("busy"); + try { + switch (action.type) { + case "archive_task": { + if (taskId) await archiveAndSwitch(taskId); + break; + } + case "delete_task": { + if (taskId) { + const { activeTaskId, activeSessionId } = store.getState().tasks; + await deleteTask(taskId); + await removeTaskFromBoard(taskId, { + wasActiveTaskId: activeTaskId, + wasActiveSessionId: activeSessionId, + }); + } + break; + } + case "ws_request": { + const client = getWebSocketClient(); + const params = action.params as + | { method: string; payload: Record } + | undefined; + // i18n-exempt: technical error for unavailable WebSocket recovery client. + if (!client || !params) throw new Error("WebSocket recovery request is unavailable"); + await client.request(params.method, params.payload); + break; + } + } + setState("done"); + if (action.type === "ws_request") onCompleted?.(); + } catch { + setState("error"); + setTimeout(() => setState("idle"), 3000); + } + }, [action, state, taskId, store, archiveAndSwitch, removeTaskFromBoard, onCompleted]); + + // Once a ws_request has been fired, hide this button: it's no longer + // actionable. If the recovery succeeds the whole ActionMessage unmounts via + // isSessionActive; if it fails, a newer status/error message renders fresh + // buttons, so this stale one would just confuse the user. + if (state === "done" && action.type === "ws_request") return null; + + const Icon = action.icon ? ACTION_ICON_MAP[action.icon] : null; + const disabled = state === "busy" || state === "done"; + const isDestructive = action.variant === "destructive"; + + const button = ( + + ); + + if (action.tooltip) { + return ( + + {button} + {action.tooltip} + + ); + } + return button; +} diff --git a/apps/web/components/task/chat/messages/action-message-recovery.test.tsx b/apps/web/components/task/chat/messages/action-message-recovery.test.tsx index 7fc8c57b3e..18aa55d7e8 100644 --- a/apps/web/components/task/chat/messages/action-message-recovery.test.tsx +++ b/apps/web/components/task/chat/messages/action-message-recovery.test.tsx @@ -11,6 +11,7 @@ import { type TaskSessionState, } from "@/lib/types/http"; import type { AppState } from "@/lib/state/store"; +import { WebSocketRequestError } from "@/lib/ws/client"; const requestMock = vi.fn().mockResolvedValue({}); @@ -26,6 +27,8 @@ afterEach(() => { const RECOVERY_MESSAGE = "Agent encountered an error"; const RESUME_TEST_ID = "recovery-resume-button"; const FRESH_TEST_ID = "recovery-fresh-button"; +const BRANCH_FAILURE_MESSAGE = "The saved branch is no longer available."; +const RECOVERY_ERROR_TEST_ID = "session-recovery-error"; const TEST_SESSION_ID = "sess-1"; const TEST_TASK_ID = "task-1"; const FAILED_AT = "2026-05-30T00:00:00Z"; @@ -174,6 +177,90 @@ describe("ActionMessage — recovery card retires once the agent is back", () => expect(screen.queryByTestId(RESUME_TEST_ID)).toBeNull(); expect(screen.queryByTestId(FRESH_TEST_ID)).toBeNull(); }); + + it("keeps a typed branch failure visible with explicit recovery choices", async () => { + requestMock.mockRejectedValueOnce( + new WebSocketRequestError(BRANCH_FAILURE_MESSAGE, "CONFLICT", { + kind: "branch_unrecoverable", + recovery_action: "resume_new_branch", + original_branch: "feature/lost", + base_branch: "main", + }), + ); + + renderWithTranscript("WAITING_FOR_INPUT", []); + fireEvent.click(screen.getByTestId(RESUME_TEST_ID)); + + expect(await screen.findByTestId(RECOVERY_ERROR_TEST_ID)).toBeTruthy(); + expect(screen.getByText(BRANCH_FAILURE_MESSAGE)).toBeTruthy(); + expect(screen.getByTestId("recovery-new-branch-button")).toBeTruthy(); + expect(screen.getByTestId("recovery-restore-workspace-button")).toBeTruthy(); + }); + + it("retains both causes when manual resume and read-only restore fail", async () => { + requestMock + .mockRejectedValueOnce( + new WebSocketRequestError(BRANCH_FAILURE_MESSAGE, "CONFLICT", { + kind: "branch_unrecoverable", + recovery_action: "resume_new_branch", + original_branch: "feature/lost", + base_branch: "main", + }), + ) + .mockRejectedValueOnce(new Error("Workspace restore failed.")); + + renderWithTranscript("WAITING_FOR_INPUT", []); + fireEvent.click(screen.getByTestId(RESUME_TEST_ID)); + expect(await screen.findByTestId(RECOVERY_ERROR_TEST_ID)).toBeTruthy(); + + fireEvent.click(screen.getByTestId("recovery-restore-workspace-button")); + + const recoveryError = await screen.findByTestId(RECOVERY_ERROR_TEST_ID); + expect(recoveryError.textContent).toContain(`Resume failed: ${BRANCH_FAILURE_MESSAGE}`); + expect(recoveryError.textContent).toContain( + "Workspace restore failed: Workspace restore failed.", + ); + expect(screen.getByTestId("recovery-new-branch-button")).toBeTruthy(); + }); + + it("does not offer branch continuation for an ordinary recovery failure", async () => { + requestMock.mockRejectedValueOnce(new Error("Provider is unavailable")); + + renderWithTranscript("WAITING_FOR_INPUT", []); + fireEvent.click(screen.getByTestId(RESUME_TEST_ID)); + + expect(await screen.findByText("Provider is unavailable")).toBeTruthy(); + expect(screen.queryByTestId("recovery-new-branch-button")).toBeNull(); + expect(screen.getByTestId("recovery-restore-workspace-button")).toBeTruthy(); + }); +}); + +describe("ActionMessage recovery retry", () => { + it("disables Retry while a repeated recovery request is pending", async () => { + let resolveRetry: (() => void) | undefined; + requestMock.mockRejectedValueOnce(new Error("Initial resume failed")).mockImplementationOnce( + () => + new Promise((resolve) => { + resolveRetry = resolve; + }), + ); + + renderWithTranscript("WAITING_FOR_INPUT", []); + fireEvent.click(screen.getByTestId(RESUME_TEST_ID)); + expect(await screen.findByTestId(RECOVERY_ERROR_TEST_ID)).toBeTruthy(); + + fireEvent.click(screen.getByTestId("ensure-session-error-retry")); + await waitFor(() => + expect((screen.getByTestId("ensure-session-error-retry") as HTMLButtonElement).disabled).toBe( + true, + ), + ); + fireEvent.click(screen.getByTestId("ensure-session-error-retry")); + expect(requestMock).toHaveBeenCalledTimes(2); + + resolveRetry?.(); + await waitFor(() => expect(screen.queryByTestId(RECOVERY_ERROR_TEST_ID)).toBeNull()); + }); }); /** Renders the card with a live store handle so a spec can land transcript rows and diff --git a/apps/web/components/task/chat/messages/action-message-recovery.tsx b/apps/web/components/task/chat/messages/action-message-recovery.tsx new file mode 100644 index 0000000000..5130a11525 --- /dev/null +++ b/apps/web/components/task/chat/messages/action-message-recovery.tsx @@ -0,0 +1,153 @@ +"use client"; + +import { useCallback } from "react"; +import { useTranslation } from "react-i18next"; +import { Button } from "@kandev/ui/button"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import { + EnsureSessionErrorBanner, + SessionRecoveryNotice, +} from "@/components/task/ensure-session-error"; +import { + useSessionRecoveryActions, + type SessionRecoveryBusyAction, +} from "@/hooks/domains/session/use-session-recovery-actions"; +import type { SessionRecoveryAction } from "@/lib/services/session-recovery-service"; +import type { MessageAction } from "@/components/task/chat/types"; +import { ACTION_ICON_MAP, ActionButton } from "./action-message-actions"; + +export function sessionRecoveryAction(action: MessageAction): SessionRecoveryAction | null { + if (action.type !== "ws_request" || !action.params) return null; + if (action.params.method !== "session.recover") return null; + const payload = action.params.payload; + if (!payload || typeof payload !== "object") return null; + const recoveryAction = (payload as { action?: unknown }).action; + switch (recoveryAction) { + case "resume": + case "resume_new_branch": + case "fresh_start": + case "runtime_retry": + return recoveryAction; + default: + return null; + } +} + +export function SessionRecoveryActionButtons({ + actions, + taskId, + sessionId, + onRecoveryRequested, +}: { + actions: MessageAction[]; + taskId: string; + sessionId: string; + onRecoveryRequested: () => void; +}) { + const { t } = useTranslation(); + const { + busyAction, + recoveryError, + branchDetails, + recoveryNotice, + handleRecover, + handleRestore, + handleRetry, + handleNewBranch, + } = useSessionRecoveryActions({ taskId, sessionId }); + + const onRecoveryAction = useCallback( + async (action: SessionRecoveryAction) => { + if (await handleRecover(action)) onRecoveryRequested(); + }, + [handleRecover, onRecoveryRequested], + ); + const onRetry = useCallback(async () => { + if (await handleRetry()) onRecoveryRequested(); + }, [handleRetry, onRecoveryRequested]); + const restoreAction = { + label: t("task:restoreReadOnlyWorkspace"), + onClick: () => void handleRestore(), + testId: "recovery-restore-workspace-button", + disabled: busyAction !== null, + }; + + return ( + <> + {recoveryError ? ( + void onRetry()} + retryDisabled={busyAction !== null} + compact + action={ + branchDetails + ? { + label: t("task:continueOnNewBranch"), + onClick: () => + void handleNewBranch().then((success) => { + if (success) onRecoveryRequested(); + }), + testId: "recovery-new-branch-button", + disabled: busyAction !== null, + } + : restoreAction + } + secondaryAction={branchDetails ? restoreAction : undefined} + testId="session-recovery-error" + /> + ) : null} + {recoveryNotice ? : null} +
+ {actions.map((action, i) => { + const recoveryAction = sessionRecoveryAction(action); + return recoveryAction ? ( + void onRecoveryAction(recoveryAction)} + /> + ) : ( + + ); + })} +
+ + ); +} + +function SessionRecoveryActionButton({ + action, + busyAction, + onClick, +}: { + action: MessageAction; + busyAction: SessionRecoveryBusyAction; + onClick: () => void; +}) { + const Icon = action.icon ? ACTION_ICON_MAP[action.icon] : null; + const button = ( + + ); + + if (action.tooltip) { + return ( + + {button} + {action.tooltip} + + ); + } + return button; +} diff --git a/apps/web/components/task/chat/messages/action-message.tsx b/apps/web/components/task/chat/messages/action-message.tsx index 2279c72b36..a4e12fd124 100644 --- a/apps/web/components/task/chat/messages/action-message.tsx +++ b/apps/web/components/task/chat/messages/action-message.tsx @@ -1,43 +1,18 @@ "use client"; -import { useState, useCallback, useEffect, useMemo, memo, type ReactElement } from "react"; +import { useState, useEffect, useMemo, memo, type ReactElement } from "react"; import { Trans, useTranslation } from "react-i18next"; -import { - IconAlertTriangle, - IconArchive, - IconTrash, - IconRefresh, - IconPlayerPlay, - IconSparkles, - IconGitCommit, - IconX, -} from "@tabler/icons-react"; +import { IconAlertTriangle } from "@tabler/icons-react"; import { cn } from "@/lib/utils"; -import { Button } from "@kandev/ui/button"; -import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; -import { getWebSocketClient } from "@/lib/ws/connection"; -import { useAppStore, useAppStoreApi } from "@/components/state-provider"; import { useActionMessageSession, useAgentBootOutcomeAfterMessage } from "./action-message-state"; -import { useArchiveAndSwitchTask } from "@/hooks/use-task-actions"; -import { useTaskRemoval } from "@/hooks/use-task-removal"; -import { deleteTask } from "@/lib/api/domains/kanban-api"; import type { Message, TaskSessionState } from "@/lib/types/http"; import type { MessageAction } from "@/components/task/chat/types"; import { ActionMessageDetails, type ActionMeta } from "./action-message-details"; import { formatDateTime } from "@/lib/i18n/formats"; import { parseRetryAt, retryCountdownLabel } from "./transient-retry"; import { hasSessionRecoveryResolutionAfter } from "@/hooks/processed-message-filtering"; - -const ICON_MAP: Record = { - archive: IconArchive, - trash: IconTrash, - refresh: IconRefresh, - "player-play": IconPlayerPlay, - sparkles: IconSparkles, - "git-commit": IconGitCommit, - "alert-triangle": IconAlertTriangle, - x: IconX, -}; +import { ActionButtons } from "./action-message-actions"; +import { SessionRecoveryActionButtons, sessionRecoveryAction } from "./action-message-recovery"; function isSessionActive(state?: TaskSessionState) { return state === "RUNNING" || state === "STARTING" || state === "COMPLETED"; @@ -101,6 +76,7 @@ export const ActionMessage = memo(function ActionMessage({ comment }: { comment: sessionError={sessionError} sessionState={sessionState} taskId={comment.task_id} + sessionId={comment.session_id} recoveryResolved={ recoveryResolvedDurably || agentRebooted || (recoveryRequested && !recoveryFailedAgain) } @@ -115,6 +91,7 @@ function SettledActionMessage({ sessionError, sessionState, taskId, + sessionId, recoveryResolved, onRecoveryRequested, }: { @@ -123,6 +100,7 @@ function SettledActionMessage({ sessionError?: string; sessionState?: TaskSessionState; taskId?: string; + sessionId?: string; recoveryResolved: boolean; onRecoveryRequested: () => void; }) { @@ -141,6 +119,7 @@ function SettledActionMessage({ sessionError={sessionError} sessionState={sessionState} taskId={taskId} + sessionId={sessionId} recoveryResolved={recoveryResolved} onRecoveryRequested={onRecoveryRequested} /> @@ -153,6 +132,7 @@ function SettledFailureMessage({ sessionError, sessionState, taskId, + sessionId, recoveryResolved, onRecoveryRequested, }: { @@ -161,6 +141,7 @@ function SettledFailureMessage({ sessionError?: string; sessionState?: TaskSessionState; taskId?: string; + sessionId?: string; recoveryResolved: boolean; onRecoveryRequested: () => void; }) { @@ -187,7 +168,6 @@ function SettledFailureMessage({ metadata?.variant === "warning" ? "text-amber-600 dark:text-amber-400" : "text-red-600 dark:text-red-400"; - return (
@@ -197,19 +177,53 @@ function SettledFailureMessage({
{message}
- {metadata?.actions && metadata.actions.length > 0 && ( - - )} + {renderSettledActionButtons({ + actions: metadata?.actions, + taskId, + sessionId, + isRecoveryMessage: metadata?.recovery_actions === true, + onRecoveryRequested, + })}
); } +function renderSettledActionButtons({ + actions, + taskId, + sessionId, + isRecoveryMessage, + onRecoveryRequested, +}: { + actions?: MessageAction[]; + taskId?: string; + sessionId?: string; + isRecoveryMessage: boolean; + onRecoveryRequested: () => void; +}): ReactElement | null { + if (!actions || actions.length === 0) return null; + const hasSessionRecoveryAction = actions.some((action) => sessionRecoveryAction(action)); + if (hasSessionRecoveryAction && taskId && sessionId) { + return ( + + ); + } + return ( + + ); +} + function transientRetryReasonKey(failureCode?: string) { switch (failureCode) { case "model_capacity": @@ -502,136 +516,3 @@ function MissingBranchRecovery({ ); } - -function ActionButtons({ - actions, - taskId, - onRecoveryRequested, - compact = false, - labelOverride, -}: { - actions: MessageAction[]; - taskId?: string; - onRecoveryRequested?: () => void; - compact?: boolean; - labelOverride?: string; -}) { - return ( -
- {actions.map((action, i) => ( - - ))} -
- ); -} - -function ActionButton({ - action, - messageTaskId, - onCompleted, - compact = false, - labelOverride, -}: { - action: MessageAction; - messageTaskId?: string; - onCompleted?: () => void; - compact?: boolean; - labelOverride?: string; -}): ReactElement | null { - const [state, setState] = useState<"idle" | "busy" | "done" | "error">("idle"); - const activeTaskId = useAppStore((s) => s.tasks.activeTaskId); - const taskId = messageTaskId || activeTaskId; - const store = useAppStoreApi(); - const archiveAndSwitch = useArchiveAndSwitchTask(); - const { removeTaskFromBoard } = useTaskRemoval({ store }); - - const execute = useCallback(async () => { - if (state === "busy") return; - setState("busy"); - try { - switch (action.type) { - case "archive_task": { - if (taskId) await archiveAndSwitch(taskId); - break; - } - case "delete_task": { - if (taskId) { - const { activeTaskId, activeSessionId } = store.getState().tasks; - await deleteTask(taskId); - await removeTaskFromBoard(taskId, { - wasActiveTaskId: activeTaskId, - wasActiveSessionId: activeSessionId, - }); - } - break; - } - case "ws_request": { - const client = getWebSocketClient(); - const params = action.params as - | { method: string; payload: Record } - | undefined; - if (!client || !params) throw new Error("WebSocket recovery request is unavailable"); - await client.request(params.method, params.payload); - break; - } - } - setState("done"); - if (action.type === "ws_request") onCompleted?.(); - } catch { - setState("error"); - setTimeout(() => setState("idle"), 3000); - } - }, [action, state, taskId, store, archiveAndSwitch, removeTaskFromBoard]); - - // Once a ws_request has been fired, hide this button: it's no longer - // actionable. If the recovery succeeds the whole ActionMessage unmounts via - // isSessionActive; if it fails, a newer status/error message renders fresh - // buttons, so this stale one would just confuse the user. - if (state === "done" && action.type === "ws_request") return null; - - const Icon = action.icon ? ICON_MAP[action.icon] : null; - const disabled = state === "busy" || state === "done"; - const isDestructive = action.variant === "destructive"; - - const button = ( - - ); - - if (action.tooltip) { - return ( - - {button} - {action.tooltip} - - ); - } - return button; -} diff --git a/apps/web/components/task/chat/messages/status-message.test.tsx b/apps/web/components/task/chat/messages/status-message.test.tsx index f3ba24c9ac..84435f79a9 100644 --- a/apps/web/components/task/chat/messages/status-message.test.tsx +++ b/apps/web/components/task/chat/messages/status-message.test.tsx @@ -43,3 +43,33 @@ describe("StatusMessage model selection warnings", () => { expect(screen.getByText("The saved model was not advertised by the executor.")).toBeTruthy(); }); }); + +describe("StatusMessage branch replacement warnings", () => { + it("states that conversation history continued but lost code did not", () => { + const comment: Message = { + id: "status-branch-1", + session_id: toSessionId("session-1"), + task_id: toTaskId("task-1"), + author_type: "agent", + content: "branch_recreated", + type: "status", + created_at: "2026-08-15T00:00:00Z", + metadata: { + variant: "warning", + kind: "branch_recreated", + original_branch: "feature/lost", + new_branch: "kandev/task-recovery-1", + base_branch: "main", + }, + }; + + render(); + + expect(screen.getByTestId("branch-recreated-warning")).toBeTruthy(); + expect(screen.getByText("feature/lost")).toBeTruthy(); + expect(screen.getByText("kandev/task-recovery-1")).toBeTruthy(); + expect(screen.getByText("main")).toBeTruthy(); + expect(screen.getByText(/conversation history continues/i)).toBeTruthy(); + expect(screen.getByText(/code changes.*not recovered/i)).toBeTruthy(); + }); +}); diff --git a/apps/web/components/task/chat/messages/status-message.tsx b/apps/web/components/task/chat/messages/status-message.tsx index 437dfeca54..3a5b7e245f 100644 --- a/apps/web/components/task/chat/messages/status-message.tsx +++ b/apps/web/components/task/chat/messages/status-message.tsx @@ -14,6 +14,8 @@ import type { StatusMetadata } from "@/components/task/chat/types"; import { useTranslation } from "react-i18next"; import { t } from "@/lib/i18n"; +const UNKNOWN_TASK_KEY = "task:unknown"; + interface ErrorMetadata extends StatusMetadata { error?: string; text?: string; @@ -31,6 +33,9 @@ interface ErrorMetadata extends StatusMetadata { executor_type?: string; executor_profile_id?: string; remediation?: string[]; + original_branch?: string; + new_branch?: string; + base_branch?: string; } function getStatusStyle(isError: boolean, isWarning: boolean) { @@ -181,11 +186,12 @@ function modelSelectionReasonLabel(reason: string | undefined): string { }; return reason ? t(keyByReason[reason] ?? "task:modelSelectionReasonUnknown", { reason }) - : t("task:unknown"); + : t(UNKNOWN_TASK_KEY); } function ModelSelectionWarningDetails({ metadata }: { metadata: ErrorMetadata }) { - const requested = metadata.requested_model || t("task:unknown"); + const unknown = t(UNKNOWN_TASK_KEY); + const requested = metadata.requested_model || unknown; const effective = metadata.effective_model || t("task:modelSelectionProviderDefault"); const remediation = Array.isArray(metadata.remediation) ? metadata.remediation : []; const remediationKey: Record = { @@ -243,6 +249,41 @@ function ModelSelectionWarningDetails({ metadata }: { metadata: ErrorMetadata }) ); } +function BranchRecreatedWarning({ metadata }: { metadata: ErrorMetadata }) { + const { t } = useTranslation(); + const unknown = t(UNKNOWN_TASK_KEY); + const originalBranch = metadata.original_branch || unknown; + const newBranch = metadata.new_branch || unknown; + const baseBranch = metadata.base_branch || unknown; + + return ( +
+ +
+

+ {t("task:branchRecreatedWarning")} +

+
+

+ {t("task:branchRecreatedOriginal")}: {originalBranch} +

+

+ {t("task:branchRecreatedNew")}: {newBranch} +

+

+ {t("task:branchRecreatedBase")}: {baseBranch} +

+
+

{t("task:branchRecreatedConversation")}

+

{t("task:branchRecreatedCodeNotRecovered")}

+
+
+ ); +} + function computeExpandableContent(isError: boolean, metadata: ErrorMetadata | undefined) { if (!isError) return { hasExpandableContent: false, errorDetails: [] as { label: string; value: string }[] }; @@ -336,6 +377,9 @@ export const StatusMessage = memo(function StatusMessage({ comment }: { comment: const [isExpanded, setIsExpanded] = useState(false); const { metadata, progress, statusLine, message, isError, isWarning } = parseStatusMetadata(comment); + if (metadata?.kind === "branch_recreated") { + return ; + } const { hasExpandableContent, errorDetails } = computeExpandableContent(isError, metadata); const isSimpleStatus = !isError && !isWarning && progress === null && !statusLine && !metadata?.message; diff --git a/apps/web/components/task/chat/session-stopped-banner.test.tsx b/apps/web/components/task/chat/session-stopped-banner.test.tsx index f16c91c9ff..8cd5c904a6 100644 --- a/apps/web/components/task/chat/session-stopped-banner.test.tsx +++ b/apps/web/components/task/chat/session-stopped-banner.test.tsx @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { useState } from "react"; import { TooltipProvider } from "@kandev/ui/tooltip"; import type { SessionStoppedBannerProps } from "./session-stopped-banner"; +import { WebSocketRequestError } from "@/lib/ws/client"; const mocks = vi.hoisted(() => ({ request: vi.fn(), @@ -48,6 +49,10 @@ vi.mock("react-i18next", () => ({ "task:starting": "Starting...", "task:startFreshSession": "Start fresh session", "task:agentProfileNoLongerExists": "The agent profile no longer exists.", + "task:continueOnNewBranch": "Continue on a new branch", + "task:restoreReadOnlyWorkspace": "Restore read-only workspace", + "task:retry": "Retry", + "task:couldnTStartASession": "Session recovery failed", })[key] ?? key, }), })); @@ -59,6 +64,8 @@ const baseProps: Omit { afterEach(cleanup); -describe("SessionStoppedBanner", () => { +describe("SessionStoppedBanner basics", () => { it("offers only New Agent for a completed session and opens the new-session dialog", async () => { render(); expect(screen.getByTestId("completed-session-banner")).toBeTruthy(); expect(screen.getByText("This session is complete.")).toBeTruthy(); expect(screen.getByRole("button", { name: "New Agent" })).toBeTruthy(); - expect(screen.queryByTestId("recovery-resume-button")).toBeNull(); - expect(screen.queryByTestId("recovery-fresh-button")).toBeNull(); + expect(screen.queryByTestId(RESUME_BUTTON_TEST_ID)).toBeNull(); + expect(screen.queryByTestId(FRESH_BUTTON_TEST_ID)).toBeNull(); fireEvent.click(screen.getByRole("button", { name: "New Agent" })); @@ -126,7 +133,7 @@ describe("SessionStoppedBanner", () => { it("keeps resume and fresh-start recovery actions for a recoverable session", async () => { render(); - fireEvent.click(screen.getByTestId("recovery-resume-button")); + fireEvent.click(screen.getByTestId(RESUME_BUTTON_TEST_ID)); await waitFor(() => expect(mocks.request).toHaveBeenCalledWith( "session.recover", @@ -135,7 +142,7 @@ describe("SessionStoppedBanner", () => { ), ); - fireEvent.click(screen.getByTestId("recovery-fresh-button")); + fireEvent.click(screen.getByTestId(FRESH_BUTTON_TEST_ID)); await waitFor(() => expect(mocks.request).toHaveBeenCalledWith( "session.recover", @@ -149,8 +156,8 @@ describe("SessionStoppedBanner", () => { mocks.agentProfiles.splice(0, mocks.agentProfiles.length); render(); - expect(screen.getByTestId("recovery-resume-button").getAttribute("disabled")).not.toBeNull(); - fireEvent.click(screen.getByTestId("recovery-fresh-button")); + expect(screen.getByTestId(RESUME_BUTTON_TEST_ID).getAttribute("disabled")).not.toBeNull(); + fireEvent.click(screen.getByTestId(FRESH_BUTTON_TEST_ID)); expect(await screen.findByTestId("new-session-dialog")).toBeTruthy(); expect(mocks.request).not.toHaveBeenCalled(); @@ -173,3 +180,48 @@ describe("SessionStoppedBanner", () => { expect(screen.getByRole("button", { name: "Start fresh session" })).toBeTruthy(); }); }); + +describe("SessionStoppedBanner recovery failures", () => { + it("keeps a typed branch error visible and offers explicit continuation", async () => { + const branchError = new WebSocketRequestError( + "The saved branch is no longer available.", + "CONFLICT", + { + kind: "branch_unrecoverable", + recovery_action: "resume_new_branch", + original_branch: "feature/lost", + base_branch: "main", + }, + ); + mocks.request.mockRejectedValueOnce(branchError); + + render(); + fireEvent.click(screen.getByTestId(RESUME_BUTTON_TEST_ID)); + + expect(await screen.findByText("The saved branch is no longer available.")).toBeTruthy(); + expect(screen.getByTestId("recovery-new-branch-button")).toBeTruthy(); + expect(screen.getByTestId("recovery-restore-workspace-button")).toBeTruthy(); + + fireEvent.click(screen.getByTestId("recovery-new-branch-button")); + + await waitFor(() => + expect(mocks.request).toHaveBeenLastCalledWith( + "session.recover", + { task_id: "task-1", session_id: "session-1", action: "resume_new_branch" }, + 30000, + ), + ); + expect(screen.queryByText("The saved branch is no longer available.")).toBeNull(); + }); + + it("does not offer branch continuation for an ordinary resume error", async () => { + mocks.request.mockRejectedValueOnce(new Error("Provider is unavailable")); + + render(); + fireEvent.click(screen.getByTestId(RESUME_BUTTON_TEST_ID)); + + expect(await screen.findByText("Provider is unavailable")).toBeTruthy(); + expect(screen.queryByTestId("recovery-new-branch-button")).toBeNull(); + expect(screen.getByTestId("recovery-restore-workspace-button")).toBeTruthy(); + }); +}); diff --git a/apps/web/components/task/chat/session-stopped-banner.tsx b/apps/web/components/task/chat/session-stopped-banner.tsx index 27dc2846b2..a9cb1913f9 100644 --- a/apps/web/components/task/chat/session-stopped-banner.tsx +++ b/apps/web/components/task/chat/session-stopped-banner.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useState } from "react"; +import { useCallback } from "react"; import { IconAlertTriangle, IconCircleCheck, @@ -12,8 +12,17 @@ import { Button } from "@kandev/ui/button"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import { useTranslation } from "react-i18next"; import { NewSessionDialog } from "@/components/task/new-session-dialog"; +import { + EnsureSessionErrorBanner, + SessionRecoveryNotice, +} from "@/components/task/ensure-session-error"; import { useAppStore } from "@/components/state-provider"; -import { getWebSocketClient } from "@/lib/ws/connection"; +import { + useSessionRecoveryActions, + type SessionRecoveryBusyAction, + type SessionRecoveryActions, +} from "@/hooks/domains/session/use-session-recovery-actions"; +import type { BranchRecoveryDetails } from "@/lib/services/session-recovery-service"; export type SessionStoppedBannerMode = "recoverable" | "completed"; @@ -28,42 +37,99 @@ export type SessionStoppedBannerProps = { detail?: string; resumeLabel?: string; resumingLabel?: string; + recoveryActions?: SessionRecoveryActions; }; -async function requestSessionRecover( - taskId: string, - sessionId: string, - action: "resume" | "fresh_start", -): Promise { - const client = getWebSocketClient(); - if (!client) return false; - try { - await client.request( - "session.recover", - { task_id: taskId, session_id: sessionId, action }, - 30000, - ); - return true; - } catch { - return false; - } +function StoppedRecoveryFeedback({ + workspaceId, + recoveryError, + recoveryNotice, + branchDetails, + busyAction, + onRetry, + onRestore, + onNewBranch, +}: { + workspaceId?: string | null; + recoveryError: Error | null; + recoveryNotice: string | null; + branchDetails: BranchRecoveryDetails | null; + busyAction: SessionRecoveryBusyAction; + onRetry: () => void; + onRestore: () => void; + onNewBranch: () => void; +}) { + const { t } = useTranslation(); + if (!recoveryError && !recoveryNotice) return null; + const restoreAction = { + label: t("task:restoreReadOnlyWorkspace"), + onClick: onRestore, + testId: "recovery-restore-workspace-button", + disabled: busyAction !== null, + }; + return ( + <> + {recoveryError ? ( + + ) : null} + {recoveryNotice ? : null} + + ); } function RecoverableSessionActions({ onShowDialog, taskId, sessionId, + workspaceId, resumeLabel, resumingLabel, + recoveryActions, }: Pick< SessionStoppedBannerProps, - "onShowDialog" | "taskId" | "sessionId" | "resumeLabel" | "resumingLabel" + | "onShowDialog" + | "taskId" + | "sessionId" + | "workspaceId" + | "resumeLabel" + | "resumingLabel" + | "recoveryActions" >) { const { t } = useTranslation(); const resumeText = resumeLabel ?? t("task:resume"); const resumingText = resumingLabel ?? t("task:resuming"); - const [isResuming, setIsResuming] = useState(false); - const [isStartingFresh, setIsStartingFresh] = useState(false); + const localRecoveryActions = useSessionRecoveryActions({ + taskId: taskId ?? "", + sessionId: sessionId ?? "", + }); + const { + busyAction, + recoveryError, + branchDetails, + recoveryNotice, + handleRecover, + handleRestore, + handleRetry, + handleNewBranch, + } = recoveryActions ?? localRecoveryActions; const profileExists = useAppStore((s) => { if (!sessionId) return false; @@ -73,26 +139,63 @@ function RecoverableSessionActions({ ); }); - const handleRecover = useCallback( - async (action: "resume" | "fresh_start") => { - if (!sessionId || !taskId) return; - const setBusy = action === "resume" ? setIsResuming : setIsStartingFresh; - setBusy(true); - const ok = await requestSessionRecover(taskId, sessionId, action); - if (!ok) setBusy(false); - }, - [sessionId, taskId], - ); - - const handleResume = useCallback(() => handleRecover("resume"), [handleRecover]); + const handleResume = useCallback(() => { + if (taskId && sessionId) void handleRecover("resume"); + }, [handleRecover, sessionId, taskId]); const handleFreshStart = useCallback(() => { if (!profileExists) { onShowDialog(true); return; } - void handleRecover("fresh_start"); + if (taskId && sessionId) void handleRecover("fresh_start"); }, [profileExists, onShowDialog, handleRecover]); + return ( +
+ void handleRestore()} + onNewBranch={handleNewBranch} + /> + +
+ ); +} + +function RecoverableSessionButtons({ + taskId, + sessionId, + profileExists, + busyAction, + resumeText, + resumingText, + onResume, + onFreshStart, +}: { + taskId: string | null; + sessionId: string | null; + profileExists: boolean; + busyAction: SessionRecoveryBusyAction; + resumeText: string; + resumingText: string; + onResume: () => void; + onFreshStart: () => void; +}) { + const { t } = useTranslation(); return (
{sessionId && taskId && ( @@ -107,11 +210,11 @@ function RecoverableSessionActions({ size="sm" data-testid="recovery-resume-button" className="min-h-11 w-full shrink-0 gap-1.5 cursor-pointer sm:w-auto" - onClick={handleResume} - disabled={isResuming || !profileExists} + onClick={onResume} + disabled={busyAction !== null || !profileExists} > - {isResuming ? resumingText : resumeText} + {busyAction === "resume" ? resumingText : resumeText} @@ -124,12 +227,12 @@ function RecoverableSessionActions({ variant="outline" size="sm" className="min-h-11 w-full shrink-0 gap-1.5 cursor-pointer sm:w-auto" - onClick={handleFreshStart} - disabled={isStartingFresh} + onClick={onFreshStart} + disabled={busyAction !== null} data-testid="recovery-fresh-button" > - {isStartingFresh ? t("task:starting") : t("task:startFreshSession")} + {busyAction === "fresh_start" ? t("task:starting") : t("task:startFreshSession")}
); @@ -146,6 +249,7 @@ export function SessionStoppedBanner({ detail, resumeLabel, resumingLabel, + recoveryActions, }: SessionStoppedBannerProps) { const { t } = useTranslation(); const isCompleted = mode === "completed"; @@ -192,8 +296,10 @@ export function SessionStoppedBanner({ onShowDialog={onShowDialog} taskId={taskId} sessionId={sessionId} + workspaceId={workspaceId} resumeLabel={resumeLabel} resumingLabel={resumingLabel} + recoveryActions={recoveryActions} /> )}
diff --git a/apps/web/components/task/ensure-session-error.test.tsx b/apps/web/components/task/ensure-session-error.test.tsx index 4a1db1e5b8..60a6a934e8 100644 --- a/apps/web/components/task/ensure-session-error.test.tsx +++ b/apps/web/components/task/ensure-session-error.test.tsx @@ -82,6 +82,22 @@ describe("EnsureSessionErrorBanner", () => { fireEvent.click(screen.getByTestId("ensure-session-error-retry")); expect(onRetry).toHaveBeenCalledTimes(1); }); + + it("disables retry while another recovery action is pending", () => { + const onRetry = vi.fn(); + render( + , + ); + const retry = screen.getByTestId("ensure-session-error-retry") as HTMLButtonElement; + expect(retry.disabled).toBe(true); + + fireEvent.click(retry); + expect(onRetry).not.toHaveBeenCalled(); + }); }); describe("EnsureSessionErrorEmptyState", () => { diff --git a/apps/web/components/task/ensure-session-error.tsx b/apps/web/components/task/ensure-session-error.tsx index 96278dad23..df1d18cb93 100644 --- a/apps/web/components/task/ensure-session-error.tsx +++ b/apps/web/components/task/ensure-session-error.tsx @@ -1,11 +1,12 @@ "use client"; import Link from "@/components/routing/app-link"; -import { IconAlertTriangle, IconRefresh } from "@tabler/icons-react"; +import { IconAlertTriangle, IconInfoCircle, IconRefresh } from "@tabler/icons-react"; import { Alert, AlertDescription, AlertTitle } from "@kandev/ui/alert"; import { Button } from "@kandev/ui/button"; import { useTranslation } from "react-i18next"; import { t } from "@/lib/i18n"; +import { cn } from "@/lib/utils"; // EnsureSessionErrorInfo wraps a parsed ensure error so UI can offer a targeted action for the missing-agent-profile case. export type EnsureSessionErrorInfo = { @@ -47,19 +48,40 @@ export function describeEnsureError( }; } +type RecoveryAction = { + label: string; + onClick: () => void; + testId: string; + disabled?: boolean; +}; + type BannerProps = { error: Error | null; onRetry: () => void; workspaceId?: string | null; + action?: RecoveryAction; + secondaryAction?: RecoveryAction; + retryDisabled?: boolean; + testId?: string; + compact?: boolean; }; /** Slim banner for the task page, rendered above the layout. */ -export function EnsureSessionErrorBanner({ error, onRetry, workspaceId }: BannerProps) { +export function EnsureSessionErrorBanner({ + error, + onRetry, + workspaceId, + action, + secondaryAction, + retryDisabled, + testId = "ensure-session-error-banner", + compact = false, +}: BannerProps) { const { t } = useTranslation(); const info = describeEnsureError(error, workspaceId); if (!info) return null; return ( -
+
{info.title} @@ -75,11 +97,36 @@ export function EnsureSessionErrorBanner({ error, onRetry, workspaceId }: Banner {info.action.label} ) : null} + {action ? ( + + ) : null} + {secondaryAction ? ( + + ) : null}