diff --git a/.changeset/fn-7490-push-to-remote-setting.md b/.changeset/fn-7490-push-to-remote-setting.md new file mode 100644 index 0000000000..fcbcd51212 --- /dev/null +++ b/.changeset/fn-7490-push-to-remote-setting.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix direct merges so Push to remote after merge honors the configured remote and branch. +category: fix +dev: Resolves remote-only push targets from the merge integration branch and preserves non-fatal push errors on done tasks. diff --git a/docs/settings-reference.md b/docs/settings-reference.md index bc9326faca..7af6b5a8fe 100644 --- a/docs/settings-reference.md +++ b/docs/settings-reference.md @@ -476,8 +476,10 @@ Sandbox backend precedence is: 2. Project `sandbox.backend` 3. Default `"native"` -| `pushAfterMerge` | `boolean` | `false` | Auto-push to remote after successful direct merge. Includes pulling latest and AI conflict resolution. | -| `pushRemote` | `string` | `"origin"` | Git remote (and optional branch) to push to after merge. | +| `pushAfterMerge` | `boolean` | `false` | Auto-push after successful direct merges only. Pull-request strategy is excluded because PR mode publishes its task branch separately before PR creation. | +| `pushRemote` | `string` | `"origin"` | Git remote target used when `pushAfterMerge` is enabled. Accepts `remote` (for example `origin`) or `remote branch` (for example `upstream main`). Empty/unset values fall back to `origin` plus the resolved merge integration branch. | + +When `pushAfterMerge` is enabled, a completed direct merge first runs `git pull --rebase ` and then `git push `. For remote-only targets, Fusion resolves `` from the merge integration branch rather than the task worktree's incidental checkout, so reuse-task-worktree and detached-HEAD merge modes still push the intended branch. If the post-merge pull or push fails, the local merge remains completed and the done task records `pushedToRemote: false` with a `pushError` for operator follow-up. | `worktreeInitCommand` | `string` | `undefined` | Shell command run after task worktree creation and in temporary merge worktrees before merge/review verification. In standalone AI merge, this runs inside each fresh `fusion-ai-merge-*` clean-room worktree after `git worktree add`; when unset, Fusion infers a package-manager install from the lockfile and may skip only when the install marker matches. Useful for project-specific setup beyond package install (for example `pnpm install --frozen-lockfile`, `cp .env.local .env`, or codegen/bootstrap scripts). | | `worktreeCopyFiles` | `string[]` | `[]` | Repository-root-relative regular files to copy into each newly assigned non-resume task worktree. Configure from Settings → Worktrees with editable rows or Browse (useful for `.env`-style files). Fusion copies these files after fresh creation or pooled-worktree preparation and before `worktreeInitCommand`, secrets-env materialization, and task execution. Blank/duplicate entries are ignored; absolute paths, `..` traversal, missing files, directories, and unreadable/non-regular sources are skipped as non-fatal task-log/audit diagnostics without logging file contents. Resume/existing worktrees are not overwritten. | | `testCommand` | `string` | `undefined` | Merge-time test command (hard gate). When unset, Fusion auto-detects from lockfile. | diff --git a/packages/dashboard/app/components/SettingsModal.tsx b/packages/dashboard/app/components/SettingsModal.tsx index 04110e9d74..9af4f73244 100644 --- a/packages/dashboard/app/components/SettingsModal.tsx +++ b/packages/dashboard/app/components/SettingsModal.tsx @@ -2537,6 +2537,11 @@ export function SettingsModal({ githubAuthToken: form.githubAuthToken?.trim() || undefined, prTitlePromptInstructions: form.prTitlePromptInstructions?.trim() || undefined, prDescriptionPromptInstructions: form.prDescriptionPromptInstructions?.trim() || undefined, + /* + FNXC:MergeSettings 2026-07-04-09:18: + Push target text is meaningful only when direct post-merge pushing is enabled. Hiding the input must not keep submitting a stale remote/branch from the form state; clearing it lets project settings fall back to the default origin target when the toggle is disabled. + */ + pushRemote: form.pushAfterMerge ? form.pushRemote?.trim() || undefined : undefined, overlapIgnorePaths: (form.overlapIgnorePaths ?? []).map((path) => path.trim()).filter((path) => path.length > 0), worktreeCopyFiles: normalizedWorktreeCopyFiles.length > 0 || initialScopedValues?.project?.worktreeCopyFiles !== undefined ? normalizedWorktreeCopyFiles diff --git a/packages/dashboard/app/components/__tests__/SettingsModal.scheduling-merge.test.tsx b/packages/dashboard/app/components/__tests__/SettingsModal.scheduling-merge.test.tsx index d55d4dcfee..339bddaab5 100644 --- a/packages/dashboard/app/components/__tests__/SettingsModal.scheduling-merge.test.tsx +++ b/packages/dashboard/app/components/__tests__/SettingsModal.scheduling-merge.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { render, screen, fireEvent, waitFor, within } from "@testing-library/react"; +import { cleanup, render, screen, fireEvent, waitFor, within } from "@testing-library/react"; import { EditorView } from "@codemirror/view"; import path from "path"; import { SettingsModal } from "../SettingsModal"; @@ -1242,14 +1242,14 @@ describe("SettingsModal", () => { expect(screen.getByPlaceholderText("origin")).toBeInTheDocument(); }); - it("includes pushAfterMerge and pushRemote in the save payload", async () => { + it("includes pushAfterMerge and trimmed pushRemote in the save payload", async () => { await settingsModalUser.click( screen.getByRole("checkbox", { name: /push to remote after merge/i }), ); const pushRemoteInput = screen.getByLabelText("Push Remote"); await settingsModalUser.clear(pushRemoteInput); - await settingsModalUser.type(pushRemoteInput, "upstream main"); + await settingsModalUser.type(pushRemoteInput, " upstream main "); await settingsModalUser.click(screen.getByText("Save")); @@ -1262,6 +1262,59 @@ describe("SettingsModal", () => { expect(payload.pushRemote).toBe("upstream main"); }); + it("renders persisted push remote settings after save and reload", async () => { + cleanup(); + vi.clearAllMocks(); + mockFetchSettings.mockResolvedValueOnce({ + ...defaultSettings, + pushAfterMerge: true, + pushRemote: "upstream main", + }); + mockFetchSettingsByScope.mockResolvedValueOnce({ + global: defaultSettings, + project: { pushAfterMerge: true, pushRemote: "upstream main" }, + }); + + renderModal({ initialSection: "merge" }); + await screen.findByRole("checkbox", { name: /push to remote after merge/i }); + + expect(screen.getByRole("checkbox", { name: /push to remote after merge/i })).toBeChecked(); + expect(screen.getByLabelText("Push Remote")).toHaveValue("upstream main"); + }); + + it("hides and clears stale Push Remote when push-after-merge is disabled", async () => { + cleanup(); + vi.clearAllMocks(); + mockFetchSettings.mockResolvedValueOnce({ + ...defaultSettings, + pushAfterMerge: true, + pushRemote: "upstream main", + }); + mockFetchSettingsByScope.mockResolvedValueOnce({ + global: defaultSettings, + project: { pushAfterMerge: true, pushRemote: "upstream main" }, + }); + + renderModal({ initialSection: "merge" }); + const pushAfterMergeToggle = await screen.findByRole("checkbox", { name: /push to remote after merge/i }); + expect(screen.getByLabelText("Push Remote")).toHaveValue("upstream main"); + + await settingsModalUser.click(pushAfterMergeToggle); + + expect(screen.queryByLabelText("Push Remote")).not.toBeInTheDocument(); + expect(screen.queryByText("Git remote to push to")).not.toBeInTheDocument(); + + await settingsModalUser.click(screen.getByText("Save")); + + await waitFor(() => { + expect(mockUpdateSettings).toHaveBeenCalledTimes(1); + }); + + const payload = mockUpdateSettings.mock.calls[0][0] as Record; + expect(payload.pushAfterMerge).toBe(false); + expect(payload.pushRemote).toBeNull(); + }); + it("round-trips plan approval mode through project settings save", async () => { const select = screen.getByLabelText("Plan approval mode"); expect(select).toHaveValue("workflow"); diff --git a/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx b/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx index fe1f18d920..5a0e7991d7 100644 --- a/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx +++ b/packages/dashboard/app/components/__tests__/settings-mobile.test.tsx @@ -43,6 +43,7 @@ const defaultSettings = { vi.mock("../../api", () => ({ fetchProjects: vi.fn(() => Promise.resolve([])), fetchGitRemotes: vi.fn(() => Promise.resolve({ remotes: [] })), + fetchGitBranches: vi.fn(() => Promise.resolve([])), fetchSettings: vi.fn(() => Promise.resolve({ ...defaultSettings })), fetchSettingsByScope: vi.fn(() => Promise.resolve({ global: { ...defaultSettings }, project: {} })), updateSettings: vi.fn(() => Promise.resolve({ ...defaultSettings })), @@ -149,7 +150,7 @@ vi.mock("../../hooks/useMemoryBackendStatus", () => ({ })), })); -import { fetchSettings } from "../../api"; +import { fetchSettings, updateSettings } from "../../api"; function mockSettingsViewport(matches: boolean): void { Object.defineProperty(window, "matchMedia", { @@ -288,6 +289,33 @@ describe("SettingsModal mobile adaptations", () => { expect(getByLabelText("Memory File")).toBeTruthy(); }); + it("keeps push remote reachable and clears its hidden shell on mobile", async () => { + mockSettingsViewport(true); + Object.defineProperty(window, "innerWidth", { configurable: true, value: 375 }); + const user = userEvent.setup(); + const { getByLabelText, queryByLabelText, getByRole, queryByText } = render(); + await waitFor(() => expect(fetchSettings).toHaveBeenCalled()); + + await user.selectOptions(getByLabelText("Settings Section"), "merge"); + await user.click(getByRole("checkbox", { name: /push to remote after merge/i })); + + const pushRemote = getByLabelText("Push Remote"); + expect(pushRemote).toHaveAttribute("placeholder", "origin"); + await user.type(pushRemote, "upstream main"); + + await user.click(getByRole("checkbox", { name: /push to remote after merge/i })); + + expect(queryByLabelText("Push Remote")).toBeNull(); + expect(queryByText("Git remote to push to")).toBeNull(); + + await user.click(getByRole("button", { name: "Save" })); + await waitFor(() => expect(updateSettings).toHaveBeenCalled()); + + const payload = vi.mocked(updateSettings).mock.calls[0][0] as Record; + expect(payload.pushAfterMerge).toBe(false); + expect(payload).not.toHaveProperty("pushRemote"); + }); + it("keeps research settings controls inside mobile containment wrappers", async () => { vi.mocked(fetchSettings).mockResolvedValueOnce({ ...defaultSettings, diff --git a/packages/dashboard/src/__tests__/routes-settings.test.ts b/packages/dashboard/src/__tests__/routes-settings.test.ts index 8ed3bd4735..88c001363b 100644 --- a/packages/dashboard/src/__tests__/routes-settings.test.ts +++ b/packages/dashboard/src/__tests__/routes-settings.test.ts @@ -448,6 +448,34 @@ describe("PUT /settings", () => { expect(store.updateSettings).toHaveBeenCalledWith({ maxConcurrent: 8 }); }); + it("persists push-after-merge remote target through PUT and subsequent GET", async () => { + const updatedSettings = { + ...DEFAULT_SETTINGS, + pushAfterMerge: true, + pushRemote: "upstream main", + }; + (store.updateSettings as ReturnType).mockResolvedValue(updatedSettings); + (store.getSettingsFast as ReturnType).mockResolvedValue(updatedSettings); + + const updateRes = await REQUEST( + buildApp(), + "PUT", + "/api/settings", + JSON.stringify({ pushAfterMerge: true, pushRemote: "upstream main" }), + { "Content-Type": "application/json" }, + ); + + expect(updateRes.status).toBe(200); + expect(store.updateSettings).toHaveBeenCalledWith({ pushAfterMerge: true, pushRemote: "upstream main" }); + expect(updateRes.body.pushAfterMerge).toBe(true); + expect(updateRes.body.pushRemote).toBe("upstream main"); + + const getRes = await GET(buildApp(), "/api/settings"); + expect(getRes.status).toBe(200); + expect(getRes.body.pushAfterMerge).toBe(true); + expect(getRes.body.pushRemote).toBe("upstream main"); + }); + it("passes defaultAgentPermissionPolicy toolRules through settings updates", async () => { const payload = { defaultAgentPermissionPolicy: { diff --git a/packages/engine/src/__tests__/merger-prompt-and-utils.test.ts b/packages/engine/src/__tests__/merger-prompt-and-utils.test.ts index 2c98cb8f2c..32a1f2856c 100644 --- a/packages/engine/src/__tests__/merger-prompt-and-utils.test.ts +++ b/packages/engine/src/__tests__/merger-prompt-and-utils.test.ts @@ -194,6 +194,10 @@ function createMockStore(taskOverrides: Partial = {}, allTasks: Task[] = [ clearStaleExecutionStartBranchReferences: vi.fn().mockReturnValue([]), getVerificationCacheHit: vi.fn().mockReturnValue(null), recordVerificationCachePass: vi.fn(), + recordActivity: vi.fn().mockResolvedValue(undefined), + recordRunAuditEvent: vi.fn(), + enqueueMergeQueue: vi.fn(), + getMergeQueueSnapshot: vi.fn().mockReturnValue([]), } as unknown as TaskStore; } @@ -308,7 +312,7 @@ describe("push-after-merge", () => { } as any); }); - function setupAiMergeExecSyncWithPush(pushBehavior?: (attempt: number) => void) { + function setupAiMergeExecSyncWithPush(pushBehavior?: (attempt: number) => void, options?: { detachedHead?: boolean }) { let pushAttempts = 0; mockedExecSync.mockImplementation((cmd: any) => { @@ -320,7 +324,14 @@ describe("push-after-merge", () => { throw err; } if (cmdStr.includes("rev-parse --verify")) return Buffer.from("abc123"); - if (cmdStr.includes("git symbolic-ref --short HEAD")) return "main" as any; + if (cmdStr.includes("git symbolic-ref --short HEAD")) { + if (options?.detachedHead) { + const err = new Error("fatal: ref HEAD is not a symbolic ref") as Error & { status?: number }; + err.status = 128; + throw err; + } + return "main" as any; + } if (cmdStr.includes("git rev-parse --abbrev-ref origin/HEAD")) return "origin/main" as any; if (cmdStr === "git rev-parse HEAD" || cmdStr.startsWith("git rev-parse HEAD ")) return "mergedcommit123" as any; if (cmdStr.includes("git log HEAD..")) return "- feat: something" as any; @@ -371,6 +382,29 @@ describe("push-after-merge", () => { ).toBe(true); }); + it("FN-7490 uses the merge target branch for remote-only pushes when HEAD is detached", async () => { + setupAiMergeExecSyncWithPush(undefined, { detachedHead: true }); + + const store = createMockStore(); + (store.getSettings as ReturnType).mockResolvedValue({ + ...DEFAULT_SETTINGS, + mergeIntegrationWorktree: "reuse-task-worktree" as const, + pushAfterMerge: true, + pushRemote: "origin", + mergeStrategy: "direct", + }); + + const result = await aiMergeTask(store, "/tmp/root", "FN-050"); + + expect(result.pushedToRemote).toBe(true); + expect( + mockedExecSync.mock.calls.some((call) => String(call[0]).includes('git pull --rebase "origin" "main"')), + ).toBe(true); + expect( + mockedExecSync.mock.calls.some((call) => String(call[0]).includes('git push "origin" "main"')), + ).toBe(true); + }); + it("does not push when pushAfterMerge is disabled (default)", async () => { setupAiMergeExecSyncWithPush(); @@ -435,6 +469,68 @@ describe("push-after-merge", () => { expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done"); }); + it.each([undefined, "", " "])("FN-7490 falls back to origin and integration branch for empty pushRemote %s", async (pushRemote) => { + mockedExecSync.mockImplementation((cmd: any) => { + const cmdStr = String(cmd); + if (cmdStr.startsWith('git pull --rebase "origin" "main"')) return Buffer.from(""); + if (cmdStr.startsWith('git push "origin" "main"')) return Buffer.from(""); + if (cmdStr.includes("git symbolic-ref --short HEAD")) { + throw new Error("fatal: ref HEAD is not a symbolic ref"); + } + if (cmdStr.includes("rev-parse --verify REBASE_HEAD")) { + const err = new Error("fatal: Needed a single revision"); + throw err; + } + return Buffer.from(""); + }); + + const store = createMockStore(); + const result = await pushToRemoteAfterMerge(store, "/tmp/root", "FN-050", { + ...DEFAULT_SETTINGS, + pushAfterMerge: true, + pushRemote, + }, { integrationBranch: "main" }); + + expect(result.pushed).toBe(true); + expect( + mockedExecSync.mock.calls.some((call) => String(call[0]).startsWith('git pull --rebase "origin" "main"')), + ).toBe(true); + expect( + mockedExecSync.mock.calls.some((call) => String(call[0]).startsWith('git push "origin" "main"')), + ).toBe(true); + }); + + it("uses integration branch fallback for remote-only push targets", async () => { + mockedExecSync.mockImplementation((cmd: any) => { + const cmdStr = String(cmd); + if (cmdStr.startsWith('git pull --rebase "origin" "release"')) return Buffer.from(""); + if (cmdStr.startsWith('git push "origin" "release"')) return Buffer.from(""); + if (cmdStr.includes("git symbolic-ref --short HEAD")) { + throw new Error("fatal: ref HEAD is not a symbolic ref"); + } + if (cmdStr.includes("rev-parse --verify REBASE_HEAD")) { + const err = new Error("fatal: Needed a single revision"); + throw err; + } + return Buffer.from(""); + }); + + const store = createMockStore(); + const result = await pushToRemoteAfterMerge(store, "/tmp/root", "FN-050", { + ...DEFAULT_SETTINGS, + pushAfterMerge: true, + pushRemote: "origin", + }, { integrationBranch: "release" }); + + expect(result.pushed).toBe(true); + expect( + mockedExecSync.mock.calls.some((call) => String(call[0]).startsWith('git pull --rebase "origin" "release"')), + ).toBe(true); + expect( + mockedExecSync.mock.calls.some((call) => String(call[0]).startsWith('git push "origin" "release"')), + ).toBe(true); + }); + it("uses custom remote and branch when configured", async () => { mockedExecSync.mockImplementation((cmd: any) => { const cmdStr = String(cmd); diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts index 878eb37e2e..bed8971c3e 100644 --- a/packages/engine/src/merger.ts +++ b/packages/engine/src/merger.ts @@ -6992,12 +6992,19 @@ function isRebaseInProgress(rootDir: string): boolean { } } -function parsePushRemoteTarget(rootDir: string, pushRemote?: string): { remote: string; branch: string } { +function parsePushRemoteTarget(rootDir: string, pushRemote?: string, fallbackBranch?: string): { remote: string; branch: string } { const rawTarget = pushRemote?.trim() || "origin"; const [remoteToken, ...branchTokens] = rawTarget.split(/\s+/).filter(Boolean); const remote = remoteToken || "origin"; let branch = branchTokens.join(" ").trim(); + if (!branch) { + /* + FNXC:MergePush 2026-07-04-09:31: + Remote-only push targets must resolve to the merge integration branch, not the incidental HEAD of the worktree running post-merge git. Reuse-task-worktree can detach HEAD after advancing refs/heads/, so the direct merge call site supplies the authoritative integration branch before this helper falls back to symbolic-ref for standalone utility callers. + */ + branch = fallbackBranch?.trim() || ""; + } if (!branch) { branch = execSyncText("git symbolic-ref --short HEAD", { cwd: rootDir, @@ -7288,13 +7295,14 @@ export async function pushToRemoteAfterMerge( runtimeHint?: string; assignedAgentRuntimeConfig?: Record; onSession?: (session: { dispose: () => void }) => void; + integrationBranch?: string; }, ): Promise<{ pushed: boolean; error?: string }> { let target: { remote: string; branch: string }; try { throwIfAborted(options?.signal, taskId); - target = parsePushRemoteTarget(rootDir, settings.pushRemote); + target = parsePushRemoteTarget(rootDir, settings.pushRemote, options?.integrationBranch); } catch (error: unknown) { rethrowIfMergeAborted(error); const message = getCommandErrorMessage(error); @@ -10806,6 +10814,7 @@ export async function aiMergeTask( runtimeHint: pushRuntimeHint, assignedAgentRuntimeConfig: pushAssignedAgent?.runtimeConfig, onSession: options.onSession, + integrationBranch: mergeTarget.branch, }); if (pushResult.pushed) { mergerLog.log(`${taskId}: pushed merged result to remote`);