Skip to content

fix(ci): publish validated conflict resolution trees #7576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cv merged 1 commit into from
Jul 26, 2026
+108 −39
Merged

fix(ci): publish validated conflict resolution trees #7576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
100 changes: 79 additions & 21 deletions test/pr-merge-conflict-fixer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,24 +99,60 @@ function createConflictFixture(): {
git(repository, ["config", "user.email", "conflict-fixer@example.test"]);
git(repository, ["config", "commit.gpgsign", "false"]);
write(repository, "conflict.txt", "shared\n");
git(repository, ["add", "conflict.txt"]);
write(repository, "clean-merge.txt", "first\nkeep-1\nkeep-2\nkeep-3\nkeep-4\nkeep-5\nlast\n");
git(repository, ["add", "conflict.txt", "clean-merge.txt"]);
git(repository, ["commit", "-m", "test: add shared file"]);

git(repository, ["checkout", "-b", "pull-request"]);
write(repository, "conflict.txt", "pull request\n");
git(repository, ["add", "conflict.txt"]);
write(
repository,
"clean-merge.txt",
"pull request\nkeep-1\nkeep-2\nkeep-3\nkeep-4\nkeep-5\nlast\n",
);
git(repository, ["add", "conflict.txt", "clean-merge.txt"]);
git(repository, ["commit", "-m", "test: change PR side"]);
const headSha = git(repository, ["rev-parse", "HEAD"]);

git(repository, ["checkout", "main"]);
write(repository, "conflict.txt", "main branch\n");
write(
repository,
"clean-merge.txt",
"first\nkeep-1\nkeep-2\nkeep-3\nkeep-4\nkeep-5\nmain branch\n",
);
write(repository, "main-only.txt", "main\n");
git(repository, ["add", "conflict.txt", "main-only.txt"]);
git(repository, ["add", "conflict.txt", "clean-merge.txt", "main-only.txt"]);
git(repository, ["commit", "-m", "test: change main side"]);
const baseSha = git(repository, ["rev-parse", "HEAD"]);
return { baseSha, headSha, repository };
}

function createMovedFileConflictFixture(): ReturnType<typeof createConflictFixture> {
const repository = temporaryDirectory();
git(repository, ["init", "--initial-branch=main"]);
git(repository, ["config", "user.name", "Conflict Fixer Test"]);
git(repository, ["config", "user.email", "conflict-fixer@example.test"]);
git(repository, ["config", "commit.gpgsign", "false"]);
write(repository, "adapter.js", "shared\n");
git(repository, ["add", "adapter.js"]);
git(repository, ["commit", "-m", "test: add shared adapter"]);

git(repository, ["checkout", "-b", "pull-request"]);
write(repository, "adapter.js", "pull request intent\n");
git(repository, ["add", "adapter.js"]);
git(repository, ["commit", "-m", "test: change PR adapter"]);
const headSha = git(repository, ["rev-parse", "HEAD"]);

git(repository, ["checkout", "main"]);
fs.rmSync(path.join(repository, "adapter.js"));
write(repository, "adapter.mts", "main migration\n");
git(repository, ["add", "-A"]);
git(repository, ["commit", "-m", "test: move main adapter"]);
const baseSha = git(repository, ["rev-parse", "HEAD"]);
return { baseSha, headSha, repository };
}

function entryFor(fixture: ReturnType<typeof createConflictFixture>): ConflictMatrixEntry {
return {
base_sha: fixture.baseSha,
Expand Down Expand Up @@ -227,7 +263,7 @@ describe("PR merge conflict fixer", () => {
expect(selected.map((item) => item.pr_number)).toEqual([2]);
});

it("accepts a patch that changes only the original conflict paths (#7542)", () => {
it("accepts a patch that resolves the original conflict paths (#7542)", () => {
const fixture = createConflictFixture();
const patchPath = path.join(temporaryDirectory(), "resolution.patch");
const expectedTree = createResolutionPatch(fixture, patchPath);
Expand All @@ -239,27 +275,43 @@ describe("PR merge conflict fixer", () => {
workDirectory: path.join(temporaryDirectory(), "publisher"),
});

expect(result.changedPaths).toEqual(["conflict.txt"]);
expect(result.finalTree).toBe(expectedTree);
expect(git(result.repository, ["show", `${result.finalTree}:main-only.txt`])).toBe("main");
});

it("rejects a patch that changes a non-conflict path (#7542)", () => {
const fixture = createConflictFixture();
it("accepts a resolution that moves PR intent to main's replacement path (#7542)", () => {
const fixture = createMovedFileConflictFixture();
const patchPath = path.join(temporaryDirectory(), "resolution.patch");
createResolutionPatch(fixture, patchPath, (repository) => {
write(repository, "unrelated.txt", "not part of the conflict\n");
git(repository, ["add", "unrelated.txt"]);
const repository = path.join(temporaryDirectory(), "resolver");
const merge = required(
prepareMerge(fixture.repository, repository, fixture.headSha, fixture.baseSha),
"expected a moved-file conflict fixture",
);
expect(merge.conflictPaths).toEqual(["adapter.js"]);
fs.rmSync(path.join(repository, "adapter.js"));
write(repository, "adapter.mts", "main migration\npull request intent\n");
git(repository, ["add", "-A"]);
const expectedTree = writeTree(repository);
const patch = execFileSync("git", ["diff", "--binary", merge.conflictTree, expectedTree], {
cwd: repository,
});
fs.writeFileSync(patchPath, patch);

expect(() =>
validateResolutionPatch({
entry: entryFor(fixture),
patchPath,
sourceRepository: fixture.repository,
workDirectory: path.join(temporaryDirectory(), "publisher"),
}),
).toThrow(/non-conflict paths: unrelated\.txt/u);
const result = validateResolutionPatch({
entry: {
...entryFor(fixture),
conflict_paths: ["adapter.js"],
},
patchPath,
sourceRepository: fixture.repository,
workDirectory: path.join(temporaryDirectory(), "publisher"),
});

expect(result.finalTree).toBe(expectedTree);
expect(git(result.repository, ["show", `${result.finalTree}:adapter.mts`])).toBe(
"main migration\npull request intent",
);
expect(() => git(result.repository, ["show", `${result.finalTree}:adapter.js`])).toThrow();
});

it("rejects changed main state without comparing the live PR head SHA (#7542)", () => {
Expand Down Expand Up @@ -358,10 +410,16 @@ describe("PR merge conflict fixer", () => {
});
expect(JSON.stringify(commitRequest?.body)).not.toMatch(/author|committer|signature/u);
const blobRequests = requests.filter((item) => item.path.endsWith("/git/blobs"));
expect(blobRequests).toHaveLength(1);
expect(
Buffer.from((blobRequests[0]?.body as { content: string }).content, "base64").toString(),
).toBe("resolved intent\n");
blobRequests
.map((item) => Buffer.from((item.body as { content: string }).content, "base64").toString())
.sort(),
).toEqual(
[
"pull request\nkeep-1\nkeep-2\nkeep-3\nkeep-4\nkeep-5\nmain branch\n",
"resolved intent\n",
].sort(),
);
expect(graphql).toHaveBeenCalledWith(expect.stringContaining("updateRefs"), {
input: {
clientMutationId: commitSha,
Expand Down
47 changes: 29 additions & 18 deletions tools/pr-merge-conflict-fixer/publish.mts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { type ConflictMatrixEntry, parseConflictMatrixEntry } from "./discover.m
import {
applyResolutionPatch,
ConflictFixerError,
listTreeChanges,
prepareMerge,
replaceWithTree,
requireSha,
Expand Down Expand Up @@ -82,7 +81,6 @@ export function validateResolutionPatch(input: {
sourceRepository: string;
workDirectory: string;
}): {
changedPaths: string[];
finalTree: string;
repository: string;
} {
Expand All @@ -100,15 +98,7 @@ export function validateResolutionPatch(input: {
replaceWithTree(merge.repository, merge.conflictTree);
applyResolutionPatch(merge.repository, input.patchPath);
const finalTree = writeTree(merge.repository);
const changedPaths = listTreeChanges(merge.repository, merge.conflictTree, finalTree);
const allowedPaths = new Set(merge.conflictPaths);
const extraPaths = changedPaths.filter((file) => !allowedPaths.has(file));
if (extraPaths.length > 0) {
throw new ConflictFixerError(
`The resolution patch changes non-conflict paths: ${extraPaths.join(", ")}`,
);
}
return { changedPaths, finalTree, repository: merge.repository };
return { finalTree, repository: merge.repository };
}

function gitBuffer(repository: string, args: readonly string[]): Buffer {
Expand Down Expand Up @@ -149,8 +139,13 @@ function changedPathStatuses(
return results;
}

function treeEntry(repository: string, tree: string, filePath: string): GitTreeEntry {
function optionalTreeEntry(
repository: string,
tree: string,
filePath: string,
): GitTreeEntry | null {
const output = gitBuffer(repository, ["ls-tree", "-z", tree, "--", filePath]).toString("utf8");
if (!output) return null;
const separator = output.indexOf("\t");
if (separator < 0) throw new ConflictFixerError(`Git tree does not contain ${filePath}`);
const [mode, type, sha] = output.slice(0, separator).split(" ");
Expand All @@ -160,15 +155,29 @@ function treeEntry(repository: string, tree: string, filePath: string): GitTreeE
return { mode, path: filePath, sha, type };
}

function treeEntry(repository: string, tree: string, filePath: string): GitTreeEntry {
const entry = optionalTreeEntry(repository, tree, filePath);
if (!entry) throw new ConflictFixerError(`Git tree does not contain ${filePath}`);
return entry;
}

function parentContainsBlob(
repository: string,
parent: string,
entry: GitTreeEntry,
): boolean {
const parentEntry = optionalTreeEntry(repository, parent, entry.path);
return parentEntry?.type === "blob" && parentEntry.sha === entry.sha;
}

async function createGitHubTree(input: {
changedPaths: readonly string[];
baseSha: string;
finalTree: string;
headSha: string;
repository: string;
repositoryName: string;
request: GitHubRequest;
}): Promise<string> {
const changedPaths = new Set(input.changedPaths);
const entries: GitTreeEntry[] = [];
for (const change of changedPathStatuses(input.repository, input.headSha, input.finalTree)) {
const sourceTree = change.status === "D" ? input.headSha : input.finalTree;
Expand All @@ -177,7 +186,11 @@ async function createGitHubTree(input: {
entries.push({ ...entry, sha: null });
continue;
}
if (entry.type === "blob" && changedPaths.has(entry.path)) {
if (
entry.type === "blob" &&
!parentContainsBlob(input.repository, input.headSha, entry) &&
!parentContainsBlob(input.repository, input.baseSha, entry)
) {
const content = gitBuffer(input.repository, ["cat-file", "blob", entry.sha ?? ""]);
const created = (await input.request("POST", `/repos/${input.repositoryName}/git/blobs`, {
content: content.toString("base64"),
Expand All @@ -201,7 +214,6 @@ async function createGitHubTree(input: {
}

async function publishValidatedTree(input: {
changedPaths: readonly string[];
entry: ConflictMatrixEntry;
finalTree: string;
graphql: GraphqlRequest;
Expand All @@ -211,7 +223,7 @@ async function publishValidatedTree(input: {
request: GitHubRequest;
}): Promise<string> {
const tree = await createGitHubTree({
changedPaths: input.changedPaths,
baseSha: input.entry.base_sha,
finalTree: input.finalTree,
headSha: input.entry.head_sha,
repository: input.repository,
Expand Down Expand Up @@ -335,7 +347,6 @@ export async function publishResolution(input: {
workDirectory: path.join(temporaryDirectory, "repository"),
});
return await publishValidatedTree({
changedPaths: validated.changedPaths,
entry: input.entry,
finalTree: validated.finalTree,
graphql: input.graphql,
Expand Down
Loading