Skip to content

fix(github): return 409 for ref lookups in an empty repository #3

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
byapparov merged 1 commit into from
Nov 6, 2026
Merged

fix(github): return 409 for ref lookups in an empty repository #3

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
24 changes: 24 additions & 0 deletions packages/@emulators/github/src/__tests__/contents-commits.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1678,6 +1678,30 @@ describe("GitHub commits routes", () => {
expect(res.status).toBe(409);
});

it("409s on a branch ref lookup in an empty repository, like GitHub", async () => {
const res = await app.request(`${base}/repos/octocat/empty-repo/git/ref/heads/main`, { headers: authHeaders() });
expect(res.status).toBe(409);
expect(((await res.json()) as { message: string }).message).toBe("Git Repository is empty.");
});

it("still 404s on a missing branch ref in a populated repository", async () => {
const res = await app.request(`${base}/repos/octocat/hello-world/git/ref/heads/no-such-branch`, {
headers: authHeaders(),
});
expect(res.status).toBe(404);
});

it("serves the branch ref once an empty repository receives its first commit", async () => {
const created = await app.request(`${base}/repos/octocat/empty-repo/contents/README.md`, {
method: "PUT",
headers: jsonHeaders(),
body: JSON.stringify({ message: "first", content: Buffer.from("hi\n").toString("base64") }),
});
expect(created.status).toBe(201);
const res = await app.request(`${base}/repos/octocat/empty-repo/git/ref/heads/main`, { headers: authHeaders() });
expect(res.status).toBe(200);
});

it("returns a single commit with files and stats", async () => {
const created = await putFile(app, "notes.txt", "one\ntwo\n");
const createdBody = (await created.json()) as { commit: { sha: string } };
Expand Down
5 changes: 5 additions & 0 deletions packages/@emulators/github/src/routes/branches.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,11 @@ export function branchesAndGitRoutes({ app, store, webhooks, baseUrl }: RouteCon
const repo = lookupRepo(gh, owner, repoName);
if (!repo) throw notFoundResponse();
assertRepoContentsRead(gh, c.get("authUser"), repo);
// GitHub answers a ref lookup in a repository with no commits with 409,
// not 404, so clients can tell "empty" from "missing branch".
if (gh.commits.findBy("repo_id", repo.id).length === 0) {

@aictrl-dev aictrl-dev Bot Nov 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 409 gate counts commits; GitHub empties have no refs.

🤖 Fix with your agent 
Fix this code review finding (aictrl-dev/github-emulator PR #3, packages/@emulators/github/src/routes/branches.ts:692-694):

Problem: 409 gate counts commits; GitHub empties have no refs
Detail: The new gate treats a repo as empty when it has zero commits, but GitHub's "Git Repository is empty." 409 fires for a repository without branches (no refs). The emulator allows creating dangling Git objects without any ref: existing tests (e.g. "keeps the branch head before newer-dated parents") POST /git/commits and only afterwards PATCH /git/refs — so the store can hold commits with zero refs. In that state this endpoint now falls through to the ref lookup and returns 404, while real GitHub would still answer 409 "Git Repository is empty." — the exact case the PR was written to match. Gating on the refs table is also the self-consistent predicate for a ref endpoint. Repro: Given octocat/empty-repo has no refs; When the client seeds dangling objects via POST /git/blobs, POST /git/trees, POST /git/commits (all 201, no ref created); And calls GET /repos/octocat/empty-repo/git/ref/heads/main; Then the emulator responds 404 but real GitHub responds 409 "Git Repository is empty.".
Suggested fix: Gate on refs instead of commits: `if (gh.refs.findBy("repo_id", repo.id).length === 0) throw new ApiError(409, "Git Repository is empty.");` — matching GitHub's "no branches" definition of an empty repository. If adopted, apply the same predicate to the commits-list handler (commits.ts:99) via the shared helper so the two endpoints stay consistent.

Implement the fix on the PR head branch and add a regression test that fails before the fix and passes after.
Why this matters

The new gate treats a repo as empty when it has zero commits, but GitHub's "Git Repository is empty." 409 fires for a repository without branches (no refs). The emulator allows creating dangling Git objects without any ref: existing tests (e.g. "keeps the branch head before newer-dated parents") POST /git/commits and only afterwards PATCH /git/refs — so the store can hold commits with zero refs. In that state this endpoint now falls through to the ref lookup and returns 404, while real GitHub would still answer 409 "Git Repository is empty." — the exact case the PR was written to match. Gating on the refs table is also the self-consistent predicate for a ref endpoint. Repro: Given octocat/empty-repo has no refs; When the client seeds dangling objects via POST /git/blobs, POST /git/trees, POST /git/commits (all 201, no ref created); And calls GET /repos/octocat/empty-repo/git/ref/heads/main; Then the emulator responds 404 but real GitHub responds 409 "Git Repository is empty.".

    const repo = lookupRepo(gh, owner, repoName);
    if (!repo) throw notFoundResponse();
    assertRepoContentsRead(gh, c.get("authUser"), repo);
    // GitHub answers a ref lookup in a repository with no commits with 409,
    // not 404, so clients can tell "empty" from "missing branch".
    if (gh.commits.findBy("repo_id", repo.id).length === 0) {
      throw new ApiError(409, "Git Repository is empty.");
    }
    const fullRef = fullRefFromParam(refParam);
    const r = gh.refs.findBy("repo_id", repo.id).find((x) => x.ref === fullRef);
    if (!r) throw notFoundResponse();
    return c.json(formatRefJson(gh, repo, r.ref, r.sha, baseUrl));

@aictrl-dev aictrl-dev Bot Nov 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Empty-repo 409 guard duplicated from commits.ts.

🤖 Fix with your agent 
Fix this code review finding (aictrl-dev/github-emulator PR #3, packages/@emulators/github/src/routes/branches.ts:692-694):

Problem: Empty-repo 409 guard duplicated from commits.ts
Detail: The new inline guard (branches.ts:692-694) is a verbatim copy of the existing block in the commits-list handler GET /repos/:owner/:repo/commits (packages/@emulators/github/src/routes/commits.ts:99-101): same predicate, same status code, same literal "Git Repository is empty." string. Two hand-maintained copies of the same rule across route files will drift (message wording or the emptiness definition), and the sibling matching-refs handler will need a third copy. The codebase already centralises route guards (notFoundResponse, assertRepoContentsRead) in route-helpers.ts, so this predicate belongs there too.
Suggested fix: Add e.g. `export function assertRepoNotEmpty(gh: GitHubStore, repo: GitHubRepo): void { if (gh.commits.findBy("repo_id", repo.id).length === 0) throw new ApiError(409, "Git Repository is empty."); }` to packages/@emulators/github/src/route-helpers.ts and call it from both the /commits handler (commits.ts:99) and the /git/ref handler (branches.ts:692), deleting the duplicated inline copies (and reuse it in matching-refs when that gap is fixed).

Implement the fix on the PR head branch and add a regression test that fails before the fix and passes after.
Why this matters

The new inline guard (branches.ts:692-694) is a verbatim copy of the existing block in the commits-list handler GET /repos/:owner/:repo/commits (packages/@emulators/github/src/routes/commits.ts:99-101): same predicate, same status code, same literal "Git Repository is empty." string. Two hand-maintained copies of the same rule across route files will drift (message wording or the emptiness definition), and the sibling matching-refs handler will need a third copy. The codebase already centralises route guards (notFoundResponse, assertRepoContentsRead) in route-helpers.ts, so this predicate belongs there too.

    const repo = lookupRepo(gh, owner, repoName);
    if (!repo) throw notFoundResponse();
    assertRepoContentsRead(gh, c.get("authUser"), repo);
    // GitHub answers a ref lookup in a repository with no commits with 409,
    // not 404, so clients can tell "empty" from "missing branch".
    if (gh.commits.findBy("repo_id", repo.id).length === 0) {
      throw new ApiError(409, "Git Repository is empty.");
    }
    const fullRef = fullRefFromParam(refParam);
    const r = gh.refs.findBy("repo_id", repo.id).find((x) => x.ref === fullRef);
    if (!r) throw notFoundResponse();
    return c.json(formatRefJson(gh, repo, r.ref, r.sha, baseUrl));

throw new ApiError(409, "Git Repository is empty.");
}
const fullRef = fullRefFromParam(refParam);
const r = gh.refs.findBy("repo_id", repo.id).find((x) => x.ref === fullRef);
if (!r) throw notFoundResponse();
Expand Down
Loading