fd79d6170f
* feat(tasks): multi-branch tasks — N (repo, branch) pairs per task Extends multi-repo into multi-branch so a single task can hold multiple worktrees on the same repo (one per branch). Sibling worktrees layout at <task>/<repo>/ + <task>/<repo>-<slug>/, dedicated PRWatch per branch, live agentctl tracker rescan + frontend store updates so the UI splits PRs / files / commits per branch without a session restart. MCP add_branch_to_task_kandev tool is shielded to the worktree executor. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(web): unify multi-branch repo label as <repo> · <branch> across panels PR Changes and Commits panels rendered repo labels differently for multi-branch tasks — Commits showed "kandev · feature/x" while PR Changes leaked the raw "kandev-feature-x" agentctl tracker tag. Reformat tracker-tagged repo names via formatRepoLabel using the longest known-repo prefix so kandev-cli-* doesn't collapse to kandev · cli-*. * fix(tasks): address multi-branch PR review — data races, slug collisions, MCP error codes Backend - agentctl process.Manager: add repoTrackersMu RWMutex guarding repoTrackers + workspaceTracker. Reads snapshot under RLock; RescanRepositories builds trackers first, takes Lock for swap/append. Validates newWorkDir via os.Stat before publishing cfg.WorkDir. - worktree.Manager: reject invalid explicit BranchSlug up-front; cache key uses sanitized wt.BranchSlug so read/write paths are symmetric. - worktree.store: narrow isDuplicateColumnError to "duplicate column name". - task service AddBranchToTask: slug-collision guard rejects feat/a vs feat-a. - orchestrator resolvePushRepo: two-pass exact-name match before subdir prefix fallback so "backend" doesn't swallow pushes for "backend-admin". - executor buildRepoSpecs: duplicate repos always get a non-empty BranchSlug (falls back to branch-N when both branch fields sanitize to empty). - MCP handler: classifyAddBranchError maps service errors to NOT_FOUND / CONFLICT / VALIDATION_ERROR; add ErrorCodeConflict constant. - MCP server: extract registerAddBranchToTaskTool, register in task mode only (external mode contract: config tools + create_task_kandev). - workspace_rescan handler: reject malformed JSON with 400 instead of silently rescanning the current root. - branch_materializer: split MaterializeBranch into prepare + finalize helpers to satisfy the 80-line cap. Frontend - existingPrUrl returns undefined when a task has >1 PR so per-repo View PR buttons take over (no silent linking to a sibling). - formatRepoLabel checks exact known-repo match before prefix splitting so "kandev-cli" isn't mislabeled as "kandev · cli". - Repo chip key now includes base_branch so siblings differing only on base render as distinct chips. Docs - multi-branch spec uniqueness wording aligned with the canonical 4-column key (task_id, repository_id, base_branch, checkout_branch). - worktree.CreateRequest.BranchSlug doc describes the sibling layout (<repo>-<slug>) instead of the obsolete nested form. * fix(tasks): scope slug-collision check to repo only + use errors.Is for ErrTaskNotFound - service_branches: drop base_branch from the slug-collision check. Worktree path is (repo, slug); two rows on the same repo cut from different bases still collide on disk. - service_branches: prepareBranchAdd wraps taskrepo.ErrTaskNotFound via fmt.Errorf with %w so classifiers don't have to substring-match the formatted task UUID. - mcp handlers: classifyAddBranchError uses errors.Is(err, ErrTaskNotFound). * fix(agentctl): sanitize rescan work_dir before os.Stat to clear CodeQL path-injection CodeQL flagged the rescan endpoint as a path-injection sink (the HTTP-supplied work_dir flowed straight into os.Stat and downstream scanRepositorySubdirs). Add validateRescanPath which: - requires the input to be a clean absolute path with no ".." segments - rejects anything that isn't equal to or an ancestor of the current cfg.WorkDir — the only legitimate use case is the branch materializer promoting <task>/<repo>/ to <task>/ Defense in depth: agentctl binds to localhost, but any colocated process could otherwise direct it at /etc or the user's home by POSTing /api/v1/workspace/rescan. * fix(agentctl): tighten validateRescanPath, close TOCTOU on existingTrackers, add tests - Drop the strings.Contains(clean, "..") guard. filepath.Clean already resolves traversal; the substring check rejects legitimate names like /home/u/..hidden without adding defense (the ancestor check via filepath.Rel is the real boundary). - Drop the rel == "." branch in the ancestor check — early return for clean == currentClean already covers identity. - Read existingTrackers inside the write-lock section (not under RLock then re-Lock) so two concurrent rescans can't both see 0 trackers and double-bootstrap the multi-repo set. - Add table-driven TestValidateRescanPath covering rejection paths (relative, descendant, sibling, traversal-cleaned, empty input), legitimate ancestor promotion, exact match, no-anchor first-launch, and a /home/u/..hidden dotted-segment case. * fix(agentctl): break taint flow into os.Stat — derive rescan path from cfg, not HTTP body CodeQL's go/path-injection trace still flagged manager_rescan.go after the prior sanitizer because the user-supplied newWorkDir flowed (after validation) into os.Stat and then into cfg.WorkDir → tracker paths → workspace_monitor os.Stat sinks. Replace validateRescanPath with resolveRescanPath: it returns a path DERIVED from currentWorkDir (no-op = currentClean, promotion = filepath.Dir(currentClean)) and uses newPath only for comparison, never as a source for the returned string. os.Stat now receives a path whose provenance is trusted manager config. Functionally the legitimate caller (branch materializer promoting <task>/<repo>/ to <task>/) is unchanged. * test(agentctl): make TestResolveRescanPath windows-portable The Linux Backend Tests job passed but the Backend (windows) job rejected every "absolute" case because filepath.IsAbs("/task/repo") is false on Windows (no volume letter). Build inputs via filepath.Join against absRoot() (filepath.Abs of "/" / "\\") so the same table covers POSIX (`/...`) and Windows (`C:\...`) without per-OS branches. Keep the traversal-after-Clean case POSIX-only since filepath.Clean is the shared primitive and that input form needs a volume to be considered absolute on Windows anyway. * fix(agentctl): serialize RescanRepositories + re-check trackers under write lock Two concurrent RescanRepositories calls could both observe an empty tracker set (between the snapshot read and the bootstrap branch) and both enter transitionToMultiRepoMode, leaking a duplicate bare-root tracker and duplicate per-repo trackers. Same race in appendNewRepoTrackers would double-append the same new child across both callers. Fix: - Add rescanMu sync.Mutex on Manager. Acquired for the full body of RescanRepositories so the snapshot → branch → mutate sequence is serialized. repoTrackersMu still guards individual field reads from unrelated goroutines (subscribe/unsubscribe/poll-mode). - appendNewRepoTrackers re-checks membership inside the write-lock before appending and drops already-present trackers (Stop + detach outside the lock). Defense in depth. - Test: add `dotted segment is legal as promotion` case so the ..hidden-segment promotion path stays exercised. --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
332 lines
10 KiB
TypeScript
332 lines
10 KiB
TypeScript
"use client";
|
|
|
|
import { hashDiff, normalizeDiffContent } from "@/components/review/types";
|
|
import type { FileInfo } from "@/lib/state/store";
|
|
import type { PRDiffFile } from "@/lib/types/github";
|
|
import type { PRChangedFile } from "./changes-panel-timeline";
|
|
|
|
export type ChangedFile = {
|
|
path: string;
|
|
status: FileInfo["status"];
|
|
staged: boolean;
|
|
plus: number | undefined;
|
|
minus: number | undefined;
|
|
oldPath: string | undefined;
|
|
/** Repository this file belongs to in multi-repo workspaces; empty for single-repo. */
|
|
repositoryName?: string;
|
|
};
|
|
|
|
/**
|
|
* Picks the group label stamped onto each PR's files so the changes panel
|
|
* can show one section per PR for multi-branch tasks.
|
|
*
|
|
* Rules:
|
|
* - Only one PR on the task -> no stamp (the section header is enough).
|
|
* - Multi-repo, one PR per repo -> stamp the repo name (legacy behavior).
|
|
* - Any repo has multiple PRs -> append the branch / PR number so the
|
|
* group label disambiguates which PR each file belongs to.
|
|
*/
|
|
export function computePRGroupStamp(args: {
|
|
needsStamp: boolean;
|
|
taskHasMultipleRepos: boolean;
|
|
anyRepoMultiPR: boolean;
|
|
repoName: string;
|
|
branch: string;
|
|
prNumber: number;
|
|
}): string {
|
|
if (!args.needsStamp) return "";
|
|
const label = args.branch || `PR #${args.prNumber}`;
|
|
if (args.anyRepoMultiPR && args.taskHasMultipleRepos) {
|
|
return `${args.repoName} · ${label}`;
|
|
}
|
|
if (args.anyRepoMultiPR) {
|
|
return label;
|
|
}
|
|
return args.repoName;
|
|
}
|
|
|
|
export function mapToChangedFiles(files: FileInfo[]): ChangedFile[] {
|
|
return files.map((file) => ({
|
|
path: file.path,
|
|
status: file.status,
|
|
staged: file.staged,
|
|
plus: file.additions,
|
|
minus: file.deletions,
|
|
oldPath: file.old_path,
|
|
repositoryName: file.repository_name,
|
|
}));
|
|
}
|
|
|
|
type CumulativeDiffFiles = Record<
|
|
string,
|
|
{ diff?: string; status?: string; additions?: number; deletions?: number }
|
|
>;
|
|
|
|
function collectReviewPaths(
|
|
uncommittedFiles: FileInfo[],
|
|
cumulativeDiffFiles: CumulativeDiffFiles | undefined,
|
|
prFiles?: PRDiffFile[],
|
|
): Set<string> {
|
|
const paths = new Set<string>();
|
|
for (const file of uncommittedFiles) {
|
|
if (file.diff && normalizeDiffContent(file.diff)) paths.add(file.path);
|
|
}
|
|
if (cumulativeDiffFiles) {
|
|
for (const [path, file] of Object.entries(cumulativeDiffFiles)) {
|
|
if (!paths.has(path) && file.diff && normalizeDiffContent(file.diff)) paths.add(path);
|
|
}
|
|
}
|
|
if (prFiles) {
|
|
for (const file of prFiles) {
|
|
if (!paths.has(file.filename) && file.patch) paths.add(file.filename);
|
|
}
|
|
}
|
|
return paths;
|
|
}
|
|
|
|
function getDiffForPath(
|
|
path: string,
|
|
uncommittedFiles: FileInfo[],
|
|
cumulativeDiffFiles: CumulativeDiffFiles | undefined,
|
|
prFiles?: PRDiffFile[],
|
|
): string {
|
|
const uncommitted = uncommittedFiles.find((f) => f.path === path);
|
|
if (uncommitted?.diff) return normalizeDiffContent(uncommitted.diff);
|
|
const cumDiff = cumulativeDiffFiles?.[path]?.diff;
|
|
if (cumDiff) return normalizeDiffContent(cumDiff);
|
|
if (prFiles) {
|
|
const prFile = prFiles.find((f) => f.filename === path);
|
|
if (prFile?.patch) return normalizeDiffContent(prFile.patch);
|
|
}
|
|
return "";
|
|
}
|
|
|
|
export function computeReviewProgress(
|
|
uncommittedFiles: FileInfo[],
|
|
cumulativeDiff: { files?: CumulativeDiffFiles } | null,
|
|
reviews: Map<string, { reviewed: boolean; diffHash?: string }>,
|
|
prFiles?: PRDiffFile[],
|
|
) {
|
|
const cumulativeDiffFiles = cumulativeDiff?.files;
|
|
const paths = collectReviewPaths(uncommittedFiles, cumulativeDiffFiles, prFiles);
|
|
let reviewed = 0;
|
|
for (const path of paths) {
|
|
const state = reviews.get(path);
|
|
if (!state?.reviewed) continue;
|
|
const diffContent = getDiffForPath(path, uncommittedFiles, cumulativeDiffFiles, prFiles);
|
|
if (diffContent && state.diffHash && state.diffHash !== hashDiff(diffContent)) continue;
|
|
reviewed++;
|
|
}
|
|
return { reviewedCount: reviewed, totalFileCount: paths.size };
|
|
}
|
|
|
|
export function computeStagedStats(stagedFiles: FileInfo[]) {
|
|
const adds = stagedFiles.reduce((sum, f) => sum + (f.additions || 0), 0);
|
|
const dels = stagedFiles.reduce((sum, f) => sum + (f.deletions || 0), 0);
|
|
return { stagedFileCount: stagedFiles.length, stagedAdditions: adds, stagedDeletions: dels };
|
|
}
|
|
|
|
export function mapPRFilesToChangedFiles(
|
|
files: PRDiffFile[],
|
|
repositoryName?: string,
|
|
): PRChangedFile[] {
|
|
return files.map((file) => {
|
|
let status: FileInfo["status"];
|
|
switch (file.status) {
|
|
case "added":
|
|
status = "added";
|
|
break;
|
|
case "removed":
|
|
status = "deleted";
|
|
break;
|
|
case "renamed":
|
|
status = "renamed";
|
|
break;
|
|
default:
|
|
status = "modified";
|
|
}
|
|
return {
|
|
path: file.filename,
|
|
status,
|
|
plus: file.additions,
|
|
minus: file.deletions,
|
|
oldPath: file.old_path,
|
|
repository_name: repositoryName ?? "",
|
|
};
|
|
});
|
|
}
|
|
|
|
export function filterUnpushedCommits<T extends { commit_sha: string }>(
|
|
localCommits: T[],
|
|
prCommits: { sha: string }[],
|
|
): T[] {
|
|
if (prCommits.length === 0) return localCommits;
|
|
return localCommits.filter(
|
|
(c) =>
|
|
!prCommits.some((pr) => pr.sha.startsWith(c.commit_sha) || c.commit_sha.startsWith(pr.sha)),
|
|
);
|
|
}
|
|
|
|
/** Which timeline section, if any, renders first (topmost) in the Changes panel. */
|
|
type FirstVisibleSection = "pr" | "unstaged" | "staged" | "commits" | null;
|
|
|
|
/** PR Changes auto-expands only when the diff is small enough to scan inline. */
|
|
export const PR_CHANGES_AUTO_EXPAND_MAX_FILES = 5;
|
|
|
|
/**
|
|
* Computes the first (topmost) visible section so the panel can auto-expand it,
|
|
* mirroring the render precedence in `ChangesPanelTimeline`:
|
|
* PR (review mode, no local changes) → Unstaged → Staged → Commits.
|
|
*
|
|
* The "review mode" PR section only sits at the top when there are no local
|
|
* working-tree changes; with local changes the PR section moves below Staged and
|
|
* is therefore never first. Large PR diffs (>5 files) skip auto-expanding PR
|
|
* Changes and expand Commits instead. Returns null when nothing is shown.
|
|
*/
|
|
export function firstVisibleSection(flags: {
|
|
hasPRFiles: boolean;
|
|
hasUnstaged: boolean;
|
|
hasStaged: boolean;
|
|
showCommitsList: boolean;
|
|
prFileCount?: number;
|
|
}): FirstVisibleSection {
|
|
const { hasPRFiles, hasUnstaged, hasStaged, showCommitsList, prFileCount = 0 } = flags;
|
|
if (hasPRFiles && !hasUnstaged && !hasStaged) {
|
|
if (prFileCount <= PR_CHANGES_AUTO_EXPAND_MAX_FILES) return "pr";
|
|
if (showCommitsList) return "commits";
|
|
return null;
|
|
}
|
|
if (hasUnstaged) return "unstaged";
|
|
if (hasStaged) return "staged";
|
|
if (showCommitsList) return "commits";
|
|
return null;
|
|
}
|
|
|
|
type MergedCommit = {
|
|
commit_sha: string;
|
|
commit_message: string;
|
|
insertions: number;
|
|
deletions: number;
|
|
pushed: boolean;
|
|
/** Multi-repo: name of the repo this commit was made in. Empty for single-repo. */
|
|
repository_name?: string;
|
|
committed_at?: string;
|
|
};
|
|
|
|
/**
|
|
* Merge local session commits and PR commits into a single list. A commit is
|
|
* pushed when EITHER source confirms it: the backend's `pushed` field
|
|
* (computed from the upstream tracking ref) or a SHA match in `prCommits` (PR
|
|
* existence implies the commit is on the remote). The git signal fixes the
|
|
* original bug — pushed commits showing as unpushed when no PR exists — while
|
|
* keeping the PR signal so a commit visible only via a PR (e.g. authored by
|
|
* another contributor, or a rebased SHA the local repo doesn't track yet)
|
|
* still renders as pushed. PR commits not represented locally are appended
|
|
* as pushed. Order: unpushed first, then pushed.
|
|
*/
|
|
export function mergeCommits(
|
|
localCommits: {
|
|
commit_sha: string;
|
|
commit_message: string;
|
|
insertions: number;
|
|
deletions: number;
|
|
/** Multi-repo: name of the repo this commit was made in. */
|
|
repository_name?: string;
|
|
pushed?: boolean;
|
|
committed_at?: string;
|
|
}[],
|
|
prCommits: {
|
|
sha: string;
|
|
message: string;
|
|
additions: number;
|
|
deletions: number;
|
|
author_date?: string;
|
|
}[],
|
|
): MergedCommit[] {
|
|
const shaMatches = (a: string, b: string) => a.startsWith(b) || b.startsWith(a);
|
|
const unpushed: MergedCommit[] = [];
|
|
const pushed: MergedCommit[] = [];
|
|
const matchedPRShas = new Set<string>();
|
|
for (const c of localCommits) {
|
|
const matchesPR = prCommits.some((pr) => shaMatches(pr.sha, c.commit_sha));
|
|
const isPushed = c.pushed === true || matchesPR;
|
|
if (isPushed) {
|
|
pushed.push({ ...c, pushed: true });
|
|
} else {
|
|
unpushed.push({ ...c, pushed: false });
|
|
}
|
|
if (matchesPR) {
|
|
for (const pr of prCommits) {
|
|
if (shaMatches(pr.sha, c.commit_sha)) {
|
|
matchedPRShas.add(pr.sha);
|
|
}
|
|
}
|
|
}
|
|
}
|
|
for (const pr of prCommits) {
|
|
if (!matchedPRShas.has(pr.sha)) {
|
|
pushed.push({
|
|
commit_sha: pr.sha,
|
|
commit_message: pr.message,
|
|
insertions: pr.additions,
|
|
deletions: pr.deletions,
|
|
pushed: true,
|
|
committed_at: pr.author_date,
|
|
});
|
|
}
|
|
}
|
|
return [...unpushed, ...pushed];
|
|
}
|
|
|
|
export function getBaseBranchDisplay(baseBranch: string | undefined): string {
|
|
return baseBranch ? baseBranch.replace(/^origin\//, "") : "main";
|
|
}
|
|
|
|
export function isMultiRepoCommits(commits: { repository_name?: string }[]): boolean {
|
|
return commits.some((c) => !!c.repository_name);
|
|
}
|
|
|
|
type TaskPRUrlInput = {
|
|
pr_url?: string;
|
|
repository_id?: string;
|
|
};
|
|
|
|
/** Workspace repository id → display name for multi-repo PR/commit grouping. */
|
|
export function buildRepoNameById(
|
|
reposByWorkspace: Record<string, Array<{ id: string; name: string }>>,
|
|
): Record<string, string> {
|
|
const out: Record<string, string> = {};
|
|
for (const list of Object.values(reposByWorkspace)) {
|
|
for (const r of list) {
|
|
out[r.id] = r.name;
|
|
}
|
|
}
|
|
return out;
|
|
}
|
|
|
|
/** Merge synced TaskPR URLs and client-only pending PR URLs by repo display name. */
|
|
export function buildPrByRepoMap(
|
|
taskPRs: TaskPRUrlInput[] | undefined,
|
|
repoNameById: Record<string, string>,
|
|
pendingByRepo: Record<string, string> | undefined,
|
|
): Record<string, string | undefined> {
|
|
const map: Record<string, string | undefined> = {};
|
|
if (taskPRs) {
|
|
for (const pr of taskPRs) {
|
|
if (!pr.pr_url) continue;
|
|
const repoKey = pr.repository_id ? (repoNameById[pr.repository_id] ?? "") : "";
|
|
map[repoKey] = map[repoKey] ?? pr.pr_url;
|
|
}
|
|
}
|
|
if (pendingByRepo) {
|
|
for (const [repoKey, url] of Object.entries(pendingByRepo)) {
|
|
if (url) map[repoKey] = map[repoKey] ?? url;
|
|
}
|
|
}
|
|
const legacySingleRepo = taskPRs?.find((p) => p.pr_url && !p.repository_id);
|
|
if (!map[""] && legacySingleRepo?.pr_url) {
|
|
map[""] = legacySingleRepo.pr_url;
|
|
}
|
|
return map;
|
|
}
|