Commit4f4cb3a
fix(editor): web editor destroyed directory structure on nested repos
fix(editor): web editor destroyed directory structure on nested repos
Both editor write paths built their commit by piping `git ls-tree -r` into
`git mktree`:
ls-tree -r emits a FLAT recursive listing whose names are full paths
("100644 blob <sha>\tsrc/lib/foo.ts")
mktree builds ONE tree object and expects bare filenames for a single
directory level
So any repository containing a subdirectory produced a rejected or malformed
tree. Creating or editing a file through the web editor lost the structure —
data loss on a core feature.
It stayed silent because neither handler checked mktree's exit status: an
empty or bogus tree sha went straight into `git commit-tree`, and the handler
redirected to the blob view regardless, so the user saw a normal-looking
"saved" flow.
Both handlers now delegate to createOrUpdateFileOnBranch(), which already
does this correctly — read-tree + update-index --cacheinfo + write-tree
against an isolated temporary index, handling arbitrary nesting and immune to
a concurrent write stomping the repo's real index. It also creates the commit
and moves the ref, so ~120 lines of hand-rolled plumbing disappear from
editor.tsx.
A failed write now redirects back to the editor with a message instead of
sending the user to a blob view that 404s or shows stale content, and a
concurrent edit is distinguished from a write failure via the helper's
existing sha-mismatch result.
The test is verified rather than assumed: reintroducing the exact
`ls-tree -r` → `mktree` pattern fails two assertions, and reverting passes.
Assertions run against comment-stripped source, because the explanation of
the bug necessarily contains the strings being asserted absent — that has
falsely tripped three separate assertions earlier in this session.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>2 files changed+145−1424f4cb3aa20f9b3143fffe1663ec3c470c5a848b2
2 changed files+145−142
Addedsrc/__tests__/editor-nested-paths.test.ts+88−0View fileUnifiedSplit
@@ -0,0 +1,88 @@
1/**
2 * The web editor must not destroy directory structure.
3 *
4 * Both editor write paths built their commit by piping `git ls-tree -r` into
5 * `git mktree`:
6 *
7 * ls-tree -r emits a FLAT, recursive listing whose names are full paths
8 * ("100644 blob <sha>\tsrc/lib/foo.ts")
9 * mktree builds ONE tree object and expects bare filenames for a
10 * single directory level
11 *
12 * So any repository with a subdirectory produced a rejected or malformed
13 * tree. Editing or creating a file in a nested repo silently lost the
14 * structure — data loss on a core feature, and invisible because the handler
15 * never checked mktree's exit status before running commit-tree.
16 *
17 * Both handlers now delegate to createOrUpdateFileOnBranch(), which uses
18 * read-tree + update-index --cacheinfo + write-tree against an isolated
19 * temporary index. That handles arbitrary nesting and cannot be stomped by a
20 * concurrent write.
21 *
22 * These are structural: a functional test needs a real bare repo with nested
23 * directories on disk, and git/repository.ts already owns that behaviour.
24 * What must not regress is that the editor no longer hand-rolls it.
25 */
26
27import { describe, expect, it } from "bun:test";
28import { readFileSync } from "fs";
29
30const SRC = readFileSync("src/routes/editor.tsx", "utf8");
31// Comments explain the old bug and necessarily contain the strings being
32// asserted absent — assert on code, not prose.
33const CODE = SRC.replace(/\/\*[\s\S]*?\*\//g, "").replace(/^\s*\/\/.*$/gm, "");
34
35describe("the editor no longer hand-rolls tree construction", () => {
36 it("calls git mktree nowhere", () => {
37 expect(CODE).not.toContain('["git", "mktree"]');
38 expect(CODE).not.toMatch(/"mktree"/);
39 });
40
41 it("does not feed a recursive ls-tree into a tree builder", () => {
42 // The specific combination that caused the loss.
43 expect(CODE).not.toMatch(/"ls-tree",\s*"-r"/);
44 });
45
46 it("does not hand-roll commit-tree either", () => {
47 // commit-tree on a malformed tree is how this stayed silent: the handler
48 // never checked mktree's exit code before committing.
49 expect(CODE).not.toContain("commit-tree");
50 });
51});
52
53describe("both write paths use the shared helper", () => {
54 it("create-file and edit-file both call createOrUpdateFileOnBranch", () => {
55 const calls = CODE.match(/createOrUpdateFileOnBranch\(\{/g) ?? [];
56 expect(calls.length).toBe(2);
57 });
58
59 it("both surface a write failure instead of redirecting to a broken blob", () => {
60 // Previously a failed write still redirected to the file view, which then
61 // 404'd or showed stale content — the user had no idea it had not saved.
62 const guards = CODE.match(/if \("error" in written\)/g) ?? [];
63 expect(guards.length).toBe(2);
64 });
65
66 it("distinguishes a concurrent edit from a write failure", () => {
67 expect(CODE).toContain('written.error === "sha-mismatch"');
68 });
69});
70
71describe("the helper it delegates to handles nesting", () => {
72 const REPO = readFileSync("src/git/repository.ts", "utf8");
73 const start = REPO.indexOf("export async function createOrUpdateFileOnBranch");
74 // To the next top-level export, not a fixed char count — 3000 chars stopped
75 // short of write-tree and the assertion failed on a correct implementation.
76 const nextExport = REPO.indexOf("\nexport ", start + 10);
77 const fn = REPO.slice(start, nextExport > -1 ? nextExport : REPO.length);
78
79 it("seeds an index from the parent tree rather than rebuilding one flat tree", () => {
80 expect(fn).toContain('"git", "read-tree"');
81 expect(fn).toContain("update-index");
82 expect(fn).toContain("write-tree");
83 });
84
85 it("uses an isolated index so concurrent writes cannot stomp each other", () => {
86 expect(fn).toContain("GIT_INDEX_FILE");
87 });
88});
Modifiedsrc/routes/editor.tsx+57−142View fileUnifiedSplit
@@ -1285,84 +1285,37 @@ editor.post("/:owner/:repo/new/:ref", requireAuth, requireRepoAccess("write"), a
12851285
12861286 const fullPath = dirPath ? `${dirPath}/${filename}` : filename;
12871287
1288 // Use git hash-object + update-index + write-tree + commit-tree
1289 const repoDir = getRepoPath(owner, repo);
1290
1291 const run = async (cmd: string[], cwd: string, stdin?: string) => {
1292 const proc = Bun.spawn(cmd, {
1293 cwd,
1294 stdout: "pipe",
1295 stderr: "pipe",
1296 stdin: stdin !== undefined ? "pipe" : undefined,
1297 });
1298 if (stdin !== undefined && proc.stdin) {
1299 proc.stdin.write(new TextEncoder().encode(stdin));
1300 proc.stdin.end();
1301 }
1302 const stdout = await new Response(proc.stdout).text();
1303 await proc.exited;
1304 return stdout.trim();
1305 };
1306
1307 // Hash the new file content
1308 const blobSha = await run(
1309 ["git", "hash-object", "-w", "--stdin"],
1310 repoDir,
1311 content
1312 );
1313
1314 // Read current tree
1315 const currentTreeSha = await run(
1316 ["git", "rev-parse", `${ref}^{tree}`],
1317 repoDir
1318 );
1319
1320 // Read current tree and add new entry
1321 const treeContent = await run(["git", "ls-tree", "-r", ref], repoDir);
1322 const entries = treeContent
1323 .split("\n")
1324 .filter(Boolean)
1325 .map((line) => line + "\n")
1326 .join("");
1327 const newEntry = `100644 blob ${blobSha}\t${fullPath}\n`;
1328
1329 const newTreeSha = await run(
1330 ["git", "mktree"],
1331 repoDir,
1332 entries + newEntry
1333 );
1334
1335 // Get parent commit
1336 const parentSha = await run(
1337 ["git", "rev-parse", ref],
1338 repoDir
1339 );
1340
1341 // Create commit
1342 const env = {
1343 GIT_AUTHOR_NAME: user.displayName || user.username,
1344 GIT_AUTHOR_EMAIL: user.email,
1345 GIT_COMMITTER_NAME: user.displayName || user.username,
1346 GIT_COMMITTER_EMAIL: user.email,
1347 };
1348
1349 const commitProc = Bun.spawn(
1350 ["git", "commit-tree", newTreeSha, "-p", parentSha, "-m", message],
1351 {
1352 cwd: repoDir,
1353 stdout: "pipe",
1354 stderr: "pipe",
1355 env: { ...process.env, ...env },
1356 }
1357 );
1358 const commitSha = (await new Response(commitProc.stdout).text()).trim();
1359 await commitProc.exited;
1360
1361 // Update branch ref
1362 await run(
1363 ["git", "update-ref", `refs/heads/${ref}`, commitSha],
1364 repoDir
1365 );
1288 // Was: `git ls-tree -r` piped into `git mktree`. ls-tree -r emits a FLAT,
1289 // recursive listing whose names are full paths (src/lib/foo.ts), while
1290 // mktree builds a SINGLE tree object and expects bare filenames. Any repo
1291 // with a subdirectory therefore produced a rejected or malformed tree —
1292 // silent data loss on the web editor.
1293 //
1294 // createOrUpdateFileOnBranch already does this correctly: read-tree +
1295 // update-index --cacheinfo + write-tree against an isolated temporary
1296 // index, which handles arbitrary nesting and cannot be stomped by a
1297 // concurrent write. It also creates the commit and moves the ref.
1298 const { createOrUpdateFileOnBranch } = await import("../git/repository");
1299 const written = await createOrUpdateFileOnBranch({
1300 owner,
1301 name: repo,
1302 branch: ref,
1303 filePath: fullPath,
1304 bytes: new TextEncoder().encode(content),
1305 message,
1306 authorName: user.displayName || user.username,
1307 authorEmail: user.email,
1308 });
1309
1310 if ("error" in written) {
1311 return c.redirect(
1312 `/${owner}/${repo}/new/${ref}?error=${encodeURIComponent(
1313 written.error === "sha-mismatch"
1314 ? "The file changed while you were editing it."
1315 : "Could not write the file."
1316 )}`
1317 );
1318 }
13661319
13671320 return c.redirect(`/${owner}/${repo}/blob/${ref}/${fullPath}`);
13681321});
@@ -1669,70 +1622,32 @@ editor.post("/:owner/:repo/edit/:ref{.+$}", requireAuth, requireRepoAccess("writ
16691622 body.message || `Update ${filePath.split("/").pop()}`
16701623 ).trim();
16711624
1672 const repoDir = getRepoPath(owner, repo);
1673
1674 const run = async (cmd: string[], cwd: string, stdin?: string) => {
1675 const proc = Bun.spawn(cmd, {
1676 cwd,
1677 stdout: "pipe",
1678 stderr: "pipe",
1679 stdin: stdin !== undefined ? "pipe" : undefined,
1680 });
1681 if (stdin !== undefined && proc.stdin) {
1682 proc.stdin.write(new TextEncoder().encode(stdin));
1683 proc.stdin.end();
1684 }
1685 const stdout = await new Response(proc.stdout).text();
1686 await proc.exited;
1687 return stdout.trim();
1688 };
1689
1690 // Hash new content
1691 const blobSha = await run(
1692 ["git", "hash-object", "-w", "--stdin"],
1693 repoDir,
1694 content
1695 );
1696
1697 // Read current tree, replace the file
1698 const treeContent = await run(["git", "ls-tree", "-r", ref], repoDir);
1699 const lines = treeContent.split("\n").filter(Boolean);
1700 const updated = lines
1701 .map((line) => {
1702 const parts = line.match(/^(\d+) (\w+) ([0-9a-f]+)\t(.+)$/);
1703 if (parts && parts[4] === filePath) {
1704 return `${parts[1]} blob ${blobSha}\t${parts[4]}`;
1705 }
1706 return line;
1707 })
1708 .join("\n") + "\n";
1709
1710 const newTreeSha = await run(["git", "mktree"], repoDir, updated);
1711 const parentSha = await run(["git", "rev-parse", ref], repoDir);
1712
1713 const env = {
1714 GIT_AUTHOR_NAME: user.displayName || user.username,
1715 GIT_AUTHOR_EMAIL: user.email,
1716 GIT_COMMITTER_NAME: user.displayName || user.username,
1717 GIT_COMMITTER_EMAIL: user.email,
1718 };
1719
1720 const commitProc = Bun.spawn(
1721 ["git", "commit-tree", newTreeSha, "-p", parentSha, "-m", message],
1722 {
1723 cwd: repoDir,
1724 stdout: "pipe",
1725 stderr: "pipe",
1726 env: { ...process.env, ...env },
1727 }
1728 );
1729 const commitSha = (await new Response(commitProc.stdout).text()).trim();
1730 await commitProc.exited;
1731
1732 await run(
1733 ["git", "update-ref", `refs/heads/${ref}`, commitSha],
1734 repoDir
1735 );
1625 // Same defect as the create-file handler above: `git ls-tree -r` piped
1626 // into `git mktree`. Recursive listing gives full paths; mktree builds one
1627 // flat tree and wants bare filenames, so every repo with a subdirectory
1628 // got a rejected or malformed tree. Delegate to the helper that already
1629 // does it correctly.
1630 const { createOrUpdateFileOnBranch } = await import("../git/repository");
1631 const written = await createOrUpdateFileOnBranch({
1632 owner,
1633 name: repo,
1634 branch: ref,
1635 filePath,
1636 bytes: new TextEncoder().encode(content),
1637 message,
1638 authorName: user.displayName || user.username,
1639 authorEmail: user.email,
1640 });
1641
1642 if ("error" in written) {
1643 return c.redirect(
1644 `/${owner}/${repo}/edit/${ref}/${filePath}?error=${encodeURIComponent(
1645 written.error === "sha-mismatch"
1646 ? "The file changed while you were editing it."
1647 : "Could not save the file."
1648 )}`
1649 );
1650 }
17361651
17371652 return c.redirect(`/${owner}/${repo}/blob/${ref}/${filePath}`);
17381653});
17391654