From c5cf47aa7fbb31bcda891fd465250eebeb8bc727 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 21 Jun 2026 07:36:25 +0200 Subject: [PATCH] fix(coding-agent/utils): handled EISDIR and ENOTDIR errors during ref resolution - Update `shouldRetry` to treat `EISDIR` and `ENOTDIR` as terminal errors, preventing unnecessary retries when encountering Git reference directory conflicts. - Add a test suite to verify graceful resolution of branches in scenarios where a packed ref conflicts with a directory path in the filesystem. --- packages/coding-agent/src/utils/git.ts | 5 +- .../test/utils/git-eisdir-fallback.test.ts | 70 +++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 packages/coding-agent/test/utils/git-eisdir-fallback.test.ts diff --git a/packages/coding-agent/src/utils/git.ts b/packages/coding-agent/src/utils/git.ts index 16dcfbc6b..33f875a80 100644 --- a/packages/coding-agent/src/utils/git.ts +++ b/packages/coding-agent/src/utils/git.ts @@ -1,7 +1,7 @@ import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; -import { $which, hasFsCode, isEnoent, Snowflake } from "@oh-my-pi/pi-utils"; +import { $which, hasFsCode, isEisdir, isEnoent, isEnotdir, Snowflake } from "@oh-my-pi/pi-utils"; import { parseDiffHunks as parseCommitDiffHunks, parseFileDiffs, @@ -376,7 +376,8 @@ async function writeTempPatch(content: string): Promise { type EntryType = "directory" | "file"; function shouldRetry(err: unknown, n: number) { - if (isEnoent(err) || hasFsCode(err, "ENFILE") || hasFsCode(err, "EMFILE")) return false; + if (isEnoent(err) || isEisdir(err) || isEnotdir(err) || hasFsCode(err, "ENFILE") || hasFsCode(err, "EMFILE")) + return false; if (hasFsCode(err, "EINTR")) return n < EINTR_MAX_RETRIES; if (n > EINTR_MAX_RETRIES) throw err; throw err; diff --git a/packages/coding-agent/test/utils/git-eisdir-fallback.test.ts b/packages/coding-agent/test/utils/git-eisdir-fallback.test.ts new file mode 100644 index 000000000..16d7f0de2 --- /dev/null +++ b/packages/coding-agent/test/utils/git-eisdir-fallback.test.ts @@ -0,0 +1,70 @@ +import { afterAll, beforeAll, describe, expect, test } from "bun:test"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; +import { $ } from "bun"; +import * as git from "../../src/utils/git"; + +describe("git reference directory fallback", () => { + let repoDir: string; + let commitSha: string; + + beforeAll(async () => { + repoDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-ref-fallback-")); + const initResult = await $`git init --initial-branch=main`.cwd(repoDir).quiet(); + if (initResult.exitCode !== 0) throw new Error("git init failed"); + await $`git config user.name "Test User"`.cwd(repoDir).quiet(); + await $`git config user.email "test@example.com"`.cwd(repoDir).quiet(); + await fs.writeFile(path.join(repoDir, "file.txt"), "hello world"); + await $`git add file.txt`.cwd(repoDir).quiet(); + await $`git commit -m "initial commit"`.cwd(repoDir).quiet(); + + commitSha = (await $`git rev-parse HEAD`.cwd(repoDir).quiet().text()).trim(); + + // We will simulate a situation where: + // There is a branch called "pi-flash" which is packed (in packed-refs). + // But there is also a subdirectory created inside refs/heads under the same name: + // refs/heads/pi-flash/... because a branch called "pi-flash/something" exists as a loose ref. + // Thus: + // 1. refs/heads/pi-flash is a directory on disk. + // 2. packed-refs contains: refs/heads/pi-flash + // In this case, trying to read refs/heads/pi-flash as a file throws EISDIR. + // We want to verify that we gracefully handle this and fallback to resolving from packed-refs. + + // Let's pack the current main branch as refs/heads/pi-flash so we have it in packed-refs + await $`git update-ref refs/heads/pi-flash ${commitSha}`.cwd(repoDir).quiet(); + // Also point HEAD to refs/heads/pi-flash so we exercise HEAD -> ref -> readRef / readRefSync + await fs.writeFile(path.join(repoDir, ".git", "HEAD"), "ref: refs/heads/pi-flash\n"); + await $`git pack-refs --all`.cwd(repoDir).quiet(); + // Check that packed-refs exists + const packedRefs = await fs.readFile(path.join(repoDir, ".git", "packed-refs"), "utf8"); + expect(packedRefs).toContain("refs/heads/pi-flash"); + + // Delete the loose ref file for refs/heads/pi-flash if git pack-refs didn't already delete it (it usually does). + await fs.rm(path.join(repoDir, ".git", "refs", "heads", "pi-flash"), { force: true }); + + // Now, create refs/heads/pi-flash as a directory to simulate another branch like "pi-flash/feature" existing. + // We can just create the directory and a file inside it, or just the directory. + await fs.mkdir(path.join(repoDir, ".git", "refs", "heads", "pi-flash"), { recursive: true }); + await fs.writeFile(path.join(repoDir, ".git", "refs", "heads", "pi-flash", "feature"), commitSha); + }); + + afterAll(async () => { + await fs.rm(repoDir, { recursive: true, force: true }).catch(() => {}); + }); + + test("resolves branch that has directory conflict via resolveSync on head", () => { + const headStateSync = git.head.resolveSync(repoDir); + expect(headStateSync).not.toBeNull(); + if (!headStateSync) return; + expect(headStateSync.commit).toBe(commitSha); + }); + + test("resolves branch that has directory conflict via resolve on ref", async () => { + const repository = await git.repo.resolve(repoDir); + expect(repository).not.toBeNull(); + if (!repository) return; + const resolved = await git.ref.resolve(repoDir, "refs/heads/pi-flash"); + expect(resolved).toBe(commitSha); + }); +});