fix(claude-swe-agent): stop a fast turn from racing the Remote Control session URL (#183)

apps/claude-code-swe-agent/src/claude-runner-remote-control.test.ts

@@ -35,6 +35,11 @@ async function installFake(name: string, body: string): Promise<void> {
 //
 // Modes:
 //   complete  - bridge line + assistant text + turn_duration, written at once
+//   latebridge- assistant text + turn_duration first, then the bridge line
+//               appended FAKE_LATE_BRIDGE_MS later. Models issue #183: a fast
+//               read-only turn that finishes before the bridge finishes its
+//               async registration with claude.ai, so the session URL is not in
+//               the transcript yet when turn_duration lands.
 //   running   - bridge line + assistant text, never completes (works, then wedges)
 //   stalled   - bridge line only (registers, but the turn never begins)
 //   streaming - bridge line, then a tool_use entry every FAKE_STEP_MS for
@@ -77,6 +82,13 @@ if (mode !== "nofile") {
       clearInterval(timer);
       fs.appendFileSync(file, assistant(process.env.FAKE_ASSISTANT_TEXT || "DONE") + "\\n" + done() + "\\n");
     }, stepMs);
+  } else if (mode === "latebridge") {
+    // Turn finishes (assistant text + turn_duration) BEFORE the bridge line is
+    // written -- the bridge's registration handshake lands FAKE_LATE_BRIDGE_MS
+    // later. Reproduces issue #183's dropped session URL on fast turns.
+    fs.writeFileSync(file, assistant(process.env.FAKE_ASSISTANT_TEXT || "DONE") + "\\n" + done() + "\\n");
+    const lateMs = Number(process.env.FAKE_LATE_BRIDGE_MS || 60);
+    setTimeout(() => { fs.appendFileSync(file, bridge() + "\\n"); }, lateMs);
   } else {
     const lines = [bridge()];
     if (mode !== "stalled") lines.push(assistant(process.env.FAKE_ASSISTANT_TEXT || "DONE"));
@@ -165,6 +177,32 @@ describe("runClaudeTurnRemoteControlled (interactive)", () => {
     expect(progress).toContainEqual({ message: "https://claude.ai/code/session_TESTBRIDGE", stage: "remote-control-url" });
   });
 
+  // Regression for issue #183 ("Review Agent Doesn't Post a Session Link"). The
+  // "watch live / take over the session" link was posted for most runs but
+  // dropped intermittently on the fast, read-only `ai-review` flow. Root cause:
+  // the turn can finish (turn_duration) BEFORE the Remote Control bridge writes
+  // its `bridgeSessionId` line (that line lands when the async registration with
+  // claude.ai completes), and the loop used to return the instant it saw
+  // completion -- so the URL was never emitted. Here the bridge line is written
+  // AFTER completion; the URL must still be reported (deterministically), not
+  // raffled off by which of the two the transcript flushed first.
+  it("still reports the session URL when the bridge line lands AFTER turn_duration (issue #183)", async () => {
+    const progress: Array<{ message: string; stage: string }> = [];
+    const result = await runClaudeTurnRemoteControlled("review the PR", {
+      cwd,
+      env: env({ FAKE_SCRIPT_MODE: "latebridge", FAKE_LATE_BRIDGE_MS: "60" }),
+      settings: {},
+      runId: "run-late-bridge",
+      pollIntervalMs: 20,
+      urlGraceMs: 5000,
+      maxWaitMs: 5000,
+      onProgress: (message, stage) => progress.push({ message, stage }),
+    });
+
+    expect(result).toMatchObject({ finalMessage: "DONE", failed: false, authError: false });
+    expect(progress).toContainEqual({ message: "https://claude.ai/code/session_TESTBRIDGE", stage: "remote-control-url" });
+  });
+
   // Regression for issue #149. A feature-sized coding turn ran correctly for
   // half an hour and was then killed by an ABSOLUTE 30-minute cap and reported
   // to the user as "Timed out after 1800000ms waiting for the remote-control

apps/claude-code-swe-agent/src/claude-runner.ts

@@ -57,6 +57,12 @@ export interface RemoteControlRunOptions extends ClaudeRunOptions {
    * {@link REMOTE_CONTROL_STARTUP_TIMEOUT_MS}; injectable for tests.
    */
   startupTimeoutMs?: number;
+  /**
+   * How long a finished turn may be held open, after completion, waiting for the
+   * Remote Control bridge to surface its session URL when it has not already.
+   * Defaults to {@link REMOTE_CONTROL_URL_GRACE_MS}; injectable for tests.
+   */
+  urlGraceMs?: number;
   /**
    * Optional ABSOLUTE wall-clock backstop. Defaults to no bound at all: a real
    * coding turn can legitimately run for hours, the idle bound above is what
@@ -417,6 +423,34 @@ const REMOTE_CONTROL_WAITING_TIMEOUT_MS = 5 * 60_000;
  */
 const REMOTE_CONTROL_STARTUP_TIMEOUT_MS = 5 * 60_000;
 
+/**
+ * How long a FINISHED turn may be held open, after `turn_duration` lands, purely
+ * to let the Remote Control bridge surface its session URL (the transcript's
+ * `bridgeSessionId` line) before we return.
+ *
+ * This is the deterministic fix for issue #183: the "watch live / take over the
+ * session" link was posted for most runs but dropped intermittently on the
+ * `ai-review` flow. The two facts behind it are the reply loop's ordering (the
+ * URL is emitted the instant `bridgeSessionId` is seen, `turn_duration` ends the
+ * wait) and that the bridge's `bridgeSessionId` line is written when its ASYNC
+ * registration handshake with claude.ai completes -- NOT necessarily before the
+ * turn does. A long triage turn registers the bridge many polls before it
+ * finishes, so the URL is always emitted first. A fast, read-only review can
+ * finish in the same poll window the bridge is still registering in, and
+ * returning the instant `turn_duration` appears drops the link -- which of the
+ * two the transcript flushed first is a race, and that race is exactly the
+ * "sometimes it fails to" the maintainer saw. Reporting the URL is deterministic
+ * once the `bridgeSessionId` line exists; this bound just stops a fast
+ * completion from racing it out of existence.
+ *
+ * Only ever paid when a turn completes WITHOUT its URL already reported (the
+ * failing case) -- the common path, where the bridge registered early and the
+ * URL was emitted mid-turn, skips this entirely. Bounded because a bridge that
+ * has not registered within seconds of the turn finishing almost certainly
+ * never will (registration failed), and a completed review must not hang on it.
+ */
+const REMOTE_CONTROL_URL_GRACE_MS = 15_000;
+
 /**
  * Builds the Remote Control URL from a transcript `bridgeSessionId`, mirroring
  * the CLI's own `toCompatSessionId` (decompiled v2.1.218): a `cse_`-prefixed
@@ -880,6 +914,7 @@ export async function runClaudeTurnRemoteControlled(
   const idleStatusGraceMs = opts.idleStatusGraceMs ?? REMOTE_CONTROL_IDLE_STATUS_GRACE_MS;
   const waitingTimeoutMs = opts.waitingTimeoutMs ?? REMOTE_CONTROL_WAITING_TIMEOUT_MS;
   const startupTimeoutMs = opts.startupTimeoutMs ?? REMOTE_CONTROL_STARTUP_TIMEOUT_MS;
+  const urlGraceMs = opts.urlGraceMs ?? REMOTE_CONTROL_URL_GRACE_MS;
   const heartbeatMs = opts.heartbeatIntervalMs ?? HEARTBEAT_INTERVAL_MS;
   // No absolute cap by default -- see `maxWaitMs`'s doc comment and issue #149.
   const absoluteDeadline = opts.maxWaitMs === undefined ? Infinity : Date.now() + opts.maxWaitMs;
@@ -999,6 +1034,28 @@ export async function runClaudeTurnRemoteControlled(
                 sessionId: session.sessionId,
               };
             }
+            // Issue #183: don't let a fast completion race the Remote Control
+            // URL out of existence. The URL is emitted the instant the
+            // transcript's `bridgeSessionId` line appears (line ~1010 above),
+            // but that line lands when the bridge's async registration with
+            // claude.ai completes -- which, for a quick read-only review, can be
+            // AFTER `turn_duration`. If we haven't reported the URL yet, hold the
+            // finished turn open briefly and keep re-reading the transcript so a
+            // just-registered bridge still surfaces its link before we return.
+            // Bounded by `urlGraceMs`; skipped entirely once the URL is known
+            // (the common case, where the bridge registered mid-turn), so it
+            // costs nothing on the happy path. The interactive child stays
+            // resident (killed only in the `finally` below), so its registration
+            // keeps progressing throughout this wait.
+            if (!urlReported) {
+              const graceDeadline = Date.now() + urlGraceMs;
+              while (!urlReported && !opts.signal?.aborted && Date.now() < graceDeadline) {
+                await sleep(pollIntervalMs, opts.signal);
+                const graceRaw = await readTranscript(homeDir, opts.cwd, session.sessionId);
+                const graceBridge = graceRaw ? parseTranscript(graceRaw).bridgeSessionId : null;
+                if (graceBridge) reportUrl(remoteControlUrlFromBridge(graceBridge));
+              }
+            }
             return { finalMessage: st.finalText, failed: false, failureDetail: null, authError: false, sessionId: session.sessionId };
           }
         }