refactor(web): drop redundant VCS split button from task top bar (#804)

* refactor(web): drop redundant VCS split button from task top bar All git operations (commit, push, pull, rebase, merge, create PR, force push) are reachable from the Changes panel — its timeline exposes stage/commit and the per-section action row provides push/force-push/create-PR, while the panel header carries the pull/rebase/merge dropdown. The session command palette mirrors the same set for keyboard users. The expanded review dialog keeps its own VcsSplitButton because the changes timeline is hidden behind the modal there. Also removes the now-orphaned 'required' overflow flag (the VCS item was its only caller) and the e2e helper + spec that targeted the removed primary-action button. * test(e2e): drop task-topbar-overflow spec relying on removed VCS button The 1100px viewport test forced overflow by counting on the VCS button's width; with VcsSplitButton gone the scenario no longer holds. Overflow priority logic is already covered by topbar-action-overflow.test.ts. Co-authored-by: João Salavisa <jcfs@users.noreply.github.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: João Salavisa <jcfs@users.noreply.github.com>
2026-05-04 00:53:21 +01:00
parent 2732c101d9
commit a77df053a6
6 changed files with 5 additions and 409 deletions
@@ -26,7 +26,6 @@ import { EditorsMenu } from "@/components/task/editors-menu";
 import { BranchPathPopover } from "@/components/task/branch-path-popover";
 import { LayoutPresetSelector } from "@/components/task/layout-preset-selector";
 import { DocumentControls } from "@/components/task/document/document-controls";
-import { VcsSplitButton } from "@/components/vcs-split-button";
 import { PRTopbarButton } from "@/components/github/pr-topbar-button";
 import { JiraTicketButton, extractJiraKey } from "@/components/jira/jira-ticket-button";
 import { JiraLinkButton } from "@/components/jira/jira-link-button";
@@ -179,7 +178,6 @@ const TaskTopBar = memo(function TaskTopBar({
      <TopBarRight
        taskId={taskId}
        activeSessionId={activeSessionId}
-        baseBranch={baseBranch}
        showDebugOverlay={showDebugOverlay}
        onToggleDebugOverlay={onToggleDebugOverlay}
        isArchived={isArchived}
@@ -487,11 +485,10 @@ function TopbarToolsGroup({
  );
 }

-/** Right section: primary VCS action, status/attention, tools menu */
+/** Right section: status/attention, tools menu */
 function TopBarRight({
  taskId,
  activeSessionId,
-  baseBranch,
  showDebugOverlay,
  onToggleDebugOverlay,
  isArchived,
@@ -502,7 +499,6 @@ function TopBarRight({
 }: {
  taskId?: string | null;
  activeSessionId?: string | null;
-  baseBranch?: string;
  showDebugOverlay?: boolean;
  onToggleDebugOverlay?: () => void;
  isArchived?: boolean;
@@ -526,24 +522,6 @@ function TopBarRight({
    });
  }

-  if (!isArchived) {
-    items.push({
-      id: "vcs",
-      label: "Primary version control action",
-      priority: 100,
-      required: true,
-      content: (
-        <TopbarCluster label="Primary version control action">
-          <VcsSplitButton
-            sessionId={activeSessionId ?? null}
-            baseBranch={baseBranch}
-            buttonSize="lg"
-          />
-        </TopbarCluster>
-      ),
-    });
-  }
-
  items.push({
    id: "attention",
    label: "Task status and attention",
@@ -2,20 +2,17 @@ import { describe, expect, it } from "vitest";
 import { getHiddenTopbarActionIds } from "./topbar-action-overflow";

 const QUICK_CHAT = "quick-chat";
-const VCS = "vcs";
 const ATTENTION = "attention";
 const TOOLS = "tools";

 const items = [
  { id: QUICK_CHAT, priority: 20 },
-  { id: VCS, priority: 100, required: true },
  { id: ATTENTION, priority: 80 },
  { id: TOOLS, priority: 10 },
 ];

 const widths = new Map([
  [QUICK_CHAT, 88],
-  [VCS, 180],
  [ATTENTION, 160],
  [TOOLS, 120],
 ]);
@@ -37,7 +34,7 @@ describe("getHiddenTopbarActionIds", () => {
    expect(
      getHiddenTopbarActionIds({
        items,
-        availableWidth: 500,
+        availableWidth: 320,
        itemWidths: widths,
        gap: 8,
        overflowTriggerWidth: 40,
@@ -49,23 +46,11 @@ describe("getHiddenTopbarActionIds", () => {
    expect(
      getHiddenTopbarActionIds({
        items,
-        availableWidth: 410,
+        availableWidth: 230,
        itemWidths: widths,
        gap: 8,
        overflowTriggerWidth: 40,
      }),
    ).toEqual([QUICK_CHAT, TOOLS]);
  });
-
-  it("keeps required actions inline even when all secondary actions overflow", () => {
-    expect(
-      getHiddenTopbarActionIds({
-        items,
-        availableWidth: 230,
-        itemWidths: widths,
-        gap: 8,
-        overflowTriggerWidth: 40,
-      }),
-    ).toEqual([QUICK_CHAT, ATTENTION, TOOLS]);
-  });
 });
@@ -11,11 +11,10 @@ export type TopbarOverflowItem = {
  id: string;
  label: string;
  priority: number;
-  required?: boolean;
  content: ReactNode;
 };

-type TopbarOverflowMetricItem = Pick<TopbarOverflowItem, "id" | "priority" | "required">;
+type TopbarOverflowMetricItem = Pick<TopbarOverflowItem, "id" | "priority">;

 type HiddenTopbarActionArgs = {
  items: TopbarOverflowMetricItem[];
@@ -77,9 +76,7 @@ export function getHiddenTopbarActionIds({
  fallbackItemWidth = DEFAULT_FALLBACK_ITEM_WIDTH,
 }: HiddenTopbarActionArgs): string[] {
  const hiddenIds = new Set<string>();
-  const hideableItems = [...items]
-    .filter((item) => !item.required)
-    .sort((a, b) => a.priority - b.priority);
+  const hideableItems = [...items].sort((a, b) => a.priority - b.priority);

  while (
    totalActionWidth({
@@ -339,14 +339,6 @@ export class SessionPage {
    return this.page.getByTestId("pr-topbar-button");
  }

-  /** VCS split button primary action. Pass action to match a specific state. */
-  vcsPrimaryButton(action?: "commit" | "push" | "pr" | "rebase"): Locator {
-    if (action) {
-      return this.page.getByTestId(`vcs-primary-${action}`);
-    }
-    return this.page.locator('[data-testid^="vcs-primary-"]');
-  }
-
  /** PR detail panel (auto-shown when task has an associated PR). */
  prDetailPanel(): Locator {
    return this.page.getByTestId("pr-detail-panel");
@@ -1,277 +0,0 @@
-import { test, expect } from "../../fixtures/test-base";
-import type { ApiClient } from "../../helpers/api-client";
-import { makeGitEnv } from "../../helpers/git-helper";
-import { KanbanPage } from "../../pages/kanban-page";
-import { SessionPage } from "../../pages/session-page";
-import type { Page } from "@playwright/test";
-import fs from "node:fs";
-import path from "node:path";
-import { execSync } from "node:child_process";
-
-// ---------------------------------------------------------------------------
-// Helpers
-// ---------------------------------------------------------------------------
-
-class GitHelper {
-  constructor(
-    private repoDir: string,
-    private env: NodeJS.ProcessEnv,
-  ) {}
-
-  exec(cmd: string): string {
-    const lockPath = path.join(this.repoDir, ".git", "index.lock");
-    for (let attempt = 0; attempt < 3; attempt++) {
-      if (fs.existsSync(lockPath)) fs.unlinkSync(lockPath);
-      try {
-        return execSync(cmd, { cwd: this.repoDir, env: this.env, encoding: "utf8" });
-      } catch (err) {
-        const msg = (err as Error).message ?? "";
-        if (msg.includes("index.lock") && attempt < 2) {
-          execSync("sleep 0.2");
-          continue;
-        }
-        throw err;
-      }
-    }
-    throw new Error(`git exec failed after 3 attempts: ${cmd}`);
-  }
-
-  createFile(name: string, content: string) {
-    fs.writeFileSync(path.join(this.repoDir, name), content);
-  }
-
-  /** Set up a bare remote so the local branch can track ahead/behind. */
-  setupRemote(tmpDir: string): void {
-    const bareDir = path.join(tmpDir, "repos", "e2e-repo-bare.git");
-    execSync(`git clone --bare "${this.repoDir}" "${bareDir}"`, { env: this.env });
-    this.exec(`git remote add origin "${bareDir}"`);
-    this.exec("git fetch origin");
-    this.exec("git branch --set-upstream-to=origin/main main");
-  }
-
-  commitFile(name: string, content: string, message: string): void {
-    this.createFile(name, content);
-    this.exec("git add -A");
-    this.exec(`git commit -m "${message}"`);
-  }
-}
-
-async function createStandardProfile(apiClient: ApiClient, name: string) {
-  const { agents } = await apiClient.listAgents();
-  const agentId = agents[0]?.id;
-  if (!agentId) {
-    throw new Error(`E2E setup failed: no agent available for profile "${name}"`);
-  }
-  return apiClient.createAgentProfile(agentId, name, {
-    model: "mock-fast",
-    auto_approve: true,
-    cli_passthrough: false,
-  });
-}
-
-async function openTaskSession(page: Page, title: string): Promise<SessionPage> {
-  const kanban = new KanbanPage(page);
-  await kanban.goto();
-
-  const card = kanban.taskCardByTitle(title);
-  await expect(card).toBeVisible({ timeout: 15_000 });
-  await card.click();
-  await expect(page).toHaveURL(/\/t\//, { timeout: 15_000 });
-
-  const session = new SessionPage(page);
-  await session.waitForLoad();
-  return session;
-}
-
-// ---------------------------------------------------------------------------
-// Tests
-// ---------------------------------------------------------------------------
-
-test.describe("VCS split button context", () => {
-  test("shows Push when PR exists and commits are ahead", async ({
-    testPage,
-    apiClient,
-    seedData,
-    backend,
-  }) => {
-    test.setTimeout(120_000);
-
-    const profile = await createStandardProfile(apiClient, "VCS Push Profile");
-
-    const task = await apiClient.createTaskWithAgent(
-      seedData.workspaceId,
-      "VCS Push Button Test",
-      profile.id,
-      {
-        description: "/e2e:simple-message",
-        workflow_id: seedData.workflowId,
-        workflow_step_id: seedData.startStepId,
-        repository_ids: [seedData.repositoryId],
-      },
-    );
-
-    const session = await openTaskSession(testPage, "VCS Push Button Test");
-    await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
-
-    // Set up git with a remote so we can get "ahead" status
-    const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo");
-    const gitEnv = makeGitEnv(backend.tmpDir);
-    const git = new GitHelper(repoDir, gitEnv);
-    git.setupRemote(backend.tmpDir);
-
-    // Make a local commit → now 1 ahead of origin/main
-    git.commitFile("push-test.txt", "push content", "feat: push test commit");
-
-    // Before associating a PR, the button should show "Create PR"
-    // (ahead > 0, no PR)
-    await expect(session.vcsPrimaryButton("pr")).toBeVisible({ timeout: 30_000 });
-
-    // Now associate an open PR with the task
-    await apiClient.mockGitHubReset();
-    await apiClient.mockGitHubSetUser("test-user");
-    await apiClient.mockGitHubAssociateTaskPR({
-      task_id: task.id,
-      owner: "testorg",
-      repo: "testrepo",
-      pr_number: 50,
-      pr_url: "https://github.com/testorg/testrepo/pull/50",
-      pr_title: "Feature branch",
-      head_branch: "main",
-      base_branch: "develop",
-      author_login: "test-user",
-      additions: 10,
-      deletions: 2,
-    });
-
-    // Wait for the PR topbar button to appear (confirms PR is in store)
-    await expect(session.prTopbarButton()).toBeVisible({ timeout: 30_000 });
-
-    // The VCS button should now show "Push" instead of "Create PR"
-    await expect(session.vcsPrimaryButton("push")).toBeVisible({ timeout: 15_000 });
-    await expect(session.vcsPrimaryButton("push")).toContainText("Push");
-  });
-
-  test("shows Create PR when ahead but no PR exists", async ({
-    testPage,
-    apiClient,
-    seedData,
-    backend,
-  }) => {
-    test.setTimeout(120_000);
-
-    const profile = await createStandardProfile(apiClient, "VCS PR Profile");
-
-    await apiClient.createTaskWithAgent(seedData.workspaceId, "VCS PR Button Test", profile.id, {
-      description: "/e2e:simple-message",
-      workflow_id: seedData.workflowId,
-      workflow_step_id: seedData.startStepId,
-      repository_ids: [seedData.repositoryId],
-    });
-
-    const session = await openTaskSession(testPage, "VCS PR Button Test");
-    await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
-
-    // Set up git with a remote
-    const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo");
-    const gitEnv = makeGitEnv(backend.tmpDir);
-    const git = new GitHelper(repoDir, gitEnv);
-
-    // Only set up remote if not already configured
-    try {
-      git.exec("git remote get-url origin");
-    } catch {
-      git.setupRemote(backend.tmpDir);
-    }
-
-    // Push current state to origin, then make a new commit to be ahead
-    git.exec("git push origin main");
-    git.commitFile("pr-test.txt", "pr content", "feat: pr test commit");
-
-    // No PR associated → should show "Create PR"
-    await expect(session.vcsPrimaryButton("pr")).toBeVisible({ timeout: 30_000 });
-    await expect(session.vcsPrimaryButton("pr")).toContainText("Create PR");
-  });
-
-  test("shows Commit when there are uncommitted files even with open PR", async ({
-    testPage,
-    apiClient,
-    seedData,
-    backend,
-  }) => {
-    test.setTimeout(120_000);
-
-    const profile = await createStandardProfile(apiClient, "VCS Commit Profile");
-
-    const task = await apiClient.createTaskWithAgent(
-      seedData.workspaceId,
-      "VCS Commit Priority Test",
-      profile.id,
-      {
-        description: "/e2e:simple-message",
-        workflow_id: seedData.workflowId,
-        workflow_step_id: seedData.startStepId,
-        repository_ids: [seedData.repositoryId],
-      },
-    );
-
-    const session = await openTaskSession(testPage, "VCS Commit Priority Test");
-    await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
-
-    // Associate an open PR
-    await apiClient.mockGitHubReset();
-    await apiClient.mockGitHubSetUser("test-user");
-    await apiClient.mockGitHubAssociateTaskPR({
-      task_id: task.id,
-      owner: "testorg",
-      repo: "testrepo",
-      pr_number: 55,
-      pr_url: "https://github.com/testorg/testrepo/pull/55",
-      pr_title: "Commit Priority PR",
-      head_branch: "main",
-      base_branch: "develop",
-      author_login: "test-user",
-      additions: 5,
-      deletions: 1,
-    });
-
-    await expect(session.prTopbarButton()).toBeVisible({ timeout: 30_000 });
-
-    // Create an uncommitted file
-    const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo");
-    const git = new GitHelper(repoDir, makeGitEnv(backend.tmpDir));
-    git.createFile("uncommitted.txt", "uncommitted content");
-
-    // Even with open PR, uncommitted files → "Commit" takes priority
-    await expect(session.vcsPrimaryButton("commit")).toBeVisible({ timeout: 30_000 });
-    await expect(session.vcsPrimaryButton("commit")).toContainText("Commit");
-  });
-
-  test("defaults to Commit when no changes and no PR", async ({
-    testPage,
-    apiClient,
-    seedData,
-  }) => {
-    test.setTimeout(120_000);
-
-    const profile = await createStandardProfile(apiClient, "VCS Default Profile");
-
-    await apiClient.createTaskWithAgent(
-      seedData.workspaceId,
-      "VCS Default Button Test",
-      profile.id,
-      {
-        description: "/e2e:simple-message",
-        workflow_id: seedData.workflowId,
-        workflow_step_id: seedData.startStepId,
-        repository_ids: [seedData.repositoryId],
-      },
-    );
-
-    const session = await openTaskSession(testPage, "VCS Default Button Test");
-    await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
-
-    // No changes, no PR → default is "Commit"
-    await expect(session.vcsPrimaryButton("commit")).toBeVisible({ timeout: 15_000 });
-    await expect(session.vcsPrimaryButton("commit")).toContainText("Commit");
-  });
-});
@@ -1,79 +0,0 @@
-import { test, expect } from "../../fixtures/test-base";
-import type { Page } from "@playwright/test";
-
-type TopbarMetrics = {
-  workflowRight: number;
-  actionsLeft: number;
-  actionsRight: number;
-  headerRight: number;
-};
-
-async function readTopbarMetrics(page: Page) {
-  return page.evaluate<TopbarMetrics | null>(() => {
-    const header = document.querySelector('[data-testid="task-topbar"]');
-    const workflow = document.querySelector('[data-testid="workflow-stepper"]');
-    const actions = document.querySelector('[data-testid="topbar-action-overflow"]');
-    if (!header || !workflow || !actions) return null;
-
-    const headerRect = header.getBoundingClientRect();
-    const workflowRect = workflow.getBoundingClientRect();
-    const actionsRect = actions.getBoundingClientRect();
-
-    return {
-      workflowRight: workflowRect.right,
-      actionsLeft: actionsRect.left,
-      actionsRight: actionsRect.right,
-      headerRight: headerRect.right,
-    };
-  });
-}
-
-test.describe("Task topbar overflow", () => {
-  test("keeps contextual PR actions inline before quick chat overflows", async ({
-    testPage,
-    apiClient,
-    seedData,
-  }) => {
-    await testPage.setViewportSize({ width: 1100, height: 720 });
-
-    const task = await apiClient.createTask(
-      seedData.workspaceId,
-      "Responsive topbar PR review task",
-      {
-        workflow_id: seedData.workflowId,
-        workflow_step_id: seedData.startStepId,
-        repository_ids: [seedData.repositoryId],
-      },
-    );
-
-    await apiClient.mockGitHubAssociateTaskPR({
-      task_id: task.id,
-      owner: "testorg",
-      repo: "testrepo",
-      pr_number: 761,
-      pr_url: "https://github.com/testorg/testrepo/pull/761",
-      pr_title: "Responsive topbar overflow",
-      head_branch: "feature/topbar-overflow",
-      base_branch: "main",
-      author_login: "test-user",
-      additions: 23,
-      deletions: 3,
-    });
-
-    await testPage.goto(`/t/${task.id}`);
-
-    await expect(testPage.getByTestId("task-topbar")).toBeVisible({ timeout: 10_000 });
-    await expect(testPage.getByTestId("pr-topbar-button")).toBeVisible({ timeout: 15_000 });
-    await expect(testPage.getByTestId("topbar-action-overflow-trigger")).toBeVisible({
-      timeout: 10_000,
-    });
-
-    const metrics = await readTopbarMetrics(testPage);
-    expect(metrics).not.toBeNull();
-    expect(metrics!.workflowRight).toBeLessThanOrEqual(metrics!.actionsLeft + 1);
-    expect(metrics!.actionsRight).toBeLessThanOrEqual(metrics!.headerRight + 1);
-
-    await testPage.getByTestId("topbar-action-overflow-trigger").click();
-    await expect(testPage.getByRole("button", { name: "Chat" })).toBeVisible();
-  });
-});