From 59ee830d71d297ba76977cd4fff83b4bc9877545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Mon, 30 Mar 2026 22:44:10 +0100 Subject: [PATCH] feat(web): unified commit list and dockview panel fix (#500) --- .../internal/orchestrator/session_launch.go | 40 ++++++ apps/backend/internal/repoclone/clone.go | 2 +- apps/backend/internal/worktree/errors.go | 7 + apps/backend/internal/worktree/manager.go | 47 ++++++- .../worktree/manager_checkout_branch_test.go | 124 ++++++++++++++++++ .../task/changes-panel-timeline.tsx | 60 +-------- .../web/components/task/changes-panel.test.ts | 78 ++++++++++- apps/web/components/task/changes-panel.tsx | 101 +++++++++++--- apps/web/components/task/commit-row.tsx | 23 +++- .../web/components/task/task-page-content.tsx | 7 +- apps/web/e2e/pages/session-page.ts | 6 +- .../e2e/tests/git/git-changes-panel.spec.ts | 48 ++++--- apps/web/e2e/tests/pr/pr-detection.spec.ts | 4 +- .../e2e/tests/pr/pr-switcher-changes.spec.ts | 14 +- .../tests/terminal/terminal-keyboard.spec.ts | 19 ++- .../workflow/workflow-automation.spec.ts | 80 ++++++++++- .../domains/session/use-session-agent.ts | 10 +- .../lib/services/session-launch-helpers.ts | 2 + .../lib/services/session-launch-service.ts | 1 + .../web/lib/state/dockview-layout-builders.ts | 18 +++ 20 files changed, 572 insertions(+), 119 deletions(-) diff --git a/apps/backend/internal/orchestrator/session_launch.go b/apps/backend/internal/orchestrator/session_launch.go index 98a87204fa..6b785f3008 100644 --- a/apps/backend/internal/orchestrator/session_launch.go +++ b/apps/backend/internal/orchestrator/session_launch.go @@ -9,6 +9,7 @@ import ( "github.com/kandev/kandev/internal/orchestrator/executor" "github.com/kandev/kandev/internal/task/models" + wfmodels "github.com/kandev/kandev/internal/workflow/models" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -38,6 +39,7 @@ type LaunchSessionRequest struct { Priority int `json:"priority,omitempty"` LaunchWorkspace bool `json:"launch_workspace,omitempty"` SkipMessageRecord bool `json:"skip_message_record,omitempty"` + AutoStart bool `json:"auto_start,omitempty"` Attachments []v1.MessageAttachment `json:"attachments,omitempty"` } @@ -118,7 +120,15 @@ func (s *Service) launchPrepare(ctx context.Context, req *LaunchSessionRequest) } // launchStart creates a new session and launches the agent. +// If the request is an auto-start and the task's current workflow step does not +// have auto_start_agent, the request is downgraded to a prepare (workspace-only, +// no agent) to prevent unwanted auto-starts from the frontend's useAutoStartSession hook. func (s *Service) launchStart(ctx context.Context, req *LaunchSessionRequest) (*LaunchSessionResponse, error) { + if req.AutoStart && s.shouldBlockAutoStart(ctx, req) { + req.LaunchWorkspace = true + return s.launchPrepare(ctx, req) + } + execution, err := s.StartTask( ctx, req.TaskID, req.AgentProfileID, req.ExecutorID, req.ExecutorProfileID, req.Priority, req.Prompt, @@ -130,6 +140,36 @@ func (s *Service) launchStart(ctx context.Context, req *LaunchSessionRequest) (* return executionToLaunchResponse(req.TaskID, execution), nil } +// shouldBlockAutoStart checks whether the task's workflow step allows auto-starting +// the agent. Returns true when the step exists but does not have auto_start_agent +// in its on_enter events. Tasks without a workflow step are never blocked. +func (s *Service) shouldBlockAutoStart(ctx context.Context, req *LaunchSessionRequest) bool { + if s.workflowStepGetter == nil { + return false + } + + task, err := s.repo.GetTask(ctx, req.TaskID) + if err != nil || task.WorkflowStepID == "" { + return false + } + + step, err := s.workflowStepGetter.GetStep(ctx, task.WorkflowStepID) + if err != nil || step == nil { + return false + } + + if step.HasOnEnterAction(wfmodels.OnEnterAutoStartAgent) { + return false + } + + s.logger.Info("auto-start downgraded to prepare: step lacks auto_start_agent", + zap.String("task_id", req.TaskID), + zap.String("workflow_step_id", task.WorkflowStepID), + zap.String("step_name", step.Name)) + + return true +} + // launchStartCreated starts agent execution on an existing CREATED session. func (s *Service) launchStartCreated(ctx context.Context, req *LaunchSessionRequest) (*LaunchSessionResponse, error) { execution, err := s.StartCreatedSession( diff --git a/apps/backend/internal/repoclone/clone.go b/apps/backend/internal/repoclone/clone.go index 4074c6ef6f..1fe6e69299 100644 --- a/apps/backend/internal/repoclone/clone.go +++ b/apps/backend/internal/repoclone/clone.go @@ -122,7 +122,7 @@ func (c *Cloner) gitCmd(ctx context.Context, args ...string) *exec.Cmd { func (c *Cloner) fetch(ctx context.Context, repoPath string) { c.logger.Debug("repository already cloned, fetching", zap.String("path", repoPath)) - cmd := c.gitCmd(ctx, "-C", repoPath, "fetch", "--all", "--prune") + cmd := c.gitCmd(ctx, "-C", repoPath, "fetch", "--all", "--prune", "--force") if out, err := cmd.CombinedOutput(); err != nil { c.logger.Warn("git fetch failed (non-fatal)", zap.String("path", repoPath), diff --git a/apps/backend/internal/worktree/errors.go b/apps/backend/internal/worktree/errors.go index c4ebff776d..4bb0bb6915 100644 --- a/apps/backend/internal/worktree/errors.go +++ b/apps/backend/internal/worktree/errors.go @@ -66,6 +66,13 @@ func isBranchCheckedOutError(output string) bool { strings.Contains(out, "is already used by worktree at") } +// isFetchRefusedCheckedOut checks if a git fetch error is caused by the branch +// being checked out in another worktree. Git refuses to update a local branch +// ref that is currently checked out. +func isFetchRefusedCheckedOut(output string) bool { + return strings.Contains(strings.ToLower(output), "refusing to fetch into branch") +} + // ClassifyGitError wraps a raw git error with a user-friendly sentinel error // based on the command output. func ClassifyGitError(output string, _ error) error { diff --git a/apps/backend/internal/worktree/manager.go b/apps/backend/internal/worktree/manager.go index 6e90fa1871..a9ecbe01bb 100644 --- a/apps/backend/internal/worktree/manager.go +++ b/apps/backend/internal/worktree/manager.go @@ -278,7 +278,11 @@ func (m *Manager) createWorktree(ctx context.Context, req CreateRequest, baseRef if err != nil { return nil, err } - startPoint = req.CheckoutBranch + if fetchResult.StartPoint != "" { + startPoint = fetchResult.StartPoint + } else { + startPoint = req.CheckoutBranch + } } worktreePath, err := m.config.WorktreePath(worktreeDirName) @@ -372,6 +376,8 @@ func (m *Manager) createInTaskDir(ctx context.Context, req CreateRequest, baseRe // addWorktreeForBranch creates the git worktree, trying the checkout branch directly first // and falling back to a suffixed branch if the checkout branch is already in use. +// When a checkout branch is specified, it sets the upstream tracking branch to +// origin/ so ahead/behind counts are relative to the PR's remote branch. func (m *Manager) addWorktreeForBranch(ctx context.Context, req CreateRequest, worktreePath, fallbackBranch, startPoint, baseRef string) (string, string, error) { if req.CheckoutBranch == "" { id, err := m.gitAddWorktree(ctx, req.RepositoryPath, fallbackBranch, worktreePath, baseRef) @@ -381,6 +387,7 @@ func (m *Manager) addWorktreeForBranch(ctx context.Context, req CreateRequest, w // Try checking out the PR branch directly (common case: single task per PR). id, err := m.gitAddWorktreeExisting(ctx, req.RepositoryPath, req.CheckoutBranch, worktreePath) if err == nil { + m.setUpstreamIfExists(ctx, worktreePath, req.CheckoutBranch, req.CheckoutBranch) return id, req.CheckoutBranch, nil } if !errors.Is(err, ErrBranchCheckedOut) { @@ -391,11 +398,36 @@ func (m *Manager) addWorktreeForBranch(ctx context.Context, req CreateRequest, w // using the original branch name with a random suffix. suffixed := req.CheckoutBranch + "-" + SmallSuffix(3) id, err = m.gitAddWorktree(ctx, req.RepositoryPath, suffixed, worktreePath, startPoint) + if err == nil { + m.setUpstreamIfExists(ctx, worktreePath, suffixed, req.CheckoutBranch) + } return id, suffixed, err } +// setUpstreamIfExists sets the upstream tracking branch for a worktree branch +// to origin/ if the remote-tracking ref exists. Non-fatal on failure. +func (m *Manager) setUpstreamIfExists(ctx context.Context, worktreePath, localBranch, remoteBranch string) { + upstream := "origin/" + remoteBranch + // Verify the remote-tracking ref exists + verifyCmd := exec.CommandContext(ctx, "git", "rev-parse", "--verify", upstream) + verifyCmd.Dir = worktreePath + if err := verifyCmd.Run(); err != nil { + return + } + cmd := exec.CommandContext(ctx, "git", "branch", "--set-upstream-to="+upstream, localBranch) + cmd.Dir = worktreePath + if out, err := cmd.CombinedOutput(); err != nil { + m.logger.Debug("failed to set upstream (non-fatal)", + zap.String("branch", localBranch), + zap.String("upstream", upstream), + zap.String("output", string(out)), + zap.Error(err)) + } +} + // FetchBranchResult holds the outcome of a fetchBranchToLocal call. type FetchBranchResult struct { + StartPoint string // Ref to use as worktree start point (e.g., "origin/branch"); empty = use local branch Warning string // User-friendly warning (non-empty when fell back to local) WarningDetail string // Raw git command output for debugging } @@ -417,6 +449,19 @@ func (m *Manager) fetchBranchToLocal(ctx context.Context, repoPath, branch strin fetchCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, "fetch", "origin", branch+":"+branch) if output, err := fetchCmd.CombinedOutput(); err != nil { outputStr := string(output) + + // If the branch is checked out in another worktree, git refuses to update + // the local ref. Retry by fetching only the remote-tracking ref (origin/branch), + // which is always safe regardless of worktree state. + if isFetchRefusedCheckedOut(outputStr) { + retryCmd := m.newNonInteractiveGitCmd(fetchCtx, repoPath, "fetch", "origin", branch) + if _, retryErr := retryCmd.CombinedOutput(); retryErr == nil { + m.logger.Info("fetched via remote-tracking ref (branch checked out elsewhere)", + zap.String("branch", branch)) + return &FetchBranchResult{StartPoint: "origin/" + branch}, nil + } + } + m.logger.Warn("fetch from origin failed, checking local branch", zap.String("branch", branch), zap.String("output", outputStr), diff --git a/apps/backend/internal/worktree/manager_checkout_branch_test.go b/apps/backend/internal/worktree/manager_checkout_branch_test.go index d60fd0144c..1144d037a6 100644 --- a/apps/backend/internal/worktree/manager_checkout_branch_test.go +++ b/apps/backend/internal/worktree/manager_checkout_branch_test.go @@ -110,6 +110,130 @@ func TestCreateWorktree_CheckoutBranchFallsBackToSuffix(t *testing.T) { } } +// TestCreateWorktree_CheckoutBranchNoFetchWarningWithRemote verifies that when +// a branch is checked out in one worktree, creating a second worktree for the +// same branch does NOT produce a fetch warning. The fetch retries with a +// remote-tracking ref update instead of trying to update the checked-out local branch. +func TestCreateWorktree_CheckoutBranchNoFetchWarningWithRemote(t *testing.T) { + cfg := newTestConfig(t) + log := newTestLogger() + store := newMockStore() + + mgr, err := NewManager(cfg, store, log) + if err != nil { + t.Fatalf("NewManager failed: %v", err) + } + + repoPath := initGitRepoWithRemote(t) + + // First worktree checks out the branch directly. + wt1, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-1", + SessionID: "session-1", + TaskTitle: "PR Review 1", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "main", + CheckoutBranch: "feature/pr-branch", + }) + if err != nil { + t.Fatalf("Create() first worktree: %v", err) + } + if wt1.Branch != "feature/pr-branch" { + t.Fatalf("first worktree: expected direct branch, got %q", wt1.Branch) + } + if wt1.FetchWarning != "" { + t.Fatalf("first worktree: unexpected fetch warning: %s", wt1.FetchWarning) + } + + // Second worktree with the same checkout branch should fall back to a + // suffixed name but should NOT produce a fetch warning — the fetch retries + // via remote-tracking ref when the local branch is checked out. + wt2, err := mgr.Create(context.Background(), CreateRequest{ + TaskID: "task-2", + SessionID: "session-2", + TaskTitle: "PR Review 2", + RepositoryID: "repo-1", + RepositoryPath: repoPath, + BaseBranch: "main", + CheckoutBranch: "feature/pr-branch", + }) + if err != nil { + t.Fatalf("Create() second worktree: %v", err) + } + if wt2.Branch == "feature/pr-branch" { + t.Fatal("second worktree should NOT use the checkout branch directly") + } + if !strings.HasPrefix(wt2.Branch, "feature/pr-branch-") { + t.Fatalf("expected suffixed branch like %q, got %q", "feature/pr-branch-xxx", wt2.Branch) + } + if wt2.FetchWarning != "" { + t.Fatalf("second worktree: unexpected fetch warning: %s", wt2.FetchWarning) + } + + // Both worktrees should point to the same commit (latest from origin). + sha1 := strings.TrimSpace(runGit(t, wt1.Path, "rev-parse", "HEAD")) + sha2 := strings.TrimSpace(runGit(t, wt2.Path, "rev-parse", "HEAD")) + if sha1 != sha2 { + t.Fatalf("worktree SHAs differ: wt1=%q, wt2=%q", sha1, sha2) + } + + // Both worktrees should have upstream set to origin/feature/pr-branch + // so that "git rev-list branch...@{upstream}" gives correct ahead/behind counts. + const wantUpstream = "origin/feature/pr-branch" + upstream1 := strings.TrimSpace(runGit(t, wt1.Path, "rev-parse", "--abbrev-ref", "@{upstream}")) + if upstream1 != wantUpstream { + t.Fatalf("first worktree upstream = %q, want %q", upstream1, wantUpstream) + } + upstream2 := strings.TrimSpace(runGit(t, wt2.Path, "rev-parse", "--abbrev-ref", "@{upstream}")) + if upstream2 != wantUpstream { + t.Fatalf("second worktree upstream = %q, want %q", upstream2, wantUpstream) + } +} + +// initGitRepoWithRemote creates a bare "origin" repo, clones it, and creates +// a feature branch with a commit. Returns the clone path (has origin remote). +func initGitRepoWithRemote(t *testing.T) string { + t.Helper() + + // Create a bare repo to serve as "origin" + bareDir := filepath.Join(t.TempDir(), "origin.git") + runGit(t, t.TempDir(), "init", "--bare", "-b", "main", bareDir) + + // Clone it to get a working repo with origin + cloneDir := filepath.Join(t.TempDir(), "clone") + cmd := exec.Command("git", "clone", bareDir, cloneDir) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git clone failed: %v\n%s", err, out) + } + runGit(t, cloneDir, "config", "user.email", "test@example.com") + runGit(t, cloneDir, "config", "user.name", "Test User") + runGit(t, cloneDir, "config", "commit.gpgsign", "false") + + // Create initial commit on main + filePath := filepath.Join(cloneDir, "README.md") + if err := os.WriteFile(filePath, []byte("initial\n"), 0644); err != nil { + t.Fatalf("failed to write README.md: %v", err) + } + runGit(t, cloneDir, "add", "README.md") + runGit(t, cloneDir, "commit", "-m", "initial commit") + runGit(t, cloneDir, "push", "origin", "main") + + // Create feature branch with a commit + runGit(t, cloneDir, "checkout", "-b", "feature/pr-branch") + if err := os.WriteFile(filePath, []byte("feature change\n"), 0644); err != nil { + t.Fatalf("failed to update README.md: %v", err) + } + runGit(t, cloneDir, "add", "README.md") + runGit(t, cloneDir, "commit", "-m", "feature commit") + runGit(t, cloneDir, "push", "origin", "feature/pr-branch") + + // Go back to main so the branch isn't checked out in the main repo + runGit(t, cloneDir, "checkout", "main") + + return cloneDir +} + func initGitRepoForWorktreeTest(t *testing.T) string { t.Helper() diff --git a/apps/web/components/task/changes-panel-timeline.tsx b/apps/web/components/task/changes-panel-timeline.tsx index 9b01c0cb65..550918a017 100644 --- a/apps/web/components/task/changes-panel-timeline.tsx +++ b/apps/web/components/task/changes-panel-timeline.tsx @@ -2,7 +2,6 @@ import { useState } from "react"; import { - IconGitCommit, IconGitPullRequest, IconCloudUpload, IconChevronDown, @@ -140,6 +139,9 @@ export function CommitsSection({ onAmendCommit, onResetToCommit, }: CommitsSectionProps) { + // Find the first unpushed commit for context menu actions (amend/revert only on latest unpushed) + const firstUnpushedIndex = commits.findIndex((c) => c.pushed !== true); + return ( ))} @@ -434,54 +436,6 @@ function PRFileRow({ ); } -// --- PR commits section --- - -type PRCommitItem = { - sha: string; - message: string; - author_login: string; - author_date: string; -}; - -type PRCommitsSectionProps = { - commits: PRCommitItem[]; - isLast: boolean; - onOpenCommitDetail?: (sha: string) => void; -}; - -export function PRCommitsSection({ commits, isLast, onOpenCommitDetail }: PRCommitsSectionProps) { - return ( - -
    - {commits.map((commit, index) => ( -
  • onOpenCommitDetail?.(commit.sha)} - > - - - {commit.sha.slice(0, 7)} - - {commit.message} - {commit.author_login && ( - - {commit.author_login} - - )} -
    • - ))} -
- - ); -} - // --- Review progress bar --- type ReviewProgressBarProps = { diff --git a/apps/web/components/task/changes-panel.test.ts b/apps/web/components/task/changes-panel.test.ts index df222b54b2..8b714a75c6 100644 --- a/apps/web/components/task/changes-panel.test.ts +++ b/apps/web/components/task/changes-panel.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { filterUnpushedCommits } from "./changes-panel"; +import { filterUnpushedCommits, mergeCommits } from "./changes-panel"; const makeLocal = (sha: string, message = "msg") => ({ commit_sha: sha, @@ -38,3 +38,79 @@ describe("filterUnpushedCommits", () => { expect(filterUnpushedCommits([], pr)).toEqual([]); }); }); + +const makePRFull = (sha: string, message = "msg", author = "user") => ({ + sha, + message, + author_login: author, + author_date: "2026-03-29T00:00:00Z", + additions: 5, + deletions: 2, + files_changed: 1, +}); + +describe("mergeCommits", () => { + it("marks all local commits as unpushed when no PR commits exist", () => { + const local = [makeLocal("aaa1111", "first"), makeLocal("bbb2222", "second")]; + const result = mergeCommits(local, []); + expect(result).toEqual([ + { + commit_sha: "aaa1111", + commit_message: "first", + insertions: 1, + deletions: 0, + pushed: false, + }, + { + commit_sha: "bbb2222", + commit_message: "second", + insertions: 1, + deletions: 0, + pushed: false, + }, + ]); + }); + + it("marks local commits as pushed when they match PR commits", () => { + const local = [makeLocal("aaa1111", "first")]; + const pr = [makePRFull("aaa1111bbbccc", "first", "user")]; + const result = mergeCommits(local, pr); + expect(result).toEqual([ + { commit_sha: "aaa1111", commit_message: "first", insertions: 1, deletions: 0, pushed: true }, + ]); + }); + + it("includes PR-only commits (from other contributors) as pushed", () => { + const local: ReturnType[] = []; + const pr = [makePRFull("ccc3333", "external fix", "other-dev")]; + const result = mergeCommits(local, pr); + expect(result).toEqual([ + { + commit_sha: "ccc3333", + commit_message: "external fix", + insertions: 5, + deletions: 2, + pushed: true, + }, + ]); + }); + + it("orders unpushed commits first, then pushed", () => { + const local = [makeLocal("aaa1111", "pushed one"), makeLocal("bbb2222", "unpushed one")]; + const pr = [makePRFull("aaa1111fff", "pushed one")]; + const result = mergeCommits(local, pr); + expect(result[0]).toMatchObject({ commit_sha: "bbb2222", pushed: false }); + expect(result[1]).toMatchObject({ commit_sha: "aaa1111", pushed: true }); + }); + + it("handles mixed local, matched, and PR-only commits", () => { + const local = [makeLocal("aaa1111", "local pushed"), makeLocal("bbb2222", "local only")]; + const pr = [makePRFull("aaa1111fff", "local pushed"), makePRFull("ddd4444", "external")]; + const result = mergeCommits(local, pr); + // unpushed first, then pushed (local matched + PR-only) + expect(result).toHaveLength(3); + expect(result[0]).toMatchObject({ commit_sha: "bbb2222", pushed: false }); + expect(result[1]).toMatchObject({ commit_sha: "aaa1111", pushed: true }); + expect(result[2]).toMatchObject({ commit_sha: "ddd4444", pushed: true }); + }); +}); diff --git a/apps/web/components/task/changes-panel.tsx b/apps/web/components/task/changes-panel.tsx index 0bc0371d70..90cd91d0cf 100644 --- a/apps/web/components/task/changes-panel.tsx +++ b/apps/web/components/task/changes-panel.tsx @@ -19,7 +19,6 @@ import { ActionButtonsSection, ReviewProgressBar, PRFilesSection, - PRCommitsSection, } from "./changes-panel-timeline"; import type { PRChangedFile } from "./changes-panel-timeline"; import { useChangesGitHandlers, useChangesDialogHandlers } from "./changes-panel-hooks"; @@ -163,6 +162,65 @@ export function filterUnpushedCommits( ); } +type MergedCommit = { + commit_sha: string; + commit_message: string; + insertions: number; + deletions: number; + pushed: boolean; +}; + +/** + * Merge local session commits and PR commits into a single list. + * Local commits matched to a PR commit are marked pushed; unmatched are unpushed. + * PR-only commits (e.g. from other contributors) are appended as pushed. + * Order: unpushed first, then pushed. + */ +export function mergeCommits( + localCommits: { + commit_sha: string; + commit_message: string; + insertions: number; + deletions: number; + }[], + prCommits: { sha: string; message: string; additions: number; deletions: number }[], +): MergedCommit[] { + const matchesPR = (localSha: string) => + prCommits.some((pr) => pr.sha.startsWith(localSha) || localSha.startsWith(pr.sha)); + + const unpushed: MergedCommit[] = []; + const pushed: MergedCommit[] = []; + const matchedPRShas = new Set(); + + for (const c of localCommits) { + if (matchesPR(c.commit_sha)) { + pushed.push({ ...c, pushed: true }); + for (const pr of prCommits) { + if (pr.sha.startsWith(c.commit_sha) || c.commit_sha.startsWith(pr.sha)) { + matchedPRShas.add(pr.sha); + } + } + } else { + unpushed.push({ ...c, pushed: false }); + } + } + + // Add PR-only commits (not matched to any local commit) + for (const pr of prCommits) { + if (!matchedPRShas.has(pr.sha)) { + pushed.push({ + commit_sha: pr.sha, + commit_message: pr.message, + insertions: pr.additions, + deletions: pr.deletions, + pushed: true, + }); + } + } + + return [...unpushed, ...pushed]; +} + function getBaseBranchDisplay(baseBranch: string | undefined): string { return baseBranch ? baseBranch.replace(/^origin\//, "") : "main"; } @@ -202,8 +260,21 @@ type ChangesPanelBodyProps = { unstagedFiles: ChangedFile[]; stagedFiles: ChangedFile[]; prFiles: PRChangedFile[]; - prCommits: { sha: string; message: string; author_login: string; author_date: string }[]; - commits: ReturnType["commits"]; + prCommits: { + sha: string; + message: string; + author_login: string; + author_date: string; + additions: number; + deletions: number; + }[]; + commits: { + commit_sha: string; + commit_message: string; + insertions: number; + deletions: number; + pushed?: boolean; + }[]; pendingStageFiles: Set; reviewedCount: number; totalFileCount: number; @@ -265,7 +336,6 @@ type TimelineProps = Pick< | "hasStaged" | "hasCommits" | "hasPRFiles" - | "hasPRCommits" | "canPush" | "canCreatePR" | "existingPrUrl" @@ -340,7 +410,7 @@ function TimelineLocalChanges(props: TimelineProps & { hasCommitsToShow?: boolea {showCommitsList && ( 0; - const hasLocalChanges = props.hasUnstaged || props.hasStaged || hasUnpushedCommits; + const mergedCommits = mergeCommits(props.commits, props.prCommits); + const hasMergedCommits = mergedCommits.length > 0; + const hasLocalChanges = props.hasUnstaged || props.hasStaged || hasMergedCommits; return (
@@ -383,24 +453,15 @@ function ChangesPanelTimeline(props: TimelineProps) {
-
- )} - {props.hasPRCommits && ( -
-
)}
); diff --git a/apps/web/components/task/commit-row.tsx b/apps/web/components/task/commit-row.tsx index d6a0a42a2a..2fb643ae03 100644 --- a/apps/web/components/task/commit-row.tsx +++ b/apps/web/components/task/commit-row.tsx @@ -1,6 +1,12 @@ "use client"; -import { IconGitCommit, IconArrowBackUp, IconPencil, IconHistoryToggle } from "@tabler/icons-react"; +import { + IconGitCommit, + IconArrowBackUp, + IconPencil, + IconHistoryToggle, + IconArrowUp, +} from "@tabler/icons-react"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; import { @@ -15,6 +21,7 @@ export type CommitItem = { commit_message: string; insertions: number; deletions: number; + pushed?: boolean; }; /** Context menu for commit items */ @@ -179,7 +186,19 @@ export function CommitRow({ } }} > - + + + {commit.pushed === true ? "Pushed commit" : "Unpushed commit"} + + {commit.pushed === true ? ( + {commit.commit_sha.slice(0, 7)} diff --git a/apps/web/components/task/task-page-content.tsx b/apps/web/components/task/task-page-content.tsx index 26e46bd6ac..b2618cf598 100644 --- a/apps/web/components/task/task-page-content.tsx +++ b/apps/web/components/task/task-page-content.tsx @@ -492,7 +492,10 @@ function useTaskDetails(activeTaskId: string | null, initialTask: Task | null) { function useAutoStartSession( task: Task | null, - handleStartAgent: (agentProfileId: string, prompt?: string) => Promise, + handleStartAgent: ( + agentProfileId: string, + opts?: { prompt?: string; autoStart?: boolean }, + ) => Promise, ) { const { isLoaded } = useTaskSessions(task?.id ?? null); const sessions = useAppStore((state) => @@ -515,7 +518,7 @@ function useAutoStartSession( if (!agentProfileId) return; if (startedRef.current) return; startedRef.current = true; - handleStartAgent(agentProfileId).catch(() => { + handleStartAgent(agentProfileId, { autoStart: true }).catch(() => { startedRef.current = false; }); }, [task?.id, isLoaded, sessions.length, agentProfileId, handleStartAgent]); diff --git a/apps/web/e2e/pages/session-page.ts b/apps/web/e2e/pages/session-page.ts index 3571197a15..3364c44f03 100644 --- a/apps/web/e2e/pages/session-page.ts +++ b/apps/web/e2e/pages/session-page.ts @@ -301,9 +301,9 @@ export class SessionPage { return this.changes.getByTestId("pr-files-section"); } - /** PR commits section within the changes panel. */ - prCommitsSection(): Locator { - return this.changes.getByTestId("pr-commits-section"); + /** Commits section within the changes panel (unified list of pushed + unpushed commits). */ + commitsSection(): Locator { + return this.changes.getByTestId("commits-section"); } /** diff --git a/apps/web/e2e/tests/git/git-changes-panel.spec.ts b/apps/web/e2e/tests/git/git-changes-panel.spec.ts index 68305f304b..5c16c72411 100644 --- a/apps/web/e2e/tests/git/git-changes-panel.spec.ts +++ b/apps/web/e2e/tests/git/git-changes-panel.spec.ts @@ -1128,7 +1128,7 @@ test.describe("Git Changes Panel", () => { * Verifies that local commits section is hidden when all commits are already * in the PR. Only unpushed commits should be shown in the local section. */ - test("hides local commits section when all commits are in PR", async ({ + test("shows pushed commits with git-commit icon when all commits are in PR", async ({ testPage, apiClient, seedData, @@ -1200,18 +1200,21 @@ test.describe("Git Changes Panel", () => { await session.clickTab("Changes"); await expect(session.changes).toBeVisible({ timeout: 10_000 }); - // PR commits section should be visible - await expect(session.prCommitsSection()).toBeVisible({ timeout: 15_000 }); - - // Local commits section should be hidden (all commits are in the PR) - await expect(testPage.getByTestId("commits-section")).not.toBeVisible({ timeout: 10_000 }); + // Unified commits section should show the commit as pushed (git-commit icon, not arrow-up) + await expect(testPage.getByTestId("commits-section")).toBeVisible({ timeout: 15_000 }); + await expect(session.changes.getByText("Commit in PR")).toBeVisible({ timeout: 10_000 }); + const commitsList = testPage.getByTestId("commits-list"); + await expect(commitsList.locator("li")).toHaveCount(1, { timeout: 5_000 }); + // Pushed commits use IconGitCommit (tabler-icon-git-commit), not IconArrowUp + await expect(commitsList.locator(".tabler-icon-git-commit")).toBeVisible({ timeout: 5_000 }); + await expect(commitsList.locator(".tabler-icon-arrow-up")).not.toBeVisible(); }); /** - * Verifies that only unpushed commits appear in the local section - * when some commits are already in the PR. + * Verifies that pushed and unpushed commits are visually distinguished + * in the unified commits list. */ - test("shows only unpushed commits when some commits are in PR", async ({ + test("distinguishes pushed and unpushed commits in unified list", async ({ testPage, apiClient, seedData, @@ -1252,7 +1255,7 @@ test.describe("Git Changes Panel", () => { git.createFile("unpushed.txt", "unpushed content"); git.stageFile("unpushed.txt"); - const unpushedSha = git.commit("Unpushed commit"); + git.commit("Unpushed commit"); // Mock a PR that only contains the first commit await apiClient.mockGitHubAssociateTaskPR({ @@ -1281,18 +1284,21 @@ test.describe("Git Changes Panel", () => { await session.clickTab("Changes"); await expect(session.changes).toBeVisible({ timeout: 10_000 }); - // PR commits section should show - await expect(session.prCommitsSection()).toBeVisible({ timeout: 15_000 }); - - // Local commits section should show only the unpushed commit + // Unified commits section should show both commits await expect(testPage.getByTestId("commits-section")).toBeVisible({ timeout: 15_000 }); - await expect(session.changes.getByText("Unpushed commit")).toBeVisible({ timeout: 10_000 }); - await expect(session.changes.getByText(unpushedSha.slice(0, 7))).toBeVisible({ - timeout: 5_000, - }); - - // The local commits section should only contain the unpushed commit const commitsList = testPage.getByTestId("commits-list"); - await expect(commitsList.locator("li")).toHaveCount(1, { timeout: 5_000 }); + await expect(commitsList.locator("li")).toHaveCount(2, { timeout: 5_000 }); + + // Unpushed commit should have arrow-up icon (emerald) + const unpushedRow = commitsList.locator("li", { hasText: "Unpushed commit" }); + await expect(unpushedRow).toBeVisible({ timeout: 10_000 }); + await expect(unpushedRow.locator(".tabler-icon-arrow-up")).toBeVisible({ timeout: 5_000 }); + + // Pushed commit should have git-commit icon (muted) + const pushedRow = commitsList.locator( + `li:has-text("Pushed commit"):not(:has-text("Unpushed"))`, + ); + await expect(pushedRow).toBeVisible({ timeout: 10_000 }); + await expect(pushedRow.locator(".tabler-icon-git-commit")).toBeVisible({ timeout: 5_000 }); }); }); diff --git a/apps/web/e2e/tests/pr/pr-detection.spec.ts b/apps/web/e2e/tests/pr/pr-detection.spec.ts index 96ddb40f66..defa0ab801 100644 --- a/apps/web/e2e/tests/pr/pr-detection.spec.ts +++ b/apps/web/e2e/tests/pr/pr-detection.spec.ts @@ -329,7 +329,7 @@ test.describe("PR external detection", () => { await expect(session.prFilesSection().getByText("feature.ts")).toBeVisible(); await expect(session.prFilesSection().getByText("index.ts")).toBeVisible(); - await expect(session.prCommitsSection()).toBeVisible(); - await expect(session.prCommitsSection().getByText("implement feature X")).toBeVisible(); + await expect(session.commitsSection()).toBeVisible(); + await expect(session.commitsSection().getByText("implement feature X")).toBeVisible(); }); }); diff --git a/apps/web/e2e/tests/pr/pr-switcher-changes.spec.ts b/apps/web/e2e/tests/pr/pr-switcher-changes.spec.ts index ab9bdc684e..cdbedfe6d9 100644 --- a/apps/web/e2e/tests/pr/pr-switcher-changes.spec.ts +++ b/apps/web/e2e/tests/pr/pr-switcher-changes.spec.ts @@ -193,8 +193,8 @@ test.describe("PR switcher changes panel", () => { await expect(session.prFilesSection().getByText("auth.go")).toBeVisible(); await expect(session.prFilesSection().getByText("auth_test.go")).toBeVisible(); - await expect(session.prCommitsSection()).toBeVisible(); - await expect(session.prCommitsSection().getByText("fix auth token expiry")).toBeVisible(); + await expect(session.commitsSection()).toBeVisible(); + await expect(session.commitsSection().getByText("fix auth token expiry")).toBeVisible(); // --- Switch to Task B --- await session.taskInSidebar("Dashboard Task").click(); @@ -210,9 +210,9 @@ test.describe("PR switcher changes panel", () => { // Verify Task A files are NOT visible await expect(session.prFilesSection().getByText("auth.go")).not.toBeVisible(); - await expect(session.prCommitsSection()).toBeVisible(); - await expect(session.prCommitsSection().getByText("add dashboard component")).toBeVisible(); - await expect(session.prCommitsSection().getByText("add api client")).toBeVisible(); + await expect(session.commitsSection()).toBeVisible(); + await expect(session.commitsSection().getByText("add dashboard component")).toBeVisible(); + await expect(session.commitsSection().getByText("add api client")).toBeVisible(); // --- Switch to Task C (no PR) --- await session.taskInSidebar("No PR Task").click(); @@ -221,7 +221,7 @@ test.describe("PR switcher changes panel", () => { // PR sections should NOT be visible for a task without a PR await expect(session.prFilesSection()).not.toBeVisible({ timeout: 10_000 }); - await expect(session.prCommitsSection()).not.toBeVisible(); + await expect(session.commitsSection()).not.toBeVisible(); // --- Switch back to Task A to confirm data reappears --- await session.taskInSidebar("Auth Fix Task").click(); @@ -231,6 +231,6 @@ test.describe("PR switcher changes panel", () => { await expect(session.prFilesSection()).toBeVisible({ timeout: 15_000 }); await expect(session.prFilesSection().getByText("auth.go")).toBeVisible(); await expect(session.prFilesSection().getByText("auth_test.go")).toBeVisible(); - await expect(session.prCommitsSection().getByText("fix auth token expiry")).toBeVisible(); + await expect(session.commitsSection().getByText("fix auth token expiry")).toBeVisible(); }); }); diff --git a/apps/web/e2e/tests/terminal/terminal-keyboard.spec.ts b/apps/web/e2e/tests/terminal/terminal-keyboard.spec.ts index 200b7b477b..3ef1984cd8 100644 --- a/apps/web/e2e/tests/terminal/terminal-keyboard.spec.ts +++ b/apps/web/e2e/tests/terminal/terminal-keyboard.spec.ts @@ -97,8 +97,9 @@ test.describe("Terminal keyboard navigation", () => { const session = await seedTaskWithSession(testPage, apiClient, seedData, "CtrlArrow Test"); await focusTerminal(testPage, session); - // Start bash explicitly — it has Ctrl+Arrow bindings via /etc/inputrc - // (zsh without .zshrc doesn't bind Ctrl+Arrow by default) + // Start bash explicitly (zsh without .zshrc doesn't bind Ctrl+Arrow). + // Use --norc --noprofile so readline starts clean, then bind Ctrl+Arrow + // manually — macOS /bin/bash (3.2) lacks /etc/inputrc with word-movement. await testPage.keyboard.type("bash --norc --noprofile"); await testPage.keyboard.press("Enter"); // Wait for bash prompt (bash --norc uses "bash-X.Y$ " pattern) @@ -109,6 +110,20 @@ test.describe("Terminal keyboard navigation", () => { }) .toBe(true); + // Bind Ctrl+Arrow to word movement (required on macOS bash 3.2 which has + // no /etc/inputrc; harmless on Linux bash 5.x where it's already bound) + await testPage.keyboard.type("bind '\"\\e[1;5D\": backward-word'"); + await testPage.keyboard.press("Enter"); + await testPage.keyboard.type("bind '\"\\e[1;5C\": forward-word'"); + await testPage.keyboard.press("Enter"); + // Wait for prompt to return after bind commands + await expect + .poll(async () => (await readTerminalBuffer(testPage)).includes("bash"), { + timeout: 5_000, + message: "Waiting for bash prompt after bind", + }) + .toBe(true); + // Type a multi-word echo command (don't press Enter yet) await testPage.keyboard.type("echo aaa bbb ccc"); diff --git a/apps/web/e2e/tests/workflow/workflow-automation.spec.ts b/apps/web/e2e/tests/workflow/workflow-automation.spec.ts index bf7285ec5e..f130718301 100644 --- a/apps/web/e2e/tests/workflow/workflow-automation.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-automation.spec.ts @@ -133,9 +133,11 @@ test.describe("Workflow automation", () => { await apiClient.createWorkflowStep(workflow.id, "Review", 2); const doneStep = await apiClient.createWorkflowStep(workflow.id, "Done", 3); - // Todo: on_turn_complete moves task to In Progress after the first agent turn. + // Todo: auto_start_agent on enter (so useAutoStartSession can start the agent), + // on_turn_complete moves task to In Progress after the first agent turn. await apiClient.updateWorkflowStep(todoStep.id, { events: { + on_enter: [{ type: "auto_start_agent" }], on_turn_complete: [{ type: "move_to_next" }], }, }); @@ -548,6 +550,82 @@ test.describe("Workflow automation", () => { }); }); + /** + * Seeds a 2-step workflow (Waiting → In Progress). + * "Waiting" has NO events — it's a manual holding step. + * "In Progress" has on_enter: [auto_start_agent]. + * + * Creates a task in "Waiting" and navigates to it. The frontend's + * useAutoStartSession hook fires session.launch, but the backend should + * block auto-start because the step lacks auto_start_agent. + * + * Verifies: + * - Opening a task in a non-auto-start step does NOT launch the agent + * - The idle input is visible (user can manually start) + * - Manually sending a message DOES start the agent + */ + test("non-auto-start step blocks agent launch on task open", async ({ + testPage, + apiClient, + seedData, + }) => { + const workflow = await apiClient.createWorkflow(seedData.workspaceId, "Waiting Step Workflow"); + + // Waiting step: no events at all + const waitingStep = await apiClient.createWorkflowStep(workflow.id, "Waiting", 0); + // In Progress: has auto_start_agent (not used in this test, just to show contrast) + await apiClient.createWorkflowStep(workflow.id, "In Progress", 1); + + await apiClient.saveUserSettings({ + workspace_id: seedData.workspaceId, + workflow_filter_id: workflow.id, + enable_preview_on_click: false, + }); + + const agentProfileId = seedData.agentProfileId; + + // Create task in Waiting step + const task = await apiClient.createTask(seedData.workspaceId, "Waiting Task", { + description: "/e2e:simple-message", + workflow_id: workflow.id, + workflow_step_id: waitingStep.id, + agent_profile_id: agentProfileId, + repository_ids: [seedData.repositoryId], + }); + + // Navigate to kanban, then click the task card to open the task page. + // useAutoStartSession fires session.launch when the task page loads. + const kanban = new KanbanPage(testPage); + await kanban.goto(); + + const card = testPage.locator('[data-testid^="task-card-"]', { + has: testPage.locator('[data-testid="task-card-title"]', { hasText: "Waiting Task" }), + }); + await expect(card).toBeVisible({ timeout: 10_000 }); + await card.click(); + await expect(testPage).toHaveURL(/\/t\//, { timeout: 15_000 }); + + const session = new SessionPage(testPage); + await session.waitForLoad(); + + // Wait for the idle input (chat is ready) + await expect(session.idleInput()).toBeVisible({ timeout: 15_000 }); + + // The backend downgraded auto-start to prepare: session exists but agent + // was NOT started. Verify session was created in CREATED state. + const sessions = await apiClient.listTaskSessions(task.id); + expect(sessions.total).toBe(1); + expect(sessions.sessions[0].state).toBe("CREATED"); + + // The chat should show the empty state — no agent messages + await expect(testPage.getByText("No messages yet")).toBeVisible(); + + // Stepper shows Waiting as current step (task didn't move) + await expect(session.stepperStep("Waiting")).toHaveAttribute("aria-current", "step", { + timeout: 5_000, + }); + }); + // Note: "step prompt without {{task_prompt}} replaces task description" is tested // by backend unit tests (TestBuildWorkflowPrompt_UsesStepPromptOnlyWithoutTaskPromptPlaceholder). // The auto_start flow for new sessions uses a different prompt resolution path diff --git a/apps/web/hooks/domains/session/use-session-agent.ts b/apps/web/hooks/domains/session/use-session-agent.ts index 9e6be89707..5a3581d164 100644 --- a/apps/web/hooks/domains/session/use-session-agent.ts +++ b/apps/web/hooks/domains/session/use-session-agent.ts @@ -14,7 +14,10 @@ interface UseSessionAgentReturn { taskSessionState: TaskSessionState | null; worktreePath: string | null; worktreeBranch: string | null; - handleStartAgent: (agentProfileId: string, prompt?: string) => Promise; + handleStartAgent: ( + agentProfileId: string, + opts?: { prompt?: string; autoStart?: boolean }, + ) => Promise; handleStopAgent: () => Promise; } @@ -47,14 +50,15 @@ export function useSessionAgent(task: Task | null): UseSessionAgentReturn { const isAgentRunning = taskSessionState === "STARTING" || taskSessionState === "RUNNING"; const handleStartAgent = useCallback( - async (agentProfileId: string, prompt?: string) => { + async (agentProfileId: string, opts?: { prompt?: string; autoStart?: boolean }) => { if (!task?.id) return; if (!agentProfileId) return; setIsAgentLoading(true); try { const { request } = buildStartRequest(task.id, agentProfileId, { - prompt: prompt ?? task.description ?? "", + prompt: opts?.prompt ?? task.description ?? "", + autoStart: opts?.autoStart, }); await launchSession(request); } catch { diff --git a/apps/web/lib/services/session-launch-helpers.ts b/apps/web/lib/services/session-launch-helpers.ts index 55c7ac192b..8a87d9c7b8 100644 --- a/apps/web/lib/services/session-launch-helpers.ts +++ b/apps/web/lib/services/session-launch-helpers.ts @@ -16,6 +16,7 @@ export function buildStartRequest( prompt?: string; planMode?: boolean; priority?: number; + autoStart?: boolean; attachments?: MessageAttachment[]; }, ): BuildResult { @@ -29,6 +30,7 @@ export function buildStartRequest( prompt: opts?.prompt, plan_mode: opts?.planMode, priority: opts?.priority, + auto_start: opts?.autoStart, attachments: opts?.attachments, }, layout: opts?.planMode ? "plan" : "default", diff --git a/apps/web/lib/services/session-launch-service.ts b/apps/web/lib/services/session-launch-service.ts index 467baac616..c7c410afbf 100644 --- a/apps/web/lib/services/session-launch-service.ts +++ b/apps/web/lib/services/session-launch-service.ts @@ -28,6 +28,7 @@ export type LaunchSessionRequest = { priority?: number; launch_workspace?: boolean; skip_message_record?: boolean; + auto_start?: boolean; attachments?: MessageAttachment[]; }; diff --git a/apps/web/lib/state/dockview-layout-builders.ts b/apps/web/lib/state/dockview-layout-builders.ts index 09e042780e..aa9cb612b9 100644 --- a/apps/web/lib/state/dockview-layout-builders.ts +++ b/apps/web/lib/state/dockview-layout-builders.ts @@ -49,6 +49,24 @@ export function focusOrAddPanel( if (!quiet) existing.api.setActive(); return; } + // Guard: if the referenced group no longer exists (stale ID after layout + // transition), fall back to the active panel's group or the first group. + const pos = options.position; + if (pos && "referenceGroup" in pos) { + const groupExists = api.groups.some((g) => g.id === pos.referenceGroup); + if (!groupExists) { + if (api.activePanel) { + options = { ...options, position: { referencePanel: api.activePanel.id } }; + } else if (api.groups.length > 0) { + options = { ...options, position: { referenceGroup: api.groups[0].id } }; + } else { + options = Object.fromEntries( + Object.entries(options).filter(([k]) => k !== "position"), + ) as typeof options; + } + } + } + const prev = quiet ? api.activePanel : null; api.addPanel(options); if (prev) prev.api.setActive();