Files
kandev/apps/web/components/task-create-dialog-helpers.ts
T
João Salavisa 6ceda6ae4e fix(local-executor): preserve commits in changes panel after refresh (#834)
* fix(local-executor): preserve commits in changes panel after refresh

For a local executor on a feature branch, the create-task dialog sent
the chip's branch as base_branch with empty checkout_branch. agentctl
then recomputed merge-base(HEAD, origin/<branch>) on every changes-panel
refresh; for a never-pushed branch that collapsed to HEAD and the panel
went empty. Repository creation also pinned default_branch to .git/HEAD,
permanently anchoring the repo's integration ref to whatever feature
branch the user happened to be on at first task-create time.

The frontend now splits the chip into base_branch=default_branch +
checkout_branch=working_branch (skipping the split when fresh-branch is
on, where the chip semantically means "fork from"). The backend probes
origin/HEAD then origin/main → main → master (new common/gitref
package) to pick a repo's integration branch, and resolveRepoInput no
longer falls back to BaseBranch when default_branch is missing.

Covered by 9 unit tests on the helper, 6 Go tests on the probe order,
and 7 e2e tests across plain creation, the fork toggle, and the
validate-path probe.

* fix: address PR review feedback

- gitref: preserve slashes in HEAD-fallback branch names (TrimPrefix
  refs/heads/ instead of Split + last component); branches like
  "feature/my-feature" no longer collapse to "my-feature" and poison
  the stored default_branch.
- gitref: drop the broken packed-refs symref scan in readOriginHEAD;
  the loose refs/remotes/origin/HEAD check + pickFirstExistingBranch
  candidates already cover every realistic clone.
- service.resolveRepoInput: validate repoInput.LocalPath via
  resolveAllowedLocalPath() before probing — closes 8 CodeQL
  path-injection alerts.
- buildRepositoriesPayload: fall through to the legacy non-split shape
  when default_branch is unknown for the row (legacy repos with unset
  default_branch). Synthesizing base_branch=rowBranch was reproducing
  the very bug this PR fixes.

* fix(gitref): explicit traversal guard at I/O boundary for CodeQL

Move the path validation from service.resolveAllowedLocalPath (which
CodeQL's taint analysis doesn't recognize as a sanitizer) into a
guardRepoPath check at gitref.DefaultBranch's entry. The new check
rejects relative paths and any '..' segments before any os.Stat /
os.ReadFile call sees the value, which CodeQL recognizes as a
go/path-injection sanitizer.

The upstream allowlist check stays in place — this is a belt-and-
suspenders second barrier at the actual I/O boundary, and it also
makes the gitref package safe for any future callers (migrations,
tests) without requiring them to thread a Service in.

* fix: address review feedback on hardening + e2e race

- gitref.guardRepoPath: check the RAW input for '..' segments before
  filepath.Clean. Clean resolves '..' away from absolute paths
  ("/allowed/../etc" -> "/etc"), so iterating the cleaned path was a
  silent no-op for the very inputs we wanted to reject. Split on both
  '/' and the OS separator. Updated the test with three traversal
  cases.
- gitref_test.go: switch from strings.EqualFold to strict equality —
  git branch names are case-sensitive, "Main" should not pass for
  "main".
- e2e: replace waitForRequest with waitForResponse in
  local-executor-branch-split.spec.ts. waitForRequest resolves on send
  (before the server has persisted the task), so the followup
  apiClient.getTask call could race the write on slow CI. The two
  tests that need the request body keep access via
  (await waitForResponse).request().

* fix(ws): forward CheckoutBranch in wsUpdateTask

The PR fix added CheckoutBranch forwarding to wsCreateTask but missed
the same conversion loop in wsUpdateTask. Any task-update payload with
a repositories array would silently zero out checkout_branch and
re-introduce the merge-base collapse on refresh — same root cause this
PR fixes, in the update path.

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
2026-05-05 22:05:01 +01:00

288 lines
10 KiB
TypeScript

import type { useRouter } from "next/navigation";
import type { Task, Branch, LocalRepository, Repository } from "@/lib/types/http";
import type { AgentProfileOption } from "@/lib/state/slices";
import type { AppState } from "@/lib/state/store";
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";
import { useContextFilesStore } from "@/lib/state/context-files-store";
import { linkToTask } from "@/lib/links";
import { INTENT_PLAN } from "@/lib/state/layout-manager";
import { createTask } from "@/lib/api";
import type { FileAttachment } from "@/components/task/chat/file-attachment";
import type { MessageAttachment } from "@/lib/services/session-launch-service";
type CreateTaskParams = Parameters<typeof createTask>[0];
export type { CreateTaskParams };
/** Converts FileAttachment array to MessageAttachment array for the launch request. */
export function toMessageAttachments(
attachments: FileAttachment[],
): MessageAttachment[] | undefined {
if (attachments.length === 0) return undefined;
return attachments.map((att) =>
att.isImage
? { type: "image" as const, data: att.data, mime_type: att.mimeType }
: {
type: "resource" as const,
data: att.data,
mime_type: att.mimeType,
name: att.fileName,
},
);
}
export function autoSelectBranch(branchList: Branch[], setBranch: (value: string) => void): void {
const lastUsedBranch = getLocalStorage<string | null>(STORAGE_KEYS.LAST_BRANCH, null);
if (
lastUsedBranch &&
branchList.some((b) => {
const displayName = b.type === "remote" && b.remote ? `${b.remote}/${b.name}` : b.name;
return displayName === lastUsedBranch;
})
) {
setBranch(lastUsedBranch);
return;
}
const preferredBranch = selectPreferredBranch(branchList);
if (preferredBranch) setBranch(preferredBranch);
}
export function computePassthroughProfile(
agentProfileId: string,
agentProfiles: AgentProfileOption[],
) {
if (!agentProfileId) return false;
return (
agentProfiles.find((p: AgentProfileOption) => p.id === agentProfileId)?.cli_passthrough === true
);
}
export function computeEffectiveStepId(
selectedWorkflowId: string | null,
workflowId: string | null,
fetchedSteps: StepType[] | null,
defaultStepId: string | null,
) {
return selectedWorkflowId && selectedWorkflowId !== workflowId && fetchedSteps
? (fetchedSteps[0]?.id ?? null)
: defaultStepId;
}
export function computeIsTaskStarted(
isEditMode: boolean,
editingTask?: { state?: Task["state"] } | null,
) {
if (!isEditMode || !editingTask?.state) return false;
return editingTask.state !== "TODO" && editingTask.state !== "CREATED";
}
export type ActivatePlanModeArgs = {
sessionId: string;
taskId: string;
setActiveDocument: AppState["setActiveDocument"];
setPlanMode: AppState["setPlanMode"];
router: ReturnType<typeof useRouter>;
};
export function activatePlanMode({
sessionId,
taskId,
setActiveDocument,
setPlanMode,
router,
}: ActivatePlanModeArgs) {
setActiveDocument(sessionId, { type: "plan", taskId });
setPlanMode(sessionId, true);
useContextFilesStore.getState().addFile(sessionId, { path: "plan:context", name: "Plan" });
router.push(linkToTask(taskId, INTENT_PLAN));
}
export type BuildCreatePayloadArgs = {
workspaceId: string;
effectiveWorkflowId: string;
trimmedTitle: string;
trimmedDescription: string;
repositoriesPayload: CreateTaskParams["repositories"];
agentProfileId: string;
executorId: string;
executorProfileId: string;
withAgent: boolean;
planMode?: boolean;
attachments?: MessageAttachment[];
parentId?: string;
};
export function buildCreateTaskPayload(args: BuildCreatePayloadArgs): CreateTaskParams {
return {
workspace_id: args.workspaceId,
workflow_id: args.effectiveWorkflowId,
title: args.trimmedTitle,
description: args.trimmedDescription,
repositories: args.repositoriesPayload,
state: args.withAgent ? "IN_PROGRESS" : "CREATED",
start_agent: args.withAgent ? true : undefined,
prepare_session: args.withAgent ? undefined : true,
agent_profile_id: args.agentProfileId || undefined,
executor_id: args.executorId || undefined,
executor_profile_id: args.executorProfileId || undefined,
plan_mode: args.planMode || undefined,
attachments: args.attachments,
parent_id: args.parentId || undefined,
};
}
export function validateCreateInputs(inputs: {
trimmedTitle: string;
workspaceId: string | null;
effectiveWorkflowId: string | 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 &&
hasRepo,
);
}
/**
* 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 function buildRepositoriesPayload(opts: {
useGitHubUrl: boolean;
githubUrl: string;
githubBranch: string;
githubPrHeadBranch: string | null;
repositories: TaskRepoRow[];
/** Used to look up `default_branch` for `localPath` rows. */
discoveredRepositories: LocalRepository[];
/** Workspace repositories — used to look up `default_branch` for `repositoryId` rows. */
workspaceRepositories?: Repository[];
/**
* For the local executor (no worktree), the chip's branch field represents
* the working branch on disk, not the parent integration branch. Send it as
* `checkout_branch` so the session's `base_branch` stays anchored to the
* repo's `default_branch` (which is what git log / cumulative diff use as
* the merge-base reference). Without this, "new branch on local executor"
* collapses the merge-base recomputation to HEAD and the changes panel
* goes empty after a refresh.
*/
isLocalExecutor?: boolean;
/**
* Optional fresh-branch metadata. The UI gates this to single-row + local
* executor; when present we apply it to every row (which is at most one).
*/
freshBranch?: { confirmDiscard: boolean; consentedDirtyFiles: string[] };
}): NonNullable<CreateTaskParams["repositories"]> {
if (opts.useGitHubUrl && opts.githubUrl) {
return [
{
repository_id: "",
base_branch: opts.githubBranch || undefined,
checkout_branch: opts.githubPrHeadBranch || undefined,
github_url: opts.githubUrl.trim(),
},
];
}
const fresh = opts.freshBranch
? {
fresh_branch: true,
confirm_discard: opts.freshBranch.confirmDiscard,
consented_dirty_files: opts.freshBranch.consentedDirtyFiles,
}
: {};
// Fresh-branch flow inverts the chip's semantics: instead of "the working
// branch I'm on", row.branch becomes "the base I want to fork from". The
// backend then creates a new branch and rewrites repos[i].BaseBranch to it.
// Splitting here would force `base_branch=default_branch, checkout_branch=
// <picked-base>` and the backend would fork from the wrong base. Skip the
// split entirely when fresh-branch is active.
const isLocalExecutor = !!opts.isLocalExecutor && !opts.freshBranch;
return opts.repositories
.filter((row) => row.repositoryId || row.localPath)
.map((row) => {
const defaultBranch = resolveRowDefaultBranch(row, opts);
const branches = splitLocalExecutorBranches({
rowBranch: row.branch,
defaultBranch,
isLocalExecutor,
});
if (row.repositoryId) {
return {
repository_id: row.repositoryId,
base_branch: branches.base_branch,
checkout_branch: branches.checkout_branch,
...fresh,
};
}
return {
repository_id: "",
base_branch: branches.base_branch,
checkout_branch: branches.checkout_branch,
local_path: row.localPath,
default_branch: defaultBranch || undefined,
...fresh,
};
});
}
function resolveRowDefaultBranch(
row: TaskRepoRow,
opts: {
discoveredRepositories: LocalRepository[];
workspaceRepositories?: Repository[];
},
): string | undefined {
if (row.repositoryId) {
return opts.workspaceRepositories?.find((r) => r.id === row.repositoryId)?.default_branch;
}
return opts.discoveredRepositories.find((d) => d.path === row.localPath)?.default_branch;
}
/**
* For the local executor: return `base_branch=defaultBranch`,
* `checkout_branch=rowBranch` so the session anchors merge-base to the repo's
* integration branch while the preparer still checks out the user's working
* branch. When `rowBranch === defaultBranch` we omit checkout_branch — the
* preparer treats matching values as "use current state" and skips git ops.
*
* For non-local executors (worktree-based): keep the historical shape where
* `base_branch=rowBranch` (the worktree creates a new branch off of it).
*/
function splitLocalExecutorBranches(args: {
rowBranch?: string;
defaultBranch?: string;
isLocalExecutor: boolean;
}): { base_branch: string | undefined; checkout_branch: string | undefined } {
// Without a known default_branch we can't anchor base_branch to the
// integration ref — and using rowBranch as a stand-in reproduces the
// exact bug this PR fixes (changes panel collapses to HEAD on refresh).
// Fall through to the legacy non-split shape: a workspace repo with an
// unset default_branch is no worse off than before, and the backend's
// resolveRepoInput probe will populate it on the next CreateRepository
// call. Wait for that probe rather than synthesizing a guess here.
if (!args.isLocalExecutor || !args.defaultBranch) {
return { base_branch: args.rowBranch || undefined, checkout_branch: undefined };
}
const base = args.defaultBranch;
const checkout =
args.rowBranch && args.rowBranch !== args.defaultBranch ? args.rowBranch : undefined;
return { base_branch: base, checkout_branch: checkout };
}