From 766e62d8c224f535e393432506b8f9e6c810bdf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 5 Mar 2026 20:55:22 +0000 Subject: [PATCH] fix(worktree): improve startup readiness and base sync handling (#374) --- .../internal/agent/handlers/git_handlers.go | 18 +++ .../agent/lifecycle/env_preparer_worktree.go | 50 +++++++ apps/backend/internal/common/config/config.go | 12 +- apps/backend/internal/worktree/config.go | 8 ++ apps/backend/internal/worktree/manager.go | 126 ++++++++++++------ .../backend/internal/worktree/manager_test.go | 31 ++++- apps/backend/internal/worktree/provider.go | 8 +- apps/backend/internal/worktree/worktree.go | 22 +++ .../components/task/commit-detail-panel.tsx | 26 ++-- .../components/task/commit-diff-request.ts | 80 +++++++++++ .../components/task/task-files-panel-hooks.ts | 32 +++-- 11 files changed, 345 insertions(+), 68 deletions(-) create mode 100644 apps/web/components/task/commit-diff-request.ts diff --git a/apps/backend/internal/agent/handlers/git_handlers.go b/apps/backend/internal/agent/handlers/git_handlers.go index a0c95e4e16..a39021a06d 100644 --- a/apps/backend/internal/agent/handlers/git_handlers.go +++ b/apps/backend/internal/agent/handlers/git_handlers.go @@ -4,6 +4,7 @@ package handlers import ( "context" "fmt" + "strings" "time" "github.com/kandev/kandev/internal/agent/lifecycle" @@ -582,6 +583,14 @@ func (h *GitHandlers) wsCommitDiff(ctx context.Context, msg *ws.Message) (*ws.Me client, err := h.getAgentCtlClient(req.SessionID) if err != nil { + if isSessionNotReadyError(err) { + return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{ + "success": false, + "ready": false, + "reason": "agent_starting", + "retry_after_ms": 500, + }) + } return nil, err } @@ -593,6 +602,15 @@ func (h *GitHandlers) wsCommitDiff(ctx context.Context, msg *ws.Message) (*ws.Me return ws.NewResponse(msg.ID, msg.Action, result) } +func isSessionNotReadyError(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "no agent running for session") || + strings.Contains(msg, "agent client not available for session") +} + // getAgentCtlClient gets the agentctl client for a session func (h *GitHandlers) getAgentCtlClient(sessionID string) (*client.Client, error) { execution, ok := h.lifecycleMgr.GetExecutionBySessionID(sessionID) diff --git a/apps/backend/internal/agent/lifecycle/env_preparer_worktree.go b/apps/backend/internal/agent/lifecycle/env_preparer_worktree.go index 2a0227cb94..6f2e4c2504 100644 --- a/apps/backend/internal/agent/lifecycle/env_preparer_worktree.go +++ b/apps/backend/internal/agent/lifecycle/env_preparer_worktree.go @@ -33,6 +33,9 @@ func (p *WorktreePreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, var steps []PrepareStep totalSteps := 2 // validate + create worktree + if req.PullBeforeWorktree { + totalSteps++ + } if req.CheckoutBranch != "" { totalSteps++ } @@ -60,6 +63,16 @@ func (p *WorktreePreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, reportProgress(onProgress, step, stepIdx, totalSteps) stepIdx++ + var syncStep *PrepareStep + syncStepIndex := -1 + if req.PullBeforeWorktree { + s := beginStep("Sync base branch") + reportProgress(onProgress, s, stepIdx, totalSteps) + syncStep = &s + syncStepIndex = stepIdx + stepIdx++ + } + // Step 2: Create worktree step = beginStep("Create worktree") reportProgress(onProgress, step, stepIdx, totalSteps) @@ -76,8 +89,25 @@ func (p *WorktreePreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, PullBeforeWorktree: req.PullBeforeWorktree, WorktreeID: req.WorktreeID, } + if syncStep != nil { + createReq.OnSyncProgress = func(event worktree.SyncProgressEvent) { + applySyncProgressEvent(syncStep, event) + reportProgress(onProgress, *syncStep, syncStepIndex, totalSteps) + } + } wt, err := p.worktreeMgr.Create(ctx, createReq) + if syncStep != nil { + if syncStep.Status == PrepareStepRunning { + if err != nil { + completeStepError(syncStep, "sync interrupted by worktree creation failure") + } else { + completeStepSuccess(syncStep) + } + } + steps = append(steps, *syncStep) + reportProgress(onProgress, *syncStep, syncStepIndex, totalSteps) + } if err != nil { completeStepError(&step, err.Error()) steps = append(steps, step) @@ -140,3 +170,23 @@ func (p *WorktreePreparer) Prepare(ctx context.Context, req *EnvPrepareRequest, MainRepoGitDir: mainRepoGitDir, }, nil } + +func applySyncProgressEvent(step *PrepareStep, event worktree.SyncProgressEvent) { + if event.StepName != "" { + step.Name = event.StepName + } + step.Output = event.Output + step.Error = event.Error + switch event.Status { + case worktree.SyncProgressRunning: + step.Status = PrepareStepRunning + if step.StartedAt == nil { + now := time.Now() + step.StartedAt = &now + } + case worktree.SyncProgressCompleted: + now := time.Now() + step.Status = PrepareStepCompleted + step.EndedAt = &now + } +} diff --git a/apps/backend/internal/common/config/config.go b/apps/backend/internal/common/config/config.go index 292c824241..4a6aa4d801 100644 --- a/apps/backend/internal/common/config/config.go +++ b/apps/backend/internal/common/config/config.go @@ -119,10 +119,12 @@ type RepositoryDiscoveryConfig struct { // WorktreeConfig holds Git worktree configuration for concurrent agent execution. type WorktreeConfig struct { - Enabled bool `mapstructure:"enabled"` // Enable worktree mode - BasePath string `mapstructure:"basePath"` // Base directory for worktrees (default: ~/.kandev/worktrees) - DefaultBranch string `mapstructure:"defaultBranch"` // Default base branch (default: main) - CleanupOnRemove bool `mapstructure:"cleanupOnRemove"` // Remove worktree directory on task deletion + Enabled bool `mapstructure:"enabled"` // Enable worktree mode + BasePath string `mapstructure:"basePath"` // Base directory for worktrees (default: ~/.kandev/worktrees) + DefaultBranch string `mapstructure:"defaultBranch"` // Default base branch (default: main) + CleanupOnRemove bool `mapstructure:"cleanupOnRemove"` // Remove worktree directory on task deletion + FetchTimeoutSeconds int `mapstructure:"fetchTimeoutSeconds"` // Git fetch timeout before worktree creation + PullTimeoutSeconds int `mapstructure:"pullTimeoutSeconds"` // Git pull timeout before worktree creation } // RepoCloneConfig holds configuration for automatic repository cloning. @@ -253,6 +255,8 @@ func setDefaults(v *viper.Viper) { v.SetDefault("worktree.basePath", "") v.SetDefault("worktree.defaultBranch", "main") v.SetDefault("worktree.cleanupOnRemove", true) + v.SetDefault("worktree.fetchTimeoutSeconds", 60) + v.SetDefault("worktree.pullTimeoutSeconds", 60) // RepoClone defaults v.SetDefault("repoClone.basePath", "") diff --git a/apps/backend/internal/worktree/config.go b/apps/backend/internal/worktree/config.go index f70905a479..5e85b8fe62 100644 --- a/apps/backend/internal/worktree/config.go +++ b/apps/backend/internal/worktree/config.go @@ -23,6 +23,14 @@ type Config struct { // BranchPrefix is the prefix used for worktree branch names. // Default: feature/ BranchPrefix string `mapstructure:"branch_prefix"` + + // FetchTimeoutSeconds is the timeout for pre-worktree git fetch. + // If <= 0, manager default is used. + FetchTimeoutSeconds int `mapstructure:"fetch_timeout_seconds"` + + // PullTimeoutSeconds is the timeout for pre-worktree git pull. + // If <= 0, manager default is used. + PullTimeoutSeconds int `mapstructure:"pull_timeout_seconds"` } // DefaultBranchPrefix is used when no repository-specific prefix is provided. diff --git a/apps/backend/internal/worktree/manager.go b/apps/backend/internal/worktree/manager.go index 475cc1bc26..56287b66b7 100644 --- a/apps/backend/internal/worktree/manager.go +++ b/apps/backend/internal/worktree/manager.go @@ -18,8 +18,8 @@ import ( ) const ( - defaultGitFetchTimeout = 8 * time.Second - defaultGitPullTimeout = 8 * time.Second + defaultGitFetchTimeout = 60 * time.Second + defaultGitPullTimeout = 60 * time.Second ) // repoLockEntry tracks a repository lock and its reference count. @@ -92,14 +92,24 @@ func NewManager(cfg Config, store Store, log *logger.Logger) (*Manager, error) { return nil, fmt.Errorf("failed to create worktree base directory: %w", err) } + fetchTimeout := defaultGitFetchTimeout + if cfg.FetchTimeoutSeconds > 0 { + fetchTimeout = time.Duration(cfg.FetchTimeoutSeconds) * time.Second + } + + pullTimeout := defaultGitPullTimeout + if cfg.PullTimeoutSeconds > 0 { + pullTimeout = time.Duration(cfg.PullTimeoutSeconds) * time.Second + } + return &Manager{ config: cfg, logger: log.WithFields(zap.String("component", "worktree-manager")), store: store, worktrees: make(map[string]*Worktree), repoLocks: make(map[string]*repoLockEntry), - fetchTimeout: defaultGitFetchTimeout, - pullTimeout: defaultGitPullTimeout, + fetchTimeout: fetchTimeout, + pullTimeout: pullTimeout, }, nil } @@ -226,7 +236,7 @@ func (m *Manager) Create(ctx context.Context, req CreateRequest) (*Worktree, err baseRef := req.BaseBranch if req.PullBeforeWorktree { - baseRef = m.pullBaseBranch(req.RepositoryPath, req.BaseBranch) + baseRef = m.pullBaseBranch(ctx, req.RepositoryPath, req.BaseBranch, req.OnSyncProgress) } // Check base branch exists @@ -811,13 +821,19 @@ func classifyGitFallbackReason(cmdErr error, cmdOutput string, ctxErr error) str // 3. baseBranch is a local branch but we're on a different branch: use origin/ instead // // On fetch/pull failure, errors are logged but the function continues with the best available ref. -func (m *Manager) pullBaseBranch(repoPath, baseBranch string) string { - // Normalize branch name - strip "origin/" prefix if present +func (m *Manager) pullBaseBranch(ctx context.Context, repoPath, baseBranch string, onProgress SyncProgressCallback) string { localBranch := strings.TrimPrefix(baseBranch, "origin/") isRemoteRef := localBranch != baseBranch + stepName := "Sync base branch" + + m.reportSyncProgress(onProgress, SyncProgressEvent{ + StepName: stepName, + Status: SyncProgressRunning, + Output: fmt.Sprintf("Fetching latest changes for %s", baseBranch), + }) // Fetch branch from origin in non-interactive mode. - fetchCtx, cancelFetch := context.WithTimeout(context.Background(), m.fetchTimeout) + fetchCtx, cancelFetch := context.WithTimeout(ctx, m.fetchTimeout) defer cancelFetch() fetchArgs := []string{"fetch", "origin"} @@ -826,47 +842,81 @@ func (m *Manager) pullBaseBranch(repoPath, baseBranch string) string { } fetchCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, fetchArgs...) if output, err := fetchCmd.CombinedOutput(); err != nil { - m.logger.Warn("git fetch failed before worktree creation; continuing with fallback ref", - zap.String("branch", baseBranch), - zap.String("reason", classifyGitFallbackReason(err, string(output), fetchCtx.Err())), - zap.String("fallback_ref", baseBranch), - zap.String("output", string(output)), - zap.Error(err)) - return baseBranch + return m.handleFetchFallback(baseBranch, stepName, onProgress, fetchCtx.Err(), output, err) } - // If the original ref was already a remote ref, use it directly if isRemoteRef { - return "origin/" + localBranch + resolved := "origin/" + localBranch + m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Synced and using %s", resolved), "") + return resolved } - // For local branches: try to update if we're on that branch, otherwise use origin/ + return m.resolveLocalBaseRef(ctx, repoPath, baseBranch, localBranch, stepName, onProgress) +} + +func (m *Manager) reportSyncProgress(cb SyncProgressCallback, event SyncProgressEvent) { + if cb != nil { + cb(event) + } +} + +func (m *Manager) reportSyncCompleted(stepName string, onProgress SyncProgressCallback, output, errOutput string) { + m.reportSyncProgress(onProgress, SyncProgressEvent{ + StepName: stepName, + Status: SyncProgressCompleted, + Output: output, + Error: strings.TrimSpace(errOutput), + }) +} + +func (m *Manager) handleFetchFallback(baseBranch, stepName string, onProgress SyncProgressCallback, ctxErr error, output []byte, cmdErr error) string { + reason := classifyGitFallbackReason(cmdErr, string(output), ctxErr) + m.logger.Warn("git fetch failed before worktree creation; continuing with fallback ref", + zap.String("branch", baseBranch), + zap.String("reason", reason), + zap.String("fallback_ref", baseBranch), + zap.String("output", string(output)), + zap.Error(cmdErr)) + m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Fetch %s; using fallback ref %s", reason, baseBranch), string(output)) + return baseBranch +} + +func (m *Manager) resolveLocalBaseRef( + ctx context.Context, repoPath, baseBranch, localBranch, stepName string, + onProgress SyncProgressCallback, +) string { remoteRef := "origin/" + localBranch - currentBranch := m.currentBranch(repoPath) - - if currentBranch == baseBranch { - // We're on the target branch - try to pull (fast-forward only) - pullCtx, cancelPull := context.WithTimeout(context.Background(), m.pullTimeout) - defer cancelPull() - - pullCmd := m.newNonInteractiveGitCmd(pullCtx, repoPath, "pull", "--ff-only", "origin", baseBranch) - if output, err := pullCmd.CombinedOutput(); err != nil { - m.logger.Warn("git pull failed before worktree creation; continuing with remote ref", - zap.String("branch", baseBranch), - zap.String("reason", classifyGitFallbackReason(err, string(output), pullCtx.Err())), - zap.String("remote_ref", remoteRef), - zap.String("output", string(output)), - zap.Error(err)) - return remoteRef - } - return baseBranch + if m.currentBranch(repoPath) == baseBranch { + return m.pullCurrentBranchOrFallback(ctx, repoPath, baseBranch, remoteRef, stepName, onProgress) } - - // Not on the target branch - use the remote ref if it exists if m.branchExists(repoPath, remoteRef) { + m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Synced and using %s", remoteRef), "") return remoteRef } + m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Remote ref not found; using %s", baseBranch), "") + return baseBranch +} +func (m *Manager) pullCurrentBranchOrFallback( + ctx context.Context, repoPath, baseBranch, remoteRef, stepName string, + onProgress SyncProgressCallback, +) string { + pullCtx, cancelPull := context.WithTimeout(ctx, m.pullTimeout) + defer cancelPull() + + pullCmd := m.newNonInteractiveGitCmd(pullCtx, repoPath, "pull", "--ff-only", "origin", baseBranch) + if output, err := pullCmd.CombinedOutput(); err != nil { + reason := classifyGitFallbackReason(err, string(output), pullCtx.Err()) + m.logger.Warn("git pull failed before worktree creation; continuing with remote ref", + zap.String("branch", baseBranch), + zap.String("reason", reason), + zap.String("remote_ref", remoteRef), + zap.String("output", string(output)), + zap.Error(err)) + m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Pull %s; using %s", reason, remoteRef), string(output)) + return remoteRef + } + m.reportSyncCompleted(stepName, onProgress, fmt.Sprintf("Synced and using %s", baseBranch), "") return baseBranch } diff --git a/apps/backend/internal/worktree/manager_test.go b/apps/backend/internal/worktree/manager_test.go index 8e19bf0c85..317faa47be 100644 --- a/apps/backend/internal/worktree/manager_test.go +++ b/apps/backend/internal/worktree/manager_test.go @@ -117,6 +117,31 @@ func TestNewManager(t *testing.T) { if !mgr.IsEnabled() { t.Error("expected manager to be enabled") } + if mgr.fetchTimeout != defaultGitFetchTimeout { + t.Fatalf("fetchTimeout = %v, want %v", mgr.fetchTimeout, defaultGitFetchTimeout) + } + if mgr.pullTimeout != defaultGitPullTimeout { + t.Fatalf("pullTimeout = %v, want %v", mgr.pullTimeout, defaultGitPullTimeout) + } +} + +func TestNewManager_CustomSyncTimeouts(t *testing.T) { + cfg := newTestConfig(t) + cfg.FetchTimeoutSeconds = 15 + cfg.PullTimeoutSeconds = 25 + log := newTestLogger() + store := newMockStore() + + mgr, err := NewManager(cfg, store, log) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + if mgr.fetchTimeout != 15*time.Second { + t.Fatalf("fetchTimeout = %v, want %v", mgr.fetchTimeout, 15*time.Second) + } + if mgr.pullTimeout != 25*time.Second { + t.Fatalf("pullTimeout = %v, want %v", mgr.pullTimeout, 25*time.Second) + } } func TestNewManager_DisabledConfig(t *testing.T) { @@ -648,7 +673,7 @@ esac } repoPath := t.TempDir() - ref := mgr.pullBaseBranch(repoPath, "origin/master") + ref := mgr.pullBaseBranch(context.Background(), repoPath, "origin/master", nil) if ref != "origin/master" { t.Fatalf("pullBaseBranch() ref = %q, want %q", ref, "origin/master") } @@ -697,7 +722,7 @@ esac repoPath := t.TempDir() start := time.Now() - ref := mgr.pullBaseBranch(repoPath, "master") + ref := mgr.pullBaseBranch(context.Background(), repoPath, "master", nil) elapsed := time.Since(start) if ref != "master" { @@ -748,7 +773,7 @@ esac mgr.pullTimeout = 300 * time.Millisecond repoPath := t.TempDir() - ref := mgr.pullBaseBranch(repoPath, "master") + ref := mgr.pullBaseBranch(context.Background(), repoPath, "master", nil) if ref != "origin/master" { t.Fatalf("pullBaseBranch() ref = %q, want %q", ref, "origin/master") } diff --git a/apps/backend/internal/worktree/provider.go b/apps/backend/internal/worktree/provider.go index d48a0ea7e0..23cb722f04 100644 --- a/apps/backend/internal/worktree/provider.go +++ b/apps/backend/internal/worktree/provider.go @@ -14,9 +14,11 @@ func Provide(writer, reader *sqlx.DB, cfg *config.Config, log *logger.Logger) (* return nil, nil, err } wtCfg := Config{ - Enabled: cfg.Worktree.Enabled, - BasePath: cfg.Worktree.BasePath, - BranchPrefix: "kandev/", + Enabled: cfg.Worktree.Enabled, + BasePath: cfg.Worktree.BasePath, + BranchPrefix: "kandev/", + FetchTimeoutSeconds: cfg.Worktree.FetchTimeoutSeconds, + PullTimeoutSeconds: cfg.Worktree.PullTimeoutSeconds, } wtCfg.SetDataDirFallback(cfg.ResolvedDataDir()) manager, err := NewManager(wtCfg, store, log) diff --git a/apps/backend/internal/worktree/worktree.go b/apps/backend/internal/worktree/worktree.go index eff86b04d6..657ad9773c 100644 --- a/apps/backend/internal/worktree/worktree.go +++ b/apps/backend/internal/worktree/worktree.go @@ -2,6 +2,25 @@ package worktree import "time" +// SyncProgressStatus represents the status of a base-branch sync progress event. +type SyncProgressStatus string + +const ( + SyncProgressRunning SyncProgressStatus = "running" + SyncProgressCompleted SyncProgressStatus = "completed" +) + +// SyncProgressEvent reports pre-worktree base-branch synchronization progress. +type SyncProgressEvent struct { + StepName string + Status SyncProgressStatus + Output string + Error string +} + +// SyncProgressCallback is called when base-branch sync status changes. +type SyncProgressCallback func(event SyncProgressEvent) + // Worktree represents a Git worktree associated with a task. type Worktree struct { // ID is the unique identifier for this worktree record. @@ -85,6 +104,9 @@ type CreateRequest struct { // WorktreeID is the ID of an existing worktree to reuse (optional). // If provided and valid, the existing worktree is returned instead of creating a new one. WorktreeID string + + // OnSyncProgress receives progress updates for pre-worktree branch sync. + OnSyncProgress SyncProgressCallback } // Validate validates the create request. diff --git a/apps/web/components/task/commit-detail-panel.tsx b/apps/web/components/task/commit-detail-panel.tsx index c756b2ac8f..dfec45b843 100644 --- a/apps/web/components/task/commit-detail-panel.tsx +++ b/apps/web/components/task/commit-detail-panel.tsx @@ -6,11 +6,11 @@ import { PanelRoot, PanelBody } from "./panel-primitives"; import { FileDiffViewer } from "@/components/diff"; import { useAppStore } from "@/components/state-provider"; import { useSessionCommits } from "@/hooks/domains/session/use-session-commits"; -import { getWebSocketClient } from "@/lib/ws/connection"; import { useToast } from "@/components/toast-provider"; import { usePanelActions } from "@/hooks/use-panel-actions"; import { setPanelTitle } from "@/lib/layout/panel-portal-manager"; import type { FileInfo } from "@/lib/state/store"; +import { requestCommitDiff } from "./commit-diff-request"; type CommitDetailPanelProps = { panelId: string; @@ -47,6 +47,15 @@ const CommitDetailPanel = memo(function CommitDetailPanel({ }: CommitDetailPanelProps) { const commitSha = params.commitSha as string; const activeSessionId = useAppStore((state) => state.tasks.activeSessionId); + const activeTaskId = useAppStore((state) => state.tasks.activeTaskId); + const sessionTaskId = useAppStore((state) => + activeSessionId ? state.taskSessions.items[activeSessionId]?.task_id : undefined, + ); + const agentctlReady = useAppStore((state) => + activeSessionId + ? state.sessionAgentctl.itemsBySessionId[activeSessionId]?.status === "ready" + : false, + ); const { commits } = useSessionCommits(activeSessionId ?? null); const { toast } = useToast(); const { openFile } = usePanelActions(); @@ -77,13 +86,12 @@ const CommitDetailPanel = memo(function CommitDetailPanel({ if (!activeSessionId) return; setLoading(true); try { - const ws = getWebSocketClient(); - if (!ws) return; - const response = await ws.request<{ success: boolean; files: Record }>( - "session.commit_diff", - { session_id: activeSessionId, commit_sha: commitSha }, - 10000, - ); + const response = await requestCommitDiff({ + sessionId: activeSessionId, + taskId: sessionTaskId ?? activeTaskId ?? null, + commitSha, + agentctlReady, + }); if (response?.success && response.files) { setFiles(response.files); } @@ -96,7 +104,7 @@ const CommitDetailPanel = memo(function CommitDetailPanel({ } finally { setLoading(false); } - }, [activeSessionId, commitSha, toast]); + }, [activeSessionId, activeTaskId, agentctlReady, commitSha, sessionTaskId, toast]); useEffect(() => { fetchDiff(); diff --git a/apps/web/components/task/commit-diff-request.ts b/apps/web/components/task/commit-diff-request.ts new file mode 100644 index 0000000000..10d8401188 --- /dev/null +++ b/apps/web/components/task/commit-diff-request.ts @@ -0,0 +1,80 @@ +"use client"; + +import type { FileInfo } from "@/lib/state/store"; +import { getWebSocketClient } from "@/lib/ws/connection"; + +type SessionStatusResponse = { + is_agent_running: boolean; +}; + +export type CommitDiffResponse = { + success: boolean; + files?: Record; + ready?: boolean; + reason?: string; + retry_after_ms?: number; +}; + +const notReadyProbeCooldownMs = 1000; +const nextProbeBySession = new Map(); + +function notReadyResult(retryAfterMs = 500): CommitDiffResponse { + return { + success: false, + ready: false, + reason: "agent_starting", + retry_after_ms: retryAfterMs, + }; +} + +export async function requestCommitDiff(params: { + sessionId: string; + taskId: string | null; + commitSha: string; + agentctlReady: boolean; +}): Promise { + const { sessionId, taskId, commitSha, agentctlReady } = params; + const ws = getWebSocketClient(); + if (!ws) return null; + + if (!agentctlReady) { + const now = Date.now(); + const nextProbeAt = nextProbeBySession.get(sessionId) ?? 0; + if (now < nextProbeAt) { + return notReadyResult(); + } + if (!taskId) { + nextProbeBySession.set(sessionId, now + notReadyProbeCooldownMs); + return notReadyResult(); + } + try { + const status = await ws.request( + "task.session.status", + { task_id: taskId, session_id: sessionId }, + 5000, + ); + if (!status?.is_agent_running) { + nextProbeBySession.set(sessionId, now + notReadyProbeCooldownMs); + return notReadyResult(); + } + } catch { + nextProbeBySession.set(sessionId, now + notReadyProbeCooldownMs); + return notReadyResult(); + } + } + + const response = await ws.request( + "session.commit_diff", + { session_id: sessionId, commit_sha: commitSha }, + 10000, + ); + if (response?.success) { + nextProbeBySession.delete(sessionId); + return response; + } + if (response?.ready === false) { + const retryAfter = response.retry_after_ms ?? notReadyProbeCooldownMs; + nextProbeBySession.set(sessionId, Date.now() + retryAfter); + } + return response; +} diff --git a/apps/web/components/task/task-files-panel-hooks.ts b/apps/web/components/task/task-files-panel-hooks.ts index c9efa2338f..9e88bd30e2 100644 --- a/apps/web/components/task/task-files-panel-hooks.ts +++ b/apps/web/components/task/task-files-panel-hooks.ts @@ -9,7 +9,6 @@ import { useGitOperations } from "@/hooks/use-git-operations"; import { useSessionFileReviews } from "@/hooks/use-session-file-reviews"; import { useCumulativeDiff } from "@/hooks/domains/session/use-cumulative-diff"; import { hashDiff, normalizeDiffContent } from "@/components/review/types"; -import { getWebSocketClient } from "@/lib/ws/connection"; import type { FileInfo } from "@/lib/state/store"; import { useToast } from "@/components/toast-provider"; import { useFileOperations } from "@/hooks/use-file-operations"; @@ -21,6 +20,7 @@ import { setUserSelectedFilesPanelTab, } from "@/lib/local-storage"; import { usePanelActions } from "@/hooks/use-panel-actions"; +import { requestCommitDiff } from "./commit-diff-request"; export type GitStatusFiles = Record< string, @@ -84,6 +84,15 @@ export function computeReviewProgress( } export function useCommitDiffs(activeSessionId: string | null | undefined) { + const activeTaskId = useAppStore((state) => state.tasks.activeTaskId); + const sessionTaskId = useAppStore((state) => + activeSessionId ? state.taskSessions.items[activeSessionId]?.task_id : undefined, + ); + const agentctlReady = useAppStore((state) => + activeSessionId + ? state.sessionAgentctl.itemsBySessionId[activeSessionId]?.status === "ready" + : false, + ); const [expandedCommit, setExpandedCommit] = useState(null); const [commitDiffs, setCommitDiffs] = useState>>({}); const [loadingCommitSha, setLoadingCommitSha] = useState(null); @@ -94,15 +103,16 @@ export function useCommitDiffs(activeSessionId: string | null | undefined) { if (!activeSessionId || commitDiffs[commitSha]) return; setLoadingCommitSha(commitSha); try { - const ws = getWebSocketClient(); - if (!ws) return; - const response = await ws.request<{ success: boolean; files: Record }>( - "session.commit_diff", - { session_id: activeSessionId, commit_sha: commitSha }, - 10000, - ); - if (response?.success && response.files) - setCommitDiffs((prev) => ({ ...prev, [commitSha]: response.files })); + const response = await requestCommitDiff({ + sessionId: activeSessionId, + taskId: sessionTaskId ?? activeTaskId ?? null, + commitSha, + agentctlReady, + }); + if (response?.success && response.files) { + const files = response.files; + setCommitDiffs((prev) => ({ ...prev, [commitSha]: files })); + } } catch (err) { toast({ title: "Failed to load commit diff", @@ -113,7 +123,7 @@ export function useCommitDiffs(activeSessionId: string | null | undefined) { setLoadingCommitSha(null); } }, - [activeSessionId, commitDiffs, toast], + [activeSessionId, activeTaskId, agentctlReady, commitDiffs, sessionTaskId, toast], ); const toggleCommit = useCallback(