feat(web): unified commit list and dockview panel fix (#500)

This commit is contained in:
Carlos Florêncio
2026-03-30 22:44:10 +01:00
committed by GitHub
parent 54b8d2939b
commit 59ee830d71
20 changed files with 572 additions and 119 deletions
@@ -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(
+1 -1
View File
@@ -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),
+7
View File
@@ -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 {
+46 -1
View File
@@ -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/<checkout-branch> 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/<remoteBranch> 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),
@@ -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()
@@ -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 (
<TimelineSection
dotColor={DOT_COLORS.commits}
@@ -153,11 +155,11 @@ export function CommitsSection({
<CommitRow
key={`${commit.commit_sha}-${index}`}
commit={commit}
isLatest={index === 0}
isLatest={index === firstUnpushedIndex}
onOpenCommitDetail={onOpenCommitDetail}
onAmendCommit={onAmendCommit}
onRevertCommit={onRevertCommit}
onResetToCommit={onResetToCommit}
onAmendCommit={commit.pushed ? undefined : onAmendCommit}
onRevertCommit={commit.pushed ? undefined : onRevertCommit}
onResetToCommit={commit.pushed ? undefined : onResetToCommit}
/>
))}
</ul>
@@ -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 (
<TimelineSection
dotColor={DOT_COLORS.pr}
label="PR Commits"
count={commits.length}
isLast={isLast}
data-testid="pr-commits-timeline-section"
>
<ul className="space-y-0.5">
{commits.map((commit, index) => (
<li
key={`${commit.sha}-${index}`}
className="group flex items-center gap-2 text-xs rounded-md px-1 py-1 -mx-1 hover:bg-muted/60 cursor-pointer"
onClick={() => onOpenCommitDetail?.(commit.sha)}
>
<IconGitCommit className="h-3.5 w-3.5 text-purple-500 shrink-0" />
<code className="font-mono text-muted-foreground text-[11px]">
{commit.sha.slice(0, 7)}
</code>
<span className="flex-1 min-w-0 truncate text-foreground">{commit.message}</span>
{commit.author_login && (
<span className="shrink-0 text-[10px] text-muted-foreground">
{commit.author_login}
</span>
)}
</li>
))}
</ul>
</TimelineSection>
);
}
// --- Review progress bar ---
type ReviewProgressBarProps = {
+77 -1
View File
@@ -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<typeof makeLocal>[] = [];
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 });
});
});
+81 -20
View File
@@ -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<T extends { commit_sha: string }>(
);
}
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<string>();
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<typeof useSessionGit>["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<string>;
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 && (
<CommitsSection
commits={props.commits}
isLast={false}
isLast={!showCommits}
onOpenCommitDetail={props.onOpenCommitDetail}
onRevertCommit={props.onRevertCommit}
onAmendCommit={props.dialogs.handleOpenAmendDialog}
@@ -373,9 +443,9 @@ function ChangesPanelTimeline(props: TimelineProps) {
);
}
const unpushedCommits = filterUnpushedCommits(props.commits, props.prCommits);
const hasUnpushedCommits = unpushedCommits.length > 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 (
<div className="flex flex-col">
@@ -383,24 +453,15 @@ function ChangesPanelTimeline(props: TimelineProps) {
<div data-testid="pr-files-section">
<PRFilesSection
files={props.prFiles}
isLast={!props.hasPRCommits && !hasLocalChanges}
onOpenDiff={props.onOpenDiffFile}
/>
</div>
)}
{props.hasPRCommits && (
<div data-testid="pr-commits-section">
<PRCommitsSection
commits={props.prCommits}
isLast={!hasLocalChanges}
onOpenCommitDetail={props.onOpenCommitDetail}
onOpenDiff={props.onOpenDiffFile}
/>
</div>
)}
<TimelineLocalChanges
{...props}
commits={unpushedCommits}
hasCommitsToShow={hasUnpushedCommits}
commits={mergedCommits}
hasCommitsToShow={hasMergedCommits}
/>
</div>
);
+21 -2
View File
@@ -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({
}
}}
>
<IconGitCommit className="h-3.5 w-3.5 text-emerald-500 shrink-0" />
<span
className="shrink-0"
title={commit.pushed === true ? "Pushed to remote" : "Local commit (not yet pushed)"}
>
<span className="sr-only">
{commit.pushed === true ? "Pushed commit" : "Unpushed commit"}
</span>
{commit.pushed === true ? (
<IconGitCommit aria-hidden="true" className="h-3.5 w-3.5 text-muted-foreground" />
) : (
<IconArrowUp aria-hidden="true" className="h-3.5 w-3.5 text-emerald-500" />
)}
</span>
<code className="font-mono text-muted-foreground text-[11px]">
{commit.commit_sha.slice(0, 7)}
</code>
@@ -492,7 +492,10 @@ function useTaskDetails(activeTaskId: string | null, initialTask: Task | null) {
function useAutoStartSession(
task: Task | null,
handleStartAgent: (agentProfileId: string, prompt?: string) => Promise<void>,
handleStartAgent: (
agentProfileId: string,
opts?: { prompt?: string; autoStart?: boolean },
) => Promise<void>,
) {
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]);
+3 -3
View File
@@ -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");
}
/**
@@ -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 });
});
});
+2 -2
View File
@@ -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();
});
});
@@ -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();
});
});
@@ -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");
@@ -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
@@ -14,7 +14,10 @@ interface UseSessionAgentReturn {
taskSessionState: TaskSessionState | null;
worktreePath: string | null;
worktreeBranch: string | null;
handleStartAgent: (agentProfileId: string, prompt?: string) => Promise<void>;
handleStartAgent: (
agentProfileId: string,
opts?: { prompt?: string; autoStart?: boolean },
) => Promise<void>;
handleStopAgent: () => Promise<void>;
}
@@ -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 {
@@ -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",
@@ -28,6 +28,7 @@ export type LaunchSessionRequest = {
priority?: number;
launch_workspace?: boolean;
skip_message_record?: boolean;
auto_start?: boolean;
attachments?: MessageAttachment[];
};
@@ -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();