From 492be9026b70611637bd9502cdd705cb282c85bf Mon Sep 17 00:00:00 2001 From: Saoud Rizwan <7799382+saoudrizwan@users.noreply.github.com> Date: Fri, 16 Oct 2026 00:31:59 +0000 Subject: [PATCH] desktop: leave commit, push, and pull to the agent Remove the commit message box and the Commit, Push, and Pull buttons from the Git view, and the matching sidecar actions, since people usually ask the agent to do these. The branch line (with ahead/behind counts), staging, discard, history, and the background fetch stay. Co-authored-by: Saoud Rizwan --- .../sidecar/source-control.test.ts | 47 ++----- .../desktop-app/sidecar/source-control.ts | 49 +------- .../views/chat/project-files-panel.test.tsx | 113 ++++------------- .../views/chat/project-files-panel.tsx | 7 +- .../views/chat/source-control-column.tsx | 118 +----------------- .../webview/hooks/use-source-control.ts | 4 +- 6 files changed, 43 insertions(+), 295 deletions(-) diff --git a/apps/examples/desktop-app/sidecar/source-control.test.ts b/apps/examples/desktop-app/sidecar/source-control.test.ts index ac80c737d1..8216339a6e 100644 --- a/apps/examples/desktop-app/sidecar/source-control.test.ts +++ b/apps/examples/desktop-app/sidecar/source-control.test.ts @@ -165,16 +165,10 @@ describe("parseSourceControlAction", () => { type: "stage", paths: ["a"], }); - expect( - parseSourceControlAction({ type: "commit", message: "m", push: true }), - ).toEqual({ - type: "commit", - message: "m", - push: true, - }); - expect(parseSourceControlAction({ type: "pull" })).toEqual({ - type: "pull", - }); + // Committing, pushing, and pulling are left to the agent. + for (const type of ["commit", "push", "pull"]) { + expect(() => parseSourceControlAction({ type })).toThrow(/Unknown/); + } expect(parseSourceControlAction({ type: "fetch" })).toEqual({ type: "fetch", }); @@ -383,7 +377,7 @@ describe("against a real repository", () => { } }); - it("pushes to and pulls from an upstream", async () => { + it("fetch surfaces commits pushed elsewhere without touching the tree", async () => { const remote = mkdtempSync(join(tmpdir(), "cline-remote-")); const other = mkdtempSync(join(tmpdir(), "cline-clone-")); const run = (cwd: string) => async (args: string[]) => @@ -398,13 +392,12 @@ describe("against a real repository", () => { remote, ]); await git(["remote", "add", "origin", remote]); - // First push publishes the branch (no upstream yet). - await runSourceControlAction(git, { type: "push" }); + await git(["push", "-q", "-u", "origin", "HEAD"]); let state = await getSourceControlState(git, "local"); expect(state.hasUpstream).toBe(true); expect(state.commits[0]?.pushed).toBe(true); - // Someone else lands a commit; pull brings it in. + // Someone else lands a commit; only a fetch can reveal it. await execFileAsync("git", ["clone", "-q", remote, other]); const otherGit = run(other); await otherGit(["config", "user.email", "o@example.com"]); @@ -417,12 +410,8 @@ describe("against a real repository", () => { await runSourceControlAction(git, { type: "fetch" }); state = await getSourceControlState(git, "local"); expect(state.behind).toBe(1); - // Park the local edits so the pull is a clean fast-forward. - await git(["stash", "-q", "--include-untracked"]); - await runSourceControlAction(git, { type: "pull" }); - state = await getSourceControlState(git, "local"); - expect(state.behind).toBe(0); - expect(state.commits[0]?.subject).toBe("remote work"); + // Fetching never touches the working tree. + expect(existsSync(join(root, "remote.txt"))).toBe(false); } finally { rmSync(remote, { recursive: true, force: true }); rmSync(other, { recursive: true, force: true }); @@ -455,7 +444,7 @@ describe("against a real repository", () => { expect(added.newText).toBe("one\ntwo\n"); }); - it("stages, unstages, discards, and commits", async () => { + it("stages, unstages, and discards", async () => { await runSourceControlAction(git, { type: "stage", paths: ["a.ts", "new.txt"], @@ -478,21 +467,5 @@ describe("against a real repository", () => { untrackedPaths: [], }); expect(readFileSync(join(root, "a.ts"), "utf8")).toBe("one\ntwo\n"); - - await runSourceControlAction(git, { - type: "commit", - message: "add staged work", - push: false, - }); - state = await getSourceControlState(git, "local"); - expect(state.staged).toEqual([]); - expect(state.commits[0]?.subject).toBe("add staged work"); - await expect( - runSourceControlAction(git, { - type: "commit", - message: " ", - push: false, - }), - ).rejects.toThrow(/message/); }); }); diff --git a/apps/examples/desktop-app/sidecar/source-control.ts b/apps/examples/desktop-app/sidecar/source-control.ts index 9cc80c309c..33689026a4 100644 --- a/apps/examples/desktop-app/sidecar/source-control.ts +++ b/apps/examples/desktop-app/sidecar/source-control.ts @@ -1,6 +1,7 @@ // Source control backing for the desktop app's workspace panel: one call // returns everything the Source Control column renders, plus the small set -// of mutations it offers (stage, unstage, discard, commit, push). +// of mutations it offers (stage, unstage, discard) plus a background fetch. +// Committing, pushing, and pulling are left to the agent. /** Runs git in the repository and resolves stdout; rejects on non-zero exit. */ export type GitRunner = (args: string[]) => Promise; @@ -485,9 +486,6 @@ export type SourceControlAction = | { type: "stage"; paths: string[] } | { type: "unstage"; paths: string[] } | { type: "discard"; paths: string[]; untrackedPaths: string[] } - | { type: "commit"; message: string; push: boolean } - | { type: "push" } - | { type: "pull" } /** Background refresh of remote-tracking refs; never touches the work tree. */ | { type: "fetch" }; @@ -515,33 +513,13 @@ export function parseSourceControlAction(value: unknown): SourceControlAction { paths: stringList(record.paths), untrackedPaths: stringList(record.untrackedPaths), }; - case "commit": - return { - type: "commit", - message: typeof record.message === "string" ? record.message : "", - push: record.push === true, - }; - case "push": - case "pull": case "fetch": - return { type: record.type }; + return { type: "fetch" }; default: throw new Error("Unknown source control action"); } } -async function hasUpstream(git: GitRunner): Promise { - return git([ - "rev-parse", - "--abbrev-ref", - "--symbolic-full-name", - "@{u}", - ]).then( - () => true, - () => false, - ); -} - async function hasHead(git: GitRunner): Promise { return git(["rev-parse", "--verify", "--quiet", "HEAD"]).then( () => true, @@ -549,12 +527,6 @@ async function hasHead(git: GitRunner): Promise { ); } -async function push(git: GitRunner): Promise { - // A branch without an upstream is published rather than failing. - if (await hasUpstream(git)) await git(["push"]); - else await git(["push", "-u", "origin", "HEAD"]); -} - // Filenames are data, never patterns: without this, `draft*.txt` would also // match (and discard) every other file the glob covers. const LITERAL = "--literal-pathspecs"; @@ -595,21 +567,6 @@ export async function runSourceControlAction( await git([LITERAL, "clean", "-f", "--", ...action.untrackedPaths]); } return; - case "commit": { - const message = action.message.trim(); - if (!message) throw new Error("A commit message is required."); - await git(["commit", "-m", message]); - if (action.push) await push(git); - return; - } - case "push": - await push(git); - return; - case "pull": - // Non-interactive: git skips the merge editor without a TTY, and - // conflicts surface as the error message in the UI. - await git(["pull"]); - return; case "fetch": await git(["fetch", "--quiet"]); return; diff --git a/apps/examples/desktop-app/webview/components/views/chat/project-files-panel.test.tsx b/apps/examples/desktop-app/webview/components/views/chat/project-files-panel.test.tsx index b7fe9b3292..12f0dfd2e4 100644 --- a/apps/examples/desktop-app/webview/components/views/chat/project-files-panel.test.tsx +++ b/apps/examples/desktop-app/webview/components/views/chat/project-files-panel.test.tsx @@ -209,16 +209,18 @@ describe("ProjectFilesPanel source control view", () => { expect(text).toContain("README.md"); expect(text).toContain("notes.txt"); expect(text).toContain("Latest work"); - // One commit ahead: Push is enabled with a count, Pull is available. - const push = byLabel("Push", container) as HTMLButtonElement; - expect(push.disabled).toBe(false); - expect(push.textContent).toContain("1"); - expect((byLabel("Pull", container) as HTMLButtonElement).disabled).toBe( - false, - ); - await click(byLabel("Pull", container)); - expect(actionCalls()).toContainEqual({ type: "pull" }); - expect(container.textContent).toContain("main"); + // Committing, pushing, and pulling are left to the agent; the branch + // line still reports how far the branch is ahead. + expect(container.querySelector('[aria-label="Commit message"]')).toBeNull(); + for (const label of ["Push", "Pull", "Publish branch"]) { + expect(container.querySelector(`[aria-label="${label}"]`)).toBeNull(); + } + expect( + Array.from(column?.querySelectorAll("button") ?? []).some((el) => + /^Commit$/.test(el.textContent ?? ""), + ), + ).toBe(false); + expect(text).toMatch(/main\s*1/); // Session-touched rows carry the marker. expect( column?.querySelector('[title="Changed in this session"]'), @@ -249,7 +251,7 @@ describe("ProjectFilesPanel source control view", () => { ).toContain("/repo/README.md"); }); - it("stages, unstages, and commits through the sidecar", async () => { + it("stages and unstages through the sidecar", async () => { await render(); await click(byLabel("Stage README.md")); expect(actionCalls()).toContainEqual({ @@ -261,26 +263,6 @@ describe("ProjectFilesPanel source control view", () => { type: "unstage", paths: ["src/app.ts"], }); - - const message = byLabel("Commit message") as HTMLTextAreaElement; - const commit = button(/^Commit$/); - expect(commit.disabled).toBe(true); - await act(async () => { - const setter = Object.getOwnPropertyDescriptor( - HTMLTextAreaElement.prototype, - "value", - )?.set; - setter?.call(message, "feat: panel"); - message.dispatchEvent(new Event("input", { bubbles: true })); - }); - expect(button(/^Commit$/).disabled).toBe(false); - await click(button(/^Commit$/)); - expect(actionCalls()).toContainEqual({ - type: "commit", - message: "feat: panel", - push: false, - }); - expect((byLabel("Commit message") as HTMLTextAreaElement).value).toBe(""); }); it("opens the first change when the panel opens, once", async () => { @@ -344,63 +326,26 @@ describe("ProjectFilesPanel source control view", () => { ).toHaveLength(1); }); - it("keeps the commit message when the commit fails", async () => { - await render(); - const message = byLabel("Commit message") as HTMLTextAreaElement; - await act(async () => { - Object.getOwnPropertyDescriptor( - HTMLTextAreaElement.prototype, - "value", - )?.set?.call(message, "wip"); - message.dispatchEvent(new Event("input", { bubbles: true })); - }); - invokeMock.mockImplementationOnce(async (command: string) => { - if (command === "run_source_control_action") { - throw new Error("pre-commit hook failed"); - } - return null; - }); - await click(button(/^Commit$/)); - expect((byLabel("Commit message") as HTMLTextAreaElement).value).toBe( - "wip", - ); - }); - - it("publishes a branch that has no upstream and disables Pull", async () => { - const previous = { ...sourceControlState }; - Object.assign(sourceControlState, { hasUpstream: false, ahead: 0 }); - try { - await render(); - expect((byLabel("Pull", container) as HTMLButtonElement).disabled).toBe( - true, - ); - await click(byLabel("Publish branch", container)); - expect(actionCalls()).toContainEqual({ type: "push" }); - } finally { - Object.assign(sourceControlState, previous); - } - }); - - it("reloads the folders expanded by the time a pull finishes", async () => { + it("reloads the folders expanded by the time an action finishes", async () => { await render(); const listings = () => invokeMock.mock.calls.filter( ([command]) => command === "list_project_entries", ).length; - // Hold the pull open and expand a folder while it runs. - let finishPull: (value: unknown) => void = () => {}; + // Hold a stage open and expand a folder while it runs. + let finishAction: (value: unknown) => void = () => {}; invokeMock.mockImplementationOnce( () => new Promise((resolve) => { - finishPull = resolve; + finishAction = resolve; }), ); - await click(byLabel("Pull", container)); + await click(byLabel("Stage README.md", container)); await click(button(/^Files$/)); await click(button(/^src/)); const before = listings(); await act(async () => { - finishPull({ environmentId: "local" }); + finishAction({ environmentId: "local" }); await Promise.resolve(); }); // Root and the newly expanded src both list again, and src keeps @@ -449,13 +394,16 @@ describe("ProjectFilesPanel source control view", () => { try { await render(); expect(fetchCount()).toBe(1); - await click(byLabel("Pull", container)); + await click(byLabel("Stage README.md", container)); expect(actionCalls()).toEqual([]); await act(async () => { finishFetch({ environmentId: "local" }); await Promise.resolve(); }); - expect(actionCalls()).toContainEqual({ type: "pull" }); + expect(actionCalls()).toContainEqual({ + type: "stage", + paths: ["README.md"], + }); } finally { invokeMock.mockImplementation(fallback as never); } @@ -472,19 +420,6 @@ describe("ProjectFilesPanel source control view", () => { } }); - it("disables Push when nothing is ahead", async () => { - const previous = { ...sourceControlState }; - Object.assign(sourceControlState, { ahead: 0 }); - try { - await render(); - expect((byLabel("Push", container) as HTMLButtonElement).disabled).toBe( - true, - ); - } finally { - Object.assign(sourceControlState, previous); - } - }); - it("restarts a diff read after closing the tab mid-flight", async () => { await render(); invokeMock.mockImplementationOnce(() => new Promise(() => {})); diff --git a/apps/examples/desktop-app/webview/components/views/chat/project-files-panel.tsx b/apps/examples/desktop-app/webview/components/views/chat/project-files-panel.tsx index 88db6aca0f..0467754b16 100644 --- a/apps/examples/desktop-app/webview/components/views/chat/project-files-panel.tsx +++ b/apps/examples/desktop-app/webview/components/views/chat/project-files-panel.tsx @@ -652,12 +652,7 @@ export function ProjectFilesPanel({ } catch (error) { toast({ variant: "destructive", - title: - action.type === "commit" - ? "Commit failed" - : action.type === "push" - ? "Push failed" - : "Git command failed", + title: "Git command failed", description: error instanceof Error ? error.message : String(error), }); return false; diff --git a/apps/examples/desktop-app/webview/components/views/chat/source-control-column.tsx b/apps/examples/desktop-app/webview/components/views/chat/source-control-column.tsx index df8fb25f0d..0f0e8f97e3 100644 --- a/apps/examples/desktop-app/webview/components/views/chat/source-control-column.tsx +++ b/apps/examples/desktop-app/webview/components/views/chat/source-control-column.tsx @@ -3,7 +3,6 @@ import { ArrowDown, ArrowUp, - Check, ChevronDown, ChevronRight, GitBranch, @@ -13,8 +12,6 @@ import { Undo2, } from "lucide-react"; import { type ReactNode, useCallback, useState } from "react"; -import { Button } from "@/components/ui/button"; -import { Textarea } from "@/components/ui/textarea"; import { type CommitFile, type SourceControlAction, @@ -84,7 +81,6 @@ export function SourceControlColumn({ onOpenCommitFile, selectedCommitFile, }: SourceControlColumnProps) { - const [message, setMessage] = useState(""); const [expandedCommits, setExpandedCommits] = useState>( () => new Set(), ); @@ -124,26 +120,6 @@ export function SourceControlColumn({ const [commitsOpen, setCommitsOpen] = useState(true); const staged = state?.staged ?? []; const changes = [...(state?.unstaged ?? []), ...(state?.untracked ?? [])]; - const canCommit = staged.length > 0 && message.trim().length > 0 && !busy; - // Pushing publishes a branch without an upstream; otherwise it needs - // something ahead. Pulling always needs an upstream. - const canPush = - !busy && - Boolean(state?.root) && - (state?.hasUpstream === false - ? state.commits.length > 0 - : (state?.ahead ?? 0) > 0); - const canPull = !busy && Boolean(state?.root && state.hasUpstream); - - const commit = useCallback(async () => { - if (!canCommit) return; - // A rejected hook or missing identity keeps the message for retry. - if ( - await onAction({ type: "commit", message: message.trim(), push: false }) - ) { - setMessage(""); - } - }, [canCommit, message, onAction]); if (state && !state.root) { return ( @@ -159,89 +135,11 @@ export function SourceControlColumn({ return (
-
- {state?.root ? ( -
- -
- ) : null} -