diff --git a/apps/web/components/task/task-top-bar.tsx b/apps/web/components/task/task-top-bar.tsx index a830e8aa17..d5d627039d 100644 --- a/apps/web/components/task/task-top-bar.tsx +++ b/apps/web/components/task/task-top-bar.tsx @@ -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({ 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: ( - - - - ), - }); - } - items.push({ id: "attention", label: "Task status and attention", diff --git a/apps/web/components/task/topbar-action-overflow.test.ts b/apps/web/components/task/topbar-action-overflow.test.ts index 16fe17f073..d0436853a4 100644 --- a/apps/web/components/task/topbar-action-overflow.test.ts +++ b/apps/web/components/task/topbar-action-overflow.test.ts @@ -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]); - }); }); diff --git a/apps/web/components/task/topbar-action-overflow.tsx b/apps/web/components/task/topbar-action-overflow.tsx index bba16a59c6..52de62e6eb 100644 --- a/apps/web/components/task/topbar-action-overflow.tsx +++ b/apps/web/components/task/topbar-action-overflow.tsx @@ -11,11 +11,10 @@ export type TopbarOverflowItem = { id: string; label: string; priority: number; - required?: boolean; content: ReactNode; }; -type TopbarOverflowMetricItem = Pick; +type TopbarOverflowMetricItem = Pick; type HiddenTopbarActionArgs = { items: TopbarOverflowMetricItem[]; @@ -77,9 +76,7 @@ export function getHiddenTopbarActionIds({ fallbackItemWidth = DEFAULT_FALLBACK_ITEM_WIDTH, }: HiddenTopbarActionArgs): string[] { const hiddenIds = new Set(); - 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({ diff --git a/apps/web/e2e/pages/session-page.ts b/apps/web/e2e/pages/session-page.ts index f5ff20cba7..5af7bffaec 100644 --- a/apps/web/e2e/pages/session-page.ts +++ b/apps/web/e2e/pages/session-page.ts @@ -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"); diff --git a/apps/web/e2e/tests/pr/vcs-button-context.spec.ts b/apps/web/e2e/tests/pr/vcs-button-context.spec.ts deleted file mode 100644 index c8a05a52fa..0000000000 --- a/apps/web/e2e/tests/pr/vcs-button-context.spec.ts +++ /dev/null @@ -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 { - 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"); - }); -}); diff --git a/apps/web/e2e/tests/task/task-topbar-overflow.spec.ts b/apps/web/e2e/tests/task/task-topbar-overflow.spec.ts deleted file mode 100644 index ea533bd410..0000000000 --- a/apps/web/e2e/tests/task/task-topbar-overflow.spec.ts +++ /dev/null @@ -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(() => { - 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(); - }); -});