Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/patch-fallback-incremental-patch-fetch.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 26 additions & 10 deletions actions/setup/js/generate_git_patch.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,9 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
// INCREMENTAL MODE (for push_to_pull_request_branch):
// Only include commits that are new since origin/branchName.
// This prevents including commits that already exist on the PR branch.
// We must explicitly fetch origin/branchName and fail if it doesn't exist.
// Prefer a fresh fetch of origin/branchName; fall back to the existing
// remote tracking ref (set up by the initial shallow checkout) when the
// fetch fails (e.g. due to shallow clone limitations or missing credentials).

debugLog(`Strategy 1 (incremental): Fetching origin/${branchName}`);
// Configure git authentication via GIT_CONFIG_* environment variables.
Expand All @@ -183,15 +185,29 @@ async function generateGitPatch(branchName, baseBranch, options = {}) {
baseRef = `origin/${branchName}`;
debugLog(`Strategy 1 (incremental): Successfully fetched, baseRef=${baseRef}`);
} catch (fetchError) {
// In incremental mode, we MUST have origin/branchName - no fallback
debugLog(`Strategy 1 (incremental): Fetch failed - ${getErrorMessage(fetchError)}`);
errorMessage = `Cannot generate incremental patch: failed to fetch origin/${branchName}. This typically happens when the remote branch doesn't exist yet or was force-pushed. Error: ${getErrorMessage(fetchError)}`;
// Don't try other strategies in incremental mode
return {
success: false,
error: errorMessage,
patchPath: patchPath,
};
// Fetch failed. Check if origin/branchName already exists from the initial shallow checkout.
// This handles cases where git fetch fails due to shallow clone limitations or when
// GITHUB_TOKEN is unavailable in the MCP server process (e.g. after clean_git_credentials.sh).
// Using the existing remote tracking ref as a fallback is safe: it represents the state
// of the branch at checkout time, so the incremental patch will include all commits
// made by the agent since then.
debugLog(`Strategy 1 (incremental): Fetch failed - ${getErrorMessage(fetchError)}, checking for existing remote tracking ref`);
try {
execGitSync(["show-ref", "--verify", "--quiet", `refs/remotes/origin/${branchName}`], { cwd });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fallback strategy! Using show-ref --verify --quiet to check for the existing remote tracking ref is a clean approach. This will work well for the shallow checkout scenario where the ref is set up during the initial checkout but git fetch fails later due to credential or network issues.

// Remote tracking ref exists from initial shallow checkout — use it as base
baseRef = `origin/${branchName}`;
debugLog(`Strategy 1 (incremental): Using existing remote tracking ref as fallback, baseRef=${baseRef}`);
} catch (refCheckError) {
// No remote tracking ref at all — cannot safely generate an incremental patch.
// Report both errors: the original fetch failure and the missing ref.
debugLog(`Strategy 1 (incremental): No existing remote tracking ref found (${getErrorMessage(refCheckError)}), failing`);
errorMessage = `Cannot generate incremental patch: failed to fetch origin/${branchName} and no existing remote tracking ref found. This typically happens when the remote branch doesn't exist yet or was force-pushed. Fetch error: ${getErrorMessage(fetchError)}`;
return {
success: false,
error: errorMessage,
patchPath: patchPath,
};
}
}
} else {
// FULL MODE (for create_pull_request):
Expand Down
45 changes: 45 additions & 0 deletions actions/setup/js/git_patch_integration.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,51 @@ describe("git patch integration tests", () => {
}
});

it("should fall back to existing remote tracking ref when fetch fails in incremental mode", async () => {
// Simulate a shallow checkout scenario:
// 1. feature-branch is created, first commit pushed to origin (origin/feature-branch exists)
// 2. Agent adds a new commit locally
// 3. Remote URL is broken so git fetch fails
// 4. We expect patch generation to succeed using the existing origin/feature-branch ref
execGit(["checkout", "-b", "shallow-fetch-fail"], { cwd: workingRepo });
fs.writeFileSync(path.join(workingRepo, "base.txt"), "Base content\n");
execGit(["add", "base.txt"], { cwd: workingRepo });
execGit(["commit", "-m", "Base commit (already on remote)"], { cwd: workingRepo });

// Push so origin/shallow-fetch-fail tracking ref is set up (simulating shallow checkout)
execGit(["push", "-u", "origin", "shallow-fetch-fail"], { cwd: workingRepo });

// Add a new commit (the agent's work)
fs.writeFileSync(path.join(workingRepo, "agent-change.txt"), "Agent change\n");
execGit(["add", "agent-change.txt"], { cwd: workingRepo });
execGit(["commit", "-m", "Agent commit - should appear in patch"], { cwd: workingRepo });

// Break the remote URL to simulate fetch failure (e.g. missing credentials or network issue)
execGit(["remote", "set-url", "origin", "https://invalid.example.invalid/nonexistent-repo.git"], { cwd: workingRepo });

const restore = setTestEnv(workingRepo);
try {
// origin/shallow-fetch-fail still points to the base commit even though fetch will fail
const result = await generateGitPatch("shallow-fetch-fail", "main", { mode: "incremental" });

// Should succeed using the existing remote tracking ref as the base
expect(result.success).toBe(true);
expect(result.patchPath).toBeDefined();

const patchContent = fs.readFileSync(result.patchPath, "utf8");

// Should contain the agent's new commit
expect(patchContent).toContain("Agent commit - should appear in patch");

// Should NOT include the already-pushed base commit
expect(patchContent).not.toContain("Base commit (already on remote)");
} finally {
// Restore remote URL before cleanup so afterEach can delete the directory
execGit(["remote", "set-url", "origin", bareRepoDir], { cwd: workingRepo });
restore();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good test coverage for the fallback scenario! Restoring the remote URL in the finally block is important to ensure afterEach cleanup works correctly. Consider adding an assertion that the error message from the original fetch failure is included in the error when the ref also doesn't exist, to ensure good debuggability.

}
});

it("should include all commits in full mode even when origin/branch exists", async () => {
// Create a feature branch with first commit
execGit(["checkout", "-b", "full-mode-branch"], { cwd: workingRepo });
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ require (
charm.land/bubbletea/v2 v2.0.2
charm.land/lipgloss/v2 v2.0.2
github.com/charmbracelet/huh v0.8.0
github.com/charmbracelet/lipgloss v1.1.1-0.20250319133953-166f707985bc
github.com/charmbracelet/x/exp/golden v0.0.0-20251215102626-e0db08df7383
github.com/cli/go-gh/v2 v2.13.0
github.com/creack/pty v1.1.24
Expand Down Expand Up @@ -42,7 +43,6 @@ require (
github.com/charmbracelet/bubbletea v1.3.10 // indirect
github.com/charmbracelet/colorprofile v0.4.2 // indirect
github.com/charmbracelet/harmonica v0.2.0 // indirect
github.com/charmbracelet/lipgloss v1.1.1-0.20250319133953-166f707985bc // indirect
github.com/charmbracelet/ultraviolet v0.0.0-20260205113103-524a6607adb8 // indirect
github.com/charmbracelet/x/ansi v0.11.6 // indirect
github.com/charmbracelet/x/cellbuf v0.0.15 // indirect
Expand Down