hearthforge

← Back to log@ fa83468

allow patch updates

AuthorKonata <konata@posteo.jp>
Date
Commitfa83468d3eaf4a191935bc33db3b6f7435364bd3
Parentacab051
7 files changed, 390 insertions(+), 53 deletions(-)
▾MREADME.md
+6-7@ fa83468@ master
@@ -34,13 +34,14 @@ bun run start # http://localhost:3000, SSH on port 2222
3434
3535 Default admin credentials: `admin` / `changeme`
3636
37-## Manual repository creation
37+## Manual repository interactions
3838
39-Repositories can bei either created through the UI, or existing ones can be copied manually to the `data/repos` directory.
39+Existing Repositories can be copied manually to the `data/repos` directory.
4040 Non-bare repos are automatically converted to bare repos on startup, discarding uncomitted changes and worktrees.
41+Pushing directly to the repositories (i.e. not through the bundled http/ssh endpoints) should generally works as well.
4142
4243 ### Docker / Podman
43-Container images are provided.
44+Container image and compose file are provided.
4445 ```bash
4546 docker compose up # or
4647 podman compose up
@@ -88,13 +89,11 @@ Pushing is also supported for the admin.
8889 bun run dev # watch mode
8990 bun run lint # Biome lint
9091 bun run format # Biome format
91-bun run test # Playwright E2E tests (don't use bun test, it doesn't respect the timeout)
92+bun run test # Run E2E and unit tests (don't use bun test, it doesn't respect the timeout)
9293 ```
9394
9495 ## Roadmap
9596 - Use [git-bug](https://github.com/git-bug/git-bug) for issue tracking instead of custom implementation
9697 - Issue labels
9798 - redirect image urls in readme
98-- registration queue
99-- edit patches
100-- show ^M in diffs
99+- registration queue
▾Mpackage.json
+1-1@ fa83468@ master
@@ -7,7 +7,7 @@
77 "css:build": "bun scripts/build-css.ts",
88 "db:init": "bun run src/db/init.ts",
99 "db:seed": "bun run src/db/seed.ts",
10- "test": "bun test tests/highlight.test.ts && bun test tests/e2e.test.ts --timeout 60000",
10+ "test": "bun test --timeout 60000",
1111 "vendor:simplewebauthn": "bun build node_modules/@simplewebauthn/browser/esm/index.js --outfile public/assets/simplewebauthn-browser.js --format esm",
1212 "vendor:jxl-polyfill": "cp node_modules/jxl-rs-polyfill/dist/auto.js public/assets/jxl-polyfill.js",
1313 "postinstall": "bun run vendor:simplewebauthn && bun run vendor:jxl-polyfill && bun run css:build",
▾Msrc/db/index.ts
+14@ fa83468@ master
@@ -86,6 +86,7 @@ interface PatchTable {
8686 created_at: string;
8787 updated_at: string;
8888 edited_at: string | null;
89+ version: string;
8990 }
9091
9192 interface PatchCommentTable {
@@ -168,6 +169,19 @@ const sqlite = new BunDatabase(DB_PATH);
168169 sqlite.run("PRAGMA journal_mode=WAL");
169170 sqlite.run("PRAGMA foreign_keys=ON");
170171
172+// Migration: add version column if missing, then populate any empty values
173+const patchCols = sqlite
174+ .query<{ name: string }, []>("PRAGMA table_info(patches)")
175+ .all();
176+if (!patchCols.some((c) => c.name === "version")) {
177+ sqlite.run(
178+ "ALTER TABLE patches ADD COLUMN version TEXT NOT NULL DEFAULT ''",
179+ );
180+}
181+sqlite.run(
182+ "UPDATE patches SET version = lower(hex(randomblob(16))) WHERE version = ''",
183+);
184+
171185 export const db = new Kysely<Database>({
172186 dialect: new BunSqliteDialect({ database: sqlite }),
173187 });
▾Msrc/db/schema.sql
+1@ fa83468@ master
@@ -84,6 +84,7 @@ CREATE TABLE IF NOT EXISTS patches (
8484 created_at TEXT NOT NULL,
8585 updated_at TEXT NOT NULL,
8686 edited_at TEXT,
87+ version TEXT NOT NULL DEFAULT '',
8788 UNIQUE(repo_id, number)
8889 );
8990
▾Msrc/routes/patches.tsx
+146-45@ fa83468@ master
@@ -266,6 +266,7 @@ export const patchRoutes = new Elysia()
266266 author_email: uploadMeta.email,
267267 created_at: now,
268268 updated_at: now,
269+ version: crypto.randomUUID(),
269270 })
270271 .returning("id")
271272 .executeTakeFirstOrThrow();
@@ -313,6 +314,7 @@ export const patchRoutes = new Elysia()
313314 "patches.created_at",
314315 "patches.updated_at",
315316 "patches.edited_at",
317+ "patches.version",
316318 "users.username as author_username",
317319 "users.avatar_version as author_avatar_version",
318320 ])
@@ -416,60 +418,159 @@ export const patchRoutes = new Elysia()
416418 },
417419 )
418420
419- .post("/:repo/patches/:number/merge", async ({ params, cookie }) => {
420- const user = await resolveSession(cookie.session.value);
421- const deny = requireAdmin(user);
422- if (deny) return deny;
421+ .post(
422+ "/:repo/patches/:number/merge",
423+ async ({ params, body, cookie }) => {
424+ const user = await resolveSession(cookie.session.value);
425+ const deny = requireAdmin(user);
426+ if (deny) return deny;
423427
424- const repo = await getRepo(params.repo, true);
425- if (!repo) return new Response("Not found", { status: 404 });
428+ const repo = await getRepo(params.repo, true);
429+ if (!repo) return new Response("Not found", { status: 404 });
426430
427- const patchNum = parseInt(params.number, 10);
428- const patch = await db
429- .selectFrom("patches")
430- .select(["id", "patch_content", "status"])
431- .where("repo_id", "=", repo.id)
432- .where("number", "=", patchNum)
433- .executeTakeFirst();
434- if (!patch) return new Response("Not found", { status: 404 });
431+ const patchNum = parseInt(params.number, 10);
432+ const patch = await db
433+ .selectFrom("patches")
434+ .select(["id", "patch_content", "status", "version"])
435+ .where("repo_id", "=", repo.id)
436+ .where("number", "=", patchNum)
437+ .executeTakeFirst();
438+ if (!patch) return new Response("Not found", { status: 404 });
435439
436- // Atomically claim the merge slot before the slow git operation to
437- // prevent two concurrent requests from both applying the same patch.
438- const claimed = await db
439- .updateTable("patches")
440- .set({ status: "merged", updated_at: new Date().toISOString() })
441- .where("id", "=", patch.id)
442- .where("status", "=", "open")
443- .executeTakeFirst();
444- if (!claimed || claimed.numUpdatedRows === 0n)
445- return new Response("Patch is not open", { status: 400 });
440+ // Reject if the patch file was changed after the admin loaded the page
441+ if (body.version !== patch.version) {
442+ return new Response(
443+ "The patch file was updated after you loaded this page. Please review the new version before merging.",
444+ { status: 409 },
445+ );
446+ }
446447
447- const mergeMeta = extractPatchMeta(patch.patch_content);
448- try {
449- await git.applyPatch(
450- repo.name,
451- patch.patch_content,
452- mergeMeta.author,
453- mergeMeta.email,
454- COMMITTER_NAME,
455- COMMITTER_EMAIL,
456- );
457- } catch (err) {
458- // Roll back the status if the git operation fails
448+ // Atomically claim the merge slot before the slow git operation to
449+ // prevent two concurrent requests from both applying the same patch.
450+ const claimed = await db
451+ .updateTable("patches")
452+ .set({ status: "merged", updated_at: new Date().toISOString() })
453+ .where("id", "=", patch.id)
454+ .where("status", "=", "open")
455+ .where("version", "=", patch.version)
456+ .executeTakeFirst();
457+ if (!claimed || claimed.numUpdatedRows === 0n)
458+ return new Response("Patch is not open", { status: 400 });
459+
460+ const mergeMeta = extractPatchMeta(patch.patch_content);
461+ try {
462+ await git.applyPatch(
463+ repo.name,
464+ patch.patch_content,
465+ mergeMeta.author,
466+ mergeMeta.email,
467+ COMMITTER_NAME,
468+ COMMITTER_EMAIL,
469+ );
470+ } catch (err) {
471+ // Roll back the status if the git operation fails
472+ await db
473+ .updateTable("patches")
474+ .set({
475+ status: "open",
476+ updated_at: new Date().toISOString(),
477+ })
478+ .where("id", "=", patch.id)
479+ .execute();
480+ throw err;
481+ }
482+ patchCache.invalidate(patch.id);
483+
484+ return new Response(null, {
485+ status: 302,
486+ headers: { Location: `/${repo.name}/patches/${patchNum}` },
487+ });
488+ },
489+ {
490+ body: t.Object({ version: t.String() }),
491+ },
492+ )
493+
494+ .post(
495+ "/:repo/patches/:number/upload",
496+ async ({ params, body, cookie }) => {
497+ const user = await resolveSession(cookie.session.value);
498+ const deny = requireAuth(user);
499+ if (deny) return deny;
500+ const repo = await getRepo(params.repo, user?.isAdmin ?? false);
501+ if (!repo) return new Response("Not found", { status: 404 });
502+
503+ const patchNum = parseInt(params.number, 10);
504+ const patch = await db
505+ .selectFrom("patches")
506+ .select(["id", "author_id", "status"])
507+ .where("repo_id", "=", repo.id)
508+ .where("number", "=", patchNum)
509+ .executeTakeFirst();
510+ if (!patch) return new Response("Not found", { status: 404 });
511+ if (patch.author_id !== user?.id && !user?.isAdmin)
512+ return new Response("Forbidden", { status: 403 });
513+ if (patch.status !== "open")
514+ return new Response("Patch is not open", { status: 400 });
515+
516+ if (!body.patch_file || body.patch_file.size === 0) {
517+ return new Response("Patch file is required", { status: 400 });
518+ }
519+ if (body.patch_file.size > MAX_USER_UPLOAD_BYTES) {
520+ return new Response("Patch file is too large", { status: 400 });
521+ }
522+
523+ const patchContent = await body.patch_file.text();
524+ if (!patchContent.trim()) {
525+ return new Response("Patch file is empty", { status: 400 });
526+ }
527+ if (!isValidPatch(patchContent)) {
528+ return new Response(
529+ "File does not appear to be a valid patch file",
530+ { status: 400 },
531+ );
532+ }
533+
534+ const uploadMeta = extractPatchMeta(patchContent);
535+ if (
536+ !uploadMeta.subject ||
537+ !uploadMeta.author ||
538+ !uploadMeta.email ||
539+ !uploadMeta.date
540+ ) {
541+ return new Response(
542+ "Patch is missing required headers (Subject, From, Date)",
543+ { status: 400 },
544+ );
545+ }
546+
547+ const newVersion = crypto.randomUUID();
459548 await db
460549 .updateTable("patches")
461- .set({ status: "open", updated_at: new Date().toISOString() })
550+ .set({
551+ patch_content: patchContent,
552+ author_name: uploadMeta.author,
553+ author_email: uploadMeta.email,
554+ version: newVersion,
555+ updated_at: new Date().toISOString(),
556+ })
462557 .where("id", "=", patch.id)
463558 .execute();
464- throw err;
465- }
466- patchCache.invalidate(patch.id);
467559
468- return new Response(null, {
469- status: 302,
470- headers: { Location: `/${repo.name}/patches/${patchNum}` },
471- });
472- })
560+ patchCache.invalidate(patch.id);
561+ runPatchCheck(repo.name, patch.id, patchContent);
562+
563+ return new Response(null, {
564+ status: 302,
565+ headers: { Location: `/${repo.name}/patches/${patchNum}` },
566+ });
567+ },
568+ {
569+ body: t.Object({
570+ patch_file: t.Optional(t.File()),
571+ }),
572+ },
573+ )
473574
474575 .post("/:repo/patches/:number/close", async ({ params, cookie }) => {
475576 const user = await resolveSession(cookie.session.value);
▾Msrc/views/patches/PatchDetail.tsx
+34@ fa83468@ master
@@ -133,6 +133,11 @@ export function PatchDetail({
133133 action={`${baseUrl}/merge`}
134134 class="inline-form"
135135 >
136+ <input
137+ type="hidden"
138+ name="version"
139+ value={patch.version}
140+ />
136141 <button
137142 type="submit"
138143 class="btn btn-sm btn-primary"
@@ -141,6 +146,35 @@ export function PatchDetail({
141146 </button>
142147 </form>
143148 )}
149+ {patch.status === "open" && (
150+ <details class="confirm-details">
151+ <summary class="btn btn-sm">
152+ Update patch file
153+ </summary>
154+ <div class="confirm-popup">
155+ <form
156+ method="POST"
157+ action={`${baseUrl}/upload`}
158+ enctype="multipart/form-data"
159+ class="upload-patch-form"
160+ >
161+ <input
162+ type="file"
163+ name="patch_file"
164+ accept=".patch"
165+ required
166+ class="form-input"
167+ />
168+ <button
169+ type="submit"
170+ class="btn btn-sm btn-primary"
171+ >
172+ Upload
173+ </button>
174+ </form>
175+ </div>
176+ </details>
177+ )}
144178 {user?.isAdmin && patch.status !== "merged" && (
145179 <form
146180 method="POST"
▾Mtests/e2e.test.ts
+188@ fa83468@ master
@@ -1037,6 +1037,194 @@ describe('patches', () => {
10371037 expect(checkResp.status()).toBe(404);
10381038 } finally { await page.close(); }
10391039 });
1040+
1041+ // ── Patch file re-upload & version protection ──────────────────────────────
1042+
1043+ let uploadTestPatchUrl: string;
1044+
1045+ // Adds upload-test.txt — applies cleanly to my-repo
1046+ const UPLOAD_TEST_PATCH = [
1047+ 'From c1d2e3f4a5b6c1d2e3f4a5b6c1d2e3f4a5b6c1d2 Mon Sep 17 00:00:00 2001',
1048+ 'From: Original Author <original@example.com>',
1049+ 'Date: Wed, 03 Jan 2024 10:00:00 +0000',
1050+ 'Subject: [PATCH] Add upload-test.txt',
1051+ '',
1052+ '---',
1053+ 'diff --git a/upload-test.txt b/upload-test.txt',
1054+ 'new file mode 100644',
1055+ 'index 0000000..9daeafb',
1056+ '--- /dev/null',
1057+ '+++ b/upload-test.txt',
1058+ '@@ -0,0 +1 @@',
1059+ '+upload test',
1060+ '',
1061+ ].join('\n');
1062+
1063+ // Replacement: different author, same diff target
1064+ const REPLACEMENT_PATCH = [
1065+ 'From d1e2f3a4b5c6d1e2f3a4b5c6d1e2f3a4b5c6d1e2 Mon Sep 17 00:00:00 2001',
1066+ 'From: Replaced Author <replaced@example.com>',
1067+ 'Date: Thu, 04 Jan 2024 10:00:00 +0000',
1068+ 'Subject: [PATCH] Add upload-test.txt (v2)',
1069+ '',
1070+ '---',
1071+ 'diff --git a/upload-test.txt b/upload-test.txt',
1072+ 'new file mode 100644',
1073+ 'index 0000000..9daeafb',
1074+ '--- /dev/null',
1075+ '+++ b/upload-test.txt',
1076+ '@@ -0,0 +1 @@',
1077+ '+upload test v2',
1078+ '',
1079+ ].join('\n');
1080+
1081+ test('create patch for re-upload tests', async () => {
1082+ writeTempFile('/tmp/upload-test.patch', UPLOAD_TEST_PATCH);
1083+ const page = await adminCtx.newPage();
1084+ try {
1085+ await page.goto(`${BASE}/my-repo/patches/new`);
1086+ await page.fill('[name=title]', 'Upload test patch');
1087+ await page.locator('[name=patch_file]').setInputFiles('/tmp/upload-test.patch');
1088+ await page.click('form[action$="/patches"] button[type=submit]');
1089+ await page.waitForURL(/\/my-repo\/patches\/\d+/);
1090+ uploadTestPatchUrl = page.url();
1091+ } finally { await page.close(); }
1092+ });
1093+
1094+ test('upload patch file button is visible for admin on open patch', async () => {
1095+ const page = await adminCtx.newPage();
1096+ try {
1097+ await page.goto(uploadTestPatchUrl);
1098+ expect(await page.locator('details:has([name=patch_file])').count()).toBe(1);
1099+ } finally { await page.close(); }
1100+ });
1101+
1102+ test('non-author non-admin cannot upload patch file', async () => {
1103+ const aliceCtx = await loggedInContext('alice', 'password123');
1104+ const page = await aliceCtx.newPage();
1105+ try {
1106+ const patchNum = uploadTestPatchUrl.split('/patches/')[1];
1107+ const resp = await page.request.post(`${BASE}/my-repo/patches/${patchNum}/upload`, {
1108+ multipart: { patch_file: { name: 'test.patch', mimeType: 'text/plain', buffer: Buffer.from(UPLOAD_TEST_PATCH) } },
1109+ maxRedirects: 0,
1110+ });
1111+ expect(resp.status()).toBe(403);
1112+ } finally { await aliceCtx.close(); }
1113+ });
1114+
1115+ test('upload button hidden for non-author non-admin', async () => {
1116+ const aliceCtx = await loggedInContext('alice', 'password123');
1117+ const page = await aliceCtx.newPage();
1118+ try {
1119+ await page.goto(uploadTestPatchUrl);
1120+ expect(await page.locator('details:has([name=patch_file])').count()).toBe(0);
1121+ } finally { await aliceCtx.close(); }
1122+ });
1123+
1124+ test('admin can upload replacement patch file', async () => {
1125+ writeTempFile('/tmp/replacement.patch', REPLACEMENT_PATCH);
1126+ const page = await adminCtx.newPage();
1127+ try {
1128+ await page.goto(uploadTestPatchUrl);
1129+ await page.locator('details:has([name=patch_file]) summary').click();
1130+ await page.locator('[name=patch_file]').setInputFiles('/tmp/replacement.patch');
1131+ await page.locator('details:has([name=patch_file]) button[type=submit]').click();
1132+ await page.waitForURL(new RegExp(uploadTestPatchUrl.replace(BASE, '')));
1133+ // Author info should reflect the replacement patch
1134+ expect(await page.locator('.patch-author-identity').textContent()).toContain('Replaced Author');
1135+ expect(await page.locator('.patch-author-identity').textContent()).toContain('replaced@example.com');
1136+ } finally { await page.close(); }
1137+ });
1138+
1139+ test('merge fails when version token is stale', async () => {
1140+ // Patch that adds stale-version.txt — applies cleanly
1141+ const STALE_PATCH = [
1142+ 'From e1f2a3b4c5d6e1f2a3b4c5d6e1f2a3b4c5d6e1f2 Mon Sep 17 00:00:00 2001',
1143+ 'From: Test User <test@example.com>',
1144+ 'Date: Fri, 05 Jan 2024 10:00:00 +0000',
1145+ 'Subject: [PATCH] Add stale-version.txt',
1146+ '',
1147+ '---',
1148+ 'diff --git a/stale-version.txt b/stale-version.txt',
1149+ 'new file mode 100644',
1150+ 'index 0000000..9daeafb',
1151+ '--- /dev/null',
1152+ '+++ b/stale-version.txt',
1153+ '@@ -0,0 +1 @@',
1154+ '+stale',
1155+ '',
1156+ ].join('\n');
1157+ const STALE_PATCH_V2 = STALE_PATCH
1158+ .replace('Add stale-version.txt', 'Add stale-version.txt (v2)')
1159+ .replace('+stale', '+stale v2');
1160+
1161+ writeTempFile('/tmp/stale.patch', STALE_PATCH);
1162+ const page = await adminCtx.newPage();
1163+ try {
1164+ // Create the patch
1165+ await page.goto(`${BASE}/my-repo/patches/new`);
1166+ await page.fill('[name=title]', 'Stale version test');
1167+ await page.locator('[name=patch_file]').setInputFiles('/tmp/stale.patch');
1168+ await page.click('form[action$="/patches"] button[type=submit]');
1169+ await page.waitForURL(/\/my-repo\/patches\/\d+/);
1170+ const stalePatchUrl = page.url();
1171+ const patchNum = stalePatchUrl.split('/patches/')[1];
1172+
1173+ // Capture the version the admin sees on the page
1174+ const staleVersion = await page.locator('form[action*="/merge"] [name=version]').inputValue();
1175+
1176+ // Author uploads a new patch file (simulated by admin here), bumping the version
1177+ writeTempFile('/tmp/stale-v2.patch', STALE_PATCH_V2);
1178+ await page.locator('details:has([name=patch_file]) summary').click();
1179+ await page.locator('[name=patch_file]').setInputFiles('/tmp/stale-v2.patch');
1180+ await page.locator('details:has([name=patch_file]) button[type=submit]').click();
1181+ await page.waitForURL(new RegExp(stalePatchUrl.replace(BASE, '')));
1182+
1183+ // Admin tries to merge with the stale version — should be rejected
1184+ const resp = await page.request.post(`${BASE}/my-repo/patches/${patchNum}/merge`, {
1185+ form: { version: staleVersion },
1186+ maxRedirects: 0,
1187+ });
1188+ expect(resp.status()).toBe(409);
1189+ expect(await resp.text()).toContain('updated');
1190+
1191+ // Patch status must still be open
1192+ const checkResp = await page.request.get(stalePatchUrl);
1193+ expect(checkResp.status()).toBe(200);
1194+ expect(await checkResp.text()).toContain('open');
1195+ } finally { await page.close(); }
1196+ });
1197+
1198+ test('merge succeeds with current version token after replacement upload', async () => {
1199+ const page = await adminCtx.newPage();
1200+ try {
1201+ await page.goto(uploadTestPatchUrl);
1202+ await page.click('form[action*="/merge"] button');
1203+ await page.waitForURL(new RegExp(uploadTestPatchUrl.replace(BASE, '')));
1204+ expect(await page.locator('.patch-badge').textContent()).toBe('merged');
1205+ // Merged commit should carry the replacement patch's author
1206+ const repoPath = `${process.cwd()}/data-test/repos/my-repo.git`;
1207+ const authorName = (await $`git -C ${repoPath} log -1 --format=%aN`.quiet()).text().trim();
1208+ expect(authorName).toBe('Replaced Author');
1209+ } finally { await page.close(); }
1210+ });
1211+
1212+ test('upload patch file button hidden on merged patch', async () => {
1213+ const page = await adminCtx.newPage();
1214+ try {
1215+ await page.goto(uploadTestPatchUrl);
1216+ expect(await page.locator('details:has([name=patch_file])').count()).toBe(0);
1217+ } finally { await page.close(); }
1218+ });
1219+
1220+ test('POST to upload on merged patch returns 400', async () => {
1221+ const patchNum = uploadTestPatchUrl.split('/patches/')[1];
1222+ const resp = await adminCtx.request.post(`${BASE}/my-repo/patches/${patchNum}/upload`, {
1223+ multipart: { patch_file: { name: 'test.patch', mimeType: 'text/plain', buffer: Buffer.from(UPLOAD_TEST_PATCH) } },
1224+ maxRedirects: 0,
1225+ });
1226+ expect(resp.status()).toBe(400);
1227+ });
10401228 });
10411229
10421230 // ─── Pagination ───────────────────────────────────────────────────────────────