Keep established reviews alive during remote outages

scripts/build-diff-data.mjs

@@ -439,6 +439,15 @@ function bareCache(remoteUrl) {
   return { path, run };
 }
 
+class RemoteConnectionError extends Error {}
+
+function remoteLookupError(message, error) {
+  const detail = error?.stderr?.toString().trim() || '';
+  const transient = /(?:failed to connect|couldn't connect|could not resolve host|could not resolve hostname|temporary failure in name resolution|connection (?:timed out|reset|refused|closed)|network is unreachable|operation timed out|i\/o timeout|TLS connection was non-properly terminated|error connecting to|HTTP 50[0234]|requested URL returned error: 50[0234])/i.test(detail);
+  const ErrorType = transient ? RemoteConnectionError : Error;
+  return new ErrorType(message);
+}
+
 function fetchInto(cache, remoteUrl, refspecs) {
   try {
     runGit(
@@ -456,8 +465,9 @@ function fetchInto(cache, remoteUrl, refspecs) {
     );
   } catch (error) {
     const detail = error?.stderr?.toString().trim();
-    throw new Error(
+    throw remoteLookupError(
       `Could not fetch the remote target${detail ? `: ${detail}` : ''}`,
+      error,
     );
   }
 }
@@ -484,8 +494,8 @@ function remoteDefaultBranchInfo(remoteUrl) {
   let raw;
   try {
     raw = runGit(['ls-remote', '--symref', remoteUrl, 'HEAD'], { remoteUrl });
-  } catch {
-    throw new Error('Could not read the remote default branch');
+  } catch (error) {
+    throw remoteLookupError('Could not read the remote default branch', error);
   }
   const match = raw.match(/^ref:\s+refs\/heads\/([^\t\n]+)\s+HEAD$/m);
   if (!match) {
@@ -659,8 +669,9 @@ function pullRequestInfo(pr, remote) {
       )
         ? ' Check gh auth status.'
         : '';
-    throw new Error(
+    throw remoteLookupError(
       `Could not read pull request ${pr} with gh${detail ? `: ${detail}` : ''}.${authHint}`,
+      error,
     );
   }
 }
@@ -1467,13 +1478,21 @@ function fingerprint() {
   ].join('|');
 }
 
-const refresh = () => {
+let remoteRefreshFailed = false;
+const refresh = ({ retainSnapshot = false } = {}) => {
   try {
     const wrote = build();
+    if (remoteRefreshFailed) console.error('Remote refresh recovered.');
+    remoteRefreshFailed = false;
     console.log(wrote ? `Wrote ${output}` : 'No diff-data changes');
     return true;
   } catch (error) {
     console.error(error.message);
+    if (retainSnapshot && error instanceof RemoteConnectionError) {
+      remoteRefreshFailed = true;
+      console.error('Keeping the last valid review; it has not been refreshed. Will retry the remote refresh.');
+      return true;
+    }
     process.exitCode = 1;
     return false;
   }
@@ -1504,10 +1523,11 @@ if (watching && started) {
       const next = fingerprint();
       remoteWait += watchInterval;
       const remoteDue = remoteMode && remoteWait >= remoteRefreshInterval;
+      if (remoteRefreshFailed && !remoteDue) return true;
       if (next !== last || remoteDue || watchContent) {
         last = next;
         remoteWait = 0;
-        if (!refresh()) {
+        if (!refresh({ retainSnapshot: true })) {
           clearInterval(watcher);
           return false;
         }

tests/presenter-recovery.test.mjs

@@ -61,14 +61,14 @@ function stop(child) {
   });
 }
 
-function present(repo, summaries, output, codex, environment = {}) {
+function present(repo, summaries, output, codex, environment = {}, target = ['--worktree']) {
   return spawn(
     process.execPath,
     [
       script,
       '--repo',
       repo,
-      '--worktree',
+      ...target,
       '--agent',
       'codex',
       '--summaries',
@@ -240,3 +240,114 @@ process.stdout.write(JSON.stringify(response));
     await rm(root, { recursive: true, force: true });
   }
 });
+
+
+test('serves completed notes and lets an active agent finish during a remote outage', async () => {
+  const root = await mkdtemp(join(tmpdir(), 'diffsplain-remote-recovery-'));
+  const repo = await makeRepo(root, ['changed.txt']);
+  const remote = join(root, 'remote.git');
+  const bin = join(root, 'bin');
+  const failure = join(root, 'offline');
+  const release = join(root, 'release-agent');
+  const active = join(root, 'agent-active');
+  const calls = join(root, 'calls.jsonl');
+  const summaries = join(root, 'notes.json');
+  const output = join(root, 'snapshot.json');
+  const codex = join(root, 'codex.mjs');
+  let presenter;
+  let logs = '';
+  try {
+    git(repo, 'branch', '-M', 'main');
+    git(repo, 'switch', '-qc', 'feature');
+    git(repo, 'commit', '-qam', 'feature');
+    execFileSync('git', ['init', '--bare', '-q', remote]);
+    git(repo, 'remote', 'add', 'origin', remote);
+    git(repo, 'push', '-q', 'origin', 'main', 'feature');
+    git(repo, 'switch', '-q', 'main');
+    await mkdir(bin);
+    await writeFile(join(bin, 'git'), `#!/usr/bin/env node
+const { existsSync } = require('node:fs');
+const { spawnSync } = require('node:child_process');
+if (process.argv.includes('fetch') && existsSync(${JSON.stringify(failure)})) {
+  process.stderr.write('fatal: Failed to connect to github.com port 443');
+  process.exit(128);
+}
+const result = spawnSync('git', process.argv.slice(2), {
+  env: { ...process.env, PATH: process.env.RECOVERY_REAL_PATH }, stdio: 'inherit',
+});
+process.exit(result.status ?? 1);
+`);
+    await chmod(join(bin, 'git'), 0o755);
+    await writeFile(codex, `#!/usr/bin/env node
+import { appendFileSync, existsSync, readFileSync, writeFileSync } from 'node:fs';
+const input = JSON.parse(readFileSync(0, 'utf8'));
+const paths = input.files.map((file) => file.path);
+appendFileSync(${JSON.stringify(calls)}, JSON.stringify(paths) + '\\n');
+if (!paths.length) {
+  writeFileSync(${JSON.stringify(active)}, 'active');
+  while (!existsSync(${JSON.stringify(release)})) {
+    await new Promise((resolve) => setTimeout(resolve, 25));
+  }
+}
+process.stdout.write(JSON.stringify({
+  change: { title: 'Feature', summary: 'Updates text.', why: 'Tests recovery.', highlights: [], risks: [] },
+  files: paths.map((path) => ({ path, title: 'Updated text', what: 'Changes text.', why: 'Tests recovery.', details: [], risks: [] })),
+}));
+`);
+    await chmod(codex, 0o755);
+    presenter = present(repo, summaries, output, codex, {
+      PATH: `${bin}:${process.env.PATH}`,
+      RECOVERY_REAL_PATH: process.env.PATH,
+      XDG_CACHE_HOME: join(root, 'cache'),
+      XDG_CONFIG_HOME: join(root, 'config'),
+      DIFFSPLAIN_WATCH_INTERVAL_MS: '50',
+      DIFFSPLAIN_REMOTE_REFRESH_INTERVAL_MS: '200',
+    }, ['--branch', 'feature', '--base', 'main']);
+    presenter.stdout.on('data', (chunk) => { logs += chunk; });
+    presenter.stderr.on('data', (chunk) => { logs += chunk; });
+    const ready = await waitFor(() => {
+      const line = logs.split('\n').find((value) => value.startsWith('{"event":"ready"'));
+      return line && JSON.parse(line);
+    });
+    const url = new URL('diff-data.json', ready.url);
+    url.searchParams.set('access', ready.access);
+    const served = async () => {
+      const response = await fetch(url);
+      assert.equal(response.status, 200);
+      return response.json();
+    };
+    await waitFor(() => readFile(active, 'utf8'));
+    const before = await served();
+    assert.equal(before.files[0].noteReady, true);
+    assert.equal(before.notes.status, 'generating');
+    await writeFile(failure, 'offline');
+    await waitFor(() => (logs.match(/Keeping the last valid review/g) || []).length >= 2);
+    assert.equal(presenter.exitCode, null, logs);
+    assert.deepEqual(await served(), before);
+    await writeFile(release, 'finish');
+    const completed = await waitFor(async () => {
+      const snapshot = await served();
+      return snapshot.notes.complete && snapshot;
+    });
+    assert.equal(completed.files[0].noteReady, true);
+    const callsBefore = await readFile(calls, 'utf8');
+    const notesBefore = await readFile(summaries, 'utf8');
+    await rm(failure);
+    await waitFor(() => logs.includes('Remote refresh recovered'));
+    const recovered = await served();
+    assert.deepEqual(recovered.files, completed.files);
+    assert.deepEqual(recovered.change, completed.change);
+    assert.deepEqual(recovered.notes, completed.notes);
+    assert.deepEqual(recovered.repo, completed.repo);
+    assert.equal(await readFile(calls, 'utf8'), callsBefore);
+    assert.equal(await readFile(summaries, 'utf8'), notesBefore);
+    const stopped = await stop(presenter);
+    assert.equal(stopped.code, 0);
+    await assert.rejects(fetch(url));
+  } catch (error) {
+    throw new Error(`${error.message}\n${logs}`, { cause: error });
+  } finally {
+    await stopIfRunning(presenter);
+    await rm(root, { recursive: true, force: true });
+  }
+});

tests/remote-targets.test.mjs

@@ -34,14 +34,24 @@ function run(repo, args, options = {}) {
   );
 }
 
-async function proxyRemote(fixture, remoteUrl) {
+async function proxyRemote(fixture, remoteUrl, failurePath) {
   const bin = join(fixture.root, "git-proxy");
   const proxy = join(bin, "git");
   await mkdir(bin);
   await writeFile(
     proxy,
     `#!/usr/bin/env node
 const { spawnSync } = require("node:child_process");
+const { existsSync, readFileSync, appendFileSync } = require("node:fs");
+const failurePath = ${JSON.stringify(failurePath) || "undefined"};
+if (failurePath && existsSync(failurePath)) {
+  const failure = JSON.parse(readFileSync(failurePath, "utf8"));
+  if (process.argv.includes(failure.command)) {
+    appendFileSync(failurePath + ".attempts", Date.now() + "\\n");
+    process.stderr.write(failure.message);
+    process.exit(128);
+  }
+}
 const args = process.argv.slice(2).map((arg) =>
   arg === ${JSON.stringify(remoteUrl)}
     ? ${JSON.stringify(fixture.remote)}
@@ -1344,3 +1354,101 @@ test("rejects conflicting remote target flags", async () => {
     await rm(fixture.root, { recursive: true, force: true });
   }
 });
+
+async function remoteWatchTargetArgs(fixture, target, failure) {
+  if (!target.startsWith("pr")) return ["--branch", "feature"];
+  const gh = join(fixture.root, "git-proxy", "gh");
+  await writeFile(gh, `#!/usr/bin/env node
+const { existsSync, readFileSync, appendFileSync } = require("node:fs");
+const failurePath = ${JSON.stringify(failure)};
+if (existsSync(failurePath)) {
+  const failure = JSON.parse(readFileSync(failurePath, "utf8"));
+  if (failure.command === "gh") {
+    appendFileSync(failurePath + ".attempts", Date.now() + "\\n");
+    process.stderr.write(failure.message);
+    process.exit(1);
+  }
+}
+process.stdout.write(JSON.stringify({
+  number: 7, title: "Feature", url: "https://github.com/example/project/pull/7",
+  baseRefName: "main", baseRefOid: ${JSON.stringify(fixture.mainOid)},
+  headRefName: "feature", headRefOid: ${JSON.stringify(fixture.featureOid)}
+}));
+`);
+  await chmod(gh, 0o755);
+  execFileSync("git", ["--git-dir", fixture.remote, "update-ref", "refs/pull/7/head", fixture.featureOid]);
+  return ["--pr", "7"];
+}
+
+for (const [target, failureMessage] of [
+  ["branch fetch", "fatal: Failed to connect to github.com port 443: Couldn't connect to server"],
+  ["branch ls-remote", "fatal: Could not resolve host: github.com"],
+  ["pr fetch", "fatal: The requested URL returned error: 500"],
+  ["pr gh", "HTTP 500: Internal Server Error (https://api.github.com/graphql)"],
+]) {
+  test(`retains the snapshot and retries after transient ${target} failures`, async () => {
+    const fixture = await makeRemoteRepo();
+    const output = join(fixture.root, "watch.json");
+    const failure = join(fixture.root, "failure.json");
+    const remoteUrl = "https://github.com/example/project.git";
+    let watched;
+    try {
+      const env = await proxyRemote(fixture, remoteUrl, failure);
+      git(fixture.repo, "remote", "set-url", "origin", remoteUrl);
+      const args = [
+        ...await remoteWatchTargetArgs(fixture, target, failure),
+        "--cache-dir", join(fixture.root, "cache"), "--watch", "--output", output,
+      ];
+      watched = startWatcher(fixture.repo, args, { env });
+      await waitForSnapshot(output, watched, (value) => value.repo.head === fixture.featureOid);
+      const original = await readFile(output, "utf8");
+      await writeFile(failure, JSON.stringify({
+        command: target.split(" ")[1],
+        message: failureMessage,
+      }));
+      await waitFor(() => watched.logs().includes("Keeping the last valid review") || watched.child.exitCode !== null);
+      assert.equal(watched.child.exitCode, null, watched.logs());
+      assert.match(watched.logs(), /Keeping the last valid review.*retry/i);
+      await waitFor(async () => (await readFile(`${failure}.attempts`, "utf8")).trim().split("\n").length >= 3);
+      const attempts = (await readFile(`${failure}.attempts`, "utf8")).trim().split("\n").map(Number);
+      assert.ok(attempts[2] - attempts[0] >= 150, "retries respect the remote refresh interval");
+      assert.equal(await readFile(output, "utf8"), original);
+      await rm(failure);
+      await waitFor(() => watched.logs().includes("Remote refresh recovered"));
+      assert.equal(await readFile(output, "utf8"), original);
+      if (!target.startsWith("pr")) {
+        const head = await publishFeatureUpdate(fixture, "recovered.txt", "recovered\n");
+        await waitForSnapshot(output, watched, (value) => value.repo.head === head);
+      }
+      await writeFile(failure, JSON.stringify({ command: "fetch", message: "fatal: couldn't find remote ref refs/heads/feature" }));
+      await waitFor(() => watched.child.exitCode !== null);
+      assert.equal(watched.child.exitCode, 1, "permanent target errors still stop the watcher");
+    } finally {
+      await stopIfRunning(watched);
+      await rm(fixture.root, { recursive: true, force: true });
+    }
+  });
+}
+
+test("fails initial transient remote lookup without serving an old snapshot", async () => {
+  const fixture = await makeRemoteRepo();
+  const output = join(fixture.root, "old.json");
+  const failure = join(fixture.root, "failure.json");
+  const remoteUrl = "https://github.com/example/project.git";
+  try {
+    const env = await proxyRemote(fixture, remoteUrl, failure);
+    git(fixture.repo, "remote", "set-url", "origin", remoteUrl);
+    await writeFile(output, '{"old":true}\n');
+    await writeFile(failure, JSON.stringify({ command: "fetch", message: "fatal: Failed to connect to github.com port 443" }));
+    const result = spawnSync(process.execPath, [script, "--repo", fixture.repo,
+      "--branch", "feature", "--watch", "--cache-dir", join(fixture.root, "cache"), "--output", output],
+    { env, encoding: "utf8", timeout: 10_000 });
+    assert.equal(result.status, 1);
+    assert.match(result.stderr, /Could not fetch the remote target/);
+    assert.doesNotMatch(result.stdout, /Wrote|No diff-data changes/);
+    assert.doesNotMatch(result.stderr, /Keeping the last valid review/);
+    assert.equal(await readFile(output, "utf8"), '{"old":true}\n');
+  } finally {
+    await rm(fixture.root, { recursive: true, force: true });
+  }
+});