feat: the inbox daemon keeps answering while git works; dismissals hold for one head; rows show timestamps

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.
 

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",

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": {

packages/cli/src/inbox/daemon.ts

@@ -265,27 +265,29 @@ 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}`));
+  }
 }
 
 /**
  * Removes a pull request's worktree, first stopping any diffity server the reviewer opened on it.
  * 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<void> {
   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). */

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<number>;
   /** 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<void>;
 }
 
 export interface OpenedSession {

packages/cli/src/inbox/page.ts

@@ -82,6 +82,28 @@ 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 metaLine(parts) {
+    const text = parts.filter(Boolean).join(' \\u00b7 ');
+    return text ? '<div class="meta">' + text + '</div>' : '';
+  }
+
+  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 +115,7 @@ export function inboxPage(): string {
       '<span class="size">' + sizeLabel(r) + '</span>' +
       '<span class="title"><div><span class="repo">' + esc(r.repo) + '#' + r.number + '</span> ' +
       '<span class="name">' + esc(r.title) + '</span></div>' +
-      '<div class="meta">by ' + esc(r.author) + ' \\u00b7 ' + r.changedFiles + ' file(s)</div></span>' +
+      '<div class="meta">by ' + esc(r.author) + ' \\u00b7 ' + r.changedFiles + ' file(s)' + (times(r) ? ' \\u00b7 ' + times(r) : '') + '</div></span>' +
       (r.stale ? '<span class="badge stale">stale</span>' : '') +
       '<span class="open-hint">open \\u2197</span>';
     return row;
@@ -106,7 +128,7 @@ export function inboxPage(): string {
       '<span class="size">' + sizeLabel(r) + '</span>' +
       '<span class="title"><div><span class="repo">' + esc(r.repo) + '#' + r.number + '</span> ' +
       '<span class="name">' + esc(r.title) + '</span></div>' +
-      (r.statusReason ? '<div class="meta">' + esc(r.statusReason) + '</div>' : '') + '</span>' +
+      metaLine([esc(r.statusReason || ''), times(r)]) + '</span>' +
       '<span class="badge ' + badgeClass + '">' + esc(badgeText) + '</span>';
     return row;
   }
@@ -118,19 +140,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();
   }

packages/cli/src/inbox/prepare.ts

@@ -29,7 +29,7 @@ export interface ExportOpts {
 export interface PrepareDeps {
   startServer(worktree: string, diffRef: string): Promise<ServerHandle>;
   runAgent(opts: RunAgentOpts): Promise<{ stdout: string; timedOut: boolean }>;
-  exportBundle(opts: ExportOpts): void;
+  exportBundle(opts: ExportOpts): void | Promise<void>;
   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,25 +78,25 @@ 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 };
     }
 
     // The head actually checked out, which may be newer than the snapshot if the author pushed
     // 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 };
     }

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;
   }
 

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<void> {
   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 } },
   );
 }

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,