fix(coding-agents): bound the 429 retry by the caller's clock, not a constant (#3425)

The retry budget was a flat 6s, so a Retry-After longer than that was never
honoured. That was defensible mid-session — the next write-back replaces the
whole document — but wrong on a session's LAST Stop, where there is no next one,
and wrong for the persistent-plugin harnesses, which have no host timer at all
and could easily have waited.

The budget is now a deadline the caller supplies, because only the caller knows
its clock:

- Hook harnesses pass the host's own kill timeout, which is per-harness and not
  a constant: 60s for Claude Code, Codex, Copilot, Devin and Grok, but 30s for
  Cursor and Antigravity. RetainHookSpec carries `hostTimeoutSec` — the same
  number the installer writes into the hook registration — and the deadline is
  process start plus that, less a 2s margin for the response to come back.
- The persistent-plugin runtime has no external timer, so it takes the default
  60s window and can ride out a rate limit a hook could not.

A retry now also has to leave room for the request itself (`req` aborts at 15s):
starting a wait that cannot finish before the deadline buys nothing.

The rule that a Retry-After is honoured in full or not at all is unchanged —
waiting less than the server asked just earns another 429.

Author: nicoloboschi

hindsight-integrations/coding-agents/src/core/chat.test.ts

@@ -367,16 +367,53 @@ describe("retainLiveSession — incremental write-back", () => {
     expect(retain).toHaveBeenCalledTimes(1);
   });
 
+  it("honours a long Retry-After when the caller's clock has room for it", async () => {
+    // A persistent-plugin runtime is not on a host's kill timer, so a 20s rate limit is worth
+    // waiting out rather than deferring — the fixed 6s budget this replaced could not express that.
+    vi.useFakeTimers();
+    try {
+      const { retain, client } = stubClient();
+      retain.mockRejectedValueOnce(new RateLimitedError(20_000));
+      const done = retainLiveSession(client, "s1", turns(3), "2026-01-01T00:00:00Z", "opencode", {
+        cursors: memoryCursorStore(),
+        retryUntil: Date.now() + 120_000,
+      });
+      await vi.advanceTimersByTimeAsync(20_000);
+      await done;
+      expect(retain).toHaveBeenCalledTimes(2);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  it("does not start a wait its caller's clock cannot finish", async () => {
+    // Same 20s rate limit, but a hook with ~10s of host timeout left: waiting would be killed
+    // mid-write. Defer instead — the next write-back replaces the whole document.
+    const { retain, client } = stubClient();
+    retain.mockRejectedValue(new RateLimitedError(20_000));
+
+    await expect(
+      retainLiveSession(client, "s1", turns(3), "2026-01-01T00:00:00Z", "codex", {
+        cursors: memoryCursorStore(),
+        retryUntil: Date.now() + 10_000,
+      })
+    ).rejects.toBeInstanceOf(RateLimitedError);
+    expect(retain).toHaveBeenCalledTimes(1);
+  });
+
   it("does not retry when Retry-After exceeds what a hook can wait", async () => {
     // Waiting less than the server asked would just earn another 429, and waiting the full 60s
     // risks the harness killing the hook mid-write. Leave it to the next write-back, which
     // replaces the whole document.
     const { retain, client } = stubClient();
     retain.mockRejectedValue(new RateLimitedError(60_000));
 
-    await expect(write(client, turns(3), memoryCursorStore())).rejects.toBeInstanceOf(
-      RateLimitedError
-    );
+    await expect(
+      retainLiveSession(client, "s1", turns(3), "2026-01-01T00:00:00Z", "codex", {
+        cursors: memoryCursorStore(),
+        retryUntil: Date.now() + 20_000,
+      })
+    ).rejects.toBeInstanceOf(RateLimitedError);
     expect(retain).toHaveBeenCalledTimes(1);
   });
 

hindsight-integrations/coding-agents/src/core/chat.ts

@@ -109,17 +109,15 @@ async function supportsAppend(client: HindsightClient): Promise<boolean> {
   }
 }
 
-/**
- * Attempts a rate-limited write-back may make, and the most total time it may spend waiting.
- *
- * A hook process is on the host's clock — Claude Code allows 60s for a Stop hook, and a cold
- * daemon can already have eaten most of it — so honouring a long `Retry-After` here would trade a
- * deferred retain for a killed hook, which is strictly worse. Short limits mean a brief rate limit
- * is ridden out and a serious one is left to the next write-back, which replaces the whole
- * document anyway.
- */
+/** Retry window for a caller that did not supply one — a long-lived host with no external clock. */
+const DEFAULT_RETRY_WINDOW_MS = 60_000;
+
+/** Attempts a rate-limited write-back may make before giving up. */
 const RETAIN_RETRY_ATTEMPTS = 2;
-const RETAIN_RETRY_BUDGET_MS = 6000;
+
+/** What a retry must still leave room for: `req` aborts a request at 15s. Waiting past the point
+ *  where the retry itself could not finish buys nothing. */
+const REQUEST_BUDGET_MS = 15_000;
 
 /**
  * Submit a write-back, retrying while the API is rate-limiting us AND our write is still the
@@ -136,20 +134,20 @@ const RETAIN_RETRY_BUDGET_MS = 6000;
  */
 async function submitWithRetry(
   send: () => Promise<void>,
-  isStillNewest: () => boolean
+  isStillNewest: () => boolean,
+  retryUntil: number
 ): Promise<void> {
-  let spent = 0;
   for (let attempt = 0; ; attempt++) {
     try {
       return await send();
     } catch (e) {
       const limited = e instanceof RateLimitedError;
       if (!limited || attempt >= RETAIN_RETRY_ATTEMPTS || !isStillNewest()) throw e;
-      // Either honour Retry-After or do not retry: waiting less than the server asked would just
-      // earn another 429, so a wait that does not fit the budget means "not on this hook's clock".
+      // Either honour Retry-After in full or do not retry at all: waiting less than the server
+      // asked would just earn another 429. Whether it fits is the CALLER's clock, not a constant —
+      // a hook process is killed by its host at a known deadline, a long-lived runtime is not.
       const wait = e.retryAfterMs || 1000;
-      if (wait > RETAIN_RETRY_BUDGET_MS - spent) throw e;
-      spent += wait;
+      if (Date.now() + wait + REQUEST_BUDGET_MS > retryUntil) throw e;
       await sleep(wait);
     }
   }
@@ -209,13 +207,23 @@ export async function retainLiveSession(
   turns: TransportTurn[],
   startTs: string,
   harness?: string,
-  opts: { cursors?: RetainCursorStore; stamp?: RetainStamp } = {}
+  opts: { cursors?: RetainCursorStore; stamp?: RetainStamp; retryUntil?: number } = {}
 ): Promise<void> {
   const cursors = opts.cursors;
-  if (!cursors) return writeSession(client, sessionId, turns, startTs, harness, opts.stamp);
+  if (!cursors)
+    return writeSession(
+      client,
+      sessionId,
+      turns,
+      startTs,
+      harness,
+      opts.stamp,
+      undefined,
+      opts.retryUntil
+    );
   // Serialised so the plan is made against the previous write-back's CONFIRMED cursor (see above).
   return serialize(cursors, sessionId, () =>
-    writeSession(client, sessionId, turns, startTs, harness, opts.stamp, cursors)
+    writeSession(client, sessionId, turns, startTs, harness, opts.stamp, cursors, opts.retryUntil)
   );
 }
 
@@ -226,7 +234,9 @@ async function writeSession(
   startTs: string,
   harness?: string,
   stamp?: RetainStamp,
-  cursors?: RetainCursorStore
+  cursors?: RetainCursorStore,
+  /** Absolute time this write-back may keep retrying until; the caller owns its own clock. */
+  retryUntil = Date.now() + DEFAULT_RETRY_WINDOW_MS
 ): Promise<void> {
   const refId = `conversation:${sessionId}`;
   const appendSupported = Boolean(cursors) && (await supportsAppend(client));
@@ -287,7 +297,8 @@ async function writeSession(
       if (!cursors) return true;
       const now = cursors.read(sessionId);
       return now?.turns === claimed.turns && now?.fingerprint === claimed.fingerprint;
-    }
+    },
+    retryUntil
   );
   cursors?.write(sessionId, next);
 }

hindsight-integrations/coding-agents/src/core/retain-hook.test.ts

@@ -127,7 +127,10 @@ describe("runRetainHook anti-recursion guard", () => {
     // No stdin is provided/mocked here — if the guard didn't return before `readFileSync(0, ...)`,
     // this call would attempt to read the real process stdin. Resolving without calling makeClient
     // proves the guard fired first.
-    await runRetainHook({ harness: "claude-code", parse: () => ({}) }, makeClient);
+    await runRetainHook(
+      { harness: "claude-code", hostTimeoutSec: 60, parse: () => ({}) },
+      makeClient
+    );
     expect(makeClient).not.toHaveBeenCalled();
   });
 });

hindsight-integrations/coding-agents/src/core/retain-hook.ts

@@ -24,6 +24,9 @@ import type { RetainCursorStore } from "./retain-cursor";
 import { buildRetainStamp, type RetainStamp } from "./retain-stamp";
 import { fileCursorStore } from "./session-cache";
 import { readClaudeTranscript } from "./transcript";
+
+/** Headroom left before the host's kill: the response still has to come back after the last wait. */
+const HOST_DEADLINE_MARGIN_MS = 2000;
 import type { TransportTurn } from "./chat";
 
 export interface RetainHookEventFields {
@@ -39,6 +42,11 @@ export type TranscriptReader = (path: string) => TransportTurn[];
 export interface RetainHookSpec {
   /** Harness name — config `harnesses.<name>` section, {harness} template field, diag records. */
   harness: string;
+  /** Seconds the HOST allows this hook before killing it — the same number the installer writes
+   *  into its hook registration. It varies (60s for Claude Code and Codex, 30s for Cursor and
+   *  Antigravity), and it is the only honest basis for deciding how long a rate-limited write-back
+   *  may wait: past it the process is killed mid-write, which is worse than deferring. */
+  hostTimeoutSec: number;
   /** Read the fields out of the harness's stdin event (shapes differ per harness). */
   parse(event: Record<string, unknown>): RetainHookEventFields;
   /** Harness-specific transcript parser. Defaults to the Claude JSONL reader. */
@@ -68,6 +76,8 @@ export async function buildRetain(args: {
   stamp?: RetainStamp;
   /** Injectable for tests; defaults to the per-session temp file (a Stop hook has no memory). */
   cursors?: RetainCursorStore;
+  /** Absolute time the host will kill this process; bounds any rate-limit retry. */
+  retryUntil?: number;
 }): Promise<void> {
   const { harness, sessionId, transcriptPath, client } = args;
   const readTranscript = args.readTranscript ?? readClaudeTranscript;
@@ -81,6 +91,7 @@ export async function buildRetain(args: {
     await retainLiveSession(client as HindsightClient, sessionId, turns, startTs, harness, {
       cursors: args.cursors ?? fileCursorStore(harness),
       stamp: args.stamp,
+      retryUntil: args.retryUntil,
     });
     diag(harness, "retain_ok", { ms: Date.now() - t0, turns: turns.length, session: sessionId });
   } catch (e) {
@@ -103,6 +114,9 @@ export async function runRetainHook(
   // Anti-recursion: the codebase survey's own headless claude session (core/survey.ts) sets this
   // so its hooks are a no-op — it must not retain its own survey session's transcript.
   if (process.env.HINDSIGHT_DISABLE_HOOKS) return;
+  // The host started counting when it spawned us, which is near enough to now: everything above
+  // is synchronous. A margin keeps the kill from landing between our last wait and its response.
+  const hostDeadline = Date.now() + spec.hostTimeoutSec * 1000 - HOST_DEADLINE_MARGIN_MS;
 
   let ev: Record<string, unknown> = {};
   try {
@@ -142,6 +156,7 @@ export async function runRetainHook(
     transcriptPath,
     client,
     readTranscript: spec.readTranscript,
+    retryUntil: hostDeadline,
     stamp: buildRetainStamp(cfg, {
       directory: cwd,
       harness: spec.harness,

hindsight-integrations/coding-agents/src/harness/hook-lifecycle.ts

@@ -150,6 +150,7 @@ export const HOOK_HARNESSES: Record<HookHarnessName, HookHarnessSpec> = {
     sessionStart: standardSessionStart("claude-code"),
     prompt: claudePrompt,
     retain: {
+      hostTimeoutSec: 60,
       harness: "claude-code",
       parse: (ev) => ({
         sessionId: ev.session_id as string | undefined,
@@ -168,6 +169,7 @@ export const HOOK_HARNESSES: Record<HookHarnessName, HookHarnessSpec> = {
     sessionStart: standardSessionStart("codex"),
     prompt: codexPrompt,
     retain: {
+      hostTimeoutSec: 60,
       harness: "codex",
       parse: (ev) => ({
         sessionId: ev.session_id as string | undefined,
@@ -196,6 +198,7 @@ export const HOOK_HARNESSES: Record<HookHarnessName, HookHarnessSpec> = {
     },
     prompt: antigravityPrompt,
     retain: {
+      hostTimeoutSec: 30,
       harness: "antigravity-cli",
       parse: (ev) => ({
         sessionId: ev.conversationId as string | undefined,
@@ -225,6 +228,7 @@ export const HOOK_HARNESSES: Record<HookHarnessName, HookHarnessSpec> = {
     },
     prompt: cursorPrompt,
     retain: {
+      hostTimeoutSec: 30,
       harness: "cursor-cli",
       parse: (ev) => ({
         sessionId:
@@ -259,6 +263,7 @@ export const HOOK_HARNESSES: Record<HookHarnessName, HookHarnessSpec> = {
     },
     prompt: copilotPrompt,
     retain: {
+      hostTimeoutSec: 60,
       harness: "copilot-cli",
       parse: (ev) => ({
         sessionId: ev.sessionId as string | undefined,
@@ -287,6 +292,7 @@ export const HOOK_HARNESSES: Record<HookHarnessName, HookHarnessSpec> = {
     },
     prompt: devinPrompt,
     retain: {
+      hostTimeoutSec: 60,
       harness: "devin-cli",
       parse: (ev) => {
         const sessionId = ev.session_id as string | undefined;
@@ -326,6 +332,7 @@ export const HOOK_HARNESSES: Record<HookHarnessName, HookHarnessSpec> = {
       }),
     },
     retain: {
+      hostTimeoutSec: 60,
       harness: "grok-build",
       parse: (ev) => ({
         sessionId: ev.sessionId as string | undefined,