diff --git a/apps/backend/cmd/kandev/adapters.go b/apps/backend/cmd/kandev/adapters.go index b748697b6c..05611beb46 100644 --- a/apps/backend/cmd/kandev/adapters.go +++ b/apps/backend/cmd/kandev/adapters.go @@ -391,7 +391,7 @@ func (a *lifecycleAdapter) GetGitLog(ctx context.Context, sessionID, baseCommit if agentClient == nil { return nil, nil } - return agentClient.GitLog(ctx, baseCommit, limit, targetBranch) + return agentClient.GitLog(ctx, baseCommit, limit, targetBranch, "") } // GetCumulativeDiff retrieves the cumulative diff for a session from baseCommit to HEAD. diff --git a/apps/backend/internal/agent/handlers/git_handlers.go b/apps/backend/internal/agent/handlers/git_handlers.go index b580beca39..c7a65b85f5 100644 --- a/apps/backend/internal/agent/handlers/git_handlers.go +++ b/apps/backend/internal/agent/handlers/git_handlers.go @@ -153,6 +153,10 @@ type GitCommitRequest struct { Message string `json:"message"` StageAll bool `json:"stage_all"` Amend bool `json:"amend"` + // Multi-repo: subpath of the repo to commit in (e.g. "kandev"). Empty for + // single-repo workspaces. Required for multi-repo — committing at the task + // root fails because it isn't itself a git repo. + Repo string `json:"repo,omitempty"` } // GitRenameBranchRequest for worktree.rename_branch action @@ -164,19 +168,22 @@ type GitRenameBranchRequest struct { // GitStageRequest for worktree.stage action type GitStageRequest struct { SessionID string `json:"session_id"` - Paths []string `json:"paths"` // Empty = stage all + Paths []string `json:"paths"` // Empty = stage all + Repo string `json:"repo,omitempty"` // Multi-repo subpath; empty for single-repo } // GitUnstageRequest for worktree.unstage action type GitUnstageRequest struct { SessionID string `json:"session_id"` - Paths []string `json:"paths"` // Empty = unstage all + Paths []string `json:"paths"` // Empty = unstage all + Repo string `json:"repo,omitempty"` // Multi-repo subpath; empty for single-repo } // GitDiscardRequest for worktree.discard action type GitDiscardRequest struct { SessionID string `json:"session_id"` - Paths []string `json:"paths"` // Required - files to discard + Paths []string `json:"paths"` // Required - files to discard + Repo string `json:"repo,omitempty"` // Multi-repo subpath; empty for single-repo } // GitCreatePRRequest for worktree.create_pr action @@ -192,13 +199,15 @@ type GitCreatePRRequest struct { type GitRevertCommitRequest struct { SessionID string `json:"session_id"` CommitSHA string `json:"commit_sha"` + Repo string `json:"repo,omitempty"` // Multi-repo subpath; empty for single-repo } // GitResetRequest for worktree.reset action type GitResetRequest struct { SessionID string `json:"session_id"` CommitSHA string `json:"commit_sha"` - Mode string `json:"mode"` // "soft", "mixed", or "hard" + Mode string `json:"mode"` // "soft", "mixed", or "hard" + Repo string `json:"repo,omitempty"` // Multi-repo subpath; empty for single-repo } // GitShowCommitRequest for session.commit_diff action @@ -359,7 +368,7 @@ func (h *GitHandlers) wsCommit(ctx context.Context, msg *ws.Message) (*ws.Messag return nil, err } - result, err := client.GitCommit(ctx, req.Message, req.StageAll, req.Amend) + result, err := client.GitCommit(ctx, req.Message, req.StageAll, req.Amend, req.Repo) if err != nil { return nil, fmt.Errorf("commit failed: %w", err) } @@ -421,7 +430,7 @@ func (h *GitHandlers) wsReset(ctx context.Context, msg *ws.Message) (*ws.Message return nil, err } - result, err := client.GitReset(ctx, req.CommitSHA, req.Mode) + result, err := client.GitReset(ctx, req.CommitSHA, req.Mode, req.Repo) if err != nil { return nil, fmt.Errorf("reset failed: %w", err) } @@ -445,7 +454,7 @@ func (h *GitHandlers) wsStage(ctx context.Context, msg *ws.Message) (*ws.Message return nil, err } - result, err := client.GitStage(ctx, req.Paths) + result, err := client.GitStage(ctx, req.Paths, req.Repo) if err != nil { return nil, fmt.Errorf("stage failed: %w", err) } @@ -469,7 +478,7 @@ func (h *GitHandlers) wsUnstage(ctx context.Context, msg *ws.Message) (*ws.Messa return nil, err } - result, err := client.GitUnstage(ctx, req.Paths) + result, err := client.GitUnstage(ctx, req.Paths, req.Repo) if err != nil { return nil, fmt.Errorf("unstage failed: %w", err) } @@ -497,7 +506,7 @@ func (h *GitHandlers) wsDiscard(ctx context.Context, msg *ws.Message) (*ws.Messa return nil, err } - result, err := client.GitDiscard(ctx, req.Paths) + result, err := client.GitDiscard(ctx, req.Paths, req.Repo) if err != nil { return nil, fmt.Errorf("discard failed: %w", err) } @@ -567,7 +576,7 @@ func (h *GitHandlers) wsRevertCommit(ctx context.Context, msg *ws.Message) (*ws. return nil, err } - result, err := client.GitRevertCommit(ctx, req.CommitSHA) + result, err := client.GitRevertCommit(ctx, req.CommitSHA, req.Repo) if err != nil { return nil, fmt.Errorf("revert commit failed: %w", err) } @@ -702,7 +711,7 @@ func (h *GitHandlers) wsGitCommits(ctx context.Context, msg *ws.Message) (*ws.Me // Use target branch for dynamic merge-base calculation. // This ensures accurate commit filtering even after rebases. - result, err := agentClient.GitLog(ctx, baseCommit, req.Limit, targetBranch) + result, err := agentClient.GitLog(ctx, baseCommit, req.Limit, targetBranch, "") if err != nil { return nil, fmt.Errorf("git log failed: %w", err) } diff --git a/apps/backend/internal/agent/lifecycle/event_types.go b/apps/backend/internal/agent/lifecycle/event_types.go index 7f6aa4f6d2..88146ab009 100644 --- a/apps/backend/internal/agent/lifecycle/event_types.go +++ b/apps/backend/internal/agent/lifecycle/event_types.go @@ -243,6 +243,10 @@ type GitCommitData struct { Deletions int `json:"deletions"` CommittedAt string `json:"committed_at"` CreatedAt string `json:"created_at,omitempty"` + // RepositoryName identifies which repo this commit belongs to in multi-repo + // task workspaces. Empty for single-repo. Carried to the frontend so the + // Commits panel can render per-repo group headers. + RepositoryName string `json:"repository_name,omitempty"` } type GitResetData struct { diff --git a/apps/backend/internal/agent/lifecycle/events.go b/apps/backend/internal/agent/lifecycle/events.go index 554719394f..3b90cd2564 100644 --- a/apps/backend/internal/agent/lifecycle/events.go +++ b/apps/backend/internal/agent/lifecycle/events.go @@ -261,15 +261,16 @@ func (p *EventPublisher) PublishGitCommit(execution *AgentExecution, commit *age AgentID: execution.ID, Timestamp: time.Now().Format(time.RFC3339Nano), Commit: &GitCommitData{ - CommitSHA: commit.CommitSHA, - ParentSHA: commit.ParentSHA, - Message: commit.Message, - AuthorName: commit.AuthorName, - AuthorEmail: commit.AuthorEmail, - FilesChanged: commit.FilesChanged, - Insertions: commit.Insertions, - Deletions: commit.Deletions, - CommittedAt: commit.CommittedAt.Format(time.RFC3339), + CommitSHA: commit.CommitSHA, + ParentSHA: commit.ParentSHA, + Message: commit.Message, + AuthorName: commit.AuthorName, + AuthorEmail: commit.AuthorEmail, + FilesChanged: commit.FilesChanged, + Insertions: commit.Insertions, + Deletions: commit.Deletions, + CommittedAt: commit.CommittedAt.Format(time.RFC3339), + RepositoryName: commit.RepositoryName, }, }) } diff --git a/apps/backend/internal/agent/lifecycle/executor_sprites_e2e_test.go b/apps/backend/internal/agent/lifecycle/executor_sprites_e2e_test.go index 1d38a45655..abe6fcb291 100644 --- a/apps/backend/internal/agent/lifecycle/executor_sprites_e2e_test.go +++ b/apps/backend/internal/agent/lifecycle/executor_sprites_e2e_test.go @@ -320,11 +320,11 @@ func TestSpritesE2E_FullFlow(t *testing.T) { _, err = client.CreateFile(ctx, "e2e-change.txt") require.NoError(t, err) - result, err := client.GitStage(ctx, []string{"e2e-change.txt"}) + result, err := client.GitStage(ctx, []string{"e2e-change.txt"}, "") require.NoError(t, err) require.True(t, result.Success, "git stage should succeed: %s", result.Error) - result, err = client.GitCommit(ctx, "e2e test commit", false) + result, err = client.GitCommit(ctx, "e2e test commit", false, false, "") require.NoError(t, err) require.True(t, result.Success, "git commit should succeed: %s", result.Error) t.Logf("git commit: %s", result.Output) diff --git a/apps/backend/internal/agentctl/client/git.go b/apps/backend/internal/agentctl/client/git.go index 0cf7cf6cbf..90f077984f 100644 --- a/apps/backend/internal/agentctl/client/git.go +++ b/apps/backend/internal/agentctl/client/git.go @@ -89,15 +89,18 @@ func (c *Client) GitAbort(ctx context.Context, operation string) (*GitOperationR // GitCommit creates a commit with the specified message. // If stageAll is true, all changes are staged before committing. // If amend is true, it amends the previous commit instead of creating a new one. -func (c *Client) GitCommit(ctx context.Context, message string, stageAll bool, amend bool) (*GitOperationResult, error) { +// repo is the multi-repo subpath (e.g. "kandev"); empty for single-repo workspaces. +func (c *Client) GitCommit(ctx context.Context, message string, stageAll bool, amend bool, repo string) (*GitOperationResult, error) { payload := struct { Message string `json:"message"` StageAll bool `json:"stage_all"` Amend bool `json:"amend"` + Repo string `json:"repo,omitempty"` }{ Message: message, StageAll: stageAll, Amend: amend, + Repo: repo, } return c.gitOperation(ctx, "/api/v1/git/commit", payload) } @@ -114,56 +117,71 @@ func (c *Client) GitRenameBranch(ctx context.Context, newName string) (*GitOpera // GitStage stages files for commit. // If paths is empty, stages all changes (git add -A). -func (c *Client) GitStage(ctx context.Context, paths []string) (*GitOperationResult, error) { +// repo is the multi-repo subpath (e.g. "kandev"); empty for single-repo workspaces. +func (c *Client) GitStage(ctx context.Context, paths []string, repo string) (*GitOperationResult, error) { payload := struct { Paths []string `json:"paths"` + Repo string `json:"repo,omitempty"` }{ Paths: paths, + Repo: repo, } return c.gitOperation(ctx, "/api/v1/git/stage", payload) } // GitUnstage unstages files from the index. // If paths is empty, unstages all changes (git reset HEAD). -func (c *Client) GitUnstage(ctx context.Context, paths []string) (*GitOperationResult, error) { +// repo is the multi-repo subpath (e.g. "kandev"); empty for single-repo workspaces. +func (c *Client) GitUnstage(ctx context.Context, paths []string, repo string) (*GitOperationResult, error) { payload := struct { Paths []string `json:"paths"` + Repo string `json:"repo,omitempty"` }{ Paths: paths, + Repo: repo, } return c.gitOperation(ctx, "/api/v1/git/unstage", payload) } // GitDiscard discards changes to files, reverting them to HEAD. // Paths must not be empty - at least one file must be specified. -func (c *Client) GitDiscard(ctx context.Context, paths []string) (*GitOperationResult, error) { +// repo is the multi-repo subpath (e.g. "kandev"); empty for single-repo workspaces. +func (c *Client) GitDiscard(ctx context.Context, paths []string, repo string) (*GitOperationResult, error) { payload := struct { Paths []string `json:"paths"` + Repo string `json:"repo,omitempty"` }{ Paths: paths, + Repo: repo, } return c.gitOperation(ctx, "/api/v1/git/discard", payload) } // GitRevertCommit undoes the latest commit using git reset --soft, keeping changes staged. -func (c *Client) GitRevertCommit(ctx context.Context, commitSHA string) (*GitOperationResult, error) { +// repo is the multi-repo subpath (e.g. "kandev"); empty for single-repo workspaces. +func (c *Client) GitRevertCommit(ctx context.Context, commitSHA, repo string) (*GitOperationResult, error) { payload := struct { CommitSHA string `json:"commit_sha"` + Repo string `json:"repo,omitempty"` }{ CommitSHA: commitSHA, + Repo: repo, } return c.gitOperation(ctx, "/api/v1/git/revert-commit", payload) } // GitReset resets HEAD to the specified commit. // Mode can be "soft" (keep changes staged), "mixed" (keep changes unstaged), or "hard" (discard all changes). -func (c *Client) GitReset(ctx context.Context, commitSHA, mode string) (*GitOperationResult, error) { +// repo is the multi-repo subpath (e.g. "kandev"); empty for single-repo workspaces. +func (c *Client) GitReset(ctx context.Context, commitSHA, mode, repo string) (*GitOperationResult, error) { payload := struct { CommitSHA string `json:"commit_sha"` Mode string `json:"mode"` + Repo string `json:"repo,omitempty"` }{ CommitSHA: commitSHA, Mode: mode, + Repo: repo, } return c.gitOperation(ctx, "/api/v1/git/reset", payload) } @@ -322,17 +340,25 @@ type GitCommitInfo struct { FilesChanged int `json:"files_changed"` Insertions int `json:"insertions"` Deletions int `json:"deletions"` + // RepositoryName tags commits when fetched from a multi-repo subpath. Empty + // for single-repo workspaces. Stamped client-side after the per-repo call + // so callers can fan out across repos and merge. + RepositoryName string `json:"repository_name,omitempty"` } // GitLog gets the commit log from baseCommit to HEAD. // If baseCommit is empty, returns recent commits (limited by limit). // If targetBranch is provided, computes merge-base dynamically for accurate filtering. -func (c *Client) GitLog(ctx context.Context, baseCommit string, limit int, targetBranch string) (*GitLogResult, error) { +// repo is the multi-repo subpath (e.g. "kandev"); empty for single-repo workspaces. +func (c *Client) GitLog(ctx context.Context, baseCommit string, limit int, targetBranch, repo string) (*GitLogResult, error) { reqURL := fmt.Sprintf("%s/api/v1/git/log?since=%s&limit=%d", c.baseURL, url.QueryEscape(baseCommit), limit) if targetBranch != "" { reqURL += "&target_branch=" + url.QueryEscape(targetBranch) } + if repo != "" { + reqURL += "&repo=" + url.QueryEscape(repo) + } req, err := http.NewRequestWithContext(ctx, "GET", reqURL, nil) if err != nil { diff --git a/apps/backend/internal/agentctl/server/api/git.go b/apps/backend/internal/agentctl/server/api/git.go index 47a329416c..0352ecf180 100644 --- a/apps/backend/internal/agentctl/server/api/git.go +++ b/apps/backend/internal/agentctl/server/api/git.go @@ -4,12 +4,29 @@ import ( "errors" "fmt" "net/http" + "sort" + "time" "github.com/gin-gonic/gin" "github.com/kandev/kandev/internal/agentctl/server/process" "go.uber.org/zap" ) +// sortCommitsByCommittedAtDesc sorts commits newest-first using committed_at. +// Commits with unparseable timestamps preserve their original relative order +// (sort.SliceStable). Used when merging logs from multiple repos so the +// combined timeline reads chronologically. +func sortCommitsByCommittedAtDesc(commits []*process.GitCommitInfo) { + sort.SliceStable(commits, func(i, j int) bool { + ti, errI := time.Parse(time.RFC3339, commits[i].CommittedAt) + tj, errJ := time.Parse(time.RFC3339, commits[j].CommittedAt) + if errI != nil || errJ != nil { + return false + } + return ti.After(tj) + }) +} + // Repo selects a repository sub-directory of the workspace for multi-repo // task roots. Optional. Empty = workspace root (single-repo behavior). // All git request structs below embed this field via the standard `repo` JSON @@ -602,7 +619,13 @@ func (s *Server) handleGitReset(c *gin.Context) { c.JSON(http.StatusOK, result) } -// handleGitLog handles GET /api/v1/git/log +// handleGitLog handles GET /api/v1/git/log. +// +// Multi-repo: when the caller doesn't pin a repo (?repo= empty) and the +// workspace contains per-repo subdirs, fan out one log per repo and merge +// the results, stamping RepositoryName on each commit so the frontend can +// group them. Without this, the call would land on the workspace root which +// for multi-repo task workspaces isn't a git repo and returns nothing. func (s *Server) handleGitLog(c *gin.Context) { var req GitLogRequest if err := c.ShouldBindQuery(&req); err != nil { @@ -629,42 +652,90 @@ func (s *Server) handleGitLog(c *gin.Context) { return } - gitOp, gitOpErr := s.procMgr.GitOperatorFor(req.Repo) - if gitOpErr != nil { - c.JSON(http.StatusBadRequest, process.GitLogResult{ - Success: false, - Error: gitOpErr.Error(), - }) - return + if req.Repo == "" { + if subs := s.procMgr.RepoSubpaths(); len(subs) > 0 { + s.handleGitLogMultiRepo(c, req, subs, limit) + return + } + } + + result, err := s.runGitLogForRepo(c, req, limit, req.Repo) + if err != nil { + s.handleGitError(c, "log", err) + return + } + c.JSON(http.StatusOK, result) +} + +// runGitLogForRepo runs git log against a single repo subpath. Returns a +// result-with-error or a non-nil error for transport failures. +func (s *Server) runGitLogForRepo( + c *gin.Context, + req GitLogRequest, + limit int, + repo string, +) (*process.GitLogResult, error) { + gitOp, gitOpErr := s.procMgr.GitOperatorFor(repo) + if gitOpErr != nil { + return nil, gitOpErr } - // If target_branch is provided, compute merge-base dynamically. - // This ensures we only show commits not yet merged into the target branch, - // which is accurate even after rebases. baseCommit := req.Since if req.TargetBranch != "" { mergeBase, err := gitOp.GetMergeBase(c.Request.Context(), "HEAD", req.TargetBranch) if err != nil { s.logger.Warn("failed to compute merge-base, falling back to since", zap.String("target_branch", req.TargetBranch), + zap.String("repo", repo), zap.Error(err)) - // Fall back to the provided base commit } else if mergeBase != "" { baseCommit = mergeBase } } - result, err := gitOp.GetLog(c.Request.Context(), baseCommit, limit) - if err != nil { - s.logger.Error("git log failed", zap.Error(err)) - c.JSON(http.StatusInternalServerError, process.GitLogResult{ - Success: false, - Error: err.Error(), - }) - return + return gitOp.GetLog(c.Request.Context(), baseCommit, limit) +} + +// handleGitLogMultiRepo fans the request out across every per-repo tracker, +// stamps RepositoryName on each returned commit, and merges into a single +// log sorted by committed_at descending. Per-repo limits are applied +// individually; the merged result is then truncated to the request limit. +func (s *Server) handleGitLogMultiRepo( + c *gin.Context, + req GitLogRequest, + subpaths []string, + limit int, +) { + merged := make([]*process.GitCommitInfo, 0) + for _, sub := range subpaths { + result, err := s.runGitLogForRepo(c, req, limit, sub) + if err != nil { + s.logger.Warn("git log for repo failed; skipping", + zap.String("repo", sub), zap.Error(err)) + continue + } + if !result.Success { + s.logger.Warn("git log for repo returned failure; skipping", + zap.String("repo", sub), zap.String("error", result.Error)) + continue + } + for _, commit := range result.Commits { + commit.RepositoryName = sub + merged = append(merged, commit) + } } - c.JSON(http.StatusOK, result) + // Sort newest-first across repos. Falls back to insertion order if either + // timestamp is unparseable. + sortCommitsByCommittedAtDesc(merged) + if len(merged) > limit { + merged = merged[:limit] + } + + c.JSON(http.StatusOK, process.GitLogResult{ + Success: true, + Commits: merged, + }) } // handleGitCumulativeDiff handles GET /api/v1/git/cumulative-diff diff --git a/apps/backend/internal/agentctl/server/api/git_log_multi_repo_test.go b/apps/backend/internal/agentctl/server/api/git_log_multi_repo_test.go new file mode 100644 index 0000000000..1ec9da72c1 --- /dev/null +++ b/apps/backend/internal/agentctl/server/api/git_log_multi_repo_test.go @@ -0,0 +1,43 @@ +package api + +import ( + "testing" + + "github.com/kandev/kandev/internal/agentctl/server/process" +) + +// sortCommitsByCommittedAtDesc must order commits newest-first so the merged +// per-repo log reads chronologically, regardless of which repo bucket each +// commit came from. Unparseable timestamps preserve their relative order via +// SliceStable so we don't randomly reshuffle bad data. +func TestSortCommitsByCommittedAtDesc(t *testing.T) { + t.Parallel() + + t.Run("interleaves commits from multiple repos by timestamp", func(t *testing.T) { + commits := []*process.GitCommitInfo{ + {CommitSHA: "a", CommittedAt: "2026-04-26T10:00:00Z", RepositoryName: "frontend"}, + {CommitSHA: "b", CommittedAt: "2026-04-26T12:00:00Z", RepositoryName: "backend"}, + {CommitSHA: "c", CommittedAt: "2026-04-26T11:00:00Z", RepositoryName: "frontend"}, + } + sortCommitsByCommittedAtDesc(commits) + got := []string{commits[0].CommitSHA, commits[1].CommitSHA, commits[2].CommitSHA} + want := []string{"b", "c", "a"} + for i := range got { + if got[i] != want[i] { + t.Errorf("position %d: got %q, want %q (full order: %v)", i, got[i], want[i], got) + } + } + }) + + t.Run("preserves order on unparseable timestamps", func(t *testing.T) { + commits := []*process.GitCommitInfo{ + {CommitSHA: "x", CommittedAt: "not-a-date"}, + {CommitSHA: "y", CommittedAt: "also-bad"}, + } + sortCommitsByCommittedAtDesc(commits) + if commits[0].CommitSHA != "x" || commits[1].CommitSHA != "y" { + t.Errorf("expected stable order on bad timestamps; got %s, %s", + commits[0].CommitSHA, commits[1].CommitSHA) + } + }) +} diff --git a/apps/backend/internal/agentctl/server/api/workspace_state.go b/apps/backend/internal/agentctl/server/api/workspace_state.go index ef6e0537b6..0397f77dcf 100644 --- a/apps/backend/internal/agentctl/server/api/workspace_state.go +++ b/apps/backend/internal/agentctl/server/api/workspace_state.go @@ -19,6 +19,19 @@ type SetPollModeRequest struct { // See plan: focus-gated git polling. The gateway computes a workspace-level // mode (fast if any session focused, slow if any subscribed, paused otherwise) // and pushes it here so the tracker can throttle expensive git scans. +// +// Multi-repo: the per-repo trackers also need the mode update — otherwise they +// keep their construction-time default (fast) and never throttle, AND they +// miss the focus-triggered immediate scan that handleMonitorModeChange does +// on a transition INTO fast. Fanning out keeps every repo in lockstep. +// +// Snapshot-on-focus: a transition to fast/slow forces a RefreshGitStatus on +// every tracker so a fresh git status reaches subscribers. monitorTick +// otherwise only pushes on detected change; without this, a page opened +// after the agent has been idle would see no events until the user makes +// a file change. Multi-repo amplifies it: per-repo state is a Map and the +// aggregator only renders the repos that have published — missing events +// = missing files in the Changes panel, not just stale data. func (s *Server) handleSetPollMode(c *gin.Context) { var req SetPollModeRequest if err := c.ShouldBindJSON(&req); err != nil { @@ -32,7 +45,7 @@ func (s *Server) handleSetPollMode(c *gin.Context) { return } - s.procMgr.GetWorkspaceTracker().SetPollMode(mode) + s.procMgr.SetWorkspacePollMode(c.Request.Context(), mode) s.logger.Debug("workspace poll mode updated", zap.String("mode", req.Mode)) c.JSON(http.StatusOK, gin.H{"mode": req.Mode}) diff --git a/apps/backend/internal/agentctl/server/process/git.go b/apps/backend/internal/agentctl/server/process/git.go index ab56f108f0..ffd182b823 100644 --- a/apps/backend/internal/agentctl/server/process/git.go +++ b/apps/backend/internal/agentctl/server/process/git.go @@ -39,6 +39,10 @@ type GitOperator struct { workDir string logger *logger.Logger workspaceTracker *WorkspaceTracker + // repoName is the multi-repo subpath this operator runs in (e.g. "kandev"). + // Empty for the workspace-root operator. Stamped on emitted commit + // notifications so the frontend can group commits per repo. + repoName string mu sync.Mutex // Prevents concurrent git operations inProgress bool @@ -54,6 +58,14 @@ func NewGitOperator(workDir string, log *logger.Logger, workspaceTracker *Worksp } } +// NewGitOperatorForRepo creates a GitOperator scoped to a multi-repo subpath +// so its emitted events (e.g. commit notifications) carry the repo name. +func NewGitOperatorForRepo(workDir, repoName string, log *logger.Logger, workspaceTracker *WorkspaceTracker) *GitOperator { + op := NewGitOperator(workDir, log, workspaceTracker) + op.repoName = repoName + return op +} + // runGitCommand executes a git command in the workDir with defense-in-depth validation. // Validates both flags and branch/ref arguments to prevent command injection. func (g *GitOperator) runGitCommand(ctx context.Context, args ...string) (string, error) { @@ -503,15 +515,16 @@ func (g *GitOperator) Commit(ctx context.Context, message string, stageAll bool, filesChanged, insertions, deletions := g.getCommitStats(ctx, strings.TrimSpace(commitSHA)) commit := &streams.GitCommitNotification{ - CommitSHA: strings.TrimSpace(commitSHA), - ParentSHA: strings.TrimSpace(parentSHA), - Message: message, - AuthorName: authorName, - AuthorEmail: authorEmail, - FilesChanged: filesChanged, - Insertions: insertions, - Deletions: deletions, - CommittedAt: time.Now().UTC(), + RepositoryName: g.repoName, + CommitSHA: strings.TrimSpace(commitSHA), + ParentSHA: strings.TrimSpace(parentSHA), + Message: message, + AuthorName: authorName, + AuthorEmail: authorEmail, + FilesChanged: filesChanged, + Insertions: insertions, + Deletions: deletions, + CommittedAt: time.Now().UTC(), } g.workspaceTracker.NotifyGitCommit(commit) diff --git a/apps/backend/internal/agentctl/server/process/git_log.go b/apps/backend/internal/agentctl/server/process/git_log.go index 06e02a988a..1e7298b9af 100644 --- a/apps/backend/internal/agentctl/server/process/git_log.go +++ b/apps/backend/internal/agentctl/server/process/git_log.go @@ -25,6 +25,9 @@ type GitCommitInfo struct { FilesChanged int `json:"files_changed"` Insertions int `json:"insertions"` Deletions int `json:"deletions"` + // RepositoryName tags commits returned from a multi-repo log fan-out. + // Empty for single-repo workspaces. Set by the API layer after the call. + RepositoryName string `json:"repository_name,omitempty"` } // CumulativeDiffResult represents the cumulative diff from base commit to HEAD. diff --git a/apps/backend/internal/agentctl/server/process/manager.go b/apps/backend/internal/agentctl/server/process/manager.go index 5a51e6e8ff..7e8e6e0ae7 100644 --- a/apps/backend/internal/agentctl/server/process/manager.go +++ b/apps/backend/internal/agentctl/server/process/manager.go @@ -321,6 +321,44 @@ func (m *Manager) ListProcesses(sessionID string) []ProcessInfo { return m.processRunner.List(sessionID) } +// RepoSubpaths returns the subpath name (relative to cfg.WorkDir) for every +// per-repo tracker discovered at construction time. Empty for single-repo +// workspaces. Used by callers that want to fan an op out across repos. +func (m *Manager) RepoSubpaths() []string { + out := make([]string, 0, len(m.repoTrackers)) + for _, t := range m.repoTrackers { + if t.repositoryName != "" { + out = append(out, t.repositoryName) + } + } + return out +} + +// SetWorkspacePollMode propagates a poll-mode change to the root tracker and +// every per-repo tracker, then forces a RefreshGitStatus on each non-paused +// tracker so a fresh snapshot reaches every subscriber. Without the refresh, +// monitorTick only pushes on detected change — multi-repo workspaces would +// leave the per-repo state map sparse (one repo present, the other missing) +// after a focus event, since the agent's initial pushes happen at boot and +// no replay path exists for clients that subscribe later. +func (m *Manager) SetWorkspacePollMode(ctx context.Context, mode PollMode) { + m.workspaceTracker.SetPollMode(mode) + for _, t := range m.repoTrackers { + t.SetPollMode(mode) + } + if mode == PollModePaused { + return + } + // Refresh in background — RefreshGitStatus blocks on git commands which can + // take seconds on large repos; the HTTP caller shouldn't wait. + go func() { + m.workspaceTracker.RefreshGitStatus(ctx) + for _, t := range m.repoTrackers { + t.RefreshGitStatus(ctx) + } + }() +} + // GitOperator returns the git operator for git operations against the // workspace root. Lazy-initialized. func (m *Manager) GitOperator() *GitOperator { @@ -357,7 +395,7 @@ func (m *Manager) GitOperatorFor(subpath string) (*GitOperator, error) { if op, ok := m.gitOperatorsBySubpath[cleaned]; ok { return op, nil } - op := NewGitOperator(full, m.logger, m.workspaceTracker) + op := NewGitOperatorForRepo(full, cleaned, m.logger, m.workspaceTracker) m.gitOperatorsBySubpath[cleaned] = op return op, nil } diff --git a/apps/backend/internal/agentctl/server/process/manager_multi_repo_stream_test.go b/apps/backend/internal/agentctl/server/process/manager_multi_repo_stream_test.go index 5db67f5ee5..8e863df276 100644 --- a/apps/backend/internal/agentctl/server/process/manager_multi_repo_stream_test.go +++ b/apps/backend/internal/agentctl/server/process/manager_multi_repo_stream_test.go @@ -82,3 +82,59 @@ func TestManager_SingleRepo_NoPerRepoTrackers(t *testing.T) { t.Errorf("single-repo workspace should not have per-repo trackers; got %d", len(mgr.repoTrackers)) } } + +// RepoSubpaths must list every per-repo tracker's directory name (and +// nothing else) so the API layer can fan git-log out across repos. +func TestManager_RepoSubpaths(t *testing.T) { + taskRoot := t.TempDir() + for _, name := range []string{"frontend", "backend"} { + repoDir, cleanup := setupTestRepo(t) + t.Cleanup(cleanup) + if err := os.Rename(repoDir, filepath.Join(taskRoot, name)); err != nil { + t.Fatalf("place %s: %v", name, err) + } + } + + mgr := NewManager(&config.InstanceConfig{WorkDir: taskRoot}, newTestLogger(t)) + subs := mgr.RepoSubpaths() + want := map[string]bool{"frontend": true, "backend": true} + if len(subs) != len(want) { + t.Fatalf("RepoSubpaths length = %d, want %d (got %v)", len(subs), len(want), subs) + } + for _, s := range subs { + if !want[s] { + t.Errorf("unexpected repo subpath %q", s) + } + } +} + +// Verifies that SetWorkspacePollMode propagates to every per-repo tracker. +// Without the fan-out, per-repo trackers stay at their construction-time +// default and never receive the focus-triggered refresh that delivers a +// fresh git status snapshot to a subscriber attached after agent boot. +func TestManager_SetWorkspacePollMode_PropagatesToPerRepoTrackers(t *testing.T) { + taskRoot := t.TempDir() + for _, name := range []string{"frontend", "backend"} { + repoDir, cleanup := setupTestRepo(t) + t.Cleanup(cleanup) + if err := os.Rename(repoDir, filepath.Join(taskRoot, name)); err != nil { + t.Fatalf("place %s: %v", name, err) + } + } + + mgr := NewManager(&config.InstanceConfig{WorkDir: taskRoot}, newTestLogger(t)) + if len(mgr.repoTrackers) != 2 { + t.Fatalf("expected 2 per-repo trackers, got %d", len(mgr.repoTrackers)) + } + + mgr.SetWorkspacePollMode(context.Background(), PollModeSlow) + + if got := mgr.workspaceTracker.GetPollMode(); got != PollModeSlow { + t.Errorf("root tracker mode = %v, want %v", got, PollModeSlow) + } + for i, tr := range mgr.repoTrackers { + if got := tr.GetPollMode(); got != PollModeSlow { + t.Errorf("repoTrackers[%d] mode = %v, want %v", i, got, PollModeSlow) + } + } +} diff --git a/apps/backend/internal/agentctl/server/process/workspace_git_commits.go b/apps/backend/internal/agentctl/server/process/workspace_git_commits.go index 4e3d20f911..0121748df4 100644 --- a/apps/backend/internal/agentctl/server/process/workspace_git_commits.go +++ b/apps/backend/internal/agentctl/server/process/workspace_git_commits.go @@ -71,16 +71,17 @@ func (wt *WorkspaceTracker) getCommitsSince(ctx context.Context, baseCommit stri } commits = append(commits, &types.GitCommitNotification{ - Timestamp: time.Now(), - CommitSHA: sha, - ParentSHA: parentSHA, - AuthorName: parts[2], - AuthorEmail: parts[3], - Message: parts[4], - FilesChanged: filesChanged, - Insertions: insertions, - Deletions: deletions, - CommittedAt: committedAt, + Timestamp: time.Now(), + RepositoryName: wt.repositoryName, + CommitSHA: sha, + ParentSHA: parentSHA, + AuthorName: parts[2], + AuthorEmail: parts[3], + Message: parts[4], + FilesChanged: filesChanged, + Insertions: insertions, + Deletions: deletions, + CommittedAt: committedAt, }) } diff --git a/apps/backend/internal/agentctl/types/streams/git.go b/apps/backend/internal/agentctl/types/streams/git.go index 8315d8bc70..956868ec7f 100644 --- a/apps/backend/internal/agentctl/types/streams/git.go +++ b/apps/backend/internal/agentctl/types/streams/git.go @@ -95,6 +95,11 @@ type GitCommitNotification struct { // Timestamp is when this notification was created. Timestamp time.Time `json:"timestamp"` + // RepositoryName identifies which repository this commit belongs to in + // multi-repo task workspaces. Empty for single-repo. Carried through to + // the frontend so the Commits panel can render per-repo group headers. + RepositoryName string `json:"repository_name,omitempty"` + // CommitSHA is the SHA of the new commit. CommitSHA string `json:"commit_sha"` diff --git a/apps/backend/internal/orchestrator/executor/executor_execute.go b/apps/backend/internal/orchestrator/executor/executor_execute.go index 04a1263ad4..f664760237 100644 --- a/apps/backend/internal/orchestrator/executor/executor_execute.go +++ b/apps/backend/internal/orchestrator/executor/executor_execute.go @@ -878,14 +878,7 @@ func (e *Executor) persistTaskEnvironment( } // Create a new task environment - workspacePath := resp.WorktreePath - if workspacePath == "" { - workspacePath = req.RepositoryPath - } - // Task directory mode: WorkspacePath = task root, WorktreePath = repo subdir - if req.TaskDirName != "" && resp.WorktreePath != "" { - workspacePath = filepath.Dir(resp.WorktreePath) - } + workspacePath := resolveTaskEnvWorkspacePath(req, resp) env := &models.TaskEnvironment{ TaskID: taskID, RepositoryID: req.RepositoryID, @@ -931,6 +924,30 @@ func buildTaskEnvironmentRepos(worktrees []RepoWorktreeResult) []*models.TaskEnv return out } +// resolveTaskEnvWorkspacePath returns the host path that should be persisted on +// the TaskEnvironment row (and used as agentctl's WorkDir on recreation). +// +// In task-directory mode the desired value is always the task root that holds +// every per-repo worktree as a sibling. The legacy single-repo path passes +// resp.WorktreePath as that repo's subdir, so the parent is the task root. +// The multi-repo path on the lifecycle adapter mirrors agentctl's WorkDir +// (already the task root) into resp.WorktreePath, so it must be used as-is — +// applying filepath.Dir would walk up one level too far and point at the +// /tasks holder instead. +func resolveTaskEnvWorkspacePath(req *LaunchAgentRequest, resp *LaunchAgentResponse) string { + workspacePath := resp.WorktreePath + if workspacePath == "" { + workspacePath = req.RepositoryPath + } + if req.TaskDirName == "" || resp.WorktreePath == "" { + return workspacePath + } + if len(resp.Worktrees) > 1 { + return resp.WorktreePath + } + return filepath.Dir(resp.WorktreePath) +} + // persistTaskEnvironmentRepos inserts per-repo rows under an existing env id, // skipping any (env, repo) pair that already exists. Used when an existing // environment is reused (resume / re-launch on the same task). diff --git a/apps/backend/internal/orchestrator/executor/executor_execute_workspace_path_test.go b/apps/backend/internal/orchestrator/executor/executor_execute_workspace_path_test.go new file mode 100644 index 0000000000..969d41200d --- /dev/null +++ b/apps/backend/internal/orchestrator/executor/executor_execute_workspace_path_test.go @@ -0,0 +1,52 @@ +package executor + +import "testing" + +// Pinpointed tests for resolveTaskEnvWorkspacePath. The persisted workspace_path +// becomes agentctl's WorkDir on cold start (GetOrEnsureExecution); pointing it +// at the wrong directory disables the per-repo tracker fan-out and silently +// drops the Changes panel back to single-repo mode. +func TestResolveTaskEnvWorkspacePath(t *testing.T) { + t.Parallel() + t.Run("multi-repo keeps task root", func(t *testing.T) { + req := &LaunchAgentRequest{TaskDirName: "do-nothing_mvo"} + resp := &LaunchAgentResponse{ + // Lifecycle adapter mirrors agentctl WorkDir (task root) into + // the legacy WorktreePath field for multi-repo launches. + WorktreePath: "/tmp/tasks/do-nothing_mvo", + Worktrees: []RepoWorktreeResult{ + {WorktreePath: "/tmp/tasks/do-nothing_mvo/kandev"}, + {WorktreePath: "/tmp/tasks/do-nothing_mvo/thm"}, + }, + } + if got := resolveTaskEnvWorkspacePath(req, resp); got != "/tmp/tasks/do-nothing_mvo" { + t.Fatalf("multi-repo: want /tmp/tasks/do-nothing_mvo, got %q", got) + } + }) + + t.Run("single-repo derives task root from repo subdir", func(t *testing.T) { + req := &LaunchAgentRequest{TaskDirName: "fix-thing_abc"} + resp := &LaunchAgentResponse{ + WorktreePath: "/tmp/tasks/fix-thing_abc/kandev", + } + if got := resolveTaskEnvWorkspacePath(req, resp); got != "/tmp/tasks/fix-thing_abc" { + t.Fatalf("single-repo: want /tmp/tasks/fix-thing_abc, got %q", got) + } + }) + + t.Run("non-task-dir mode passes worktree path through", func(t *testing.T) { + req := &LaunchAgentRequest{} // TaskDirName empty + resp := &LaunchAgentResponse{WorktreePath: "/legacy/worktrees/xyz"} + if got := resolveTaskEnvWorkspacePath(req, resp); got != "/legacy/worktrees/xyz" { + t.Fatalf("non-task-dir: want /legacy/worktrees/xyz, got %q", got) + } + }) + + t.Run("no worktree falls back to repository path", func(t *testing.T) { + req := &LaunchAgentRequest{RepositoryPath: "/repos/myrepo"} + resp := &LaunchAgentResponse{} // no WorktreePath + if got := resolveTaskEnvWorkspacePath(req, resp); got != "/repos/myrepo" { + t.Fatalf("fallback: want /repos/myrepo, got %q", got) + } + }) +} diff --git a/apps/backend/internal/task/handlers/repository_handlers.go b/apps/backend/internal/task/handlers/repository_handlers.go index 086f782685..9810fee86a 100644 --- a/apps/backend/internal/task/handlers/repository_handlers.go +++ b/apps/backend/internal/task/handlers/repository_handlers.go @@ -38,10 +38,13 @@ func (h *RepositoryHandlers) registerHTTP(router *gin.Engine) { api.GET("/workspaces/:id/repositories", h.httpListRepositories) api.POST("/workspaces/:id/repositories", h.httpCreateRepository) api.GET("/workspaces/:id/repositories/discover", h.httpDiscoverRepositories) - api.GET("/workspaces/:id/repositories/branches", h.httpListLocalRepositoryBranches) + // Unified branch listing — accepts either ?repository_id= for an imported + // workspace repo, or ?path= for an on-machine folder discovered but not + // yet imported. Both paths bottom out in `listGitBranches`; the only + // difference is how the absolute path is resolved. + api.GET("/workspaces/:id/branches", h.httpListBranches) api.GET("/workspaces/:id/repositories/validate", h.httpValidateRepositoryPath) api.GET("/repositories/:id", h.httpGetRepository) - api.GET("/repositories/:id/branches", h.httpListRepositoryBranches) api.PATCH("/repositories/:id", h.httpUpdateRepository) api.DELETE("/repositories/:id", h.httpDeleteRepository) api.GET("/repositories/:id/scripts", h.httpListRepositoryScripts) @@ -150,20 +153,33 @@ func (h *RepositoryHandlers) httpValidateRepositoryPath(c *gin.Context) { }) } -func (h *RepositoryHandlers) httpListLocalRepositoryBranches(c *gin.Context) { +// httpListBranches handles the unified branch endpoint: +// +// GET /api/v1/workspaces/:id/branches?repository_id=X +// GET /api/v1/workspaces/:id/branches?path=/abs/path +// +// Exactly one of the two query params must be set. Both bottom out in the +// same `listGitBranches` call; the difference is just where the absolute +// path is resolved from (DB row vs request param). +func (h *RepositoryHandlers) httpListBranches(c *gin.Context) { + repoID := c.Query("repository_id") path := c.Query("path") - if path == "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "path is required"}) + if repoID == "" && path == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "repository_id or path is required"}) return } - branches, err := h.service.ListLocalRepositoryBranches(c.Request.Context(), path) + if repoID != "" && path != "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "specify only one of repository_id or path"}) + return + } + branches, err := h.service.ListBranches(c.Request.Context(), repoID, path) if err != nil { if errors.Is(err, service.ErrPathNotAllowed) { c.JSON(http.StatusBadRequest, gin.H{"error": "path is not within allowed roots"}) return } - h.logger.Error("failed to list local repository branches", zap.Error(err)) - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list local repository branches"}) + h.logger.Error("failed to list branches", zap.Error(err)) + c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list branches"}) return } dtoBranches := make([]dto.BranchDTO, len(branches)) @@ -239,26 +255,6 @@ func (h *RepositoryHandlers) httpGetRepository(c *gin.Context) { c.JSON(http.StatusOK, dto.FromRepository(repository)) } -func (h *RepositoryHandlers) httpListRepositoryBranches(c *gin.Context) { - branches, err := h.service.ListRepositoryBranches(c.Request.Context(), c.Param("id")) - if err != nil { - if errors.Is(err, service.ErrPathNotAllowed) { - c.JSON(http.StatusBadRequest, gin.H{"error": "repository path is not allowed"}) - return - } - h.logger.Error("failed to list repository branches", zap.Error(err)) - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list repository branches"}) - return - } - dtoBranches := make([]dto.BranchDTO, len(branches)) - for i, branch := range branches { - dtoBranches[i] = dto.FromBranch(branch) - } - c.JSON(http.StatusOK, dto.RepositoryBranchesResponse{ - Branches: dtoBranches, - Total: len(dtoBranches), - }) -} type httpUpdateRepositoryRequest struct { Name *string `json:"name"` diff --git a/apps/backend/internal/task/service/repository_discovery.go b/apps/backend/internal/task/service/repository_discovery.go index 5f51cbfec0..eefc17d653 100644 --- a/apps/backend/internal/task/service/repository_discovery.go +++ b/apps/backend/internal/task/service/repository_discovery.go @@ -127,50 +127,50 @@ func (s *Service) ValidateLocalRepositoryPath(ctx context.Context, path string) }, nil } -func (s *Service) ListRepositoryBranches(ctx context.Context, repoID string) ([]Branch, error) { - repo, err := s.repoEntities.GetRepository(ctx, repoID) +// ListBranches lists git branches for either an imported workspace repo +// (by id, path resolved from the DB row) or an on-machine folder (by path +// directly). Exactly one of `repoID` or `path` should be set; the handler +// validates that. +func (s *Service) ListBranches(ctx context.Context, repoID, path string) ([]Branch, error) { + resolvedPath, err := s.resolveBranchListingPath(ctx, repoID, path) if err != nil { return nil, err } - if repo.LocalPath == "" { - return nil, fmt.Errorf("repository local path is empty") - } - absPath, err := filepath.Abs(repo.LocalPath) - if err != nil { - return nil, fmt.Errorf("invalid repository path: %w", err) - } - if !isPathAllowed(absPath, s.discoveryRoots()) { - return nil, ErrPathNotAllowed - } - info, err := os.Stat(absPath) - if err != nil { - return nil, err - } - if !info.IsDir() { - return nil, fmt.Errorf("repository path is not a directory") - } - return listGitBranches(absPath) + return listGitBranches(resolvedPath) } -func (s *Service) ListLocalRepositoryBranches(ctx context.Context, path string) ([]Branch, error) { - if path == "" { - return nil, fmt.Errorf("repository path is required") +// resolveBranchListingPath turns the request inputs into a validated absolute +// path to list branches in. Validation is identical for both inputs (must be +// allowed root, must exist, must be a directory) — only the source differs. +func (s *Service) resolveBranchListingPath(ctx context.Context, repoID, path string) (string, error) { + switch { + case repoID != "": + repo, err := s.repoEntities.GetRepository(ctx, repoID) + if err != nil { + return "", err + } + if repo.LocalPath == "" { + return "", fmt.Errorf("repository local path is empty") + } + path = repo.LocalPath + case path == "": + return "", fmt.Errorf("repository_id or path is required") } absPath, err := filepath.Abs(path) if err != nil { - return nil, fmt.Errorf("invalid repository path: %w", err) + return "", fmt.Errorf("invalid repository path: %w", err) } if !isPathAllowed(absPath, s.discoveryRoots()) { - return nil, ErrPathNotAllowed + return "", ErrPathNotAllowed } info, err := os.Stat(absPath) if err != nil { - return nil, err + return "", err } if !info.IsDir() { - return nil, fmt.Errorf("repository path is not a directory") + return "", fmt.Errorf("repository path is not a directory") } - return listGitBranches(absPath) + return absPath, nil } func (s *Service) discoveryRoots() []string { diff --git a/apps/backend/internal/task/service/service_turns.go b/apps/backend/internal/task/service/service_turns.go index d502abd621..f6b9dd066e 100644 --- a/apps/backend/internal/task/service/service_turns.go +++ b/apps/backend/internal/task/service/service_turns.go @@ -5,6 +5,7 @@ import ( "database/sql" "errors" "fmt" + "path/filepath" "time" "github.com/google/uuid" @@ -202,9 +203,16 @@ func (s *Service) GetWorkspaceInfoForSession(ctx context.Context, taskID, sessio taskID = session.TaskID } - // Get workspace path from the session's worktree + // Get workspace path from the session's worktree(s). + // Multi-repo: every per-repo worktree sits as a sibling under the task root + // (~/.kandev/tasks/{taskDirName}/{repoName}/), so the workspace path agentctl + // needs is the parent of any one of them. Picking the first repo's path here + // would point agentctl at a single repo subdir and disable the per-repo + // tracker fan-out that scanRepositorySubdirs relies on. var workspacePath string - if len(session.Worktrees) > 0 { + if len(session.Worktrees) > 1 { + workspacePath = filepath.Dir(session.Worktrees[0].WorktreePath) + } else if len(session.Worktrees) == 1 { workspacePath = session.Worktrees[0].WorktreePath } diff --git a/apps/backend/internal/task/service/service_turns_test.go b/apps/backend/internal/task/service/service_turns_test.go index 65b504a881..98a7bdf0d5 100644 --- a/apps/backend/internal/task/service/service_turns_test.go +++ b/apps/backend/internal/task/service/service_turns_test.go @@ -2,6 +2,7 @@ package service import ( "context" + "fmt" "testing" "time" @@ -208,3 +209,52 @@ func TestGetWorkspaceInfoForSession_SessionNotFound(t *testing.T) { t.Fatal("expected error for nonexistent session") } } + +// Multi-repo: the workspace path agentctl boots with must be the task root +// (parent of every per-repo subdir) so its scanRepositorySubdirs detects all +// repos and starts a per-repo tracker for each. Returning a single repo's +// path would collapse fan-out into the legacy single-tracker mode and +// suppress the per-repo events the Changes panel needs to render headers. +func TestGetWorkspaceInfoForSession_MultiRepoReturnsTaskRoot(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + + setupTestTask(t, repo) + now := time.Now().UTC() + + session := &models.TaskSession{ + ID: "session-multi", + TaskID: "task-123", + State: models.TaskSessionStateCompleted, + StartedAt: now, + UpdatedAt: now, + } + if err := repo.CreateTaskSession(ctx, session); err != nil { + t.Fatalf("create session: %v", err) + } + for i, path := range []string{ + "/tmp/tasks/do-nothing_mvo/kandev", + "/tmp/tasks/do-nothing_mvo/thm", + } { + if err := repo.CreateTaskSessionWorktree(ctx, &models.TaskSessionWorktree{ + ID: fmt.Sprintf("wt%d", i), + SessionID: session.ID, + WorktreeID: fmt.Sprintf("wid%d", i), + RepositoryID: fmt.Sprintf("repo%d", i), + Position: i, + WorktreePath: path, + CreatedAt: now, + }); err != nil { + t.Fatalf("create worktree %d: %v", i, err) + } + } + + info, err := svc.GetWorkspaceInfoForSession(ctx, "task-123", session.ID) + if err != nil { + t.Fatalf("GetWorkspaceInfoForSession: %v", err) + } + if info.WorkspacePath != "/tmp/tasks/do-nothing_mvo" { + t.Errorf("expected WorkspacePath '/tmp/tasks/do-nothing_mvo' (task root), got %q", + info.WorkspacePath) + } +} diff --git a/apps/web/app/actions/workspaces.ts b/apps/web/app/actions/workspaces.ts index c608120b5c..b8f9aaede5 100644 --- a/apps/web/app/actions/workspaces.ts +++ b/apps/web/app/actions/workspaces.ts @@ -164,16 +164,6 @@ export async function listRepositoriesAction( return fetchJson(url); } -export async function listLocalRepositoryBranchesAction( - workspaceId: string, - path: string, -): Promise { - const params = `?path=${encodeURIComponent(path)}`; - return fetchJson( - `${apiBaseUrl}/api/v1/workspaces/${workspaceId}/repositories/branches${params}`, - ); -} - export async function discoverRepositoriesAction( workspaceId: string, root?: string, diff --git a/apps/web/components/review/review-diff-list-groups.tsx b/apps/web/components/review/review-diff-list-groups.tsx index 8fdf14f534..2f3d054ed0 100644 --- a/apps/web/components/review/review-diff-list-groups.tsx +++ b/apps/web/components/review/review-diff-list-groups.tsx @@ -1,32 +1,10 @@ "use client"; -import type { ReviewFile } from "./types"; - -/** - * Splits files into per-repository groups, preserving file order within each - * group. Files with no repository_name fall into a single trailing group with - * empty name (rendered without a repo header). - */ -export function groupFilesByRepository( - files: ReviewFile[], -): Array<{ repositoryName: string; files: ReviewFile[] }> { - const order: string[] = []; - const buckets = new Map(); - for (const file of files) { - const name = file.repository_name ?? ""; - if (!buckets.has(name)) { - buckets.set(name, []); - order.push(name); - } - buckets.get(name)!.push(file); - } - return order.map((name) => ({ repositoryName: name, files: buckets.get(name)! })); -} - /** * Sticky per-repository section header rendered above each group of files in - * the changes panel. Only shown when the panel actually has multiple repos - * (or any single named repo) — see ReviewDiffList for the gating logic. + * the changes panel. The grouping logic lives in @/lib/group-by-repo so the + * Review dialog and the dockview Changes tab share the same Map-based + * bucketer. Only this presentational header is review-specific. */ export function RepoGroupHeader({ name, fileCount }: { name: string; fileCount: number }) { // The empty-name group ("uncategorised" — no repository_name on its files) diff --git a/apps/web/components/review/review-diff-list.tsx b/apps/web/components/review/review-diff-list.tsx index b3e9547906..34615dabe2 100644 --- a/apps/web/components/review/review-diff-list.tsx +++ b/apps/web/components/review/review-diff-list.tsx @@ -31,7 +31,8 @@ import { useRunComment } from "@/hooks/domains/comments/use-run-comment"; import type { DiffComment } from "@/lib/diff/types"; import { diffSkipReasonLabel } from "./types"; import type { ReviewFile } from "./types"; -import { groupFilesByRepository, RepoGroupHeader } from "./review-diff-list-groups"; +import { RepoGroupHeader } from "./review-diff-list-groups"; +import { groupByRepositoryName } from "@/lib/group-by-repo"; function isMarkdownPath(filePath: string): boolean { const ext = filePath.split(".").pop()?.toLowerCase(); @@ -70,7 +71,10 @@ export const ReviewDiffList = memo(function ReviewDiffList({ const scrollContainerRef = useRef(null); // Find index of selected file - we need to force-load all files up to it const selectedIndex = selectedFile ? files.findIndex((f) => f.path === selectedFile) : -1; - const groups = useMemo(() => groupFilesByRepository(files), [files]); + const groups = useMemo( + () => groupByRepositoryName(files, (f) => f.repository_name), + [files], + ); const showRepoHeaders = groups.length > 1 || (groups[0]?.repositoryName ?? "") !== ""; return (
@@ -81,9 +85,9 @@ export const ReviewDiffList = memo(function ReviewDiffList({ data-repository-name={group.repositoryName || ""} > {showRepoHeaders && ( - + )} - {group.files.map((file) => ( + {group.items.map((file) => ( ; - branchesLoading: boolean; agentProfileOptions: ReturnType; executorProfileOptions: Array<{ value: string; @@ -31,53 +22,33 @@ type CreateModeSelectorsProps = { executorsLoading: boolean; isCreatingSession: boolean; fs: DialogFormState; - onBranchChange: (v: string) => void; onAgentProfileChange: (v: string) => void; onExecutorProfileChange: (v: string) => void; - isLocalExecutor: boolean; workflowAgentLocked: boolean; - repositories: Repository[]; }; /** - * Create/edit-mode form body section: branch + agent + executor selectors, - * followed by the multi-repo extra-repository rows. Extracted from - * task-create-dialog.tsx to keep the parent file under the lint line cap. + * Create/edit-mode form body section: agent + executor profile selectors. + * Repo and branch selection moved into the chip row above the description + * (see RepoChipsRow), so this row no longer carries those controls. */ export function CreateModeSelectors(props: CreateModeSelectorsProps) { return ( - <> - - - + ); } diff --git a/apps/web/components/task-create-dialog-effects.ts b/apps/web/components/task-create-dialog-effects.ts index 55e06f5bb3..8f65eecd58 100644 --- a/apps/web/components/task-create-dialog-effects.ts +++ b/apps/web/components/task-create-dialog-effects.ts @@ -1,14 +1,11 @@ "use client"; import { useEffect } from "react"; -import type { Repository, Executor, Branch } from "@/lib/types/http"; +import type { Repository, Executor } from "@/lib/types/http"; import type { AgentProfileOption } from "@/lib/state/slices"; import { DEFAULT_LOCAL_EXECUTOR_TYPE } from "@/lib/utils"; import { useToast } from "@/components/toast-provider"; -import { - discoverRepositoriesAction, - listLocalRepositoryBranchesAction, -} from "@/app/actions/workspaces"; +import { discoverRepositoriesAction } from "@/app/actions/workspaces"; import { getLocalStorage } from "@/lib/local-storage"; import { STORAGE_KEYS } from "@/lib/settings/constants"; import { listWorkflowSteps } from "@/lib/api/domains/workflow-api"; @@ -18,7 +15,6 @@ import type { StoreSelections, TaskCreateEffectsArgs, } from "@/components/task-create-dialog-types"; -import { autoSelectBranch } from "@/components/task-create-dialog-helpers"; export function useWorkflowAgentProfileEffect( fs: DialogFormState, @@ -79,25 +75,27 @@ export function useRepositoryAutoSelectEffect( workspaceId: string | null, repositories: Repository[], ) { - const { repositoryId, selectedLocalRepo, useGitHubUrl, setRepositoryId } = fs; + // Auto-seed the unified repos list with the user's last-used repo (or the + // workspace's only repo) on open. Skipped when URL mode is active or the + // user already has at least one row picked. + const { repositories: rows, useGitHubUrl, setRepositories } = fs; useEffect(() => { - if (!open || !workspaceId || repositoryId || selectedLocalRepo || useGitHubUrl) return; + if (!open || !workspaceId || useGitHubUrl) return; + const hasPickedRow = rows.some((r) => r.repositoryId || r.localPath); + if (hasPickedRow) return; const lastUsedRepoId = getLocalStorage(STORAGE_KEYS.LAST_REPOSITORY_ID, null); + let pickId: string | null = null; if (lastUsedRepoId && repositories.some((r: Repository) => r.id === lastUsedRepoId)) { - void Promise.resolve().then(() => setRepositoryId(lastUsedRepoId)); - return; + pickId = lastUsedRepoId; + } else if (repositories.length === 1) { + pickId = repositories[0].id; } - if (repositories.length === 1) - void Promise.resolve().then(() => setRepositoryId(repositories[0].id)); - }, [ - open, - repositories, - repositoryId, - selectedLocalRepo, - useGitHubUrl, - workspaceId, - setRepositoryId, - ]); + if (!pickId) return; + const id = pickId; + void Promise.resolve().then(() => + setRepositories([{ key: "row-0", repositoryId: id, branch: "" }]), + ); + }, [open, repositories, rows, useGitHubUrl, workspaceId, setRepositories]); } export function useDiscoverReposEffect( @@ -147,35 +145,9 @@ export function useDiscoverReposEffect( ]); } -export function useLocalBranchesEffect( - fs: DialogFormState, - open: boolean, - workspaceId: string | null, - toast: ReturnType["toast"], -) { - const { selectedLocalRepo, setLocalBranches, setLocalBranchesLoading } = fs; - useEffect(() => { - if (!open || !workspaceId || !selectedLocalRepo) return; - const repoPath = selectedLocalRepo.path; - void Promise.resolve() - .then(() => setLocalBranchesLoading(true)) - .then(() => listLocalRepositoryBranchesAction(workspaceId, repoPath)) - .then((r) => { - setLocalBranches(r.branches); - }) - .catch((e) => { - toast({ - title: "Failed to load branches", - description: e instanceof Error ? e.message : "Request failed", - variant: "error", - }); - setLocalBranches([]); - }) - .finally(() => { - setLocalBranchesLoading(false); - }); - }, [open, selectedLocalRepo, toast, workspaceId, setLocalBranches, setLocalBranchesLoading]); -} +// useLocalBranchesEffect removed: the chip row's RepoChip loads branches per +// repo via useRepositoryBranches now (the store dedupes by id), so we don't +// need a per-form "primary local repo branches" cache anymore. export function useDefaultSelectionsEffect( fs: DialogFormState, @@ -267,35 +239,30 @@ export function useDefaultSelectionsEffect( }, [executorProfileId, executors, setExecutorId]); } -export function useBranchAutoSelectEffect(fs: DialogFormState, branches: Branch[]) { - const { repositoryId, branch, localBranches, githubBranches, useGitHubUrl, setBranch } = fs; +/** + * Auto-selects a sensible branch in the GitHub URL flow. Per-repo (workspace + * or discovered) branch auto-select happens inside RepoChip when a row's + * branches load — that keeps the per-row state confined to the chip. + */ +export function useBranchAutoSelectEffect(fs: DialogFormState) { + const { githubBranch, githubBranches, useGitHubUrl, setGitHubBranch, githubPrHeadBranch } = fs; useEffect(() => { - if (!repositoryId || branch) return; - autoSelectBranch(branches, setBranch); - }, [branch, branches, repositoryId, setBranch]); - useEffect(() => { - if (repositoryId || localBranches.length === 0 || branch) return; - autoSelectBranch(localBranches, setBranch); - }, [branch, localBranches, repositoryId, setBranch]); - const { githubPrHeadBranch } = fs; - useEffect(() => { - if (!useGitHubUrl || githubBranches.length === 0 || branch) return; - // If a PR URL was detected, prefer the PR's head branch. + if (!useGitHubUrl || githubBranches.length === 0 || githubBranch) return; if (githubPrHeadBranch) { const prBranch = githubBranches.find((b) => b.name === githubPrHeadBranch); if (prBranch) { - setBranch(prBranch.name); + setGitHubBranch(prBranch.name); return; } } - // GitHub URL branches are referenced by name only (no remote prefix). + // GitHub URL branches are referenced by name only (no remote prefix); // selectPreferredBranch expects origin-prefixed remotes, so pick directly. const preferred = githubBranches.find((b) => b.name === "main") ?? githubBranches.find((b) => b.name === "master") ?? githubBranches[0]; - if (preferred) setBranch(preferred.name); - }, [branch, githubBranches, useGitHubUrl, setBranch, githubPrHeadBranch]); + if (preferred) setGitHubBranch(preferred.name); + }, [githubBranch, githubBranches, useGitHubUrl, setGitHubBranch, githubPrHeadBranch]); } /** Parse a GitHub URL to extract owner, repo, and optional PR number. Returns null if invalid. */ @@ -405,14 +372,13 @@ export function useGitHubUrlBranchesEffect(fs: DialogFormState, open: boolean) { } export function useTaskCreateDialogEffects(fs: DialogFormState, args: TaskCreateEffectsArgs) { - const { open, workspaceId, workflowId, repositories, repositoriesLoading, branches } = args; + const { open, workspaceId, workflowId, repositories, repositoriesLoading } = args; const { agentProfiles, executors, workspaceDefaults, toast, workflows } = args; useWorkflowStepsEffect(fs, workflowId); useWorkflowAgentProfileEffect(fs, workflows, agentProfiles); useRepositoryAutoSelectEffect(fs, open, workspaceId, repositories); useDiscoverReposEffect(fs, open, workspaceId, repositoriesLoading, toast); - useBranchAutoSelectEffect(fs, branches); - useLocalBranchesEffect(fs, open, workspaceId, toast); + useBranchAutoSelectEffect(fs); useDefaultSelectionsEffect(fs, open, { agentProfiles, executors, workspaceDefaults }, workflows); useGitHubUrlBranchesEffect(fs, open); } diff --git a/apps/web/components/task-create-dialog-extra-repos-state.ts b/apps/web/components/task-create-dialog-extra-repos-state.ts deleted file mode 100644 index c57cdd03ce..0000000000 --- a/apps/web/components/task-create-dialog-extra-repos-state.ts +++ /dev/null @@ -1,49 +0,0 @@ -"use client"; - -import { useCallback, useRef, useState } from "react"; -import type { ExtraRepositoryRow } from "@/components/task-create-dialog-types"; - -/** - * Manages the extra-repository rows for multi-repo task creation. The primary - * repo lives on the form's repositoryId/branch; extras here represent the - * 2nd, 3rd, ... rows that the user adds via "+ Add repository". - * - * `nextKey` increments to give each row a stable client-side key without - * relying on array indices (which would shift on removal and break inputs). - */ -export function useExtraRepositoriesState() { - const [extraRepositories, setExtraRepositories] = useState([]); - const nextKeyRef = useRef(0); - - const addExtraRepository = useCallback(() => { - nextKeyRef.current += 1; - const key = `extra-${nextKeyRef.current}`; - setExtraRepositories((rows) => [...rows, { key, repositoryId: "", branch: "" }]); - }, []); - - const removeExtraRepository = useCallback((key: string) => { - setExtraRepositories((rows) => rows.filter((r) => r.key !== key)); - }, []); - - const updateExtraRepository = useCallback( - (key: string, patch: Partial) => { - setExtraRepositories((rows) => - rows.map((r) => (r.key === key ? { ...r, ...patch } : r)), - ); - }, - [], - ); - - const resetExtraRepositories = useCallback(() => { - setExtraRepositories([]); - nextKeyRef.current = 0; - }, []); - - return { - extraRepositories, - addExtraRepository, - removeExtraRepository, - updateExtraRepository, - resetExtraRepositories, - }; -} diff --git a/apps/web/components/task-create-dialog-extra-repos.test.tsx b/apps/web/components/task-create-dialog-extra-repos.test.tsx deleted file mode 100644 index 4f875d2454..0000000000 --- a/apps/web/components/task-create-dialog-extra-repos.test.tsx +++ /dev/null @@ -1,152 +0,0 @@ -import { describe, it, expect, vi, afterEach } from "vitest"; -import { render, screen, fireEvent, cleanup } from "@testing-library/react"; -import type { Repository } from "@/lib/types/http"; -import type { DialogFormState, ExtraRepositoryRow } from "./task-create-dialog-types"; - -// Mock branches hook so the BranchSelector inside the row doesn't blow up. -// vi.mock is hoisted, so it applies to the static import below. -vi.mock("@/hooks/domains/workspace/use-repository-branches", () => ({ - useRepositoryBranches: () => ({ branches: [], isLoading: false }), -})); - -import { ExtraRepositoryRows } from "./task-create-dialog-extra-repos"; - -afterEach(cleanup); - -const REPO_FRONT_ID = "repo-front"; -const REPO_BACK_ID = "repo-back"; - -function makeRepo(id: string, name: string): Repository { - return { - id, - workspace_id: "ws-1", - name, - source_type: "local", - local_path: `/repos/${name}`, - default_branch: "main", - created_at: new Date().toISOString(), - updated_at: new Date().toISOString(), - } as Repository; -} - -function makeFs(overrides: Partial): DialogFormState { - // Only the fields ExtraRepositoryRows actually reads need to be real. - return { - repositoryId: REPO_FRONT_ID, - extraRepositories: [] as ExtraRepositoryRow[], - addExtraRepository: vi.fn(), - removeExtraRepository: vi.fn(), - updateExtraRepository: vi.fn(), - ...overrides, - } as unknown as DialogFormState; -} - -describe("ExtraRepositoryRows", () => { - it("disables the Add button until a primary repo is selected", async () => { - - render( - , - ); - const btn = screen.getByTestId("add-extra-repository") as HTMLButtonElement; - expect(btn.disabled).toBe(true); - }); - - it("calls addExtraRepository when the Add button is clicked", async () => { - - const fs = makeFs({}); - render( - , - ); - fireEvent.click(screen.getByTestId("add-extra-repository")); - expect(fs.addExtraRepository).toHaveBeenCalledOnce(); - }); - - it("renders one editable row per extra repository", async () => { - - render( - , - ); - expect(screen.getAllByTestId("extra-repository-row")).toHaveLength(2); - }); - - it("hides the section when the task is already started", async () => { - - const { container } = render( - , - ); - expect(container.querySelector("[data-testid='extra-repositories']")).toBeNull(); - }); - - it("opening a row's dropdown shows the unused workspace repos", async () => { - - render( - , - ); - // Open the row's repository combobox. - const triggers = screen.getAllByTestId("repository-selector"); - fireEvent.click(triggers[0]); - // The primary repo (frontend) is excluded; the other two should be offered. - expect(screen.queryByText("backend")).not.toBeNull(); - expect(screen.queryByText("shared")).not.toBeNull(); - expect(screen.queryByText("frontend")).toBeNull(); - }); - - it("disables the Add button when no other workspace repos are available", async () => { - - render( - , - ); - const btn = screen.getByTestId("add-extra-repository") as HTMLButtonElement; - expect(btn.disabled).toBe(true); - }); -}); diff --git a/apps/web/components/task-create-dialog-extra-repos.tsx b/apps/web/components/task-create-dialog-extra-repos.tsx deleted file mode 100644 index bbcc480d58..0000000000 --- a/apps/web/components/task-create-dialog-extra-repos.tsx +++ /dev/null @@ -1,202 +0,0 @@ -"use client"; - -import { useMemo, useCallback } from "react"; -import { IconPlus, IconX } from "@tabler/icons-react"; -import { Button } from "@kandev/ui/button"; -import { RepositorySelector, BranchSelector } from "@/components/task-create-dialog-selectors"; -import { useRepositoryBranches } from "@/hooks/domains/workspace/use-repository-branches"; -import { useBranchOptions, useRepositoryOptions } from "@/components/task-create-dialog-options"; -import type { Repository } from "@/lib/types/http"; -import type { - ExtraRepositoryRow, - DialogFormState, -} from "@/components/task-create-dialog-types"; - -type ExtraRepositoryRowsProps = { - fs: DialogFormState; - repositories: Repository[]; - isTaskStarted: boolean; - /** - * Disable the Add button when no primary repository is selected — extra - * rows only make sense when the task has at least one repo defined. - */ - primarySelected: boolean; -}; - -/** - * Renders the "+ Add repository" control plus one editable row per extra - * repo on the task. The primary repo is rendered elsewhere (header + - * CreateEditSelectors); this component only handles the 2nd, 3rd, … - * - * Each row is a pair of (repository combobox, base-branch combobox). - * Branches for the chosen repo are fetched on demand via the existing - * useRepositoryBranches hook so the user gets the right list per repo. - */ -export function ExtraRepositoryRows({ - fs, - repositories, - isTaskStarted, - primarySelected, -}: ExtraRepositoryRowsProps) { - // Hide the section in started/edit modes where the repo set is fixed. - if (isTaskStarted) return null; - - // Count workspace repositories the user could still add: total minus primary - // minus all already-chosen extras. When zero, we disable the Add button so - // the user gets a clear signal instead of a row with an empty dropdown. - const usedIds = collectUsedRepoIds(fs, ""); - const remainingCount = repositories.filter((r) => !usedIds.has(r.id)).length; - const canAddMore = primarySelected && remainingCount > 0; - const addButtonHint = computeAddButtonHint(primarySelected, remainingCount); - - return ( -
- {fs.extraRepositories.map((row) => ( - fs.updateExtraRepository(row.key, { repositoryId: value })} - onBranchChange={(value) => fs.updateExtraRepository(row.key, { branch: value })} - onRemove={() => fs.removeExtraRepository(row.key)} - /> - ))} - -
- ); -} - -function computeAddButtonHint( - primarySelected: boolean, - remainingCount: number, -): string | undefined { - if (!primarySelected) return "Select the primary repository first"; - if (remainingCount === 0) return "All workspace repositories are already added"; - return undefined; -} - -function computeBranchPlaceholder( - repoId: string, - branchesLoading: boolean, - branchCount: number, -): string { - if (!repoId) return "Select repository first"; - if (branchesLoading) return "Loading branches..."; - if (branchCount === 0) return "No branches found"; - return "Select branch"; -} - -function collectUsedRepoIds(fs: DialogFormState, exceptKey: string): Set { - const ids = new Set(); - if (fs.repositoryId) ids.add(fs.repositoryId); - for (const r of fs.extraRepositories) { - if (r.key !== exceptKey && r.repositoryId) ids.add(r.repositoryId); - } - return ids; -} - -type ExtraRepositoryRowItemProps = { - row: ExtraRepositoryRow; - repositories: Repository[]; - excludedRepoIds: Set; - onRepositoryChange: (value: string) => void; - onBranchChange: (value: string) => void; - onRemove: () => void; -}; - -function ExtraRepositoryRowItem({ - row, - repositories, - excludedRepoIds, - onRepositoryChange, - onBranchChange, - onRemove, -}: ExtraRepositoryRowItemProps) { - // Filter the repo list to drop already-used repos (prevents duplicates - // before the user even sees them in the dropdown). - const filteredRepos = useMemo( - () => repositories.filter((r) => !excludedRepoIds.has(r.id)), - [repositories, excludedRepoIds], - ); - const { headerRepositoryOptions } = useRepositoryOptions(filteredRepos, []); - const { branches, isLoading: branchesLoading } = useRepositoryBranches( - row.repositoryId || null, - Boolean(row.repositoryId), - ); - const branchOptions = useBranchOptions(branches); - - const branchPlaceholder = computeBranchPlaceholder( - row.repositoryId, - branchesLoading, - branchOptions.length, - ); - - const handleRepoChange = useCallback( - (value: string) => { - onRepositoryChange(value); - // Reset branch when the repo changes — the previous branch may not exist - // on the new repo, and stale text would mislead the user. - onBranchChange(""); - }, - [onRepositoryChange, onBranchChange], - ); - - return ( -
-
- -
-
- -
- -
- ); -} diff --git a/apps/web/components/task-create-dialog-footer.test.ts b/apps/web/components/task-create-dialog-footer.test.ts index 8bcc92a00f..69b4395f97 100644 --- a/apps/web/components/task-create-dialog-footer.test.ts +++ b/apps/web/components/task-create-dialog-footer.test.ts @@ -29,7 +29,7 @@ function makeProps( hasTitle: true, hasDescription: true, hasRepositorySelection: true, - branch: "main", + hasAllBranches: true, agentProfileId: "agent-1", workspaceId: "ws-1", effectiveWorkflowId: "wf-1", @@ -69,7 +69,9 @@ describe("computeDisabledReason (start-task)", () => { }); it("flags missing branch", () => { - expect(computeDisabledReason(makeProps({ branch: "" }), KIND_START)).toBe(REASON_BRANCH); + expect(computeDisabledReason(makeProps({ hasAllBranches: false }), KIND_START)).toBe( + REASON_BRANCH, + ); }); it("flags missing workspace in create mode", () => { @@ -157,7 +159,7 @@ describe("computeDisabledReason (default)", () => { isSessionMode: true, hasTitle: false, hasRepositorySelection: false, - branch: "", + hasAllBranches: false, }), KIND_DEFAULT, ), diff --git a/apps/web/components/task-create-dialog-footer.tsx b/apps/web/components/task-create-dialog-footer.tsx index d245c192f6..e03245c461 100644 --- a/apps/web/components/task-create-dialog-footer.tsx +++ b/apps/web/components/task-create-dialog-footer.tsx @@ -216,7 +216,12 @@ export type TaskCreateDialogFooterProps = { hasTitle: boolean; hasDescription: boolean; hasRepositorySelection: boolean; - branch: string; + /** + * True when every selected repo has a base branch picked (and the URL flow's + * branch, if active). Was previously `branch: string` for the single-repo + * primary; now aggregated upstream because each row has its own branch. + */ + hasAllBranches: boolean; agentProfileId: string; workspaceId: string | null; effectiveWorkflowId: string | null; @@ -245,7 +250,7 @@ function computeBaseDisabled(props: TaskCreateDialogFooterProps) { props.isCreatingTask || !props.hasTitle || !props.hasRepositorySelection || - !props.branch || + !props.hasAllBranches || missingCtx ); } @@ -263,7 +268,7 @@ export const REASON_DESCRIPTION = "Add a session description"; function baseReason(props: TaskCreateDialogFooterProps): string | null { if (!props.hasTitle) return REASON_TITLE; if (!props.hasRepositorySelection) return REASON_REPO; - if (!props.branch) return REASON_BRANCH; + if (!props.hasAllBranches) return REASON_BRANCH; if (props.isCreateMode && !props.workspaceId) return REASON_WORKSPACE; if (props.isCreateMode && !props.effectiveWorkflowId) return REASON_WORKFLOW; return null; diff --git a/apps/web/components/task-create-dialog-form-body.tsx b/apps/web/components/task-create-dialog-form-body.tsx index f059e454ea..0fb50c51da 100644 --- a/apps/web/components/task-create-dialog-form-body.tsx +++ b/apps/web/components/task-create-dialog-form-body.tsx @@ -19,12 +19,6 @@ type SelectorOption = { type CreateEditSelectorsProps = { isTaskStarted: boolean; - hasRepositorySelection: boolean; - branchOptions: SelectorOption[]; - branch: string; - onBranchChange: (value: string) => void; - branchesLoading: boolean; - localBranchesLoading: boolean; agentProfiles: AgentProfileOption[]; agentProfilesLoading: boolean; agentProfileOptions: SelectorOption[]; @@ -39,15 +33,6 @@ type CreateEditSelectorsProps = { executorProfileId: string; onExecutorProfileChange: (value: string) => void; executorsLoading: boolean; - BranchSelectorComponent: React.ComponentType<{ - options: SelectorOption[]; - value: string; - onValueChange: (value: string) => void; - disabled: boolean; - placeholder: string; - searchPlaceholder: string; - emptyMessage: string; - }>; AgentSelectorComponent: React.ComponentType<{ options: SelectorOption[]; value: string; @@ -64,19 +49,11 @@ type CreateEditSelectorsProps = { placeholder: string; triggerClassName?: string; }>; - isLocalExecutor: boolean; - useGitHubUrl: boolean; workflowAgentLocked: boolean; }; export const CreateEditSelectors = memo(function CreateEditSelectors({ isTaskStarted, - hasRepositorySelection, - branchOptions, - branch, - onBranchChange, - branchesLoading, - localBranchesLoading, agentProfiles, agentProfilesLoading, agentProfileOptions, @@ -87,10 +64,7 @@ export const CreateEditSelectors = memo(function CreateEditSelectors({ executorProfileId, onExecutorProfileChange, executorsLoading, - isLocalExecutor, - useGitHubUrl, workflowAgentLocked, - BranchSelectorComponent, AgentSelectorComponent, ExecutorProfileSelectorComponent, }: CreateEditSelectorsProps) { @@ -98,41 +72,16 @@ export const CreateEditSelectors = memo(function CreateEditSelectors({ const agentLockedByWorkflow = workflowAgentLocked; - const isLocalWithoutGitHubUrl = isLocalExecutor && !useGitHubUrl; - - const branchPlaceholder = (() => { - if (isLocalWithoutGitHubUrl) return "Uses current branch"; - if (!hasRepositorySelection) return "Select repository first"; - if (branchesLoading || localBranchesLoading) return "Loading branches..."; - return branchOptions.length > 0 ? "Select branch" : "No branches found"; - })(); - - const branchDisabled = - isLocalWithoutGitHubUrl || - !hasRepositorySelection || - branchesLoading || - localBranchesLoading || - branchOptions.length === 0; - const agentPlaceholder = (() => { if (agentProfilesLoading) return "Loading agents..."; if (agentProfiles.length === 0) return "No agents available"; return "Select agent"; })(); + // Branch + repo selection moved into the chip row above the description; + // this row carries only agent and executor profile selectors now. return ( -
-
- -
+
{agentProfiles.length === 0 && !agentProfilesLoading ? (
diff --git a/apps/web/components/task-create-dialog-handlers.ts b/apps/web/components/task-create-dialog-handlers.ts index e11098fa3b..7749eeb04b 100644 --- a/apps/web/components/task-create-dialog-handlers.ts +++ b/apps/web/components/task-create-dialog-handlers.ts @@ -2,43 +2,42 @@ import { useCallback } from "react"; import type { Repository } from "@/lib/types/http"; -import type { DialogFormState } from "@/components/task-create-dialog-types"; +import type { DialogFormState, TaskRepoRow } from "@/components/task-create-dialog-types"; import { setLocalStorage } from "@/lib/local-storage"; import { STORAGE_KEYS } from "@/lib/settings/constants"; /** - * Centralizes form-field change handlers for the task-create dialog. Lives - * in its own file so the main state hook stays under the file-length lint cap. + * Centralizes form-field change handlers for the task-create dialog. + * + * The dialog now stores all repos in a single `fs.repositories` list (no + * "primary vs extras" split), so the per-row handlers are uniform: changing + * a repo on row N is the same op whether N==0 or N==5. */ export function useDialogHandlers(fs: DialogFormState, repositories: Repository[]) { - const handleSelectLocalRepository = useCallback( - (path: string) => { - fs.setDiscoveredRepoPath(path); - fs.setSelectedLocalRepo(fs.discoveredRepositories.find((r) => r.path === path) ?? null); - fs.setRepositoryId(""); - fs.setBranch(""); - fs.setLocalBranches([]); + /** + * Resolves a picker value into the right shape for a row: + * - If it matches a workspace repo id → `{ repositoryId, localPath: undefined }`. + * - Otherwise treat as a discovered on-machine path → `{ localPath, repositoryId: undefined }`. + * The branch is reset because the previous branch may not exist on the new repo. + */ + const handleRowRepositoryChange = useCallback( + (key: string, value: string) => { + const isWorkspaceRepo = repositories.some((r: Repository) => r.id === value); + const patch: Partial = isWorkspaceRepo + ? { repositoryId: value, localPath: undefined, branch: "" } + : { repositoryId: undefined, localPath: value, branch: "" }; + fs.updateRepository(key, patch); + if (isWorkspaceRepo) setLocalStorage(STORAGE_KEYS.LAST_REPOSITORY_ID, value); }, - [fs], + [repositories, fs], ); - const handleRepositoryChange = useCallback( - (value: string) => { - if (repositories.find((r: Repository) => r.id === value)) { - fs.setRepositoryId(value); - setLocalStorage(STORAGE_KEYS.LAST_REPOSITORY_ID, value); - fs.setDiscoveredRepoPath(""); - fs.setSelectedLocalRepo(null); - fs.setLocalBranches([]); - fs.setBranch(""); - fs.setUseGitHubUrl(false); - fs.setGitHubUrl(""); - fs.setGitHubBranches([]); - return; - } - handleSelectLocalRepository(value); + const handleRowBranchChange = useCallback( + (key: string, value: string) => { + fs.updateRepository(key, { branch: value }); + setLocalStorage(STORAGE_KEYS.LAST_BRANCH, value); }, - [repositories, fs, handleSelectLocalRepository], + [fs], ); const handleAgentProfileChange = useCallback( @@ -62,9 +61,9 @@ export function useDialogHandlers(fs: DialogFormState, repositories: Repository[ }, [fs], ); - const handleBranchChange = useCallback( + const handleGitHubBranchChange = useCallback( (value: string) => { - fs.setBranch(value); + fs.setGitHubBranch(value); setLocalStorage(STORAGE_KEYS.LAST_BRANCH, value); }, [fs], @@ -76,27 +75,31 @@ export function useDialogHandlers(fs: DialogFormState, repositories: Repository[ [fs], ); + /** + * Toggles between "repo chips" mode and "GitHub URL" mode. URL mode + * replaces the chip row with a single URL input; flipping back restores + * the chip flow with whatever rows were already there. + */ const handleToggleGitHubUrl = useCallback(() => { const next = !fs.useGitHubUrl; fs.setUseGitHubUrl(next); if (next) { - fs.setRepositoryId(""); - fs.setDiscoveredRepoPath(""); - fs.setSelectedLocalRepo(null); - fs.setLocalBranches([]); + // Stash the rows aside while in URL mode; flipping back keeps them. + // (Nothing to do here — chips re-render off `fs.repositories` once + // useGitHubUrl flips back to false.) } else { fs.setGitHubUrl(""); fs.setGitHubBranches([]); fs.setGitHubUrlError(null); fs.setGitHubPrHeadBranch(null); } - fs.setBranch(""); + fs.setGitHubBranch(""); }, [fs]); const handleGitHubUrlChange = useCallback( (value: string) => { fs.setGitHubUrl(value); - fs.setBranch(""); + fs.setGitHubBranch(""); fs.setGitHubBranches([]); fs.setGitHubUrlError(null); fs.setGitHubPrHeadBranch(null); @@ -105,11 +108,12 @@ export function useDialogHandlers(fs: DialogFormState, repositories: Repository[ ); return { - handleRepositoryChange, + handleRowRepositoryChange, + handleRowBranchChange, handleAgentProfileChange, handleExecutorProfileChange, handleTaskNameChange, - handleBranchChange, + handleGitHubBranchChange, handleWorkflowChange, handleToggleGitHubUrl, handleGitHubUrlChange, diff --git a/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts b/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts index 1d52591731..101e819dd1 100644 --- a/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts +++ b/apps/web/components/task-create-dialog-helpers.multi-repo.test.ts @@ -1,68 +1,82 @@ import { describe, it, expect } from "vitest"; import { buildRepositoriesPayload } from "./task-create-dialog-helpers"; -describe("buildRepositoriesPayload — multi-repo", () => { - it("appends extra repositories after the primary in order", () => { +describe("buildRepositoriesPayload — unified rows", () => { + it("maps each row in order, dropping empty ones silently", () => { const payload = buildRepositoriesPayload({ useGitHubUrl: false, githubUrl: "", - branch: "main", + githubBranch: "", githubPrHeadBranch: null, - repositoryId: "repo-front", - selectedLocalRepo: null, - extraRepositories: [ - { repositoryId: "repo-back", branch: "develop" }, - { repositoryId: "repo-shared", branch: "" }, + repositories: [ + { key: "r0", repositoryId: "repo-front", branch: "main" }, + { key: "r1", repositoryId: "repo-back", branch: "develop" }, + { key: "r2", branch: "" }, // no repo picked yet — dropped + { key: "r3", repositoryId: "repo-shared", branch: "" }, ], + discoveredRepositories: [], }); expect(payload).toEqual([ - { repository_id: "repo-front", base_branch: "main", checkout_branch: undefined }, + { repository_id: "repo-front", base_branch: "main" }, { repository_id: "repo-back", base_branch: "develop" }, { repository_id: "repo-shared", base_branch: undefined }, ]); }); - it("drops extra rows with empty repository_id", () => { + it("emits local_path + default_branch for discovered (on-machine) rows", () => { const payload = buildRepositoriesPayload({ useGitHubUrl: false, githubUrl: "", - branch: "main", + githubBranch: "", githubPrHeadBranch: null, - repositoryId: "repo-front", - selectedLocalRepo: null, - extraRepositories: [ - { repositoryId: "", branch: "" }, - { repositoryId: "repo-back", branch: "main" }, + repositories: [ + { key: "r0", localPath: "/home/me/projects/local-project", branch: "trunk" }, + { key: "r1", repositoryId: "repo-back", branch: "main" }, ], - }); - expect(payload).toHaveLength(2); - expect(payload[1].repository_id).toBe("repo-back"); - }); - - it("returns extras even when no primary is set (recovery shape)", () => { - const payload = buildRepositoriesPayload({ - useGitHubUrl: false, - githubUrl: "", - branch: "", - githubPrHeadBranch: null, - repositoryId: "", - selectedLocalRepo: null, - extraRepositories: [{ repositoryId: "repo-x", branch: "main" }], - }); - expect(payload).toEqual([{ repository_id: "repo-x", base_branch: "main" }]); - }); - - it("single-repo single call: behaves identically to before extraRepositories was added", () => { - const payload = buildRepositoriesPayload({ - useGitHubUrl: false, - githubUrl: "", - branch: "main", - githubPrHeadBranch: null, - repositoryId: "repo-only", - selectedLocalRepo: null, + discoveredRepositories: [ + { path: "/home/me/projects/local-project", default_branch: "trunk" }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ] as any, }); expect(payload).toEqual([ - { repository_id: "repo-only", base_branch: "main", checkout_branch: undefined }, + { + repository_id: "", + base_branch: "trunk", + local_path: "/home/me/projects/local-project", + default_branch: "trunk", + }, + { repository_id: "repo-back", base_branch: "main" }, ]); }); + + it("URL mode produces a single github_url entry and ignores the rows", () => { + const payload = buildRepositoriesPayload({ + useGitHubUrl: true, + githubUrl: "github.com/owner/repo", + githubBranch: "feature-x", + githubPrHeadBranch: null, + repositories: [{ key: "r0", repositoryId: "ignored", branch: "ignored" }], + discoveredRepositories: [], + }); + expect(payload).toEqual([ + { + repository_id: "", + base_branch: "feature-x", + checkout_branch: undefined, + github_url: "github.com/owner/repo", + }, + ]); + }); + + it("single-row workspace repo: payload mirrors the row", () => { + const payload = buildRepositoriesPayload({ + useGitHubUrl: false, + githubUrl: "", + githubBranch: "", + githubPrHeadBranch: null, + repositories: [{ key: "r0", repositoryId: "repo-only", branch: "main" }], + discoveredRepositories: [], + }); + expect(payload).toEqual([{ repository_id: "repo-only", base_branch: "main" }]); + }); }); diff --git a/apps/web/components/task-create-dialog-helpers.ts b/apps/web/components/task-create-dialog-helpers.ts index 6850c90462..02b49bae61 100644 --- a/apps/web/components/task-create-dialog-helpers.ts +++ b/apps/web/components/task-create-dialog-helpers.ts @@ -2,7 +2,7 @@ import type { useRouter } from "next/navigation"; import type { Task, Branch, LocalRepository } from "@/lib/types/http"; import type { AgentProfileOption } from "@/lib/state/slices"; import type { AppState } from "@/lib/state/store"; -import type { StepType } from "@/components/task-create-dialog-types"; +import type { StepType, TaskRepoRow } from "@/components/task-create-dialog-types"; import { selectPreferredBranch } from "@/lib/utils"; import { getLocalStorage } from "@/lib/local-storage"; import { STORAGE_KEYS } from "@/lib/settings/constants"; @@ -138,93 +138,66 @@ export function validateCreateInputs(inputs: { trimmedTitle: string; workspaceId: string | null; effectiveWorkflowId: string | null; - repositoryId: string; - selectedLocalRepo: LocalRepository | null; + /** Unified repos list. The form is valid if any row has a repo set OR URL mode is filled. */ + repositories: TaskRepoRow[]; githubUrl?: string; agentProfileId: string; }): boolean { + const hasRepo = + inputs.repositories.some((r) => r.repositoryId || r.localPath) || + Boolean(inputs.githubUrl?.trim()); return Boolean( inputs.trimmedTitle && - inputs.workspaceId && - inputs.effectiveWorkflowId && - inputs.agentProfileId && - (inputs.repositoryId || inputs.selectedLocalRepo || inputs.githubUrl?.trim()), + inputs.workspaceId && + inputs.effectiveWorkflowId && + inputs.agentProfileId && + hasRepo, ); } /** - * One additional repository row from the multi-repo task create form. - * The primary repo is built from useGitHubUrl/repositoryId/selectedLocalRepo - * fields (see buildRepositoriesPayload below); these extras are appended. + * Builds the repositories payload for task creation from the unified list. + * + * - URL mode produces a single entry with `github_url`. + * - Otherwise each row maps to either a workspace `repository_id` or a + * discovered `local_path`. Empty rows are dropped silently so a user + * can leave an unfinished chip without blocking submit; duplicate + * detection happens on the backend. */ -export type ExtraRepoInput = { - repositoryId: string; - branch: string; -}; - -/** Builds the repositories payload for task creation from the current form state. */ export function buildRepositoriesPayload(opts: { useGitHubUrl: boolean; githubUrl: string; - branch: string; + githubBranch: string; githubPrHeadBranch: string | null; - repositoryId: string; - selectedLocalRepo: LocalRepository | null; - /** - * Additional repository rows entered via the "+ Add repository" button. - * Empty rows (no repository_id) are dropped silently so a user can leave - * an unfinished row without blocking submit; duplicate detection happens - * at the backend (see Phase 8). - */ - extraRepositories?: ExtraRepoInput[]; -}): NonNullable { - const primary = buildPrimaryRepoPayload(opts); - const extras = (opts.extraRepositories ?? []) - .filter((row) => row.repositoryId.trim() !== "") - .map((row) => ({ - repository_id: row.repositoryId, - base_branch: row.branch || undefined, - })); - if (primary.length === 0) return extras; - return [...primary, ...extras]; -} - -function buildPrimaryRepoPayload(opts: { - useGitHubUrl: boolean; - githubUrl: string; - branch: string; - githubPrHeadBranch: string | null; - repositoryId: string; - selectedLocalRepo: LocalRepository | null; + repositories: TaskRepoRow[]; + /** Used to look up `default_branch` for `localPath` rows. */ + discoveredRepositories: LocalRepository[]; }): NonNullable { if (opts.useGitHubUrl && opts.githubUrl) { return [ { repository_id: "", - base_branch: opts.branch || undefined, + base_branch: opts.githubBranch || undefined, checkout_branch: opts.githubPrHeadBranch || undefined, github_url: opts.githubUrl.trim(), }, ]; } - if (opts.repositoryId) { - return [ - { - repository_id: opts.repositoryId, - base_branch: opts.branch || undefined, - checkout_branch: opts.githubPrHeadBranch || undefined, - }, - ]; - } - if (opts.selectedLocalRepo) { - return [ - { + return opts.repositories + .filter((row) => row.repositoryId || row.localPath) + .map((row) => { + if (row.repositoryId) { + return { + repository_id: row.repositoryId, + base_branch: row.branch || undefined, + }; + } + const local = opts.discoveredRepositories.find((d) => d.path === row.localPath); + return { repository_id: "", - base_branch: opts.branch || undefined, - local_path: opts.selectedLocalRepo.path, - default_branch: opts.selectedLocalRepo.default_branch || undefined, - }, - ]; - } - return []; + base_branch: row.branch || undefined, + local_path: row.localPath, + default_branch: local?.default_branch || undefined, + }; + }); } diff --git a/apps/web/components/task-create-dialog-repo-chips.test.tsx b/apps/web/components/task-create-dialog-repo-chips.test.tsx new file mode 100644 index 0000000000..8ef46d42b4 --- /dev/null +++ b/apps/web/components/task-create-dialog-repo-chips.test.tsx @@ -0,0 +1,313 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, fireEvent, cleanup } from "@testing-library/react"; +import type { Branch, Repository } from "@/lib/types/http"; +import type { DialogFormState, TaskRepoRow } from "./task-create-dialog-types"; + +// One mocked branches hook now — id-based and path-based rows go through +// the same loader. Tests can override the return value when they need +// branch-specific assertions; the mockBySource handle lets a test prove +// which kind of source the chip passed in. +const lastBranchSource = vi.hoisted((): { value: unknown } => ({ value: null })); +const mockBranches = vi.hoisted( + (): { value: { branches: Branch[]; isLoading: boolean } } => ({ + value: { branches: [], isLoading: false }, + }), +); + +vi.mock("@/hooks/domains/workspace/use-repository-branches", () => ({ + useBranches: (source: unknown) => { + lastBranchSource.value = source; + return mockBranches.value; + }, +})); + +import { RepoChipsRow } from "./task-create-dialog-repo-chips"; + +afterEach(cleanup); + +const REPO_FRONT_ID = "repo-front"; +const REPO_BACK_ID = "repo-back"; + +function makeRepo(id: string, name: string): Repository { + return { + id, + workspace_id: "ws-1", + name, + source_type: "local", + local_path: `/repos/${name}`, + default_branch: "main", + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + } as Repository; +} + +function row(overrides: Partial = {}): TaskRepoRow { + return { key: `row-${Math.random()}`, branch: "", ...overrides }; +} + +function makeFs(overrides: Partial): DialogFormState { + // Only the fields RepoChipsRow actually reads/sets need to be real. + return { + repositories: [] as TaskRepoRow[], + useGitHubUrl: false, + discoveredRepositories: [], + addRepository: vi.fn(), + removeRepository: vi.fn(), + updateRepository: vi.fn(), + setRepositories: vi.fn(), + ...overrides, + } as unknown as DialogFormState; +} + +const NOOP = (_key: string, _value: string) => undefined; + +describe("RepoChipsRow", () => { + it("renders one chip per row plus an Add button", () => { + render( + , + ); + expect(screen.getAllByTestId("repo-chip")).toHaveLength(1); + expect(screen.getByTestId("add-repository")).toBeTruthy(); + }); + + it("renders one chip per row across multiple repos", () => { + render( + , + ); + expect(screen.getAllByTestId("repo-chip")).toHaveLength(2); + }); + + it("renders the GitHub URL input in URL mode (chips suppressed)", () => { + render( + undefined} + onGitHubUrlChange={() => undefined} + />, + ); + expect(screen.getByTestId("repo-chips-row")).toBeTruthy(); + expect(screen.queryAllByTestId("repo-chip")).toHaveLength(0); + expect(screen.getByTestId("github-url-input")).toBeTruthy(); + }); + + it("hides the chip row when the task is already started", () => { + render( + , + ); + expect(screen.queryByTestId("repo-chips-row")).toBeNull(); + }); + + it("disables Add when no more repositories are available", () => { + render( + , + ); + const btn = screen.getByTestId("add-repository") as HTMLButtonElement; + expect(btn.disabled).toBe(true); + }); + + it("calls fs.addRepository when the + button is clicked", () => { + const fs = makeFs({ repositories: [row({ key: "r0", repositoryId: REPO_FRONT_ID })] }); + render( + , + ); + fireEvent.click(screen.getByTestId("add-repository")); + expect(fs.addRepository).toHaveBeenCalledOnce(); + }); + + it("removing a chip calls fs.removeRepository(key)", () => { + const fs = makeFs({ + repositories: [ + row({ key: "r0", repositoryId: REPO_FRONT_ID, branch: "main" }), + row({ key: "r1", repositoryId: REPO_BACK_ID, branch: "develop" }), + ], + }); + render( + , + ); + fireEvent.click(screen.getAllByTestId("remove-repo-chip")[0]); + expect(fs.removeRepository).toHaveBeenCalledWith("r0"); + }); + + // Regression: discovered (on-machine) repos must surface in the picker + // dropdown alongside workspace repos. A previous rewrite passed [] for + // discovered repos and lost them. + it("includes discovered (on-machine) repos in the picker dropdown", () => { + render( + , + ); + fireEvent.click(screen.getByTestId("repo-chip-trigger")); + expect(screen.getByText("frontend")).toBeTruthy(); + expect(screen.getByText("projects/local-project")).toBeTruthy(); + expect(screen.queryByText("frontend-dup")).toBeNull(); + }); + + // Regression: picking a discovered repo passes its path as the value; + // onRowRepositoryChange must receive it (the handler then resolves to a + // workspace id or local path). Previously the chip wrote the path into + // a workspace `repository_id`, causing FK failures on submit. + it("calls onRowRepositoryChange with the discovered path when picked", () => { + const onRowRepositoryChange = vi.fn(); + render( + , + ); + fireEvent.click(screen.getByTestId("repo-chip-trigger")); + fireEvent.click(screen.getByText("projects/local-project")); + expect(onRowRepositoryChange).toHaveBeenCalledWith( + "r0", + "/home/me/projects/local-project", + ); + }); + + // Regression: discovered (path-keyed) rows used to call the branch loader + // with no path source, so their branch picker stayed empty. The chip must + // build a `kind: "path"` source for discovered rows so the unified hook + // hits the path-based query param instead of trying an id lookup. + it("discovered rows build a path-source for the branch loader", () => { + render( + , + ); + expect(lastBranchSource.value).toEqual({ + kind: "path", + workspaceId: "ws-1", + path: "/home/me/projects/proj", + }); + }); + + it("workspace rows build an id-source for the branch loader", () => { + render( + , + ); + expect(lastBranchSource.value).toEqual({ + kind: "id", + workspaceId: "ws-1", + repositoryId: REPO_FRONT_ID, + }); + }); + + // Regression: remote branches must keep their "origin/" prefix so users + // can distinguish a local "main" from "origin/main". A prior rewrite + // dropped the prefix, producing two indistinguishable rows. + it("remote branches keep their origin/ prefix and don't collide with local names", () => { + mockBranches.value = { + isLoading: false, + branches: [ + { name: "main", type: "local" }, + { name: "main", type: "remote", remote: "origin" }, + ] as unknown as Branch[], + }; + render( + , + ); + fireEvent.click(screen.getByTestId("branch-chip-trigger")); + expect(screen.getByText("main")).toBeTruthy(); + expect(screen.getByText("origin/main")).toBeTruthy(); + // Reset for sibling tests. + mockBranches.value = { branches: [], isLoading: false }; + }); +}); diff --git a/apps/web/components/task-create-dialog-repo-chips.tsx b/apps/web/components/task-create-dialog-repo-chips.tsx new file mode 100644 index 0000000000..7a14fd0e49 --- /dev/null +++ b/apps/web/components/task-create-dialog-repo-chips.tsx @@ -0,0 +1,396 @@ +"use client"; + +import { useMemo, useState } from "react"; +import { IconPlus, IconX, IconCode, IconGitBranch } from "@tabler/icons-react"; +import { cn } from "@/lib/utils"; +import { Popover, PopoverContent, PopoverTrigger } from "@kandev/ui/popover"; +import { + Command, + CommandEmpty, + CommandGroup, + CommandInput, + CommandItem, + CommandList, +} from "@kandev/ui/command"; +import { useBranches, type BranchSource } from "@/hooks/domains/workspace/use-repository-branches"; +import type { Branch, LocalRepository, Repository } from "@/lib/types/http"; +import type { DialogFormState, TaskRepoRow } from "@/components/task-create-dialog-types"; + +/** + * Chip row for the task-create dialog. Renders one chip per row in + * `fs.repositories`, plus a trailing "+" to add another. Each chip is two + * pills (repo, branch) and a remove (×). All chips are equivalent — there + * is no "primary" — and any row can hold either a workspace repo or a + * discovered on-machine path. + * + * In GitHub URL mode the chips are replaced by an inline URL input pill; + * the trailing toggle flips between the two modes. + */ +type RepoChipsRowProps = { + fs: DialogFormState; + repositories: Repository[]; + isTaskStarted: boolean; + /** Required for loading branches on discovered (path-keyed) rows. */ + workspaceId: string | null; + /** + * Per-row repo change handler. Resolves the picked value into either a + * workspace `repositoryId` or a discovered `localPath` and writes that + * into the row. Comes from useDialogHandlers so the resolution logic + * stays in one place. + */ + onRowRepositoryChange: (key: string, value: string) => void; + onRowBranchChange: (key: string, value: string) => void; + /** GitHub URL flow lives alongside the chips so users can switch in place. */ + onToggleGitHubUrl?: () => void; + onGitHubUrlChange?: (value: string) => void; +}; + +export function RepoChipsRow({ + fs, + repositories, + isTaskStarted, + workspaceId, + onRowRepositoryChange, + onRowBranchChange, + onToggleGitHubUrl, + onGitHubUrlChange, +}: RepoChipsRowProps) { + // No early returns above hooks. URL mode and started-state checks happen below. + const usedIds = useMemo(() => collectUsedRepoIds(fs.repositories), [fs.repositories]); + if (isTaskStarted) return null; + + const remainingCount = repositories.filter((r) => !usedIds.has(r.id)).length; + // Add stays enabled when no workspace repos are left, since users can also + // pick from discovered on-machine paths. + const hasDiscovered = fs.discoveredRepositories.length > 0; + const canAddMore = remainingCount > 0 || hasDiscovered; + const addHint = canAddMore ? undefined : "All workspace repositories are already added"; + + return ( +
+ {fs.useGitHubUrl ? ( + + ) : ( + <> + {fs.repositories.map((row) => ( + onRowRepositoryChange(row.key, value)} + onBranchChange={(value) => onRowBranchChange(row.key, value)} + onRemove={() => fs.removeRepository(row.key)} + /> + ))} + + + )} + {onToggleGitHubUrl && ( + + )} +
+ ); +} + +/** Build the set of repo identifiers (workspace id or path) currently in use. */ +function collectUsedRepoIds(rows: TaskRepoRow[], exceptKey?: string): Set { + const ids = new Set(); + for (const r of rows) { + if (r.key === exceptKey) continue; + if (r.repositoryId) ids.add(r.repositoryId); + if (r.localPath) ids.add(r.localPath); + } + return ids; +} + +type RepoChipProps = { + row: TaskRepoRow; + /** Required for path-based branch loading on discovered rows. */ + workspaceId: string | null; + repositories: Repository[]; + discoveredRepositories: LocalRepository[]; + /** Repo IDs/paths to filter out of the dropdown (already in use elsewhere). */ + excludedRepoIds: Set; + onRepositoryChange: (value: string) => void; + onBranchChange: (value: string) => void; + onRemove: () => void; +}; + +function RepoChip({ + row, + workspaceId, + repositories, + discoveredRepositories, + excludedRepoIds, + onRepositoryChange, + onBranchChange, + onRemove, +}: RepoChipProps) { + const filteredRepos = useMemo( + () => + repositories.filter((r) => !excludedRepoIds.has(r.id) || r.id === row.repositoryId), + [repositories, excludedRepoIds, row.repositoryId], + ); + // Discovered (on-machine) repos not yet imported into the workspace. Drop + // ones whose path is already a workspace repo so the dropdown doesn't show + // the same folder twice. + const filteredDiscovered = useMemo(() => { + const workspaceRepoPaths = new Set( + filteredRepos + .map((r) => r.local_path) + .filter(Boolean) + .map((path: string) => normalizeRepoPath(path)), + ); + return discoveredRepositories.filter( + (r) => + !workspaceRepoPaths.has(normalizeRepoPath(r.path)) && + (!excludedRepoIds.has(r.path) || r.path === row.localPath), + ); + }, [filteredRepos, discoveredRepositories, excludedRepoIds, row.localPath]); + + // One hook for both row sources (workspace repo by id OR on-machine path). + // Backend resolves either to an absolute path; the Zustand cache keys + // path-based entries with a synthetic key so workspace + discovered share + // one slice (and the same fetch dedupe). + const branchSource = useMemo(() => { + if (!workspaceId) return null; + if (row.repositoryId) { + return { kind: "id", workspaceId, repositoryId: row.repositoryId }; + } + if (row.localPath) { + return { kind: "path", workspaceId, path: row.localPath }; + } + return null; + }, [workspaceId, row.repositoryId, row.localPath]); + const { branches, isLoading: branchesLoading } = useBranches(branchSource, !!branchSource); + + const repoOptions: PillOption[] = useMemo(() => { + return [ + ...filteredRepos.map((r) => ({ value: r.id, label: r.name })), + ...filteredDiscovered.map((r) => ({ + value: r.path, + label: shortRepoPath(r.path), + keywords: [r.path], + })), + ]; + }, [filteredRepos, filteredDiscovered]); + const branchOptions: PillOption[] = useMemo(() => branches.map(branchToOption), [branches]); + + const repoLabel = + repositories.find((r) => r.id === row.repositoryId)?.name ?? + discoveredRepositories.find((r) => r.path === row.localPath)?.path?.split("/").pop() ?? + ""; + const hasRepo = !!(row.repositoryId || row.localPath); + const branchPlaceholder = !hasRepo + ? "branch" + : branchesLoading + ? "loading…" + : branchOptions.length === 0 + ? "no branches" + : "branch"; + + return ( + + } + value={repoLabel} + placeholder="repository" + options={repoOptions} + onSelect={onRepositoryChange} + searchPlaceholder="Search repositories..." + emptyMessage="No repositories" + testId="repo-chip-trigger" + /> + } + value={row.branch} + placeholder={branchPlaceholder} + options={branchOptions} + onSelect={onBranchChange} + disabled={!hasRepo || branchesLoading || branchOptions.length === 0} + searchPlaceholder="Search branches..." + emptyMessage="No branches" + testId="branch-chip-trigger" + /> + + + ); +} + +type PillOption = { value: string; label: string; keywords?: string[] }; + +function normalizeRepoPath(path: string): string { + return path.replace(/\\/g, "/").replace(/\/+$/g, ""); +} + +function shortRepoPath(path: string): string { + // Show the trailing folder name with one parent for context (e.g. "myorg/myrepo"). + const parts = path.replace(/\\/g, "/").split("/").filter(Boolean); + if (parts.length <= 1) return path; + return parts.slice(-2).join("/"); +} + +function branchToOption(b: Branch): PillOption { + // Remote branches keep their "origin/" prefix so they're distinguishable + // from local branches with the same short name (e.g. "main" vs "origin/main"). + // Without the prefix, the dropdown shows two indistinguishable rows. + const display = b.type === "remote" && b.remote ? `${b.remote}/${b.name}` : b.name; + return { value: display, label: display }; +} + +/** + * Compact pill trigger that opens a popover with a search list. Auto-widths + * to its content (no `w-full`, no chevron) so multiple pills can sit on one + * line without overlapping or stretching to fill the row. + */ +function Pill({ + icon, + value, + placeholder, + options, + onSelect, + disabled = false, + searchPlaceholder, + emptyMessage, + testId, +}: { + icon: React.ReactNode; + value: string; + placeholder: string; + options: PillOption[]; + onSelect: (value: string) => void; + disabled?: boolean; + searchPlaceholder: string; + emptyMessage: string; + testId?: string; +}) { + const [open, setOpen] = useState(false); + const hasValue = !!value; + return ( + + + + + + + + + {emptyMessage} + + {options.map((option) => ( + { + onSelect(option.value); + setOpen(false); + }} + > + {option.label} + + ))} + + + + + + ); +} + +function GitHubUrlPill({ + githubUrl, + githubUrlError, + onGitHubUrlChange, +}: { + githubUrl: string; + githubUrlError: string | null; + onGitHubUrlChange?: (value: string) => void; +}) { + return ( +
+ onGitHubUrlChange?.(e.target.value)} + placeholder="github.com/owner/repo" + data-testid="github-url-input" + aria-label="GitHub repository URL" + aria-invalid={!!githubUrlError} + aria-describedby={githubUrlError ? "github-url-error" : undefined} + className={cn( + "h-7 rounded-md px-2.5 text-xs bg-muted/30 border border-border/60", + "outline-none focus:bg-muted focus:border-border placeholder:text-muted-foreground", + githubUrlError && "border-destructive text-destructive", + )} + autoFocus + /> + {githubUrlError && ( + + )} +
+ ); +} diff --git a/apps/web/components/task-create-dialog-repositories-state.ts b/apps/web/components/task-create-dialog-repositories-state.ts new file mode 100644 index 0000000000..3368a1258d --- /dev/null +++ b/apps/web/components/task-create-dialog-repositories-state.ts @@ -0,0 +1,33 @@ +"use client"; + +import { useCallback, useRef, useState } from "react"; +import type { TaskRepoRow } from "@/components/task-create-dialog-types"; + +/** + * Manages the unified `repositories` list for task creation. Every chip + * (one or many) is an entry; there is no "primary" or "extras" split. + * + * `nextKey` increments to give each row a stable client-side key without + * relying on array indices (which would shift on removal and break + * uncontrolled inputs). + */ +export function useRepositoriesState() { + const [repositories, setRepositories] = useState([]); + const nextKeyRef = useRef(0); + + const addRepository = useCallback(() => { + nextKeyRef.current += 1; + const key = `row-${nextKeyRef.current}`; + setRepositories((rows) => [...rows, { key, branch: "" }]); + }, []); + + const removeRepository = useCallback((key: string) => { + setRepositories((rows) => rows.filter((r) => r.key !== key)); + }, []); + + const updateRepository = useCallback((key: string, patch: Partial) => { + setRepositories((rows) => rows.map((r) => (r.key === key ? { ...r, ...patch } : r))); + }, []); + + return { repositories, setRepositories, addRepository, removeRepository, updateRepository }; +} diff --git a/apps/web/components/task-create-dialog-selectors.tsx b/apps/web/components/task-create-dialog-selectors.tsx index a16bbf7a9f..8b092fa46b 100644 --- a/apps/web/components/task-create-dialog-selectors.tsx +++ b/apps/web/components/task-create-dialog-selectors.tsx @@ -79,6 +79,7 @@ type BranchSelectorProps = { placeholder: string; searchPlaceholder: string; emptyMessage: string; + triggerClassName?: string; }; export const BranchSelector = memo(function BranchSelector({ @@ -89,6 +90,7 @@ export const BranchSelector = memo(function BranchSelector({ placeholder, searchPlaceholder, emptyMessage, + triggerClassName, }: BranchSelectorProps) { return ( ); @@ -233,10 +236,9 @@ export const InlineTaskName = memo(function InlineTaskName({ type="text" value={value} onChange={(e) => onChange(e.target.value)} - placeholder="task-name" + placeholder="Task name" data-testid="task-title-input" - size={Math.max(value.length, 9)} - className="bg-transparent border-none outline-none focus:ring-0 text-sm font-medium min-w-0 rounded-md px-1.5 py-0.5 -mx-1.5 hover:bg-muted focus:bg-muted transition-colors" + className="w-full bg-transparent border-none outline-none focus:ring-0 text-sm font-medium rounded-md px-2 py-1 -mx-2 hover:bg-muted/30 focus:bg-muted/30 transition-colors placeholder:text-muted-foreground/70" /> ); }); diff --git a/apps/web/components/task-create-dialog-state.ts b/apps/web/components/task-create-dialog-state.ts index 69c52168c8..d965803222 100644 --- a/apps/web/components/task-create-dialog-state.ts +++ b/apps/web/components/task-create-dialog-state.ts @@ -10,7 +10,6 @@ import type { import type { TaskFormInputsHandle } from "@/components/task-create-dialog-types"; import { useAppStore } from "@/components/state-provider"; import { useRepositories } from "@/hooks/domains/workspace/use-repositories"; -import { useRepositoryBranches } from "@/hooks/domains/workspace/use-repository-branches"; import { useSettingsData } from "@/hooks/domains/settings/use-settings-data"; import { useRepositoryOptions, @@ -27,8 +26,9 @@ import type { DialogFormState, DialogComputedValues, DialogComputedArgs, + TaskRepoRow, } from "@/components/task-create-dialog-types"; -import { useExtraRepositoriesState } from "@/components/task-create-dialog-extra-repos-state"; +import { useRepositoriesState } from "@/components/task-create-dialog-repositories-state"; import { computePassthroughProfile, computeEffectiveStepId, @@ -44,17 +44,14 @@ type FormResetters = { setTaskName: (v: string) => void; setHasTitle: (v: boolean) => void; setHasDescription: (v: boolean) => void; - setRepositoryId: (v: string) => void; - setBranch: (v: string) => void; + setRepositories: (v: TaskRepoRow[]) => void; + setGitHubBranch: (v: string) => void; setAgentProfileId: (v: string) => void; setExecutorId: (v: string) => void; setExecutorProfileId: (v: string) => void; setSelectedWorkflowId: (v: string | null) => void; setFetchedSteps: (v: StepType[] | null) => void; setDiscoveredRepositories: (v: LocalRepository[]) => void; - setDiscoveredRepoPath: (v: string) => void; - setSelectedLocalRepo: (v: LocalRepository | null) => void; - setLocalBranches: (v: Branch[]) => void; setDiscoverReposLoaded: (v: boolean) => void; setUseGitHubUrl: (v: boolean) => void; setGitHubUrl: (v: string) => void; @@ -145,8 +142,21 @@ function resetTaskForm( resetters.setTaskName(name); resetters.setHasTitle(name.trim().length > 0); resetters.setHasDescription(description.trim().length > 0); - resetters.setRepositoryId(initialValues?.repositoryId ?? ""); - resetters.setBranch(initialValues?.branch ?? ""); + // Seed the unified repos list from initialValues. A repo + branch pre-fill + // becomes a single row; nothing seeds an empty list (the auto-select + // effect later picks the user's last-used repo or the first workspace one). + if (initialValues?.repositoryId) { + resetters.setRepositories([ + { + key: "row-0", + repositoryId: initialValues.repositoryId, + branch: initialValues.branch ?? "", + }, + ]); + } else { + resetters.setRepositories([]); + } + resetters.setGitHubBranch(initialValues?.branch ?? ""); resetters.setAgentProfileId(""); resetters.setExecutorId(""); resetters.setExecutorProfileId(""); @@ -158,9 +168,6 @@ function resetTaskForm( function resetDiscoveryState(resetters: FormResetters, iv?: TaskCreateDialogInitialValues) { const ghUrl = iv?.githubUrl ?? ""; resetters.setDiscoveredRepositories([]); - resetters.setDiscoveredRepoPath(""); - resetters.setSelectedLocalRepo(null); - resetters.setLocalBranches([]); resetters.setDiscoverReposLoaded(false); resetters.setUseGitHubUrl(Boolean(ghUrl)); resetters.setGitHubUrl(ghUrl); @@ -266,8 +273,9 @@ function useFormStateValues( const [draftDescription, setDraftDescription] = useState(""); const descriptionInputRef = useRef(null); - const [repositoryId, setRepositoryId] = useState(initialValues?.repositoryId ?? ""); - const [branch, setBranch] = useState(initialValues?.branch ?? ""); + // GitHub URL flow has its own branch field (the per-repo branch lives on + // each row in `repositories`). Seed from initialValues for the URL flow only. + const [githubBranch, setGitHubBranch] = useState(initialValues?.branch ?? ""); const [agentProfileId, setAgentProfileId] = useState(""); const [executorId, setExecutorId] = useState(""); const [executorProfileId, setExecutorProfileId] = useState(""); @@ -285,10 +293,8 @@ function useFormStateValues( draftDescription, setDraftDescription, descriptionInputRef, - repositoryId, - setRepositoryId, - branch, - setBranch, + githubBranch, + setGitHubBranch, agentProfileId, setAgentProfileId, executorId, @@ -311,26 +317,17 @@ function useFormStateValues( }; } -/** Repository discovery state */ +/** Repository discovery state — just the discovered list. The previous + * per-form `selectedLocalRepo` / `discoveredRepoPath` / `localBranches` + * primary-only fields are gone; discovered repos now live as ordinary rows + * in `fs.repositories` with `localPath` set. */ function useDiscoveryState() { const [discoveredRepositories, setDiscoveredRepositories] = useState([]); - const [discoveredRepoPath, setDiscoveredRepoPath] = useState(""); - const [selectedLocalRepo, setSelectedLocalRepo] = useState(null); - const [localBranches, setLocalBranches] = useState([]); - const [localBranchesLoading, setLocalBranchesLoading] = useState(false); const [discoverReposLoading, setDiscoverReposLoading] = useState(false); const [discoverReposLoaded, setDiscoverReposLoaded] = useState(false); return { discoveredRepositories, setDiscoveredRepositories, - discoveredRepoPath, - setDiscoveredRepoPath, - selectedLocalRepo, - setSelectedLocalRepo, - localBranches, - setLocalBranches, - localBranchesLoading, - setLocalBranchesLoading, discoverReposLoading, setDiscoverReposLoading, discoverReposLoaded, @@ -348,7 +345,7 @@ export function useDialogFormState( const discovery = useDiscoveryState(); const ghUrl = useGitHubUrlState(); const wfAgent = useWorkflowAgentProfileState(); - const extraRepos = useExtraRepositoriesState(); + const repos = useRepositoriesState(); useFormResetEffects({ open, @@ -363,17 +360,14 @@ export function useDialogFormState( setTaskName: form.setTaskName, setHasTitle: form.setHasTitle, setHasDescription: form.setHasDescription, - setRepositoryId: form.setRepositoryId, - setBranch: form.setBranch, + setRepositories: repos.setRepositories, + setGitHubBranch: form.setGitHubBranch, setAgentProfileId: form.setAgentProfileId, setExecutorId: form.setExecutorId, setExecutorProfileId: form.setExecutorProfileId, setSelectedWorkflowId: form.setSelectedWorkflowId, setFetchedSteps: form.setFetchedSteps, setDiscoveredRepositories: discovery.setDiscoveredRepositories, - setDiscoveredRepoPath: discovery.setDiscoveredRepoPath, - setSelectedLocalRepo: discovery.setSelectedLocalRepo, - setLocalBranches: discovery.setLocalBranches, setDiscoverReposLoaded: discovery.setDiscoverReposLoaded, setUseGitHubUrl: ghUrl.setUseGitHubUrl, setGitHubUrl: ghUrl.setGitHubUrl, @@ -391,13 +385,7 @@ export function useDialogFormState( form.descriptionInputRef, ); - // Reset extra repositories whenever the dialog re-opens with a fresh form. - useEffect(() => { - if (!open) extraRepos.resetExtraRepositories(); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [open]); - - return { ...form, ...discovery, ...ghUrl, ...wfAgent, ...extraRepos, clearDraft }; + return { ...form, ...discovery, ...ghUrl, ...wfAgent, ...repos, clearDraft }; } export type { DialogFormState } from "@/components/task-create-dialog-types"; @@ -417,7 +405,6 @@ export function useDialogComputed({ workspaceId, workflowId, defaultStepId, - branches, settingsData, agentProfiles, workspaces, @@ -450,15 +437,17 @@ export function useDialogComputed({ const workspaceDefaults = workspaceId ? workspaces.find((ws: Workspace) => ws.id === workspaceId) : null; + // The form has a repo selection when either: (a) any chip in the unified + // list has a repo set, or (b) URL mode has a non-empty URL. The chip row + // takes care of branch state per row; there is no global branch. const hasRepositorySelection = Boolean( - fs.repositoryId || fs.selectedLocalRepo || (fs.useGitHubUrl && fs.githubUrl.trim()), + fs.repositories.some((r) => r.repositoryId || r.localPath) || + (fs.useGitHubUrl && fs.githubUrl.trim()), ); - const effectiveBranches = (() => { - if (fs.useGitHubUrl) return fs.githubBranches; - if (fs.repositoryId) return branches; - return fs.localBranches; - })(); - const branchOptions = useBranchOptions(effectiveBranches); + // Branch options are only used by the URL-mode flow now (the chip's branch + // pill loads branches per-repo). Keep the computed value but always feed it + // the URL branches when in URL mode. + const branchOptions = useBranchOptions(fs.useGitHubUrl ? fs.githubBranches : []); const agentProfileOptions = useAgentProfileOptions(agentProfiles); const allExecutorProfiles = useMemo(() => { return executors.flatMap((executor) => @@ -528,17 +517,16 @@ export function useTaskCreateDialogData( useSettingsData(open); const { repositories, isLoading: repositoriesLoading } = useRepositories(workspaceId, open); - const { branches, isLoading: branchesLoading } = useRepositoryBranches( - fs.repositoryId || null, - Boolean(open && fs.repositoryId), - ); + // Per-repo branch loading lives in each chip now (RepoChipsRow). No + // global branch query is needed here — the chip uses useRepositoryBranches + // for its own row, and the store dedupes by repositoryId. + const branchesLoading = false; const computed = useDialogComputed({ fs, open, workspaceId, workflowId, defaultStepId, - branches, settingsData, agentProfiles, workspaces, @@ -554,7 +542,6 @@ export function useTaskCreateDialogData( snapshots, repositories, repositoriesLoading, - branches, branchesLoading, computed, }; diff --git a/apps/web/components/task-create-dialog-submit.tsx b/apps/web/components/task-create-dialog-submit.tsx index 4d50d842a0..2825b2513d 100644 --- a/apps/web/components/task-create-dialog-submit.tsx +++ b/apps/web/components/task-create-dialog-submit.tsx @@ -31,13 +31,12 @@ export function useTaskSubmitHandlers({ workflowId, effectiveWorkflowId, effectiveDefaultStepId, - repositoryId, - selectedLocalRepo, + repositories, + discoveredRepositories, useGitHubUrl, githubUrl, githubPrHeadBranch, - branch, - extraRepositories, + githubBranch, agentProfileId, executorId, executorProfileId, @@ -53,8 +52,8 @@ export function useTaskSubmitHandlers({ setHasTitle, setHasDescription, setTaskName, - setRepositoryId, - setBranch, + setRepositories, + setGitHubBranch, setAgentProfileId, setExecutorId, setSelectedWorkflowId, @@ -70,8 +69,8 @@ export function useTaskSubmitHandlers({ setHasTitle(false); setHasDescription(false); setTaskName(""); - setRepositoryId(""); - setBranch(""); + setRepositories([]); + setGitHubBranch(""); setAgentProfileId(""); setExecutorId(""); setSelectedWorkflowId(workflowId); @@ -82,8 +81,8 @@ export function useTaskSubmitHandlers({ setHasTitle, setHasDescription, setTaskName, - setRepositoryId, - setBranch, + setRepositories, + setGitHubBranch, setAgentProfileId, setExecutorId, setSelectedWorkflowId, @@ -95,23 +94,18 @@ export function useTaskSubmitHandlers({ buildRepositoriesPayload({ useGitHubUrl, githubUrl, - branch, + githubBranch, githubPrHeadBranch, - repositoryId, - selectedLocalRepo, - extraRepositories: extraRepositories.map((row) => ({ - repositoryId: row.repositoryId, - branch: row.branch, - })), + repositories, + discoveredRepositories, }), [ useGitHubUrl, - repositoryId, - branch, githubUrl, + githubBranch, githubPrHeadBranch, - selectedLocalRepo, - extraRepositories, + repositories, + discoveredRepositories, ], ); @@ -403,8 +397,7 @@ export function useTaskSubmitHandlers({ trimmedTitle, workspaceId, effectiveWorkflowId, - repositoryId, - selectedLocalRepo, + repositories, githubUrl, agentProfileId, }) @@ -433,8 +426,7 @@ export function useTaskSubmitHandlers({ taskName, workspaceId, effectiveWorkflowId, - repositoryId, - selectedLocalRepo, + repositories, githubUrl, agentProfileId, getRepositoriesPayload, @@ -454,8 +446,7 @@ export function useTaskSubmitHandlers({ trimmedTitle, workspaceId, effectiveWorkflowId, - repositoryId, - selectedLocalRepo, + repositories, githubUrl, agentProfileId, }) @@ -488,8 +479,7 @@ export function useTaskSubmitHandlers({ taskName, workspaceId, effectiveWorkflowId, - repositoryId, - selectedLocalRepo, + repositories, githubUrl, agentProfileId, isPassthroughProfile, @@ -509,8 +499,7 @@ export function useTaskSubmitHandlers({ trimmedTitle, workspaceId, effectiveWorkflowId, - repositoryId, - selectedLocalRepo, + repositories, githubUrl, agentProfileId, }) @@ -550,8 +539,7 @@ export function useTaskSubmitHandlers({ taskName, workspaceId, effectiveWorkflowId, - repositoryId, - selectedLocalRepo, + repositories, githubUrl, agentProfileId, effectiveDefaultStepId, diff --git a/apps/web/components/task-create-dialog-types.ts b/apps/web/components/task-create-dialog-types.ts index 7c8dd626eb..b3516bfeab 100644 --- a/apps/web/components/task-create-dialog-types.ts +++ b/apps/web/components/task-create-dialog-types.ts @@ -16,16 +16,22 @@ import type { import type { useToast } from "@/components/toast-provider"; /** - * One extra repository row in the multi-repo task create form. The form - * tracks these in addition to the primary (repositoryId/branch) so the - * existing single-repo controls keep working for the common case. + * One repository row in the task-create form. The form tracks every repo + * (whether one or many) as an entry in `repositories[]`. There is no + * "primary" — the backend treats them all equally and uses array order + * for position. Removing the last row clears the form's repo selection. * + * Exactly one of `repositoryId` (workspace repo) or `localPath` (on-machine + * git folder discovered via the workspace's repo discovery) is set per row. * `key` is a stable client-side id used as React key and for * add/update/remove ops; it's not sent to the backend. */ -export type ExtraRepositoryRow = { +export type TaskRepoRow = { key: string; - repositoryId: string; + /** Workspace repo id, when the user picked from the workspace's repos. */ + repositoryId?: string; + /** On-machine repo path, when the user picked from discovered repos. */ + localPath?: string; branch: string; }; @@ -86,7 +92,6 @@ export type DialogComputedArgs = { workspaceId: string | null; workflowId: string | null; defaultStepId: string | null; - branches: Branch[]; settingsData: { agentsLoaded: boolean; executorsLoaded: boolean }; agentProfiles: AgentProfileOption[]; workspaces: Workspace[]; @@ -101,7 +106,6 @@ export type TaskCreateEffectsArgs = { workflowId: string | null; repositories: Repository[]; repositoriesLoading: boolean; - branches: Branch[]; agentProfiles: AgentProfileOption[]; executors: Executor[]; workspaceDefaults: Workspace | null | undefined; @@ -131,19 +135,19 @@ export type DialogFormState = { /** Computed defaults for current open cycle (includes draft restoration) */ currentDefaults: { name: string; description: string }; descriptionInputRef: import("react").RefObject; - repositoryId: string; - setRepositoryId: (v: string) => void; - branch: string; - setBranch: (v: string) => void; /** - * Extra repository rows beyond the primary, for multi-repo tasks. - * The primary repo (repositoryId/branch above) is always position 0; - * each extra row gets the next position. Empty array = single-repo task. + * Unified list of repos on this task. Each row carries either a workspace + * `repositoryId` or a discovered `localPath`, plus its base `branch`. The + * order is the position the backend sees. There is no "primary" concept. */ - extraRepositories: ExtraRepositoryRow[]; - addExtraRepository: () => void; - removeExtraRepository: (key: string) => void; - updateExtraRepository: (key: string, patch: Partial) => void; + repositories: TaskRepoRow[]; + setRepositories: (v: TaskRepoRow[]) => void; + addRepository: () => void; + removeRepository: (key: string) => void; + updateRepository: (key: string, patch: Partial) => void; + /** GitHub URL mode: a separate flow that replaces the chip row with a URL input. */ + githubBranch: string; + setGitHubBranch: (v: string) => void; agentProfileId: string; setAgentProfileId: (v: string) => void; executorId: string; @@ -152,14 +156,6 @@ export type DialogFormState = { setExecutorProfileId: (v: string) => void; discoveredRepositories: LocalRepository[]; setDiscoveredRepositories: (v: LocalRepository[]) => void; - discoveredRepoPath: string; - setDiscoveredRepoPath: (v: string) => void; - selectedLocalRepo: LocalRepository | null; - setSelectedLocalRepo: (v: LocalRepository | null) => void; - localBranches: Branch[]; - setLocalBranches: (v: Branch[]) => void; - localBranchesLoading: boolean; - setLocalBranchesLoading: (v: boolean) => void; discoverReposLoading: boolean; setDiscoverReposLoading: (v: boolean) => void; discoverReposLoaded: boolean; @@ -200,17 +196,15 @@ export type SubmitHandlersDeps = { workflowId: string | null; effectiveWorkflowId: string | null; effectiveDefaultStepId: string | null; - repositoryId: string; - selectedLocalRepo: LocalRepository | null; + /** Unified repo list from the form. Empty when in GitHub URL mode. */ + repositories: TaskRepoRow[]; + /** All on-machine discovered repos — used to look up `default_branch` for `localPath` rows. */ + discoveredRepositories: LocalRepository[]; useGitHubUrl: boolean; githubUrl: string; githubPrHeadBranch: string | null; - branch: string; - /** - * Extra repository rows from the multi-repo form. Submit handlers append - * these to the primary in the same payload position order. - */ - extraRepositories: ExtraRepositoryRow[]; + /** Branch for the GitHub URL flow. Per-row branches live on `repositories[i].branch`. */ + githubBranch: string; agentProfileId: string; executorId: string; executorProfileId: string; @@ -237,8 +231,8 @@ export type SubmitHandlersDeps = { setHasTitle: (v: boolean) => void; setHasDescription: (v: boolean) => void; setTaskName: (v: string) => void; - setRepositoryId: (v: string) => void; - setBranch: (v: string) => void; + setRepositories: (v: TaskRepoRow[]) => void; + setGitHubBranch: (v: string) => void; setAgentProfileId: (v: string) => void; setExecutorId: (v: string) => void; setSelectedWorkflowId: (v: string | null) => void; diff --git a/apps/web/components/task-create-dialog.tsx b/apps/web/components/task-create-dialog.tsx index 2b34939904..aa88379da0 100644 --- a/apps/web/components/task-create-dialog.tsx +++ b/apps/web/components/task-create-dialog.tsx @@ -21,13 +21,13 @@ import { useAgentProfileOptions, } from "@/components/task-create-dialog-options"; import { - RepositorySelector, AgentSelector, ExecutorProfileSelector, InlineTaskName, } from "@/components/task-create-dialog-selectors"; import { useTaskSubmitHandlers } from "@/components/task-create-dialog-submit"; import { CreateModeSelectors } from "@/components/task-create-dialog-create-mode-selectors"; +import { RepoChipsRow } from "@/components/task-create-dialog-repo-chips"; import { useToast } from "@/components/toast-provider"; import { useDialogFormState, @@ -74,145 +74,22 @@ interface TaskCreateDialogProps { parentTaskId?: string; } -function getRepositoryPlaceholder( - workspaceId: string | null, - repositoriesLoading: boolean, - discoverReposLoading: boolean, -) { - if (!workspaceId) return "Select workspace first"; - if (repositoriesLoading || discoverReposLoading) return "Loading..."; - return "Select repository"; -} - type DialogHeaderContentProps = { isCreateMode: boolean; isEditMode: boolean; - isTaskStarted: boolean; sessionRepoName?: string; initialTitle?: string; - taskName: string; - repositoryId: string; - discoveredRepoPath: string; - workspaceId: string | null; - repositoriesLoading: boolean; - discoverReposLoading: boolean; - headerRepositoryOptions: ReturnType["headerRepositoryOptions"]; - onRepositoryChange: (v: string) => void; - onTaskNameChange: (v: string) => void; - useGitHubUrl: boolean; - githubUrl: string; - githubUrlError: string | null; - onToggleGitHubUrl: () => void; - onGitHubUrlChange: (v: string) => void; }; -function RepoSourceInput({ - useGitHubUrl, - githubUrl, - githubUrlError, - onGitHubUrlChange, - isTaskStarted, - headerRepositoryOptions, - repositoryId, - discoveredRepoPath, - onRepositoryChange, - workspaceId, - repositoriesLoading, - discoverReposLoading, -}: Pick< - DialogHeaderContentProps, - | "useGitHubUrl" - | "githubUrl" - | "githubUrlError" - | "onGitHubUrlChange" - | "isTaskStarted" - | "headerRepositoryOptions" - | "repositoryId" - | "discoveredRepoPath" - | "onRepositoryChange" - | "workspaceId" - | "repositoriesLoading" - | "discoverReposLoading" ->): React.ReactNode { - if (useGitHubUrl) { - return ( -
- onGitHubUrlChange(e.target.value)} - placeholder="github.com/owner/repo" - data-testid="github-url-input" - aria-label="GitHub repository URL" - aria-invalid={!!githubUrlError} - aria-describedby={githubUrlError ? "github-url-error" : undefined} - size={Math.max((githubUrl || "").length + 1, "github.com/owner/repo".length)} - className={`bg-transparent border-none outline-none focus:ring-0 text-sm font-medium min-w-0 h-7 rounded-md px-2 hover:bg-muted focus:bg-muted transition-colors placeholder:text-muted-foreground ${githubUrlError ? "text-destructive" : ""}`} - disabled={isTaskStarted} - autoFocus - /> - {githubUrlError && ( - - )} -
- ); - } - return ( - - ); -} - function DialogHeaderContent(props: DialogHeaderContentProps) { - const { isCreateMode, isEditMode, isTaskStarted, sessionRepoName, initialTitle } = props; - const { taskName, onTaskNameChange, useGitHubUrl, onToggleGitHubUrl } = props; + const { isCreateMode, isEditMode, sessionRepoName, initialTitle } = props; if (isCreateMode || isEditMode) { + // Header is intentionally minimal — the dialog itself signals "create" + // and the task name input lives in the body below the repo chips. The + // GitHub URL input + toggle moved into the chip row as well. return ( - <> - -
- - / - -
- - {!isTaskStarted && ( -
- -
- )} - + {isEditMode ? "Edit task" : "New task"} ); } return ( @@ -235,14 +112,13 @@ function DialogHeaderContent(props: DialogHeaderContentProps) { type DialogFormBodyProps = { isSessionMode: boolean; isCreateMode: boolean; + isEditMode: boolean; isTaskStarted: boolean; isPassthroughProfile: boolean; initialDescription: string; hasDescription: boolean; workspaceId: string | null; onJiraImport?: (ticket: JiraTicket) => void; - branchOptions: ReturnType; - branchesLoading: boolean; agentProfileOptions: ReturnType; executorProfileOptions: Array<{ value: string; @@ -258,29 +134,30 @@ type DialogFormBodyProps = { effectiveWorkflowId: string | null; fs: DialogFormState; handleKeyDown: ReturnType; - onBranchChange: (v: string) => void; + onTaskNameChange: (v: string) => void; + onRowRepositoryChange: (key: string, value: string) => void; + onRowBranchChange: (key: string, value: string) => void; onAgentProfileChange: (v: string) => void; onExecutorProfileChange: (v: string) => void; onWorkflowChange: (v: string) => void; - hasRepositorySelection: boolean; - isLocalExecutor: boolean; + onToggleGitHubUrl: () => void; + onGitHubUrlChange: (v: string) => void; enhance?: { onEnhance: () => void; isLoading: boolean; isConfigured: boolean }; workflowAgentLocked: boolean; - /** Workspace repositories — used by the multi-repo extra rows. */ + /** Workspace repositories — driven into the chip row for repo + branch picks. */ repositories: Repository[]; }; function DialogFormBody({ isSessionMode, isCreateMode, + isEditMode, isTaskStarted, isPassthroughProfile, initialDescription, hasDescription, workspaceId, onJiraImport, - branchOptions, - branchesLoading, agentProfileOptions, executorProfileOptions, agentProfiles, @@ -292,18 +169,42 @@ function DialogFormBody({ effectiveWorkflowId, fs, handleKeyDown, - onBranchChange, + onTaskNameChange, + onRowRepositoryChange, + onRowBranchChange, onAgentProfileChange, onExecutorProfileChange, onWorkflowChange, - hasRepositorySelection, - isLocalExecutor, + onToggleGitHubUrl, + onGitHubUrlChange, enhance, workflowAgentLocked, repositories, }: DialogFormBodyProps) { + // Task name renders below the chip row in create/edit mode (per UX request); + // session mode and started tasks keep the header-only label and skip this. + const showTaskName = !isSessionMode && (isCreateMode || isEditMode) && !isTaskStarted; return (
+ {!isSessionMode && ( + + )} + {showTaskName && ( + + )} )}
0 && fs.repositories.every((r) => !!r.branch) + } agentProfileId={computed.effectiveAgentProfileId} workspaceId={workspaceId} effectiveWorkflowId={computed.effectiveWorkflowId ?? null} diff --git a/apps/web/components/task/changes-panel-file-row.tsx b/apps/web/components/task/changes-panel-file-row.tsx index f50dcdaf48..51d2572579 100644 --- a/apps/web/components/task/changes-panel-file-row.tsx +++ b/apps/web/components/task/changes-panel-file-row.tsx @@ -32,6 +32,8 @@ type ChangedFile = { plus: number | undefined; minus: number | undefined; oldPath: string | undefined; + /** Multi-repo: name of the repo this file lives in. Empty for single-repo. */ + repositoryName?: string; }; type FileRowProps = { @@ -40,9 +42,12 @@ type FileRowProps = { isSelected?: boolean; onSelect?: (path: string, e: React.MouseEvent) => boolean; onOpenDiff: (path: string) => void; - onStage: (path: string) => void; - onUnstage: (path: string) => void; - onDiscard: (path: string) => void; + // Multi-repo: handlers receive the file's repository_name so the per-file + // staging op routes to the right git repo. Same-named files (e.g. README.md + // in two repos) cannot be disambiguated by path alone. + onStage: (path: string, repo?: string) => void; + onUnstage: (path: string, repo?: string) => void; + onDiscard: (path: string, repo?: string) => void; onEditFile: (path: string) => void; }; @@ -83,6 +88,7 @@ export function FileRow({ isPending={isPending} staged={file.staged} path={file.path} + repo={file.repositoryName} onStage={onStage} onUnstage={onUnstage} /> @@ -94,7 +100,12 @@ export function FileRow({
- +
@@ -106,14 +117,16 @@ function StageButton({ isPending, staged, path, + repo, onStage, onUnstage, }: { isPending: boolean; staged: boolean; path: string; - onStage: (path: string) => void; - onUnstage: (path: string) => void; + repo?: string; + onStage: (path: string, repo?: string) => void; + onUnstage: (path: string, repo?: string) => void; }) { if (isPending) { return ( @@ -130,7 +143,7 @@ function StageButton({ className="group/unstage flex-shrink-0 flex items-center justify-center size-4 rounded bg-emerald-500/20 text-emerald-600 hover:bg-rose-500/20 hover:text-rose-600 cursor-pointer" onClick={(e) => { e.stopPropagation(); - onUnstage(path); + onUnstage(path, repo); }} > @@ -145,7 +158,7 @@ function StageButton({ className="flex-shrink-0 flex items-center justify-center size-4 rounded border border-dashed border-muted-foreground/50 text-muted-foreground hover:border-emerald-500 hover:text-emerald-500 hover:bg-emerald-500/10 cursor-pointer" onClick={(e) => { e.stopPropagation(); - onStage(path); + onStage(path, repo); }} > @@ -155,11 +168,13 @@ function StageButton({ function FileRowActions({ path, + repo, onDiscard, onEditFile, }: { path: string; - onDiscard: (path: string) => void; + repo?: string; + onDiscard: (path: string, repo?: string) => void; onEditFile: (path: string) => void; }) { return ( @@ -171,7 +186,7 @@ function FileRowActions({ className="text-muted-foreground hover:text-foreground cursor-pointer" onClick={(e) => { e.stopPropagation(); - onDiscard(path); + onDiscard(path, repo); }} > diff --git a/apps/web/components/task/changes-panel-hooks.ts b/apps/web/components/task/changes-panel-hooks.ts index 8eb0ffd3cb..a81cc891bc 100644 --- a/apps/web/components/task/changes-panel-hooks.ts +++ b/apps/web/components/task/changes-panel-hooks.ts @@ -9,12 +9,17 @@ interface GitOps { pull: (rebase?: boolean) => Promise; push: (options?: { force?: boolean; setUpstream?: boolean }) => Promise; rebase: (baseBranch: string) => Promise; - commit: (message: string, stageAll?: boolean, amend?: boolean) => Promise; - stage: (paths?: string[]) => Promise; - unstage: (paths?: string[]) => Promise; - discard: (paths?: string[]) => Promise; - revertCommit: (commitSHA: string) => Promise; - reset: (commitSHA: string, mode: "soft" | "hard") => Promise; + commit: ( + message: string, + stageAll?: boolean, + amend?: boolean, + repo?: string, + ) => Promise; + stage: (paths?: string[], repo?: string) => Promise; + unstage: (paths?: string[], repo?: string) => Promise; + discard: (paths?: string[], repo?: string) => Promise; + revertCommit: (commitSHA: string, repo?: string) => Promise; + reset: (commitSHA: string, mode: "soft" | "hard", repo?: string) => Promise; createPR: ( title: string, body: string, @@ -72,8 +77,8 @@ export function useChangesGitHandlers( handleGitOperation(() => gitOps.push({ force: true }), "Force push"); }, [handleGitOperation, gitOps]); const handleRevertCommit = useCallback( - (sha: string) => { - handleGitOperation(() => gitOps.revertCommit(sha), "Revert commit"); + (sha: string, repo?: string) => { + handleGitOperation(() => gitOps.revertCommit(sha, repo), "Revert commit"); }, [handleGitOperation, gitOps], ); @@ -96,22 +101,27 @@ function useChangesDiscardAmendHandlers( const [showDiscardDialog, setShowDiscardDialog] = useState(false); const [fileToDiscard, setFileToDiscard] = useState(null); const [filesToDiscard, setFilesToDiscard] = useState(null); + // Multi-repo: remember the clicked file's repo so the discard op routes to + // the right git repo. Path alone is ambiguous when two repos share a name. + const [repoToDiscard, setRepoToDiscard] = useState(undefined); - const handleDiscardClick = useCallback((filePath: string) => { + const handleDiscardClick = useCallback((filePath: string, repo?: string) => { setFileToDiscard(filePath); + setRepoToDiscard(repo); setFilesToDiscard(null); setShowDiscardDialog(true); }, []); const handleBulkDiscardClick = useCallback((paths: string[]) => { setFilesToDiscard(paths); setFileToDiscard(null); + setRepoToDiscard(undefined); setShowDiscardDialog(true); }, []); const handleDiscardConfirm = useCallback(async () => { const paths = filesToDiscard ?? (fileToDiscard ? [fileToDiscard] : null); if (!paths) return; try { - const result = await gitOps.discard(paths); + const result = await gitOps.discard(paths, repoToDiscard); if (!result.success) toast({ title: "Failed to discard changes", @@ -128,24 +138,34 @@ function useChangesDiscardAmendHandlers( setShowDiscardDialog(false); setFileToDiscard(null); setFilesToDiscard(null); + setRepoToDiscard(undefined); } - }, [fileToDiscard, filesToDiscard, gitOps, toast]); + }, [fileToDiscard, filesToDiscard, repoToDiscard, gitOps, toast]); // Amend dialog state (for editing last commit message directly) const [amendDialogOpen, setAmendDialogOpen] = useState(false); const [amendMessage, setAmendMessage] = useState(""); + // Multi-repo: capture the commit's repo at click time so the amend lands in + // the right git repo. Path/SHA alone can't be disambiguated when each repo + // has its own HEAD. + const [amendRepo, setAmendRepo] = useState(undefined); - const handleOpenAmendDialog = useCallback((currentMessage: string) => { + const handleOpenAmendDialog = useCallback((currentMessage: string, repo?: string) => { setAmendMessage(currentMessage); + setAmendRepo(repo); setAmendDialogOpen(true); }, []); const handleAmend = useCallback(async () => { if (!amendMessage.trim()) return; setAmendDialogOpen(false); - await handleGitOperation(() => gitOps.commit(amendMessage.trim(), false, true), "Amend commit"); + await handleGitOperation( + () => gitOps.commit(amendMessage.trim(), false, true, amendRepo), + "Amend commit", + ); setAmendMessage(""); - }, [amendMessage, handleGitOperation, gitOps]); + setAmendRepo(undefined); + }, [amendMessage, amendRepo, handleGitOperation, gitOps]); return { showDiscardDialog, @@ -168,9 +188,13 @@ function useChangesDiscardAmendHandlers( function useChangesResetHandlers(gitOps: GitOps, handleGitOperation: GitOperationFn) { const [resetDialogOpen, setResetDialogOpen] = useState(false); const [resetCommitSha, setResetCommitSha] = useState(null); + // Multi-repo: capture the commit's repo so reset runs against the right + // git repo. Without it, reset hits the workspace root and fails. + const [resetRepo, setResetRepo] = useState(undefined); - const handleOpenResetDialog = useCallback((sha: string) => { + const handleOpenResetDialog = useCallback((sha: string, repo?: string) => { setResetCommitSha(sha); + setResetRepo(repo); setResetDialogOpen(true); }, []); @@ -179,10 +203,11 @@ function useChangesResetHandlers(gitOps: GitOps, handleGitOperation: GitOperatio if (!resetCommitSha) return; setResetDialogOpen(false); const operationName = mode === "hard" ? "Hard reset" : "Soft reset"; - await handleGitOperation(() => gitOps.reset(resetCommitSha, mode), operationName); + await handleGitOperation(() => gitOps.reset(resetCommitSha, mode, resetRepo), operationName); setResetCommitSha(null); + setResetRepo(undefined); }, - [resetCommitSha, handleGitOperation, gitOps], + [resetCommitSha, resetRepo, handleGitOperation, gitOps], ); return { diff --git a/apps/web/components/task/changes-panel-timeline-grouping.test.tsx b/apps/web/components/task/changes-panel-timeline-grouping.test.tsx index 2345693499..c26b875fca 100644 --- a/apps/web/components/task/changes-panel-timeline-grouping.test.tsx +++ b/apps/web/components/task/changes-panel-timeline-grouping.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi, afterEach } from "vitest"; -import { render, screen, cleanup } from "@testing-library/react"; +import { render, screen, cleanup, fireEvent, within } from "@testing-library/react"; import type { ComponentProps } from "react"; vi.mock("./changes-panel-file-row", () => ({ @@ -11,7 +11,11 @@ vi.mock("./changes-panel-file-row", () => ({ })); vi.mock("./commit-row", () => ({ - CommitRow: () => null, + CommitRow: ({ commit, isLatest }: { commit: { commit_sha: string }; isLatest: boolean }) => ( +
  • + {commit.commit_sha} +
    • + ), })); vi.mock("@/hooks/use-multi-select", () => ({ @@ -23,7 +27,9 @@ vi.mock("@/hooks/use-multi-select", () => ({ }), })); -import { FileListSection } from "./changes-panel-timeline"; +import { FileListSection, CommitsSection } from "./changes-panel-timeline"; + +const REPO_HEADER_TID = "changes-repo-header"; afterEach(cleanup); @@ -62,7 +68,7 @@ describe("FileListSection — multi-repo grouping", () => { files={[file("a.ts"), file("b.ts")]} />, ); - expect(screen.queryAllByTestId("changes-repo-header")).toHaveLength(0); + expect(screen.queryAllByTestId(REPO_HEADER_TID)).toHaveLength(0); expect(screen.getAllByTestId("file-row")).toHaveLength(2); }); @@ -81,7 +87,7 @@ describe("FileListSection — multi-repo grouping", () => { ]} />, ); - const headers = screen.getAllByTestId("changes-repo-header"); + const headers = screen.getAllByTestId(REPO_HEADER_TID); expect(headers).toHaveLength(2); expect(headers[0].textContent).toContain("frontend"); expect(headers[0].textContent).toContain("2"); @@ -100,6 +106,130 @@ describe("FileListSection — multi-repo grouping", () => { files={[file("a.ts", "only-repo")]} />, ); - expect(screen.getByTestId("changes-repo-header").textContent).toContain("only-repo"); + expect(screen.getByTestId(REPO_HEADER_TID).textContent).toContain("only-repo"); + }); + + it("collapses one repo independently when its header is clicked", () => { + render( + undefined} + files={[ + file("src/app.tsx", "frontend"), + file("src/api.ts", "frontend"), + file("handlers/task.go", "backend"), + ]} + />, + ); + + expect(screen.getAllByTestId("file-row")).toHaveLength(3); + + const groups = screen.getAllByTestId("changes-repo-group"); + const frontendGroup = groups.find( + (g) => g.getAttribute("data-repository-name") === "frontend", + )!; + const frontendHeader = within(frontendGroup).getByTestId(REPO_HEADER_TID); + + fireEvent.click(frontendHeader); + + // frontend's two rows hidden, backend's one row still visible + expect(screen.getAllByTestId("file-row")).toHaveLength(1); + expect(frontendHeader.getAttribute("aria-expanded")).toBe("false"); + + fireEvent.click(frontendHeader); + expect(screen.getAllByTestId("file-row")).toHaveLength(3); + expect(frontendHeader.getAttribute("aria-expanded")).toBe("true"); + }); +}); + +describe("CommitsSection", () => { + type CommitProps = ComponentProps; + + function commit(sha: string, message: string, repo?: string): CommitProps["commits"][number] { + return { + commit_sha: sha, + commit_message: message, + insertions: 1, + deletions: 0, + pushed: false, + repository_name: repo, + }; + } + + it("starts collapsed by default — commits are reference info, not the primary signal", () => { + render( + , + ); + // Header is rendered, but commit rows are hidden until the user expands. + expect(screen.queryByTestId("commit-row")).toBeNull(); + const toggle = screen.getByTestId("commits-section-collapse-toggle"); + expect(toggle.getAttribute("aria-expanded")).toBe("false"); + }); + + it("groups commits per repo when 2+ repos are present", () => { + render( + , + ); + // Expand the section first so the inner repo headers and commit rows render. + fireEvent.click(screen.getByTestId("commits-section-collapse-toggle")); + + const headers = screen.getAllByTestId("commits-repo-header"); + expect(headers).toHaveLength(2); + expect(headers[0].textContent).toContain("frontend"); + expect(headers[0].textContent).toContain("2"); + expect(headers[1].textContent).toContain("backend"); + expect(headers[1].textContent).toContain("1"); + expect(screen.getAllByTestId("commit-row")).toHaveLength(3); + }); + + it("renders flat (no per-repo headers) for single-repo or untagged commits", () => { + render( + , + ); + fireEvent.click(screen.getByTestId("commits-section-collapse-toggle")); + expect(screen.queryAllByTestId("commits-repo-header")).toHaveLength(0); + expect(screen.getAllByTestId("commit-row")).toHaveLength(2); + }); + + // Regression: previously isLatest was computed against the merged list, so + // only ONE commit globally was marked latest. Each repo's newest unpushed + // commit must be latest in its own group — otherwise revert/amend buttons + // are absent on every repo except one, and clicking revert via context menu + // hits the backend's "can only revert latest commit" gate. + it("marks each repo's newest unpushed commit as latest (per-repo, not global)", () => { + render( + undefined} + />, + ); + fireEvent.click(screen.getByTestId("commits-section-collapse-toggle")); + const rows = screen.getAllByTestId("commit-row"); + const latestByShas = rows + .filter((r) => r.getAttribute("data-is-latest") === "true") + .map((r) => r.getAttribute("data-sha")); + expect(latestByShas.sort()).toEqual(["backend-only", "frontend-new"]); }); }); diff --git a/apps/web/components/task/changes-panel-timeline.tsx b/apps/web/components/task/changes-panel-timeline.tsx index 3e69d5ebae..0cface5047 100644 --- a/apps/web/components/task/changes-panel-timeline.tsx +++ b/apps/web/components/task/changes-panel-timeline.tsx @@ -24,6 +24,7 @@ import { LineStat } from "@/components/diff-stat"; import { FileStatusIcon } from "./file-status-icon"; import { FileRow, BulkActionBar, DefaultActionButtons } from "./changes-panel-file-row"; import { CommitRow, type CommitItem } from "./commit-row"; +import { groupByRepositoryName } from "@/lib/group-by-repo"; // --- Timeline visual components --- @@ -38,27 +39,7 @@ type ChangedFile = { repositoryName?: string; }; -/** - * Groups changed files by repository name, preserving original order within - * each group. Returns an empty-name leading group when files lack a - * repository name (single-repo workspaces) so the renderer can skip the - * per-repo header in that case. - */ -function groupChangedFilesByRepository( - files: ChangedFile[], -): Array<{ repositoryName: string; files: ChangedFile[] }> { - const order: string[] = []; - const buckets = new Map(); - for (const file of files) { - const name = file.repositoryName ?? ""; - if (!buckets.has(name)) { - buckets.set(name, []); - order.push(name); - } - buckets.get(name)!.push(file); - } - return order.map((name) => ({ repositoryName: name, files: buckets.get(name)! })); -} +// Per-repo grouping is shared with the Review dialog — see @/lib/group-by-repo. // --- Timeline section dot colors --- const DOT_COLORS = { @@ -81,6 +62,7 @@ function TimelineSection({ isLast, children, collapsible = true, + defaultCollapsed = false, "data-testid": testId, }: { dotColor: string; @@ -90,9 +72,10 @@ function TimelineSection({ isLast?: boolean; children?: React.ReactNode; collapsible?: boolean; + defaultCollapsed?: boolean; "data-testid"?: string; }) { - const [collapsed, setCollapsed] = useState(false); + const [collapsed, setCollapsed] = useState(defaultCollapsed); const canCollapse = collapsible && !!label; return ( @@ -151,11 +134,16 @@ type CommitsSectionProps = { commits: CommitItem[]; isLast: boolean; onOpenCommitDetail?: (sha: string) => void; - onRevertCommit?: (sha: string) => void; - onAmendCommit?: (currentMessage: string) => void; - onResetToCommit?: (sha: string) => void; + // Multi-repo: handlers receive the commit's repository_name so amend/revert/ + // reset land in the right git repo. Without it the op runs at the workspace + // root, which fails ("can only revert latest commit") for multi-repo tasks. + onRevertCommit?: (sha: string, repo?: string) => void; + onAmendCommit?: (currentMessage: string, repo?: string) => void; + onResetToCommit?: (sha: string, repo?: string) => void; }; +// Commits grouping shares the helper above with files — see @/lib/group-by-repo. + export function CommitsSection({ commits, isLast, @@ -164,8 +152,8 @@ 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); + const groups = groupByRepositoryName(commits, (c) => c.repository_name); + const showRepoHeaders = groups.length > 1 || (groups[0]?.repositoryName ?? "") !== ""; return (
      - {commits.map((commit, index) => ( - - ))} + {showRepoHeaders + ? groups.map((g) => ( + + )) + : commits.map((commit, index) => ( + c.pushed !== true)} + onOpenCommitDetail={onOpenCommitDetail} + onAmendCommit={commit.pushed ? undefined : onAmendCommit} + onRevertCommit={commit.pushed ? undefined : onRevertCommit} + onResetToCommit={commit.pushed ? undefined : onResetToCommit} + /> + ))}
    ); } +function CommitsRepoGroup({ + repositoryName, + groupCommits, + onOpenCommitDetail, + onAmendCommit, + onRevertCommit, + onResetToCommit, +}: { + repositoryName: string; + groupCommits: CommitItem[]; + onOpenCommitDetail?: (sha: string) => void; + onAmendCommit?: (currentMessage: string, repo?: string) => void; + onRevertCommit?: (sha: string, repo?: string) => void; + onResetToCommit?: (sha: string, repo?: string) => void; +}) { + const [collapsed, setCollapsed] = useState(false); + // Each repo has its own "latest unpushed commit" — revert/amend in this + // group must target THIS repo's newest, not the merged-list newest. Using + // the global firstUnpushedIndex would mark only one commit across all repos + // as latest and trigger "can only revert latest commit" everywhere else. + const firstUnpushedInGroup = groupCommits.findIndex((c) => c.pushed !== true); + return ( +
  • + + {!collapsed && ( +
      + {groupCommits.map((commit, index) => ( + + ))} +
    + )} +
    • + ); +} + // --- Action buttons section (Create PR / Push) --- type ActionButtonsSectionProps = { @@ -313,9 +380,11 @@ type FileListSectionProps = { onSecondaryAction?: () => void; onOpenDiff: (path: string) => void; onEditFile: (path: string) => void; - onStage: (path: string) => void; - onUnstage: (path: string) => void; - onDiscard: (path: string) => void; + // Multi-repo: handlers receive the file's repositoryName so each per-file op + // hits the right git repo. Same-named files across repos collide by path. + onStage: (path: string, repo?: string) => void; + onUnstage: (path: string, repo?: string) => void; + onDiscard: (path: string, repo?: string) => void; onBulkStage?: (paths: string[]) => void; onBulkUnstage?: (paths: string[]) => void; onBulkDiscard?: (paths: string[]) => void; @@ -335,13 +404,28 @@ function FileListBody(props: { onKeyDown: (e: React.KeyboardEvent) => void; onOpenDiff: (path: string) => void; onEditFile: (path: string) => void; - onStage: (path: string) => void; - onUnstage: (path: string) => void; - onDiscard: (path: string) => void; + onStage: (path: string, repo?: string) => void; + onUnstage: (path: string, repo?: string) => void; + onDiscard: (path: string, repo?: string) => void; }) { const { variant, files, pendingStageFiles, multiSelect } = props; - const groups = useMemo(() => groupChangedFilesByRepository(files), [files]); + const groups = useMemo( + () => groupByRepositoryName(files, (f) => f.repositoryName), + [files], + ); const showRepoHeaders = groups.length > 1 || (groups[0]?.repositoryName ?? "") !== ""; + // Per-repo collapsed state: keyed by repositoryName. Default expanded; + // setting an entry to true collapses that group. Persists across re-renders + // for the lifetime of this section instance (resets on unmount). + const [collapsedRepos, setCollapsedRepos] = useState>(() => new Set()); + const toggleRepo = useCallback((name: string) => { + setCollapsedRepos((prev) => { + const next = new Set(prev); + if (next.has(name)) next.delete(name); + else next.add(name); + return next; + }); + }, []); const renderRow = (file: ChangedFile) => ( -
    toggleRepo(group.repositoryName)} > + {collapsedRepos.has(group.repositoryName) ? ( + + ) : ( + + )} {group.repositoryName || "Other"} - {group.files.length} + {group.items.length} -
    -
      {group.files.map(renderRow)}
    + + {!collapsedRepos.has(group.repositoryName) && ( +
      {group.items.map(renderRow)}
    + )} ) : ( // Single-repo flat list: render rows inline so the DOM shape stays // identical to the pre-multi-repo behavior. - group.files.map(renderRow) + group.items.map(renderRow) ), )} diff --git a/apps/web/components/task/changes-panel.tsx b/apps/web/components/task/changes-panel.tsx index 23f0f11795..2a531b50cb 100644 --- a/apps/web/components/task/changes-panel.tsx +++ b/apps/web/components/task/changes-panel.tsx @@ -171,6 +171,8 @@ type MergedCommit = { insertions: number; deletions: number; pushed: boolean; + /** Multi-repo: name of the repo this commit was made in. Empty for single-repo. */ + repository_name?: string; }; /** @@ -185,6 +187,8 @@ export function mergeCommits( commit_message: string; insertions: number; deletions: number; + /** Multi-repo: name of the repo this commit was made in. */ + repository_name?: string; }[], prCommits: { sha: string; message: string; additions: number; deletions: number }[], ): MergedCommit[] { @@ -289,11 +293,17 @@ type ChangesPanelBodyProps = { onEditFile: (path: string) => void; onOpenCommitDetail?: (sha: string) => void; onOpenReview?: () => void; - onRevertCommit?: (sha: string) => void; + // Multi-repo: handlers carry the commit's repository_name so revert/amend/ + // reset target the right git repo. Without it the op runs at the workspace + // root and fails ("can only revert latest commit") on multi-repo tasks. + onRevertCommit?: (sha: string, repo?: string) => void; onStageAll: () => void; onUnstageAll: () => void; - onStage: (path: string) => Promise; - onUnstage: (path: string) => Promise; + // Multi-repo: callers pass the file's repository_name so the agentctl op + // routes to the right repo. Same-named files across repos can't be + // disambiguated by path alone. + onStage: (path: string, repo?: string) => Promise; + onUnstage: (path: string, repo?: string) => Promise; onBulkStage: (paths: string[]) => void; onBulkUnstage: (paths: string[]) => void; onBulkDiscard: (paths: string[]) => void; @@ -625,8 +635,8 @@ const ChangesPanel = memo(function ChangesPanel({ onOpenReview={onOpenReview} onStageAll={git.stageAll} onUnstageAll={git.unstageAll} - onStage={(path) => git.stageFile([path]).then(() => undefined)} - onUnstage={(path) => git.unstageFile([path]).then(() => undefined)} + onStage={(path, repo) => git.stageFile([path], repo).then(() => undefined)} + onUnstage={(path, repo) => git.unstageFile([path], repo).then(() => undefined)} onBulkStage={(paths) => { git.stageFile(paths).catch(() => {}); }} diff --git a/apps/web/components/task/commit-row.tsx b/apps/web/components/task/commit-row.tsx index 2fb643ae03..e0f2f9a53b 100644 --- a/apps/web/components/task/commit-row.tsx +++ b/apps/web/components/task/commit-row.tsx @@ -22,6 +22,8 @@ export type CommitItem = { insertions: number; deletions: number; pushed?: boolean; + /** Multi-repo: name of the repo this commit was made in. Empty for single-repo. */ + repository_name?: string; }; /** Context menu for commit items */ @@ -36,9 +38,12 @@ function CommitContextMenu({ children: React.ReactNode; commit: CommitItem; isLatest: boolean; - onAmendCommit?: (currentMessage: string) => void; - onRevertCommit?: (sha: string) => void; - onResetToCommit?: (sha: string) => void; + // Multi-repo: handlers receive the commit's repository_name so the + // amend/revert/reset op runs in the right git repo. Without it, ops hit + // the workspace root which fails on multi-repo task workspaces. + onAmendCommit?: (currentMessage: string, repo?: string) => void; + onRevertCommit?: (sha: string, repo?: string) => void; + onResetToCommit?: (sha: string, repo?: string) => void; }) { const hasActions = onAmendCommit || onRevertCommit || onResetToCommit; @@ -51,19 +56,25 @@ function CommitContextMenu({ {children} {isLatest && onAmendCommit && ( - onAmendCommit(commit.commit_message)}> + onAmendCommit(commit.commit_message, commit.repository_name)} + > Amend message )} {isLatest && onRevertCommit && ( - onRevertCommit(commit.commit_sha)}> + onRevertCommit(commit.commit_sha, commit.repository_name)} + > Revert commit )} {onResetToCommit && ( - onResetToCommit(commit.commit_sha)}> + onResetToCommit(commit.commit_sha, commit.repository_name)} + > Reset to this commit @@ -83,9 +94,12 @@ function CommitRowActions({ }: { commit: CommitItem; isLatest: boolean; - onAmendCommit?: (currentMessage: string) => void; - onRevertCommit?: (sha: string) => void; - onResetToCommit?: (sha: string) => void; + // Multi-repo: handlers receive the commit's repository_name so the + // amend/revert/reset op runs in the right git repo. Without it, ops hit + // the workspace root which fails on multi-repo task workspaces. + onAmendCommit?: (currentMessage: string, repo?: string) => void; + onRevertCommit?: (sha: string, repo?: string) => void; + onResetToCommit?: (sha: string, repo?: string) => void; }) { return ( @@ -98,7 +112,7 @@ function CommitRowActions({ className="p-0.5 text-muted-foreground hover:text-foreground cursor-pointer" onClick={(e) => { e.stopPropagation(); - onAmendCommit(commit.commit_message); + onAmendCommit(commit.commit_message, commit.repository_name); }} > @@ -116,7 +130,7 @@ function CommitRowActions({ className="p-0.5 text-muted-foreground hover:text-foreground cursor-pointer" onClick={(e) => { e.stopPropagation(); - onRevertCommit(commit.commit_sha); + onRevertCommit(commit.commit_sha, commit.repository_name); }} > @@ -134,7 +148,7 @@ function CommitRowActions({ className="p-0.5 text-muted-foreground hover:text-foreground cursor-pointer" onClick={(e) => { e.stopPropagation(); - onResetToCommit(commit.commit_sha); + onResetToCommit(commit.commit_sha, commit.repository_name); }} > @@ -159,9 +173,12 @@ export function CommitRow({ commit: CommitItem; isLatest: boolean; onOpenCommitDetail?: (sha: string) => void; - onAmendCommit?: (currentMessage: string) => void; - onRevertCommit?: (sha: string) => void; - onResetToCommit?: (sha: string) => void; + // Multi-repo: handlers receive the commit's repository_name so the + // amend/revert/reset op runs in the right git repo. Without it, ops hit + // the workspace root which fails on multi-repo task workspaces. + onAmendCommit?: (currentMessage: string, repo?: string) => void; + onRevertCommit?: (sha: string, repo?: string) => void; + onResetToCommit?: (sha: string, repo?: string) => void; }) { const showActions = onResetToCommit || (isLatest && (onAmendCommit || onRevertCommit)); diff --git a/apps/web/components/task/mobile/session-mobile-top-bar-git-controls.tsx b/apps/web/components/task/mobile/session-mobile-top-bar-git-controls.tsx index c195738547..b8b0454e32 100644 --- a/apps/web/components/task/mobile/session-mobile-top-bar-git-controls.tsx +++ b/apps/web/components/task/mobile/session-mobile-top-bar-git-controls.tsx @@ -37,6 +37,7 @@ import { Textarea } from "@kandev/ui/textarea"; import { Checkbox } from "@kandev/ui/checkbox"; import { Label } from "@kandev/ui/label"; import { useGitOperations } from "@/hooks/use-git-operations"; +import { useSessionGit } from "@/hooks/domains/session/use-session-git"; import type { FileInfo } from "@/lib/state/slices"; import { useToast } from "@/components/toast-provider"; import { @@ -433,15 +434,13 @@ export function useMobileGitActions( setPrDialogOpen: (v: boolean) => void, ) { const { toast } = useToast(); - const { - pull, - push, - rebase, - merge, - commit, - createPR, - isLoading: isGitLoading, - } = useGitOperations(sessionId ?? null); + // SessionGit's commit fans out per-repo for multi-repo workspaces; the raw + // useGitOperations.commit hits the workspace root and fails on multi-repo + // tasks. Pull from useGitOperations only what SessionGit doesn't override. + const { pull, push, rebase, merge, createPR, isLoading: isGitLoading } = useGitOperations( + sessionId ?? null, + ); + const { commit } = useSessionGit(sessionId ?? null); const handleGitOperation = useGitToast(); const handlePull = useCallback( diff --git a/apps/web/components/task/task-files-panel-hooks.ts b/apps/web/components/task/task-files-panel-hooks.ts index 9e88bd30e2..09859f8ef3 100644 --- a/apps/web/components/task/task-files-panel-hooks.ts +++ b/apps/web/components/task/task-files-panel-hooks.ts @@ -143,7 +143,7 @@ export function useCommitDiffs(activeSessionId: string | null | undefined) { export function useGitStagingActions( activeSessionId: string | null | undefined, - changedFiles: unknown[], + changedFiles: Array<{ path?: string; repository_name?: string } | unknown>, ) { const gitOps = useGitOperations(activeSessionId ?? null); const [pendingStageFiles, setPendingStageFiles] = useState>(new Set()); @@ -153,11 +153,31 @@ export function useGitStagingActions( // eslint-disable-next-line react-hooks/exhaustive-deps }, [changedFiles]); + // Multi-repo: each agentctl git op needs the repo subpath, otherwise the + // command runs at the workspace root (the task holder dir) and silently + // no-ops. Build a path → repository_name map from changedFiles. + const repoForPath = useMemo(() => { + const m = new Map(); + for (const f of changedFiles) { + if (!f || typeof f !== "object") continue; + const path = (f as { path?: unknown }).path; + const repo = (f as { repository_name?: unknown }).repository_name; + if (typeof path === "string" && typeof repo === "string" && repo) { + m.set(path, repo); + } + } + return m; + }, [changedFiles]); + + // The FileRow now passes the file's repository_name explicitly, which avoids + // path-based lookup collisions when two repos share a filename. Fall back to + // the lookup only if the caller didn't pass one (older call sites). const handleStage = useCallback( - async (path: string) => { + async (path: string, repo?: string) => { + const effectiveRepo = repo ?? repoForPath.get(path); setPendingStageFiles((prev) => new Set(prev).add(path)); try { - await gitOps.stage([path]); + await gitOps.stage([path], effectiveRepo); } catch { setPendingStageFiles((prev) => { const next = new Set(prev); @@ -166,14 +186,15 @@ export function useGitStagingActions( }); } }, - [gitOps], + [gitOps, repoForPath], ); const handleUnstage = useCallback( - async (path: string) => { + async (path: string, repo?: string) => { + const effectiveRepo = repo ?? repoForPath.get(path); setPendingStageFiles((prev) => new Set(prev).add(path)); try { - await gitOps.unstage([path]); + await gitOps.unstage([path], effectiveRepo); } catch { setPendingStageFiles((prev) => { const next = new Set(prev); @@ -182,7 +203,7 @@ export function useGitStagingActions( }); } }, - [gitOps], + [gitOps, repoForPath], ); return { pendingStageFiles, handleStage, handleUnstage }; @@ -241,18 +262,22 @@ export function useFilesPanelTab( export function useDiscardDialog(activeSessionId: string | null | undefined) { const [showDiscardDialog, setShowDiscardDialog] = useState(false); const [fileToDiscard, setFileToDiscard] = useState(null); + // Multi-repo: capture the file's repo at click time so the discard op runs + // against the right git repo (path-only routing collides on same-named files). + const [repoToDiscard, setRepoToDiscard] = useState(undefined); const gitOps = useGitOperations(activeSessionId ?? null); const { toast } = useToast(); - const handleDiscardClick = useCallback((filePath: string) => { + const handleDiscardClick = useCallback((filePath: string, repo?: string) => { setFileToDiscard(filePath); + setRepoToDiscard(repo); setShowDiscardDialog(true); }, []); const handleDiscardConfirm = useCallback(async () => { if (!fileToDiscard) return; try { - const result = await gitOps.discard([fileToDiscard]); + const result = await gitOps.discard([fileToDiscard], repoToDiscard); if (!result.success) toast({ title: "Failed to discard changes", @@ -268,8 +293,9 @@ export function useDiscardDialog(activeSessionId: string | null | undefined) { } finally { setShowDiscardDialog(false); setFileToDiscard(null); + setRepoToDiscard(undefined); } - }, [fileToDiscard, gitOps, toast]); + }, [fileToDiscard, repoToDiscard, gitOps, toast]); return { showDiscardDialog, diff --git a/apps/web/components/vcs/vcs-dialogs.tsx b/apps/web/components/vcs/vcs-dialogs.tsx index 269ac271a5..20d06ad70a 100644 --- a/apps/web/components/vcs/vcs-dialogs.tsx +++ b/apps/web/components/vcs/vcs-dialogs.tsx @@ -22,6 +22,7 @@ import { PRBranchSummary, } from "./vcs-dialog-fields"; import { useSessionGitStatus } from "@/hooks/domains/session/use-session-git-status"; +import { useSessionGit } from "@/hooks/domains/session/use-session-git"; import { useGitOperations } from "@/hooks/use-git-operations"; import { useGitWithFeedback } from "@/hooks/use-git-with-feedback"; import { useUtilityAgentGenerator } from "@/hooks/use-utility-agent-generator"; @@ -396,7 +397,10 @@ export function VcsDialogsProvider({ const { toast } = useToast(); const gitWithFeedback = useGitWithFeedback(); const gitStatus = useSessionGitStatus(sessionId); - const { commit, createPR, isLoading: isGitLoading } = useGitOperations(sessionId); + // Use SessionGit so commit fans out per-repo for multi-repo workspaces. + // useGitOperations.commit hits the workspace root, which fails for multi-repo + // tasks because the task root isn't itself a git repo (exit 1). + const { commit, createPR, isLoading: isGitLoading } = useSessionGit(sessionId); const isUtilityConfigured = useIsUtilityConfigured(); const { isGeneratingCommitMessage, diff --git a/apps/web/hooks/domains/session/use-session-git-grouping.test.ts b/apps/web/hooks/domains/session/use-session-git-grouping.test.ts new file mode 100644 index 0000000000..e0c3851b8b --- /dev/null +++ b/apps/web/hooks/domains/session/use-session-git-grouping.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect } from "vitest"; +import { groupPathsByRepoName } from "./use-session-git"; + +describe("groupPathsByRepoName", () => { + it("splits paths by repository_name, preserving insertion order per bucket", () => { + const lookup = new Map([ + ["src/app.tsx", "frontend"], + ["src/api.ts", "frontend"], + ["handlers/task.go", "backend"], + ]); + const out = groupPathsByRepoName(["src/app.tsx", "handlers/task.go", "src/api.ts"], lookup); + expect(out.get("frontend")).toEqual(["src/app.tsx", "src/api.ts"]); + expect(out.get("backend")).toEqual(["handlers/task.go"]); + expect(out.size).toBe(2); + }); + + it("falls back to empty-string bucket for paths missing a repo", () => { + const out = groupPathsByRepoName(["unknown.txt"], new Map()); + expect(out.get("")).toEqual(["unknown.txt"]); + }); + + it("preserves single-repo callers (everything in one bucket)", () => { + const lookup = new Map([ + ["a.ts", "only"], + ["b.ts", "only"], + ]); + const out = groupPathsByRepoName(["a.ts", "b.ts"], lookup); + expect(out.size).toBe(1); + expect(out.get("only")).toEqual(["a.ts", "b.ts"]); + }); +}); diff --git a/apps/web/hooks/domains/session/use-session-git.ts b/apps/web/hooks/domains/session/use-session-git.ts index 688a8ea095..53a528f79f 100644 --- a/apps/web/hooks/domains/session/use-session-git.ts +++ b/apps/web/hooks/domains/session/use-session-git.ts @@ -53,14 +53,27 @@ export type SessionGit = { rebase: (baseBranch: string) => Promise; merge: (baseBranch: string) => Promise; abort: (operation: "merge" | "rebase") => Promise; - commit: (message: string, stageAll?: boolean, amend?: boolean) => Promise; - stage: (paths?: string[]) => Promise; - stageFile: (paths: string[]) => Promise; + // Multi-repo: when `repo` is omitted, commit fans out one call per repo with + // staged changes so each repo gets its own commit. With `repo`, only that + // repo is committed. + commit: ( + message: string, + stageAll?: boolean, + amend?: boolean, + repo?: string, + ) => Promise; + // Multi-repo: when `repo` is provided, the op runs against that repo only — + // use this for per-file actions to avoid path-based lookup collisions + // (same-named files like README.md exist in multiple repos). When `repo` + // is omitted, the op falls back to a path-based lookup against allFiles + // (works for unique paths) or fans out across every repo present. + stage: (paths?: string[], repo?: string) => Promise; + stageFile: (paths: string[], repo?: string) => Promise; stageAll: () => Promise; - unstage: (paths?: string[]) => Promise; - unstageFile: (paths: string[]) => Promise; + unstage: (paths?: string[], repo?: string) => Promise; + unstageFile: (paths: string[], repo?: string) => Promise; unstageAll: () => Promise; - discard: (paths?: string[]) => Promise; + discard: (paths?: string[], repo?: string) => Promise; revertCommit: (commitSHA: string) => Promise; renameBranch: (newName: string) => Promise; reset: (commitSHA: string, mode: "soft" | "hard") => Promise; @@ -72,6 +85,29 @@ export type SessionGit = { ) => Promise; }; +/** + * Groups paths into per-repo buckets using a path → repository_name lookup. + * Paths missing a known repo land under "" (the single-repo bucket) so legacy + * single-repo workspaces and stray entries stay correct. Insertion order in + * `paths` is preserved within each bucket. + * + * Exported for testing — also used internally by useSessionGit's stage/unstage + * fan-out, where every per-repo bucket becomes one agentctl call. + */ +export function groupPathsByRepoName( + paths: string[], + repoForPath: Map, +): Map { + const buckets = new Map(); + for (const p of paths) { + const repo = repoForPath.get(p) ?? ""; + const list = buckets.get(repo); + if (list) list.push(p); + else buckets.set(repo, [p]); + } + return buckets; +} + /** * Builds the SessionGit's flat file list. For multi-repo workspaces it * stamps each FileInfo with its repository_name so consumers can group; @@ -125,16 +161,122 @@ export function useSessionGit(sessionId: string | null | undefined): SessionGit const hasStaged = stagedFiles.length > 0; const hasCommits = commits.length > 0; - // eslint-disable-next-line react-hooks/exhaustive-deps -- depend on stable fn ref, not the whole gitOps object - const stageAll = useCallback(async () => gitOps.stage(), [gitOps.stage]); - // eslint-disable-next-line react-hooks/exhaustive-deps -- depend on stable fn ref, not the whole gitOps object - const unstageAll = useCallback(async () => gitOps.unstage(), [gitOps.unstage]); + // Multi-repo path-to-repo lookup: agentctl's stage/unstage/discard endpoints + // need an explicit repo subpath (the repo dir basename). Without it the op + // runs at the workspace root which, for multi-repo task workspaces, isn't + // a git repo and silently no-ops. + const repoForPath = useMemo(() => { + const m = new Map(); + for (const f of allFiles) { + if (f.repository_name) m.set(f.path, f.repository_name); + } + return m; + }, [allFiles]); + // List of distinct repo names present in the current files; empty when single-repo. + const reposInFiles = useMemo(() => { + const seen = new Set(); + for (const f of allFiles) if (f.repository_name) seen.add(f.repository_name); + return Array.from(seen); + }, [allFiles]); + + const groupPathsByRepo = useCallback( + (paths: string[]): Map => groupPathsByRepoName(paths, repoForPath), + [repoForPath], + ); + + // Fan out a stage-all / unstage-all (no paths) across every repo when multi-repo. + // Returns the last op's result (UI only checks success/error of the most recent). + const runOnAllRepos = useCallback( + async (op: (paths: string[] | undefined, repo?: string) => Promise) => { + if (reposInFiles.length <= 1) { + return op(undefined, reposInFiles[0]); + } + let last: GitOperationResult | undefined; + for (const repo of reposInFiles) { + last = await op(undefined, repo); + } + return last as GitOperationResult; + }, + [reposInFiles], + ); + + const stageAll = useCallback( + async () => runOnAllRepos((paths, repo) => gitOps.stage(paths, repo)), + // eslint-disable-next-line react-hooks/exhaustive-deps -- depend on stable fn ref, not the whole gitOps object + [runOnAllRepos, gitOps.stage], + ); + const unstageAll = useCallback( + async () => runOnAllRepos((paths, repo) => gitOps.unstage(paths, repo)), + // eslint-disable-next-line react-hooks/exhaustive-deps -- depend on stable fn ref, not the whole gitOps object + [runOnAllRepos, gitOps.unstage], + ); + + // Multi-repo commit: when the caller doesn't pin a repo, commit each repo + // that has staged files separately (with the same message). Committing at + // the workspace root for a multi-repo task fails because the task root + // isn't itself a git repo — that's the "exit 1" the user saw. + const commit = useCallback( + async ( + message: string, + stageAll: boolean = true, + amend: boolean = false, + repo?: string, + ): Promise => { + if (repo !== undefined) { + return gitOps.commit(message, stageAll, amend, repo || undefined); + } + const reposWithStaged = Array.from( + new Set( + stagedFiles + .map((f) => f.repository_name) + .filter((n): n is string => Boolean(n)), + ), + ); + // Single-repo (or repo-less): one call, no repo arg. + if (reposWithStaged.length === 0) { + return gitOps.commit(message, stageAll, amend); + } + // Multi-repo: one commit per repo with staged changes. Stop on first + // failure so the user sees the actual error instead of cascading ones. + let last: GitOperationResult | undefined; + for (const r of reposWithStaged) { + last = await gitOps.commit(message, stageAll, amend, r); + if (!last.success) return last; + } + return last as GitOperationResult; + }, + // eslint-disable-next-line react-hooks/exhaustive-deps -- depend on stable fn ref, not the whole gitOps object + [gitOps.commit, stagedFiles], + ); + + // runPerRepo invokes `op` once per repo bucket. When the caller supplies an + // explicit `repo`, all paths route there as a single call (skipping the + // path → repo lookup that can collide on same-named files). Otherwise paths + // are split by repository_name from allFiles. + const runPerRepo = useCallback( + async ( + paths: string[], + explicitRepo: string | undefined, + op: (paths: string[], repo: string | undefined) => Promise, + ): Promise => { + if (explicitRepo !== undefined) { + return op(paths, explicitRepo || undefined); + } + const buckets = groupPathsByRepo(paths); + let last: GitOperationResult | undefined; + for (const [repo, repoPaths] of buckets) { + last = await op(repoPaths, repo || undefined); + } + return last as GitOperationResult; + }, + [groupPathsByRepo], + ); const stageFile = useCallback( - async (paths: string[]) => { + async (paths: string[], repo?: string) => { for (const p of paths) setPendingStageFiles((prev) => new Set(prev).add(p)); try { - return await gitOps.stage(paths); + return await runPerRepo(paths, repo, (rp, r) => gitOps.stage(rp, r)); } catch (err) { setPendingStageFiles((prev) => { const next = new Set(prev); @@ -145,14 +287,14 @@ export function useSessionGit(sessionId: string | null | undefined): SessionGit } }, // eslint-disable-next-line react-hooks/exhaustive-deps -- depend on stable fn ref, not the whole gitOps object - [gitOps.stage], + [gitOps.stage, runPerRepo], ); const unstageFile = useCallback( - async (paths: string[]) => { + async (paths: string[], repo?: string) => { for (const p of paths) setPendingStageFiles((prev) => new Set(prev).add(p)); try { - return await gitOps.unstage(paths); + return await runPerRepo(paths, repo, (rp, r) => gitOps.unstage(rp, r)); } catch (err) { setPendingStageFiles((prev) => { const next = new Set(prev); @@ -163,7 +305,19 @@ export function useSessionGit(sessionId: string | null | undefined): SessionGit } }, // eslint-disable-next-line react-hooks/exhaustive-deps -- depend on stable fn ref, not the whole gitOps object - [gitOps.unstage], + [gitOps.unstage, runPerRepo], + ); + + const discard = useCallback( + async (paths?: string[], repo?: string) => { + if (!paths || paths.length === 0) { + // Discard with no paths is unsupported by the API; passthrough to surface the error. + return gitOps.discard(paths, repo); + } + return runPerRepo(paths, repo, (rp, r) => gitOps.discard(rp, r)); + }, + // eslint-disable-next-line react-hooks/exhaustive-deps -- depend on stable fn ref, not the whole gitOps object + [gitOps.discard, runPerRepo], ); return { @@ -199,14 +353,18 @@ export function useSessionGit(sessionId: string | null | undefined): SessionGit rebase: gitOps.rebase, merge: gitOps.merge, abort: gitOps.abort, - commit: gitOps.commit, - stage: gitOps.stage, + commit, + // stage/unstage route through the fan-out wrappers so multi-repo workspaces + // hit the right repo subpath in agentctl. Empty/undefined paths = stage all. + stage: (paths?: string[], repo?: string) => + paths && paths.length > 0 ? stageFile(paths, repo) : stageAll(), stageFile, stageAll, - unstage: gitOps.unstage, + unstage: (paths?: string[], repo?: string) => + paths && paths.length > 0 ? unstageFile(paths, repo) : unstageAll(), unstageFile, unstageAll, - discard: gitOps.discard, + discard, revertCommit: gitOps.revertCommit, renameBranch: gitOps.renameBranch, reset: gitOps.reset, diff --git a/apps/web/hooks/domains/workspace/use-repository-branches.ts b/apps/web/hooks/domains/workspace/use-repository-branches.ts index 306d1d2664..fba5fa311b 100644 --- a/apps/web/hooks/domains/workspace/use-repository-branches.ts +++ b/apps/web/hooks/domains/workspace/use-repository-branches.ts @@ -1,46 +1,72 @@ +"use client"; + import { useEffect, useRef } from "react"; import { useAppStore } from "@/components/state-provider"; -import { listRepositoryBranches } from "@/lib/api"; +import { listBranches } from "@/lib/api"; import type { Branch } from "@/lib/types/http"; const EMPTY_BRANCHES: Branch[] = []; -export function useRepositoryBranches(repositoryId: string | null, enabled = true) { +/** + * Source of branches for a row: either a workspace repo (by id) or an + * on-machine folder (by path). Both routes go through one backend endpoint + * (`/workspaces/:id/branches`) and share one Zustand cache slice — id-based + * entries are keyed by the repo id, path-based entries get a synthetic key. + * + * `workspaceId` is always required because the route segment needs it. + */ +export type BranchSource = + | { kind: "id"; workspaceId: string; repositoryId: string } + | { kind: "path"; workspaceId: string; path: string }; + +function cacheKeyFor(source: BranchSource | null): string { + if (!source) return ""; + return source.kind === "id" ? source.repositoryId : `path::${source.workspaceId}::${source.path}`; +} + +/** + * Loads git branches for a workspace repo or an on-machine path. One hook, + * one cache, one backend endpoint — the source shape decides which query + * param goes on the wire and which key the cache uses. + */ +export function useBranches( + source: BranchSource | null, + enabled = true, +): { branches: Branch[]; isLoading: boolean } { + const key = cacheKeyFor(source); const branches = useAppStore((state) => - repositoryId - ? (state.repositoryBranches.itemsByRepositoryId[repositoryId] ?? EMPTY_BRANCHES) - : EMPTY_BRANCHES, + key ? (state.repositoryBranches.itemsByRepositoryId[key] ?? EMPTY_BRANCHES) : EMPTY_BRANCHES, ); const isLoaded = useAppStore((state) => - repositoryId ? (state.repositoryBranches.loadedByRepositoryId[repositoryId] ?? false) : false, + key ? (state.repositoryBranches.loadedByRepositoryId[key] ?? false) : false, ); const isLoading = useAppStore((state) => - repositoryId ? (state.repositoryBranches.loadingByRepositoryId[repositoryId] ?? false) : false, + key ? (state.repositoryBranches.loadingByRepositoryId[key] ?? false) : false, ); const setRepositoryBranches = useAppStore((state) => state.setRepositoryBranches); const setRepositoryBranchesLoading = useAppStore((state) => state.setRepositoryBranchesLoading); const inFlightRef = useRef(false); useEffect(() => { - if (!enabled || !repositoryId) return; + if (!enabled || !source) return; if (isLoaded || inFlightRef.current) return; inFlightRef.current = true; - setRepositoryBranchesLoading(repositoryId, true); - // Capture the repositoryId at effect start for the closure - const fetchRepoId = repositoryId; - listRepositoryBranches(fetchRepoId, { cache: "no-store" }) - .then((response) => { - // Safe to always store: data is keyed by repositoryId in the store - setRepositoryBranches(fetchRepoId, response.branches); - }) - .catch(() => { - setRepositoryBranches(fetchRepoId, []); - }) + setRepositoryBranchesLoading(key, true); + + const promise = + source.kind === "id" + ? listBranches(source.workspaceId, { repositoryId: source.repositoryId }) + : listBranches(source.workspaceId, { path: source.path }); + + promise + .then((response) => setRepositoryBranches(key, response.branches)) + .catch(() => setRepositoryBranches(key, [])) .finally(() => { inFlightRef.current = false; - setRepositoryBranchesLoading(fetchRepoId, false); + setRepositoryBranchesLoading(key, false); }); - }, [enabled, isLoaded, repositoryId, setRepositoryBranches, setRepositoryBranchesLoading]); + // eslint-disable-next-line react-hooks/exhaustive-deps -- key encodes source identity; listing every field re-fires on every render + }, [enabled, isLoaded, key, setRepositoryBranches, setRepositoryBranchesLoading]); return { branches, isLoading }; } diff --git a/apps/web/hooks/use-git-operations.ts b/apps/web/hooks/use-git-operations.ts index 1619872701..8bb4a99be6 100644 --- a/apps/web/hooks/use-git-operations.ts +++ b/apps/web/hooks/use-git-operations.ts @@ -21,19 +21,26 @@ export interface PRCreateResult { } interface UseGitOperationsReturn { - // Operation methods + // Operation methods. The optional `repo` parameter is the multi-repo subpath + // (e.g. "kandev"); pass empty/undefined for single-repo workspaces. Multi-repo + // workspaces MUST scope each call to one repo — bulk callers fan out themselves. pull: (rebase?: boolean) => Promise; push: (options?: { force?: boolean; setUpstream?: boolean }) => Promise; rebase: (baseBranch: string) => Promise; merge: (baseBranch: string) => Promise; abort: (operation: "merge" | "rebase") => Promise; - commit: (message: string, stageAll?: boolean, amend?: boolean) => Promise; - stage: (paths?: string[]) => Promise; - unstage: (paths?: string[]) => Promise; - discard: (paths?: string[]) => Promise; - revertCommit: (commitSHA: string) => Promise; + commit: ( + message: string, + stageAll?: boolean, + amend?: boolean, + repo?: string, + ) => Promise; + stage: (paths?: string[], repo?: string) => Promise; + unstage: (paths?: string[], repo?: string) => Promise; + discard: (paths?: string[], repo?: string) => Promise; + revertCommit: (commitSHA: string, repo?: string) => Promise; renameBranch: (newName: string) => Promise; - reset: (commitSHA: string, mode: "soft" | "hard") => Promise; + reset: (commitSHA: string, mode: "soft" | "hard", repo?: string) => Promise; createPR: ( title: string, body: string, @@ -72,30 +79,47 @@ function buildGitOperationCallbacks(executeOperation: ExecuteOperation) { const abort = async (operation: "merge" | "rebase") => executeOperation("worktree.abort", { operation }); - const commit = async (message: string, stageAll = true, amend = false) => + const commit = async (message: string, stageAll = true, amend = false, repo?: string) => executeOperation("worktree.commit", { message, stage_all: stageAll, amend, + ...(repo ? { repo } : {}), }); - const stage = async (paths?: string[]) => - executeOperation("worktree.stage", { paths: paths ?? [] }); + const stage = async (paths?: string[], repo?: string) => + executeOperation("worktree.stage", { + paths: paths ?? [], + ...(repo ? { repo } : {}), + }); - const unstage = async (paths?: string[]) => - executeOperation("worktree.unstage", { paths: paths ?? [] }); + const unstage = async (paths?: string[], repo?: string) => + executeOperation("worktree.unstage", { + paths: paths ?? [], + ...(repo ? { repo } : {}), + }); - const discard = async (paths?: string[]) => - executeOperation("worktree.discard", { paths: paths ?? [] }); + const discard = async (paths?: string[], repo?: string) => + executeOperation("worktree.discard", { + paths: paths ?? [], + ...(repo ? { repo } : {}), + }); - const revertCommit = async (commitSHA: string) => - executeOperation("worktree.revert_commit", { commit_sha: commitSHA }); + const revertCommit = async (commitSHA: string, repo?: string) => + executeOperation("worktree.revert_commit", { + commit_sha: commitSHA, + ...(repo ? { repo } : {}), + }); const renameBranch = async (newName: string) => executeOperation("worktree.rename_branch", { new_name: newName }); - const reset = async (commitSHA: string, mode: "soft" | "hard") => - executeOperation("worktree.reset", { commit_sha: commitSHA, mode }); + const reset = async (commitSHA: string, mode: "soft" | "hard", repo?: string) => + executeOperation("worktree.reset", { + commit_sha: commitSHA, + mode, + ...(repo ? { repo } : {}), + }); const createPR = async ( title: string, diff --git a/apps/web/lib/api/domains/workspace-api.ts b/apps/web/lib/api/domains/workspace-api.ts index caa9c51a6c..114ccf3b39 100644 --- a/apps/web/lib/api/domains/workspace-api.ts +++ b/apps/web/lib/api/domains/workspace-api.ts @@ -37,9 +37,22 @@ export async function listRepositories( return fetchJson(url, options); } -export async function listRepositoryBranches(repositoryId: string, options?: ApiRequestOptions) { +/** + * Lists git branches for a workspace repo. Pass exactly one of `repositoryId` + * (an imported workspace repo) or `path` (an on-machine folder discovered + * but not yet imported). The backend resolves either to an absolute path and + * runs the same `listGitBranches`. + */ +export async function listBranches( + workspaceId: string, + source: { repositoryId: string } | { path: string }, + options?: ApiRequestOptions, +) { + const params = new URLSearchParams(); + if ("repositoryId" in source) params.set("repository_id", source.repositoryId); + else params.set("path", source.path); return fetchJson( - `/api/v1/repositories/${repositoryId}/branches`, + `/api/v1/workspaces/${workspaceId}/branches?${params.toString()}`, options, ); } diff --git a/apps/web/lib/group-by-repo.test.ts b/apps/web/lib/group-by-repo.test.ts new file mode 100644 index 0000000000..c4df1ab390 --- /dev/null +++ b/apps/web/lib/group-by-repo.test.ts @@ -0,0 +1,29 @@ +import { describe, it, expect } from "vitest"; +import { groupByRepositoryName } from "./group-by-repo"; + +describe("groupByRepositoryName", () => { + it("buckets by name in first-seen order, preserving item order within each bucket", () => { + const items = [ + { id: "a", repo: "frontend" }, + { id: "b", repo: "backend" }, + { id: "c", repo: "frontend" }, + { id: "d", repo: "backend" }, + ]; + const groups = groupByRepositoryName(items, (i) => i.repo); + expect(groups.map((g) => g.repositoryName)).toEqual(["frontend", "backend"]); + expect(groups[0].items.map((i) => i.id)).toEqual(["a", "c"]); + expect(groups[1].items.map((i) => i.id)).toEqual(["b", "d"]); + }); + + it("collapses missing/undefined names into a single empty-name bucket", () => { + const items = [{ id: "x" }, { id: "y", repo: "named" }, { id: "z" }]; + const groups = groupByRepositoryName(items, (i) => (i as { repo?: string }).repo); + expect(groups.map((g) => g.repositoryName)).toEqual(["", "named"]); + expect(groups[0].items.map((i) => i.id)).toEqual(["x", "z"]); + expect(groups[1].items.map((i) => i.id)).toEqual(["y"]); + }); + + it("returns an empty array for an empty input", () => { + expect(groupByRepositoryName([], () => "x")).toEqual([]); + }); +}); diff --git a/apps/web/lib/group-by-repo.ts b/apps/web/lib/group-by-repo.ts new file mode 100644 index 0000000000..d090653c42 --- /dev/null +++ b/apps/web/lib/group-by-repo.ts @@ -0,0 +1,26 @@ +/** + * Buckets a list into per-repository groups, preserving the original item + * order within each bucket and producing groups in first-seen order. Items + * whose `getName` returns undefined or empty land in a single empty-named + * group so callers can decide whether to render a header for it. + * + * One copy of this lived in three places before — review-diff-list-groups, + * changes-panel-timeline (files), and changes-panel-timeline (commits) — + * differing only in element type. The generic shape collapses them all. + */ +export function groupByRepositoryName( + items: T[], + getName: (item: T) => string | undefined, +): Array<{ repositoryName: string; items: T[] }> { + const order: string[] = []; + const buckets = new Map(); + for (const item of items) { + const name = getName(item) ?? ""; + if (!buckets.has(name)) { + buckets.set(name, []); + order.push(name); + } + buckets.get(name)!.push(item); + } + return order.map((name) => ({ repositoryName: name, items: buckets.get(name)! })); +} diff --git a/apps/web/lib/state/slices/session-runtime/types.ts b/apps/web/lib/state/slices/session-runtime/types.ts index b24531aeed..58f7ab0a47 100644 --- a/apps/web/lib/state/slices/session-runtime/types.ts +++ b/apps/web/lib/state/slices/session-runtime/types.ts @@ -110,6 +110,8 @@ export type SessionCommit = { insertions: number; deletions: number; created_at: string; + /** Multi-repo: name of the repo this commit was made in. Empty for single-repo. */ + repository_name?: string; }; export type CumulativeDiff = { diff --git a/apps/web/lib/types/git-events.ts b/apps/web/lib/types/git-events.ts index e1469a8503..93de9aa61b 100644 --- a/apps/web/lib/types/git-events.ts +++ b/apps/web/lib/types/git-events.ts @@ -45,6 +45,8 @@ export type GitCommitData = { deletions: number; committed_at: string; created_at?: string; + /** Multi-repo: name of the repo this commit was made in. Empty for single-repo. */ + repository_name?: string; }; // Git reset data diff --git a/apps/web/lib/ws/handlers/git-status.ts b/apps/web/lib/ws/handlers/git-status.ts index 34c39a2e03..0967a8bc76 100644 --- a/apps/web/lib/ws/handlers/git-status.ts +++ b/apps/web/lib/ws/handlers/git-status.ts @@ -61,6 +61,8 @@ const gitEventHandlers: GitEventHandlers = { deletions: event.commit.deletions, committed_at: event.commit.committed_at, created_at: event.commit.created_at ?? event.timestamp, + // Multi-repo: tag the commit so the Commits panel can group per repo. + repository_name: event.commit.repository_name, }); // Invalidate cumulative diff cache when new commit is created invalidateCumulativeDiffCache(resolveEnvKey(store, event.session_id));