From 104ac00d04a007f305a118733760bf9fcbe3ce4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Tue, 13 Jan 2026 16:10:01 +0100 Subject: [PATCH 1/4] Don't error on symlinks and executables when they don't have to be touched anyway Co-authored-by: Dotan Simha --- src/git.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/git.ts b/src/git.ts index a6e2a80..1306a9b 100644 --- a/src/git.ts +++ b/src/git.ts @@ -74,6 +74,12 @@ export const commitChangesFromRepo = async ({ ) { return null; } + const prevOid = await commit?.oid(); + const currentOid = await workdir?.oid(); + // Don't include files that haven't changed, and exist in both trees + if (prevOid === currentOid && !commit === !workdir) { + return null; + } if ( (await commit?.mode()) === FILE_MODES.symlink || (await workdir?.mode()) === FILE_MODES.symlink @@ -87,12 +93,6 @@ export const commitChangesFromRepo = async ({ `Unexpected executable file at ${filepath}, GitHub API only supports non-executable files and directories. You may need to add this file to .gitignore`, ); } - const prevOid = await commit?.oid(); - const currentOid = await workdir?.oid(); - // Don't include files that haven't changed, and exist in both trees - if (prevOid === currentOid && !commit === !workdir) { - return null; - } // Iterate through anything that may be a directory in either the // current commit or the working directory if ( From 6cf7c10b2482da8e162b4b1c0cae768c9d511dd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Wed, 14 Jan 2026 11:43:21 +0100 Subject: [PATCH 2/4] add tests --- src/test/integration/git.test.ts | 89 +++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 2 deletions(-) diff --git a/src/test/integration/git.test.ts b/src/test/integration/git.test.ts index 2e46c60..456a88e 100644 --- a/src/test/integration/git.test.ts +++ b/src/test/integration/git.test.ts @@ -85,7 +85,9 @@ const makeFileChanges = async ( | "with-executable-file" | "with-ignored-symlink" | "with-included-valid-symlink" - | "with-included-invalid-symlink", + | "with-included-invalid-symlink" + | "with-unchanged-symlink" + | "with-changed-symlink", ) => { // Update an existing file await fs.promises.writeFile( @@ -164,6 +166,40 @@ const makeFileChanges = async ( path.join(repoDirectory, "some-dir", "nested"), ); } + if ( + changegroup === "with-unchanged-symlink" || + changegroup === "with-changed-symlink" + ) { + // Create a symlink and commit it locally first + await fs.promises.mkdir(path.join(repoDirectory, "some-dir"), { + recursive: true, + }); + await fs.promises.symlink( + path.join(repoDirectory, "README.md"), + path.join(repoDirectory, "some-dir", "nested"), + ); + // Commit the symlink locally so it's in the base commit tree + await git.add({ + fs, + dir: repoDirectory, + filepath: "some-dir/nested", + }); + await git.commit({ + fs, + dir: repoDirectory, + message: "Add symlink", + author: { name: "Test", email: "test@test.com" }, + }); + + if (changegroup === "with-changed-symlink") { + // Change the symlink to point to a different target + await fs.promises.rm(path.join(repoDirectory, "some-dir", "nested")); + await fs.promises.symlink( + path.join(repoDirectory, "LICENSE"), + path.join(repoDirectory, "some-dir", "nested"), + ); + } + } }; const makeFileChangeAssertions = async (branch: string) => { @@ -264,7 +300,11 @@ describe("git", () => { describe("commitChangesFromRepo", () => { const testDir = path.join(ROOT_TEMP_DIRECTORY, "commitChangesFromRepo"); - for (const group of ["standard", "with-ignored-symlink"] as const) { + for (const group of [ + "standard", + "with-ignored-symlink", + "with-unchanged-symlink", + ] as const) { it(`should correctly commit all changes for group: ${group}`, async () => { const branch = `${TEST_BRANCH_PREFIX}-multiple-changes-${group}`; branches.push(branch); @@ -414,6 +454,51 @@ describe("git", () => { "Unexpected symlink at some-dir/nested, GitHub API only supports files and directories. You may need to add this file to .gitignore", ); }); + + it(`and symlink was changed`, async () => { + const branch = `${TEST_BRANCH_PREFIX}-changed-symlink-error`; + branches.push(branch); + + await fs.promises.mkdir(testDir, { recursive: true }); + const repoDirectory = path.join(testDir, `repo-changed-symlink`); + + // Clone the git repo locally using the git cli and child-process + await new Promise((resolve, reject) => { + const p = execFile( + "git", + ["clone", process.cwd(), `repo-changed-symlink`], + { cwd: testDir }, + (error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }, + ); + p.stdout?.pipe(process.stdout); + p.stderr?.pipe(process.stderr); + }); + + await makeFileChanges(repoDirectory, "with-changed-symlink"); + + // Push the changes + await expect(() => + commitChangesFromRepo({ + octokit, + ...REPO, + branch, + message: { + headline: "Test commit", + body: "This is a test commit", + }, + cwd: repoDirectory, + log, + }), + ).rejects.toThrow( + "Unexpected symlink at some-dir/nested, GitHub API only supports files and directories. You may need to add this file to .gitignore", + ); + }); }); it(`should throw appropriate error when executable file is present`, async () => { From d560c1a5e0c2bf10f10677b7af26ca311b255eeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Thu, 15 Jan 2026 10:16:58 +0100 Subject: [PATCH 3/4] try this --- src/test/integration/git.test.ts | 67 +++++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 5 deletions(-) diff --git a/src/test/integration/git.test.ts b/src/test/integration/git.test.ts index 456a88e..9e73a86 100644 --- a/src/test/integration/git.test.ts +++ b/src/test/integration/git.test.ts @@ -300,11 +300,7 @@ describe("git", () => { describe("commitChangesFromRepo", () => { const testDir = path.join(ROOT_TEMP_DIRECTORY, "commitChangesFromRepo"); - for (const group of [ - "standard", - "with-ignored-symlink", - "with-unchanged-symlink", - ] as const) { + for (const group of ["standard", "with-ignored-symlink"] as const) { it(`should correctly commit all changes for group: ${group}`, async () => { const branch = `${TEST_BRANCH_PREFIX}-multiple-changes-${group}`; branches.push(branch); @@ -364,6 +360,67 @@ describe("git", () => { }); } + it(`should allow unchanged symlinks without throwing symlink error`, async () => { + const branch = `${TEST_BRANCH_PREFIX}-multiple-changes-with-unchanged-symlink`; + branches.push(branch); + + await fs.promises.mkdir(testDir, { recursive: true }); + const repoDirectory = path.join( + testDir, + `repo-1-with-unchanged-symlink`, + ); + + // Clone the git repo locally using the git cli and child-process + await new Promise((resolve, reject) => { + const p = execFile( + "git", + ["clone", process.cwd(), `repo-1-with-unchanged-symlink`], + { cwd: testDir }, + (error) => { + if (error) { + reject(error); + } else { + resolve(); + } + }, + ); + p.stdout?.pipe(process.stdout); + p.stderr?.pipe(process.stderr); + }); + + await makeFileChanges(repoDirectory, "with-unchanged-symlink"); + + // The symlink was committed locally and is unchanged in workdir. + // When using local HEAD as base, the symlink should be detected as + // unchanged and skipped (no symlink error thrown during tree walk). + // The actual GitHub push may fail because the local commit doesn't + // exist on GitHub, but that's OK - we're testing the tree walk logic. + try { + await commitChangesFromRepo({ + octokit, + ...REPO, + branch, + message: { + headline: "Test commit", + body: "This is a test commit", + }, + cwd: repoDirectory, + log, + }); + + // If push somehow succeeded, verify the file changes + await waitForGitHubToBeReady(); + await makeFileChangeAssertions(branch); + } catch (error) { + // The error should NOT be about symlinks - that would mean tree walk + // failed to skip the unchanged symlink + expect((error as Error).message).not.toContain("Unexpected symlink"); + expect((error as Error).message).not.toContain( + "Unexpected executable", + ); + } + }); + describe(`should throw appropriate error when symlink is present`, () => { it(`and file does not exist`, async () => { const branch = `${TEST_BRANCH_PREFIX}-invalid-symlink-error`; From 7f865d1c65b9010646204216f3ae1441e78c1e33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Thu, 15 Jan 2026 11:59:18 +0100 Subject: [PATCH 4/4] fmt --- src/test/integration/git.test.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/test/integration/git.test.ts b/src/test/integration/git.test.ts index 9e73a86..3875050 100644 --- a/src/test/integration/git.test.ts +++ b/src/test/integration/git.test.ts @@ -365,10 +365,7 @@ describe("git", () => { branches.push(branch); await fs.promises.mkdir(testDir, { recursive: true }); - const repoDirectory = path.join( - testDir, - `repo-1-with-unchanged-symlink`, - ); + const repoDirectory = path.join(testDir, `repo-1-with-unchanged-symlink`); // Clone the git repo locally using the git cli and child-process await new Promise((resolve, reject) => { @@ -415,9 +412,7 @@ describe("git", () => { // The error should NOT be about symlinks - that would mean tree walk // failed to skip the unchanged symlink expect((error as Error).message).not.toContain("Unexpected symlink"); - expect((error as Error).message).not.toContain( - "Unexpected executable", - ); + expect((error as Error).message).not.toContain("Unexpected executable"); } });