From e6c49af5e84d1c1d815bcd14e01c8a8c034020b9 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Fable 5.1)" Date: Thu, 3 Sep 2026 15:11:40 +0200 Subject: [PATCH 1/3] feat: the inbox daemon keeps answering while git works, a dismissal holds for one head, and rows show the forge's timestamps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The daemon ran git synchronously — fetch and worktree add while preparing, worktree remove on a dismissal or a retirement, and the bundle export and import — and every one of them froze its own HTTP server for as long as it took, which on a node_modules-laden checkout is minutes: an open spun "waiting for localhost" through a tick, and a dismiss looked like it did nothing until the removal finished. All of it is async now. A dismissal marks the row and answers before the directory goes, the page drops the row at once, and the dismissal holds only for that head: new commits are a new change and are taken from the top, as a skip already was. Rows carry the pull request's created and updated times. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Bwp5QefjsjMFeq6CK6cT6w --- README.md | 2 +- package-lock.json | 12 +++--- packages/api/package.json | 2 +- packages/cli/package.json | 2 +- packages/cli/src/inbox/daemon.ts | 14 ++++--- packages/cli/src/inbox/open-session.ts | 11 +++--- packages/cli/src/inbox/page.ts | 31 +++++++++++---- packages/cli/src/inbox/prepare.ts | 12 +++--- packages/cli/src/inbox/reconcile.ts | 8 ++-- packages/cli/src/inbox/runtime.ts | 9 +++-- packages/cli/src/inbox/store.ts | 37 +++++++++++------ packages/cli/src/inbox/tick.ts | 4 +- packages/cli/src/inbox/view.ts | 4 ++ packages/cli/src/inbox/worktree.ts | 46 ++++++++++++---------- packages/cli/tests/inbox-open.test.ts | 2 +- packages/cli/tests/inbox-prepare.test.ts | 2 +- packages/cli/tests/inbox-reconcile.test.ts | 10 +++-- packages/cli/tests/inbox-store.test.ts | 24 ++++++++++- packages/cli/tests/inbox-tick.test.ts | 26 ++++++++++-- packages/cli/tests/inbox-units.test.ts | 5 ++- packages/git/package.json | 2 +- packages/github/package.json | 2 +- packages/github/src/inbox.ts | 4 +- packages/parser/package.json | 2 +- packages/ui/package.json | 2 +- 25 files changed, 184 insertions(+), 91 deletions(-) diff --git a/README.md b/README.md index 42ecfb76..ba5695d6 100644 --- a/README.md +++ b/README.md @@ -332,7 +332,7 @@ running past the end would otherwise be counted and highlighted with nothing to ## The review inbox -`diffity inbox` watches the pull requests awaiting your review and prepares each one ahead of time, so the review is ready the moment you look. It polls GitHub (`gh search prs --review-requested=@me`), and for each pull request worth your attention it cuts a worktree at the PR head, runs a diffity session over the diff, has an agent prepare a review with a walkthrough, and saves the result as a bundle. New commits redo a stale review; a merged, closed, or no-longer-requested PR is retired. At most `maxPrepared` reviews are kept prepared at a time — the rest wait in the queue, smallest first — and a prepared review leaves the inbox once you have posted it (GitHub withdraws the request) or dismissed it from the page. +`diffity inbox` watches the pull requests awaiting your review and prepares each one ahead of time, so the review is ready the moment you look. It polls GitHub (`gh search prs --review-requested=@me`), and for each pull request worth your attention it cuts a worktree at the PR head, runs a diffity session over the diff, has an agent prepare a review with a walkthrough, and saves the result as a bundle. New commits redo a stale review; a merged, closed, or no-longer-requested PR is retired. At most `maxPrepared` reviews are kept prepared at a time — the rest wait in the queue, smallest first — and a prepared review leaves the inbox once you have posted it (GitHub withdraws the request) or dismissed it from the page — a dismissal holds until the pull request gets new commits. The daemon never posts your prepared reviews to GitHub — they are local drafts you open and submit yourself — and it runs the review agent with your GitHub credentials stripped from its environment. That said, the agent executes the pull request's own repository code (see the warning below), so treat the "never posts" behaviour as the daemon's design, not a sandbox. diff --git a/package-lock.json b/package-lock.json index 43a9ccf7..53ffeff0 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8457,7 +8457,7 @@ }, "packages/api": { "name": "@diffity/api", - "version": "0.10.14", + "version": "0.10.15", "dependencies": { "@diffity/parser": "*" }, @@ -8468,7 +8468,7 @@ }, "packages/cli": { "name": "@naturalcycles/diffity", - "version": "0.10.14", + "version": "0.10.15", "license": "MIT", "dependencies": { "commander": "^14.0.3", @@ -8492,7 +8492,7 @@ }, "packages/git": { "name": "@diffity/git", - "version": "0.10.14", + "version": "0.10.15", "devDependencies": { "@types/node": "^25.5.0", "typescript": "^5.9.3", @@ -8501,7 +8501,7 @@ }, "packages/github": { "name": "@diffity/github", - "version": "0.10.14", + "version": "0.10.15", "dependencies": { "@diffity/api": "*", "@diffity/parser": "*" @@ -8514,7 +8514,7 @@ }, "packages/parser": { "name": "@diffity/parser", - "version": "0.10.14", + "version": "0.10.15", "devDependencies": { "typescript": "^5.9.3", "vitest": "^4.1.0" @@ -8522,7 +8522,7 @@ }, "packages/ui": { "name": "@diffity/ui", - "version": "0.10.14", + "version": "0.10.15", "dependencies": { "@diffity/api": "*", "@diffity/parser": "*", diff --git a/packages/api/package.json b/packages/api/package.json index 589ef7a9..7f07def8 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/api", - "version": "0.10.14", + "version": "0.10.15", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/cli/package.json b/packages/cli/package.json index e2f203e6..8a12172b 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@naturalcycles/diffity", - "version": "0.10.14", + "version": "0.10.15", "description": "Agent-agnostic, GitHub-style diff viewer and code review tool with a live agent loop", "type": "module", "bin": { diff --git a/packages/cli/src/inbox/daemon.ts b/packages/cli/src/inbox/daemon.ts index 325b4140..32fd5fda 100644 --- a/packages/cli/src/inbox/daemon.ts +++ b/packages/cli/src/inbox/daemon.ts @@ -265,14 +265,16 @@ function handleDismiss(store: InboxStore, config: InboxConfig, id: string, log: return; } const { pr } = resolution; - if (pr.worktreePath) { - reclaimWorktree(config, pr.worktreePath, pr.repo); - store.setPaths(pr.id, { worktreePath: null }); - } store.setStatus(pr.id, 'dismissed', 'dismissed by the reviewer'); + store.setPaths(pr.id, { worktreePath: null }); log(`dismissed ${pr.id}`); + // The row is gone as far as the page is concerned; the directory can go at its own pace. res.writeHead(204); res.end(); + if (pr.worktreePath) { + void reclaimWorktree(config, pr.worktreePath, pr.repo) + .catch(err => log(`could not remove ${pr.worktreePath}: ${err instanceof Error ? err.message : err}`)); + } } /** @@ -280,12 +282,12 @@ function handleDismiss(store: InboxStore, config: InboxConfig, id: string, log: * That session lives in the reviewer's own registry and would otherwise keep serving a directory * that no longer exists. */ -export function reclaimWorktree(config: InboxConfig, worktree: string, repo: string): void { +export async function reclaimWorktree(config: InboxConfig, worktree: string, repo: string): Promise { const instance = findInstanceForRepo(repoHash(worktree)); if (instance) { killInstance(instance); } - removeWorktree(cloneDir(config.reposDir, repo), worktree); + await removeWorktree(cloneDir(config.reposDir, repo), worktree); } /** A request whose Host is this loopback server's own address (localhost or 127.0.0.1, right port). */ diff --git a/packages/cli/src/inbox/open-session.ts b/packages/cli/src/inbox/open-session.ts index 20373dbc..f4ea43c0 100644 --- a/packages/cli/src/inbox/open-session.ts +++ b/packages/cli/src/inbox/open-session.ts @@ -1,4 +1,5 @@ -import { spawn, execFileSync } from 'node:child_process'; +import { spawn, execFile } from 'node:child_process'; +import { promisify } from 'node:util'; import { readFileSync, realpathSync } from 'node:fs'; import { createHash } from 'node:crypto'; import { checkInstanceHealth, findInstanceForRepo } from '../registry.js'; @@ -14,7 +15,7 @@ export async function openPreparedSession(worktree: string, bundlePath: string, const port = await deps.ensureServer(worktree, ref, prNumber); const url = sessionUrl(port, ref); try { - deps.importBundle(worktree, bundlePath); + await deps.importBundle(worktree, bundlePath); } catch (err) { return { url, imported: false, importError: err instanceof Error ? err.message : String(err) }; } @@ -47,8 +48,8 @@ export function realOpenSessionDeps(nodePath: string, entry: string): OpenSessio return { baseRefOf, ensureServer: (worktree, ref, prNumber) => ensureServer(nodePath, entry, worktree, ref, prNumber), - importBundle: (worktree, bundlePath) => { - execFileSync(nodePath, [entry, '--repo', worktree, 'agent', 'import-bundle', bundlePath], { stdio: 'pipe' }); + importBundle: async (worktree, bundlePath) => { + await promisify(execFile)(nodePath, [entry, '--repo', worktree, 'agent', 'import-bundle', bundlePath]); }, }; } @@ -103,7 +104,7 @@ export interface OpenSessionDeps { /** Ensures a diffity server for the worktree at that ref, told its pull request, and returns its port. */ ensureServer(worktree: string, ref: string, prNumber: number): Promise; /** Adds the prepared review's threads and tours to the running session. */ - importBundle(worktree: string, bundlePath: string): void; + importBundle(worktree: string, bundlePath: string): void | Promise; } export interface OpenedSession { diff --git a/packages/cli/src/inbox/page.ts b/packages/cli/src/inbox/page.ts index 8bfe9014..aa2e4bee 100644 --- a/packages/cli/src/inbox/page.ts +++ b/packages/cli/src/inbox/page.ts @@ -82,6 +82,23 @@ export function inboxPage(): string { function sizeLabel(r) { return '+' + r.additions + ' \\u2212' + r.deletions; } + function ago(iso) { + const seconds = Math.max(0, (Date.now() - new Date(iso).getTime()) / 1000); + if (seconds < 90) return 'just now'; + const minutes = seconds / 60; + if (minutes < 90) return Math.round(minutes) + ' min ago'; + const hours = minutes / 60; + if (hours < 36) return Math.round(hours) + ' h ago'; + return Math.round(hours / 24) + ' d ago'; + } + + function times(r) { + const parts = []; + if (r.createdAt) parts.push('opened ' + ago(r.createdAt)); + if (r.updatedAt) parts.push('updated ' + ago(r.updatedAt)); + return parts.join(' \\u00b7 '); + } + function readyRow(r) { const row = document.createElement('a'); row.className = 'row open'; @@ -93,7 +110,7 @@ export function inboxPage(): string { '' + sizeLabel(r) + '' + '
' + esc(r.repo) + '#' + r.number + ' ' + '' + esc(r.title) + '
' + - '
by ' + esc(r.author) + ' \\u00b7 ' + r.changedFiles + ' file(s)
' + + '
by ' + esc(r.author) + ' \\u00b7 ' + r.changedFiles + ' file(s)' + (times(r) ? ' \\u00b7 ' + times(r) : '') + '
' + (r.stale ? 'stale' : '') + 'open \\u2197'; return row; @@ -106,7 +123,7 @@ export function inboxPage(): string { '' + sizeLabel(r) + '' + '
' + esc(r.repo) + '#' + r.number + ' ' + '' + esc(r.title) + '
' + - (r.statusReason ? '
' + esc(r.statusReason) + '
' : '') + '
' + + '
' + [esc(r.statusReason || ''), times(r)].filter(Boolean).join(' \\u00b7 ') + '
' + '' + esc(badgeText) + ''; return row; } @@ -118,19 +135,19 @@ export function inboxPage(): string { const button = document.createElement('button'); button.type = 'button'; button.className = 'dismiss'; - button.title = 'Dismiss: you will not review this one, and it will not come back'; + button.title = 'Dismiss this version of the pull request; new commits bring it back'; button.textContent = '\\u00d7'; - button.onclick = () => dismiss(r); + button.onclick = () => dismiss(r, wrap); wrap.append(row, button); return wrap; } - async function dismiss(r) { - if (!confirm('Dismiss ' + r.repo + '#' + r.number + '? It leaves the inbox for good.')) return; + async function dismiss(r, entry) { + if (!confirm('Dismiss ' + r.repo + '#' + r.number + '? It comes back if the pull request gets new commits.')) return; + entry.remove(); const res = await fetch(r.dismissUrl, { method: 'POST' }); if (!res.ok) { el('status').textContent = 'could not dismiss ' + r.repo + '#' + r.number + ': ' + await res.text(); - return; } refresh(); } diff --git a/packages/cli/src/inbox/prepare.ts b/packages/cli/src/inbox/prepare.ts index 3b301bfe..738322af 100644 --- a/packages/cli/src/inbox/prepare.ts +++ b/packages/cli/src/inbox/prepare.ts @@ -29,7 +29,7 @@ export interface ExportOpts { export interface PrepareDeps { startServer(worktree: string, diffRef: string): Promise; runAgent(opts: RunAgentOpts): Promise<{ stdout: string; timedOut: boolean }>; - exportBundle(opts: ExportOpts): void; + exportBundle(opts: ExportOpts): void | Promise; now(): string; } @@ -61,7 +61,7 @@ export async function preparePr(snapshot: PrSnapshot, config: InboxConfig, deps: let head: string; let diffRef: string; try { - ({ head, diffRef } = prepareWorktree(clone, dest, snapshot, snapshot.baseRef)); + ({ head, diffRef } = await prepareWorktree(clone, dest, snapshot, snapshot.baseRef)); } catch (err) { return { kind: 'failed', reason: err instanceof Error ? err.message : String(err), worktree: null, logPath: null }; } @@ -78,17 +78,17 @@ export async function preparePr(snapshot: PrSnapshot, config: InboxConfig, deps: }); if (timedOut) { - removeWorktree(clone, dest); + await removeWorktree(clone, dest); return { kind: 'failed', reason: `the agent did not finish within ${config.prepareTimeoutMinutes} minutes`, worktree: null, logPath }; } const verdict = verdictOf(stdout); if (verdict.kind === 'skipped') { - removeWorktree(clone, dest); + await removeWorktree(clone, dest); return { kind: 'skipped', reason: verdict.reason, logPath }; } if (verdict.kind === 'none') { - removeWorktree(clone, dest); + await removeWorktree(clone, dest); return { kind: 'failed', reason: 'the agent ended without SKIP or PREPARED', worktree: null, logPath }; } @@ -96,7 +96,7 @@ export async function preparePr(snapshot: PrSnapshot, config: InboxConfig, deps: // between the search and the fetch; recording it keeps the next tick from calling it stale. const bundlePath = join(bundlesDir(), `${snapshot.owner}-${snapshot.repo}-${snapshot.number}-${head.slice(0, 12)}.json`); try { - deps.exportBundle({ worktree: dest, prNumber: snapshot.number, outPath: bundlePath }); + await deps.exportBundle({ worktree: dest, prNumber: snapshot.number, outPath: bundlePath }); } catch (err) { return { kind: 'failed', reason: `the review was prepared but its bundle could not be written: ${err instanceof Error ? err.message : err}`, worktree: dest, logPath }; } diff --git a/packages/cli/src/inbox/reconcile.ts b/packages/cli/src/inbox/reconcile.ts index e51a1eb5..016f1870 100644 --- a/packages/cli/src/inbox/reconcile.ts +++ b/packages/cli/src/inbox/reconcile.ts @@ -28,8 +28,8 @@ export interface ReconcileInput { * * Nothing prepares a draft, the reviewer's own pull request, or a bot's. A closed or merged one, or * one no longer asking for the review, is retired but keeps whatever was prepared. A new commit - * makes a prepared review stale and worth redoing. One the reviewer dismissed stays dismissed, - * whatever the forge says next. Everything else asked of the reviewer is queued. + * makes a prepared review stale and worth redoing. One the reviewer dismissed stays dismissed until + * it gets new commits. Everything else asked of the reviewer is queued. */ export function reconcile(input: ReconcileInput): Transition | null { const { existing, snapshot, requested, viewerLogin } = input; @@ -39,7 +39,9 @@ export function reconcile(input: ReconcileInput): Transition | null { return null; } - if (existing?.status === 'dismissed') { + // A dismissal is the reviewer's word on this version of the pull request; a new head is a new + // change, and the poll takes it from the top. + if (existing?.status === 'dismissed' && existing.headSha === snapshot.headSha) { return null; } diff --git a/packages/cli/src/inbox/runtime.ts b/packages/cli/src/inbox/runtime.ts index a27ee24a..f7655180 100644 --- a/packages/cli/src/inbox/runtime.ts +++ b/packages/cli/src/inbox/runtime.ts @@ -1,4 +1,5 @@ -import { spawn, execFileSync } from 'node:child_process'; +import { spawn, execFile } from 'node:child_process'; +import { promisify } from 'node:util'; import { createWriteStream, mkdirSync, readFileSync, rmSync } from 'node:fs'; import { dirname, join } from 'node:path'; import type { ExportOpts, PrepareDeps, RunAgentOpts, ServerHandle } from './prepare.js'; @@ -188,11 +189,11 @@ function killGroup(pid: number | undefined, signal: NodeJS.Signals): void { } } -function exportBundle(nodePath: string, entry: string, opts: ExportOpts, dataDir: string): void { +async function exportBundle(nodePath: string, entry: string, opts: ExportOpts, dataDir: string): Promise { mkdirSync(dirname(opts.outPath), { recursive: true }); - execFileSync( + await promisify(execFile)( nodePath, [entry, '--repo', opts.worktree, 'agent', 'export-bundle', '--pr', String(opts.prNumber), '--out', opts.outPath], - { stdio: 'pipe', env: { ...process.env, DIFFITY_DATA_DIR: dataDir } }, + { env: { ...process.env, DIFFITY_DATA_DIR: dataDir } }, ); } diff --git a/packages/cli/src/inbox/store.ts b/packages/cli/src/inbox/store.ts index 8ef35fd0..e4e110a1 100644 --- a/packages/cli/src/inbox/store.ts +++ b/packages/cli/src/inbox/store.ts @@ -33,6 +33,9 @@ export interface InboxPr { additions: number; deletions: number; changedFiles: number; + /** The forge's own timestamps for the pull request; null on a row from before they were kept. */ + createdAt: string | null; + updatedAt: string | null; /** Whether the last poll still listed it as awaiting the reviewer. */ requested: boolean; status: InboxStatus; @@ -104,16 +107,20 @@ export class InboxStore { worktree_path TEXT, log_path TEXT, first_seen_at TEXT NOT NULL, - last_seen_at TEXT NOT NULL + last_seen_at TEXT NOT NULL, + created_at TEXT, + updated_at TEXT ) `); - // A table from before `attempts` existed gains it here; a fresh one already has it. - try { - this.db.exec('ALTER TABLE inbox_prs ADD COLUMN attempts INTEGER NOT NULL DEFAULT 0'); - } catch (err) { - // "duplicate column" means it is already there; anything else is a real problem. - if (!/duplicate column/i.test(err instanceof Error ? err.message : String(err))) { - throw err; + // A table from an earlier build gains the columns it lacks; a fresh one already has them. + for (const column of ['attempts INTEGER NOT NULL DEFAULT 0', 'created_at TEXT', 'updated_at TEXT']) { + try { + this.db.exec(`ALTER TABLE inbox_prs ADD COLUMN ${column}`); + } catch (err) { + // "duplicate column" means it is already there; anything else is a real problem. + if (!/duplicate column/i.test(err instanceof Error ? err.message : String(err))) { + throw err; + } } } } @@ -140,8 +147,9 @@ export class InboxStore { this.db.prepare(` INSERT INTO inbox_prs ( id, owner, repo, number, title, url, author, is_draft, head_sha, base_ref, - additions, deletions, changed_files, requested, status, status_reason, first_seen_at, last_seen_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'queued', NULL, ?, ?) + additions, deletions, changed_files, requested, status, status_reason, first_seen_at, last_seen_at, + created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, 'queued', NULL, ?, ?, ?, ?) ON CONFLICT(id) DO UPDATE SET title = excluded.title, url = excluded.url, @@ -155,11 +163,14 @@ export class InboxStore { deletions = excluded.deletions, changed_files = excluded.changed_files, requested = excluded.requested, - last_seen_at = excluded.last_seen_at + last_seen_at = excluded.last_seen_at, + created_at = excluded.created_at, + updated_at = excluded.updated_at `).run( id, snapshot.owner, snapshot.repo, snapshot.number, snapshot.title, snapshot.url, snapshot.author, snapshot.isDraft ? 1 : 0, snapshot.headSha, snapshot.baseRef, snapshot.additions, snapshot.deletions, snapshot.changedFiles, requested ? 1 : 0, now, now, + snapshot.createdAt || null, snapshot.updatedAt || null, ); return this.get(id)!; } @@ -219,6 +230,8 @@ interface Row { log_path: string | null; first_seen_at: string; last_seen_at: string; + created_at: string | null; + updated_at: string | null; } function rowToPr(row: Row): InboxPr { @@ -236,6 +249,8 @@ function rowToPr(row: Row): InboxPr { additions: row.additions, deletions: row.deletions, changedFiles: row.changed_files, + createdAt: row.created_at, + updatedAt: row.updated_at, requested: row.requested === 1, status: normaliseStatus(row.status), statusReason: row.status_reason, diff --git a/packages/cli/src/inbox/tick.ts b/packages/cli/src/inbox/tick.ts index 095b48c4..29f8c5c3 100644 --- a/packages/cli/src/inbox/tick.ts +++ b/packages/cli/src/inbox/tick.ts @@ -14,7 +14,7 @@ export interface TickDeps { forge: Forge; /** Prepares one pull request; the daemon passes the real preparer, a test a fake. */ prepare(snapshot: PrSnapshot): Promise; - removeWorktree(worktree: string, repo: string): void; + removeWorktree(worktree: string, repo: string): void | Promise; log(message: string): void; now(): string; /** False once the daemon is shutting down, so the drain stops starting new preparations. */ @@ -67,7 +67,7 @@ export async function runTick(store: InboxStore, deps: TickDeps): Promise if (transition) { store.setStatus(pr.id, transition.status, transition.reason); if (pr.worktreePath) { - deps.removeWorktree(pr.worktreePath, pr.repo); + await deps.removeWorktree(pr.worktreePath, pr.repo); store.setPaths(pr.id, { worktreePath: null }); } } diff --git a/packages/cli/src/inbox/view.ts b/packages/cli/src/inbox/view.ts index 22a9d31a..2dd7d181 100644 --- a/packages/cli/src/inbox/view.ts +++ b/packages/cli/src/inbox/view.ts @@ -13,6 +13,8 @@ export interface InboxRow { changedFiles: number; additions: number; deletions: number; + createdAt: string | null; + updatedAt: string | null; /** A prepared review whose head has since moved: openable, but out of date. */ stale: boolean; preparedAt: string | null; @@ -56,6 +58,8 @@ function toRow(pr: InboxPr, openBase: string): InboxRow { changedFiles: pr.changedFiles, additions: pr.additions, deletions: pr.deletions, + createdAt: pr.createdAt, + updatedAt: pr.updatedAt, stale, preparedAt: pr.preparedAt, openUrl: openable ? `${openBase}/open/${encodeURIComponent(pr.id)}` : null, diff --git a/packages/cli/src/inbox/worktree.ts b/packages/cli/src/inbox/worktree.ts index 38124100..7fec73c5 100644 --- a/packages/cli/src/inbox/worktree.ts +++ b/packages/cli/src/inbox/worktree.ts @@ -1,8 +1,11 @@ -import { execFileSync } from 'node:child_process'; +import { execFile } from 'node:child_process'; import { existsSync } from 'node:fs'; import { join } from 'node:path'; +import { promisify } from 'node:util'; import type { PrRef } from '@diffity/github'; +const execFileAsync = promisify(execFile); + /** The base clone a pull request's worktree is cut from — one directory per repository name. */ export function cloneDir(reposDir: string, repo: string): string { return join(reposDir, repo); @@ -12,8 +15,13 @@ export function worktreePath(worktreesDir: string, ref: PrRef): string { return join(worktreesDir, `${ref.owner}-${ref.repo}-${ref.number}`); } -function runGit(cwd: string, args: string[]): void { - execFileSync('git', args, { cwd, stdio: 'pipe' }); +/** + * Off the event loop on purpose: a fetch or a worktree add on a large clone takes as long as it + * takes, and the daemon's own server has to keep answering the inbox page and its opens meanwhile. + */ +async function runGit(cwd: string, args: string[]): Promise { + const { stdout } = await execFileAsync('git', args, { cwd, encoding: 'utf-8', maxBuffer: 16 * 1024 * 1024 }); + return stdout.trim(); } /** @@ -23,30 +31,30 @@ function runGit(cwd: string, args: string[]): void { * the pull request without asking the forge anything. Idempotent and self-healing: an existing * worktree, even one a killed agent left dirty, is forced to the new head rather than re-created. */ -export function prepareWorktree(clone: string, dest: string, ref: PrRef, baseRef: string): { head: string; diffRef: string } { +export async function prepareWorktree(clone: string, dest: string, ref: PrRef, baseRef: string): Promise<{ head: string; diffRef: string }> { if (!existsSync(clone)) { throw new Error(`No local clone at ${clone}. Clone ${ref.owner}/${ref.repo} there first.`); } if (!baseRef) { throw new Error(`No base branch for ${ref.owner}/${ref.repo}#${ref.number}; cannot tell what the change is against.`); } - requireMatchingOrigin(clone, ref); + await requireMatchingOrigin(clone, ref); - runGit(clone, ['fetch', 'origin', `refs/pull/${ref.number}/head`]); - const head = revParse(clone, 'FETCH_HEAD'); + await runGit(clone, ['fetch', 'origin', `refs/pull/${ref.number}/head`]); + const head = await runGit(clone, ['rev-parse', 'FETCH_HEAD']); // `refs/heads/` so a tag sharing the branch's name cannot be fetched in its place. - runGit(clone, ['fetch', 'origin', `refs/heads/${baseRef}`]); - const diffRef = revParse(clone, 'FETCH_HEAD'); + await runGit(clone, ['fetch', 'origin', `refs/heads/${baseRef}`]); + const diffRef = await runGit(clone, ['rev-parse', 'FETCH_HEAD']); if (existsSync(join(dest, '.git'))) { - runGit(dest, ['checkout', '--detach', '--force', head]); + await runGit(dest, ['checkout', '--detach', '--force', head]); } else { try { - runGit(clone, ['worktree', 'add', '--detach', '--force', dest, head]); + await runGit(clone, ['worktree', 'add', '--detach', '--force', dest, head]); } catch (err) { // A directory git no longer tracks (after `worktree prune`) blocks `add`; clear and retry. - removeWorktree(clone, dest); - runGit(clone, ['worktree', 'add', '--detach', '--force', dest, head]); + await removeWorktree(clone, dest); + await runGit(clone, ['worktree', 'add', '--detach', '--force', dest, head]); if (!existsSync(join(dest, '.git'))) { throw err; } @@ -55,15 +63,11 @@ export function prepareWorktree(clone: string, dest: string, ref: PrRef, baseRef return { head, diffRef }; } -function revParse(cwd: string, ref: string): string { - return execFileSync('git', ['rev-parse', ref], { cwd, encoding: 'utf-8' }).trim(); -} - /** The clone must actually be the pull request's repository, not another of the same name. */ -function requireMatchingOrigin(clone: string, ref: PrRef): void { +async function requireMatchingOrigin(clone: string, ref: PrRef): Promise { let url: string; try { - url = execFileSync('git', ['remote', 'get-url', 'origin'], { cwd: clone, encoding: 'utf-8' }).trim(); + url = await runGit(clone, ['remote', 'get-url', 'origin']); } catch { throw new Error(`${clone} has no origin remote; cannot confirm it is ${ref.owner}/${ref.repo}.`); } @@ -75,12 +79,12 @@ function requireMatchingOrigin(clone: string, ref: PrRef): void { } /** Removes the worktree, forcing past a dirty tree — a prepared review leaves none, but a killed agent might. */ -export function removeWorktree(clone: string, dest: string): void { +export async function removeWorktree(clone: string, dest: string): Promise { if (!existsSync(clone) || !existsSync(dest)) { return; } try { - runGit(clone, ['worktree', 'remove', '--force', dest]); + await runGit(clone, ['worktree', 'remove', '--force', dest]); } catch { // A worktree git no longer tracks is already as gone as this needs it to be. } diff --git a/packages/cli/tests/inbox-open.test.ts b/packages/cli/tests/inbox-open.test.ts index c642fcab..72738507 100644 --- a/packages/cli/tests/inbox-open.test.ts +++ b/packages/cli/tests/inbox-open.test.ts @@ -20,7 +20,7 @@ function snapshot(): PrSnapshot { return { owner: 'o', repo: 'r', number: 4, title: 'A change', url: 'https://github.com/o/r/pull/4', author: 'alice', isBot: false, isDraft: false, state: 'OPEN', headSha: 'aaa', baseRef: 'main', - additions: 3, deletions: 1, changedFiles: 2, updatedAt: 'now', + additions: 3, deletions: 1, changedFiles: 2, createdAt: 'now', updatedAt: 'now', }; } diff --git a/packages/cli/tests/inbox-prepare.test.ts b/packages/cli/tests/inbox-prepare.test.ts index 62604ad0..33d60256 100644 --- a/packages/cli/tests/inbox-prepare.test.ts +++ b/packages/cli/tests/inbox-prepare.test.ts @@ -24,7 +24,7 @@ function snapshot(): PrSnapshot { return { owner: 'o', repo: 'demo', number: 4, title: 'A change', url: 'https://github.com/o/demo/pull/4', author: 'alice', isBot: false, isDraft: false, state: 'OPEN', headSha: head, baseRef: 'main', - additions: 1, deletions: 0, changedFiles: 1, updatedAt: 'now', + additions: 1, deletions: 0, changedFiles: 1, createdAt: 'now', updatedAt: 'now', }; } diff --git a/packages/cli/tests/inbox-reconcile.test.ts b/packages/cli/tests/inbox-reconcile.test.ts index 0aeb2e08..41b156df 100644 --- a/packages/cli/tests/inbox-reconcile.test.ts +++ b/packages/cli/tests/inbox-reconcile.test.ts @@ -7,7 +7,7 @@ function snapshot(over: Partial = {}): PrSnapshot { return { owner: 'o', repo: 'r', number: 1, title: 'A change', url: 'https://github.com/o/r/pull/1', author: 'alice', isBot: false, isDraft: false, state: 'OPEN', headSha: 'aaa', baseRef: 'main', - additions: 10, deletions: 2, changedFiles: 3, updatedAt: '2026-09-02T10:00:00Z', ...over, + additions: 10, deletions: 2, changedFiles: 3, createdAt: '2026-09-02T10:00:00Z', updatedAt: '2026-09-02T10:00:00Z', ...over, }; } @@ -15,7 +15,7 @@ function existing(over: Partial = {}): InboxPr { return { id: 'o/r#1', owner: 'o', repo: 'r', number: 1, title: 'A change', url: 'https://github.com/o/r/pull/1', author: 'alice', isDraft: false, headSha: 'aaa', baseRef: 'main', additions: 10, deletions: 2, changedFiles: 3, - requested: true, status: 'prepared', statusReason: null, attempts: 0, preparedHeadSha: 'aaa', preparedAt: '2026-09-02T09:00:00Z', + createdAt: null, updatedAt: null, requested: true, status: 'prepared', statusReason: null, attempts: 0, preparedHeadSha: 'aaa', preparedAt: '2026-09-02T09:00:00Z', bundlePath: '/b.json', worktreePath: '/wt', logPath: '/l.log', firstSeenAt: 'x', lastSeenAt: 'y', ...over, }; } @@ -87,9 +87,11 @@ describe('reconcile', () => { expect(reconcile({ existing: { ...failing, attempts: 3 }, snapshot: snapshot({ headSha: 'ddd' }), requested: true, viewerLogin: 'me' })!.prepare).toBe(true); }); - it('keeps a dismissed pull request dismissed, whatever the forge says next', () => { + it('keeps a dismissed pull request dismissed at that head, and takes new commits from the top', () => { const dismissed = existing({ status: 'dismissed', statusReason: 'dismissed by the reviewer' }); - expect(reconcile({ existing: dismissed, snapshot: snapshot({ headSha: 'bbb' }), requested: true, viewerLogin: 'me' })).toBeNull(); + expect(reconcile({ existing: dismissed, snapshot: snapshot(), requested: true, viewerLogin: 'me' })).toBeNull(); expect(reconcile({ existing: dismissed, snapshot: snapshot(), requested: false, viewerLogin: 'me' })).toBeNull(); + expect(reconcile({ existing: dismissed, snapshot: snapshot({ headSha: 'bbb' }), requested: true, viewerLogin: 'me' })) + .toEqual({ status: 'queued', reason: null, prepare: true }); }); }); diff --git a/packages/cli/tests/inbox-store.test.ts b/packages/cli/tests/inbox-store.test.ts index a04b6163..782be8eb 100644 --- a/packages/cli/tests/inbox-store.test.ts +++ b/packages/cli/tests/inbox-store.test.ts @@ -13,7 +13,7 @@ function snapshot(): PrSnapshot { return { owner: 'o', repo: 'r', number: 1, title: 'T', url: 'https://github.com/o/r/pull/1', author: 'alice', isBot: false, isDraft: false, state: 'OPEN', headSha: 'aaa', baseRef: 'main', - additions: 1, deletions: 0, changedFiles: 1, updatedAt: 'now', + additions: 1, deletions: 0, changedFiles: 1, createdAt: 'now', updatedAt: 'now', }; } @@ -47,6 +47,28 @@ describe('InboxStore migration', () => { store.close(); }); + it('adds the forge timestamps to a table created before they were kept', () => { + const seed = new DatabaseSync(path); + seed.exec(`CREATE TABLE inbox_prs ( + id TEXT PRIMARY KEY, owner TEXT NOT NULL, repo TEXT NOT NULL, number INTEGER NOT NULL, + title TEXT NOT NULL, url TEXT NOT NULL, author TEXT NOT NULL, is_draft INTEGER NOT NULL, + head_sha TEXT NOT NULL, base_ref TEXT NOT NULL, additions INTEGER NOT NULL, deletions INTEGER NOT NULL, + changed_files INTEGER NOT NULL, requested INTEGER NOT NULL, status TEXT NOT NULL, status_reason TEXT, + attempts INTEGER NOT NULL DEFAULT 0, prepared_head_sha TEXT, prepared_at TEXT, bundle_path TEXT, + worktree_path TEXT, log_path TEXT, first_seen_at TEXT NOT NULL, last_seen_at TEXT NOT NULL)`); + seed.exec(`INSERT INTO inbox_prs (id, owner, repo, number, title, url, author, is_draft, head_sha, base_ref, + additions, deletions, changed_files, requested, status, first_seen_at, last_seen_at) + VALUES ('o/r#1', 'o', 'r', 1, 'T', 'u', 'alice', 0, 'aaa', 'main', 1, 0, 1, 1, 'queued', 'x', 'y')`); + seed.close(); + + const store = new InboxStore(path); + expect(store.get('o/r#1')!.createdAt).toBeNull(); + const pr = store.observe({ ...snapshot(), createdAt: '2026-09-01T08:00:00Z', updatedAt: '2026-09-02T10:00:00Z' }, true, 'now'); + expect(pr.createdAt).toBe('2026-09-01T08:00:00Z'); + expect(pr.updatedAt).toBe('2026-09-02T10:00:00Z'); + store.close(); + }); + it('opens a fresh database and round-trips a prepared row', () => { const store = new InboxStore(path); store.observe(snapshot(), true, 'now'); diff --git a/packages/cli/tests/inbox-tick.test.ts b/packages/cli/tests/inbox-tick.test.ts index eb5abc54..f581f800 100644 --- a/packages/cli/tests/inbox-tick.test.ts +++ b/packages/cli/tests/inbox-tick.test.ts @@ -9,7 +9,7 @@ function snapshot(over: Partial = {}): PrSnapshot { return { owner: 'o', repo: 'r', number: 1, title: 'A change', url: 'https://github.com/o/r/pull/1', author: 'alice', isBot: false, isDraft: false, state: 'OPEN', headSha: 'aaa', baseRef: 'main', - additions: 10, deletions: 2, changedFiles: 3, updatedAt: '2026-09-02T10:00:00Z', ...over, + additions: 10, deletions: 2, changedFiles: 3, createdAt: '2026-09-02T10:00:00Z', updatedAt: '2026-09-02T10:00:00Z', ...over, }; } @@ -213,14 +213,13 @@ describe('runTick', () => { expect(store.get('o/r#2')!.status).toBe('queued'); }); - it('never re-queues a dismissed pull request, and its slot goes to the next in line', async () => { + it('leaves a dismissed pull request alone at that head, and its slot goes to the next in line', async () => { forge.set(snapshot({ number: 1, additions: 10, deletions: 0 })); forge.set(snapshot({ number: 2, additions: 400, deletions: 0 })); await runTick(store, deps({ maxPrepared: 1 })); store.setStatus('o/r#1', 'dismissed', 'dismissed by the reviewer'); prepared = []; - forge.snapshots.set('o/r#1', snapshot({ number: 1, additions: 10, deletions: 0, headSha: 'bbb' })); await runTick(store, deps({ maxPrepared: 1 })); expect(store.get('o/r#1')!.status).toBe('dismissed'); expect(prepared).toEqual(['o/r#2']); @@ -230,6 +229,27 @@ describe('runTick', () => { expect(view.other).toEqual([]); }); + it('takes a dismissed pull request from the top once it has new commits', async () => { + forge.set(snapshot({ number: 1 })); + await runTick(store, deps()); + store.setStatus('o/r#1', 'dismissed', 'dismissed by the reviewer'); + + prepared = []; + forge.snapshots.set('o/r#1', snapshot({ number: 1, headSha: 'bbb' })); + await runTick(store, deps()); + expect(prepared).toEqual(['o/r#1']); + expect(store.get('o/r#1')!.status).toBe('prepared'); + expect(store.get('o/r#1')!.preparedHeadSha).toBe('bbb'); + }); + + it('carries the forge\'s timestamps onto the rows', async () => { + forge.set(snapshot({ number: 1, createdAt: '2026-09-01T08:00:00Z', updatedAt: '2026-09-02T10:00:00Z' })); + await runTick(store, deps()); + const [row] = buildView(store, 'http://localhost:5390', '2026-09-02T12:00:00.000Z').ready; + expect(row.createdAt).toBe('2026-09-01T08:00:00Z'); + expect(row.updatedAt).toBe('2026-09-02T10:00:00Z'); + }); + it('offers a dismiss link for every row but one being prepared', async () => { forge.set(snapshot({ number: 1, additions: 10, deletions: 0 })); forge.set(snapshot({ number: 2, additions: 300, deletions: 0 })); diff --git a/packages/cli/tests/inbox-units.test.ts b/packages/cli/tests/inbox-units.test.ts index 8afc90b9..9dc297d7 100644 --- a/packages/cli/tests/inbox-units.test.ts +++ b/packages/cli/tests/inbox-units.test.ts @@ -35,7 +35,7 @@ describe('composePrompt', () => { const snapshot: PrSnapshot = { owner: 'o', repo: 'r', number: 7, title: 'Add a widget', url: 'https://github.com/o/r/pull/7', author: 'alice', isBot: false, isDraft: false, state: 'OPEN', headSha: 'abc', baseRef: 'main', - additions: 12, deletions: 3, changedFiles: 2, updatedAt: '2026-09-02T10:00:00Z', + additions: 12, deletions: 3, changedFiles: 2, createdAt: '2026-09-02T10:00:00Z', updatedAt: '2026-09-02T10:00:00Z', }; it('tells the agent the worktree, forbids the forge, and asks for a verdict', () => { @@ -84,10 +84,11 @@ describe('the forge parsers', () => { const ref = { owner: 'o', repo: 'r', number: 1 }; const ok = parsePrSnapshot(ref, JSON.stringify({ title: 'T', url: 'https://github.com/o/r/pull/1', author: { login: 'alice', is_bot: false }, - isDraft: false, state: 'OPEN', headRefOid: 'abc', baseRefName: 'main', additions: 1, deletions: 0, changedFiles: 1, updatedAt: 'now', + isDraft: false, state: 'OPEN', headRefOid: 'abc', baseRefName: 'main', additions: 1, deletions: 0, changedFiles: 1, createdAt: 'now', updatedAt: 'now', })); expect(ok?.headSha).toBe('abc'); expect(ok?.state).toBe('OPEN'); + expect(ok?.createdAt).toBe('now'); const bad = parsePrSnapshot(ref, JSON.stringify({ url: 'u', state: 'OPEN' })); expect(bad).toBeNull(); diff --git a/packages/git/package.json b/packages/git/package.json index 049bc2ff..0daebbc4 100644 --- a/packages/git/package.json +++ b/packages/git/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/git", - "version": "0.10.14", + "version": "0.10.15", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/github/package.json b/packages/github/package.json index 55fd0760..13b2b56e 100644 --- a/packages/github/package.json +++ b/packages/github/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/github", - "version": "0.10.14", + "version": "0.10.15", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/github/src/inbox.ts b/packages/github/src/inbox.ts index c80ca7ef..ebc2c707 100644 --- a/packages/github/src/inbox.ts +++ b/packages/github/src/inbox.ts @@ -23,6 +23,7 @@ export interface PrSnapshot extends PrRef { additions: number; deletions: number; changedFiles: number; + createdAt: string; updatedAt: string; } @@ -64,7 +65,7 @@ export async function viewPr(ref: PrRef): Promise { const json = await ghAsync([ 'pr', 'view', String(ref.number), '--repo', `${ref.owner}/${ref.repo}`, - '--json', 'number,title,url,author,isDraft,state,headRefOid,baseRefName,additions,deletions,changedFiles,updatedAt', + '--json', 'number,title,url,author,isDraft,state,headRefOid,baseRefName,additions,deletions,changedFiles,createdAt,updatedAt', ]); return parsePrSnapshot(ref, json); } catch { @@ -92,6 +93,7 @@ export function parsePrSnapshot(ref: PrRef, json: string): PrSnapshot | null { additions: Number(data.additions ?? 0), deletions: Number(data.deletions ?? 0), changedFiles: Number(data.changedFiles ?? 0), + createdAt: String(data.createdAt ?? ''), updatedAt: String(data.updatedAt ?? ''), }; } diff --git a/packages/parser/package.json b/packages/parser/package.json index 486d5216..ce76889e 100644 --- a/packages/parser/package.json +++ b/packages/parser/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/parser", - "version": "0.10.14", + "version": "0.10.15", "private": true, "type": "module", "main": "./dist/index.js", diff --git a/packages/ui/package.json b/packages/ui/package.json index c2c08866..62b56750 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -1,6 +1,6 @@ { "name": "@diffity/ui", - "version": "0.10.14", + "version": "0.10.15", "type": "module", "private": true, "scripts": { From 82aa0b14c38230c1d1ffc2b007dd35f75ce95590 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Fable 5.1)" Date: Thu, 3 Sep 2026 15:13:36 +0200 Subject: [PATCH 2/3] fix: a row dismissed while the tick is busy is not prepared over, and an empty meta line is not drawn Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Bwp5QefjsjMFeq6CK6cT6w --- packages/cli/src/inbox/page.ts | 7 ++++++- packages/cli/src/inbox/tick.ts | 4 ++++ packages/cli/tests/inbox-tick.test.ts | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/inbox/page.ts b/packages/cli/src/inbox/page.ts index aa2e4bee..5d49187c 100644 --- a/packages/cli/src/inbox/page.ts +++ b/packages/cli/src/inbox/page.ts @@ -92,6 +92,11 @@ export function inboxPage(): string { return Math.round(hours / 24) + ' d ago'; } + function metaLine(parts) { + const text = parts.filter(Boolean).join(' \\u00b7 '); + return text ? '
' + text + '
' : ''; + } + function times(r) { const parts = []; if (r.createdAt) parts.push('opened ' + ago(r.createdAt)); @@ -123,7 +128,7 @@ export function inboxPage(): string { '' + sizeLabel(r) + '' + '
' + esc(r.repo) + '#' + r.number + ' ' + '' + esc(r.title) + '
' + - '
' + [esc(r.statusReason || ''), times(r)].filter(Boolean).join(' \\u00b7 ') + '
' + + metaLine([esc(r.statusReason || ''), times(r)]) + '' + '' + esc(badgeText) + ''; return row; } diff --git a/packages/cli/src/inbox/tick.ts b/packages/cli/src/inbox/tick.ts index 29f8c5c3..fabd927d 100644 --- a/packages/cli/src/inbox/tick.ts +++ b/packages/cli/src/inbox/tick.ts @@ -84,6 +84,10 @@ export async function runTick(store: InboxStore, deps: TickDeps): Promise if (deps.shouldContinue && !deps.shouldContinue()) { break; } + // The reviewer may have dismissed it from the page while this tick was busy with another. + if (store.get(prId(snapshot))?.status === 'dismissed') { + continue; + } if (!refresh && countReady(store) >= deps.maxPrepared) { store.setStatus(prId(snapshot), 'queued', `waiting: ${deps.maxPrepared} reviews already prepared`); waiting++; diff --git a/packages/cli/tests/inbox-tick.test.ts b/packages/cli/tests/inbox-tick.test.ts index f581f800..24fe61fd 100644 --- a/packages/cli/tests/inbox-tick.test.ts +++ b/packages/cli/tests/inbox-tick.test.ts @@ -229,6 +229,23 @@ describe('runTick', () => { expect(view.other).toEqual([]); }); + it('does not prepare a pull request dismissed while the tick was busy with another', async () => { + forge.set(snapshot({ number: 1, additions: 10, deletions: 0 })); + forge.set(snapshot({ number: 2, additions: 20, deletions: 0 })); + const prepare = (snap: PrSnapshot) => { + prepared.push(prId(snap)); + // The reviewer dismisses #2 from the page while #1 is being prepared. + if (snap.number === 1) { + store.setStatus('o/r#2', 'dismissed', 'dismissed by the reviewer'); + } + return Promise.resolve(prepareResult(snap)); + }; + await runTick(store, deps({ prepare })); + + expect(prepared).toEqual(['o/r#1']); + expect(store.get('o/r#2')!.status).toBe('dismissed'); + }); + it('takes a dismissed pull request from the top once it has new commits', async () => { forge.set(snapshot({ number: 1 })); await runTick(store, deps()); From ada5f438eaad6afe4a498e73d01435ef3c1b3306 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Fable 5.1)" Date: Thu, 3 Sep 2026 15:16:56 +0200 Subject: [PATCH 3/3] test: spawn tests wait for the server to exit before removing its directory Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Bwp5QefjsjMFeq6CK6cT6w --- packages/cli/tests/inbox-open.test.ts | 2 +- packages/cli/tests/pr-number-flag.test.ts | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/cli/tests/inbox-open.test.ts b/packages/cli/tests/inbox-open.test.ts index 72738507..b4dc7a8a 100644 --- a/packages/cli/tests/inbox-open.test.ts +++ b/packages/cli/tests/inbox-open.test.ts @@ -32,7 +32,7 @@ function preparedStore(): InboxStore { } beforeEach(() => { root = mkdtempSync(join(tmpdir(), 'diffity-open-')); }); -afterEach(() => { rmSync(root, { recursive: true, force: true }); }); +afterEach(() => { rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); }); /** Whether the child is gone within a few seconds of being told to go. */ function exited(child: ChildProcess, ms = 3000): Promise { diff --git a/packages/cli/tests/pr-number-flag.test.ts b/packages/cli/tests/pr-number-flag.test.ts index cfb6354c..00ac8f01 100644 --- a/packages/cli/tests/pr-number-flag.test.ts +++ b/packages/cli/tests/pr-number-flag.test.ts @@ -48,7 +48,19 @@ describe('--pr on the command line', () => { execFileSync('git', ['remote', 'add', 'origin', 'git@github.com:o/r.git'], { cwd: repo, stdio: 'pipe' }); }); - afterAll(() => { rmSync(root, { recursive: true, force: true }); }); + afterAll(() => { rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); }); + + /** Waits for a server told to stop to be gone, so the directory it writes into can be removed. */ + function exited(child: ChildProcess, ms = 5000): Promise { + return new Promise(resolve => { + if (child.exitCode !== null || child.signalCode !== null) { + resolve(); + return; + } + const timer = setTimeout(resolve, ms); + child.once('exit', () => { clearTimeout(timer); resolve(); }); + }); + } const env = () => ({ ...process.env, DIFFITY_DATA_DIR: join(root, 'data') }); @@ -109,6 +121,7 @@ describe('--pr on the command line', () => { expect(res.headers.get('location')).toBe('/diff?ref=HEAD'); } finally { child.kill('SIGTERM'); + await exited(child); } }, 30_000); @@ -118,6 +131,7 @@ describe('--pr on the command line', () => { expect(ref).toBe('HEAD'); } finally { child.kill('SIGTERM'); + await exited(child); } }, 30_000); });