diff --git a/setup/js/generate_git_patch.cjs b/setup/js/generate_git_patch.cjs index 6e2b222..01e30a4 100644 --- a/setup/js/generate_git_patch.cjs +++ b/setup/js/generate_git_patch.cjs @@ -442,16 +442,77 @@ async function generateGitPatch(branchName, baseBranch, options = {}) { // The measurement itself (stream to temp file via `git diff --output`, stat, // cleanup) is extracted into git_patch_utils.computeIncrementalDiffSize so // it is O(1) memory and independently unit-testable against a real repo. + // + // When the agent has merged the default branch into the PR branch (to resolve + // conflicts or sync a stale branch), the naive diff base of `origin/` + // (the PR's old head) inflates diffSize to include all of the default branch's + // new commits — even though those commits are already on origin/ + // and represent no new content in the PR. Fix: when the merge-base between + // origin/ and the local branch is NOT an ancestor of the PR's + // current head (baseCommitSha), the agent merged default-branch commits ahead + // of the PR head. Use the merge-base as the effective diff base to exclude those + // merged upstream commits from the size measurement. + let diffBaseForSize = baseCommitSha; + if (mode === "incremental" && baseCommitSha && branchName && defaultBranch) { + try { + let baseBranchRemoteRef = null; + try { + execGitSync(["show-ref", "--verify", "--quiet", `refs/remotes/origin/${defaultBranch}`], { cwd }); + baseBranchRemoteRef = `refs/remotes/origin/${defaultBranch}`; + } catch { + // origin/ not available locally; skip the adjustment + } + if (baseBranchRemoteRef) { + // Only adjust the diff base when baseCommitSha is an ancestor of the local + // branch tip. If it is NOT an ancestor the branch was rewritten (rebase / + // force-push); in that case the merge-base adjustment could undercount by + // ignoring commits that changed relative to the remote, so keep the original + // baseCommitSha as the diff base. + let baseIsAncestorOfBranch = false; + try { + execGitSync(["merge-base", "--is-ancestor", "--", baseCommitSha, branchName], { cwd }); + baseIsAncestorOfBranch = true; + } catch { + // baseCommitSha is not an ancestor of branchName (rebase / force-push) + debugLog(`Strategy 1 (incremental): baseCommitSha ${baseCommitSha} is not an ancestor of ${branchName} (rebase/force-push?); skipping merge-base adjustment`); + } + + if (baseIsAncestorOfBranch) { + const mb = execGitSync(["merge-base", "--", baseBranchRemoteRef, branchName], { cwd }).trim(); + // Check if mb is already an ancestor of baseCommitSha. + // If it is, baseCommitSha is "later" and the agent did NOT merge the default + // branch ahead of the PR head — keep baseCommitSha as the diff base. + // If mb is NOT an ancestor of baseCommitSha, the agent merged default-branch + // commits that are beyond the PR head. Use mb to exclude those commits from + // the incremental diff size measurement. + let mbIsAncestorOfBase = false; + try { + execGitSync(["merge-base", "--is-ancestor", "--", mb, baseCommitSha], { cwd }); + mbIsAncestorOfBase = true; + } catch { + // mb is not an ancestor of baseCommitSha + } + if (!mbIsAncestorOfBase) { + debugLog(`Strategy 1 (incremental): agent merged ${defaultBranch} ahead of PR head; using merge-base ${mb} as diff base instead of PR head ${baseCommitSha}`); + diffBaseForSize = mb; + } + } + } + } catch (adjustErr) { + debugLog(`Strategy 1 (incremental): diff-base adjustment failed (${getErrorMessage(adjustErr)}); using original base`); + } + } + let diffSize = null; - if (mode === "incremental" && baseCommitSha && branchName) { + if (mode === "incremental" && diffBaseForSize && branchName) { diffSize = computeIncrementalDiffSize({ - baseRef: baseCommitSha, + baseRef: diffBaseForSize, headRef: branchName, cwd, tmpPath: `${patchPath}.diff.tmp`, excludedFiles: options.excludedFiles, }); - debugLog(`Final: diffSize=${diffSize ?? "(n/a)"} bytes (baseRef=${baseCommitSha}..${branchName})`); + debugLog(`Final: diffSize=${diffSize ?? "(n/a)"} bytes (baseRef=${diffBaseForSize}..${branchName})`); } debugLog(`Final: SUCCESS - patchSize=${patchSize} bytes, patchLines=${patchLines}, diffSize=${diffSize ?? "(n/a)"} bytes, baseCommit=${baseCommitSha || "(unknown)"}`); diff --git a/setup/js/safe_outputs_handlers.cjs b/setup/js/safe_outputs_handlers.cjs index 4d8e317..ba942cb 100644 --- a/setup/js/safe_outputs_handlers.cjs +++ b/setup/js/safe_outputs_handlers.cjs @@ -405,6 +405,26 @@ function createHandlers(server, appendSafeOutput, config = {}) { entry.branch = detectedBranch; } + // Reject if branch still equals base_branch after detection. + // This means the base branch was incorrectly resolved (e.g., resolved to the + // feature branch itself due to a confused event context). Writing a safe output + // in this state would cause a cryptic git exit-1 in the safe_outputs job when + // it tries to fetch a non-existent remote ref. + if (entry.branch === entry.base_branch) { + return { + content: [ + { + type: "text", + text: JSON.stringify({ + result: "error", + error: `Branch '${entry.branch}' equals base_branch '${entry.base_branch}'. Cannot create a pull request from a branch into itself. Ensure 'branch' is your feature branch and that the base branch resolves to the target (e.g., 'main' or 'master').`, + }), + }, + ], + isError: true, + }; + } + const allowedBranches = parseAllowedBranchPatterns(prConfig.allowed_branches); if (allowedBranches.length > 0 && !isAllowedBranch(entry.branch, allowedBranches)) { return { @@ -698,6 +718,26 @@ function createHandlers(server, appendSafeOutput, config = {}) { entry.branch = detectedBranch; } + // Reject if branch still equals base_branch after detection. + // This means the base branch was incorrectly resolved (e.g., resolved to the + // feature branch itself due to a confused event context). Writing a safe output + // in this state would cause a cryptic git exit-1 in the safe_outputs job when + // it tries to fetch a non-existent remote ref. + if (entry.branch === entry.base_branch) { + return { + content: [ + { + type: "text", + text: JSON.stringify({ + result: "error", + error: `Branch '${entry.branch}' equals base_branch '${entry.base_branch}'. Cannot push to a pull request branch that targets itself. Ensure 'branch' is your feature branch and that the base branch resolves to the target (e.g., 'main' or 'master').`, + }), + }, + ], + isError: true, + }; + } + const intentValidationError = validatePushToPullRequestBranchIntent(entry); if (intentValidationError) { return buildIntentErrorResponse(intentValidationError);