Stop foreground PR polling from burning the whole GitHub quota

apps/ade-cli/src/adeRpcServer.ts

@@ -5265,13 +5265,24 @@ async function runTool(args: {
   if (name === "pr_get_review_comments") {
     const prId = assertNonEmptyString(toolArgs.prId, "prId");
     const prSvc = requirePrService(runtime);
+    // A failed thread read is reported, not flattened to `[]`. Swallowing it
+    // let "GitHub refused" render as "No actionable PR comments." — the same
+    // failure-reads-as-empty conflation this lane removed from `getChecks`, and
+    // the one an agent reads right before deciding a PR is clean.
+    let reviewThreadsUnavailable: string | null = null;
     const [comments, reviews, checks, reviewThreads] = await Promise.all([
       prSvc.getComments(prId),
       prSvc.getReviews(prId),
       prSvc.getChecks(prId),
-      prSvc.getReviewThreads(prId).catch(() => []),
+      prSvc.getReviewThreads(prId).catch((error: unknown) => {
+        reviewThreadsUnavailable = error instanceof Error ? error.message : String(error);
+        return [];
+      }),
     ]);
-    return summarizePrReviewComments(prId, comments, reviews, checks, reviewThreads);
+    return {
+      ...summarizePrReviewComments(prId, comments, reviews, checks, reviewThreads),
+      ...(reviewThreadsUnavailable ? { reviewThreadsUnavailable } : {}),
+    };
   }
 
   if (name === "pr_rerun_failed_checks") {

apps/ade-cli/src/headlessLinearServices.test.ts

@@ -281,6 +281,51 @@ describe("headlessLinearServices", () => {
     }
   });
 
+  it("times out a response body that stalls after headers arrive", async () => {
+    // The header timer is cleared the moment headers land, so a body that
+    // stalls mid-stream used to leave the read pending forever: the transport
+    // failure was never recorded and the poller tick that awaited it never
+    // completed. The desktop owner has always bounded this phase.
+    vi.useFakeTimers();
+    const environment = isolateHeadlessGithubAuth("ade-headless-github-body-timeout-", {
+      emptyGhConfig: true,
+    });
+    const previousFetch = globalThis.fetch;
+    const fetchImpl = vi.fn(async (_input: RequestInfo | URL, init?: RequestInit) => {
+      // Headers arrive; the body never does, until the request is aborted.
+      const signal = init?.signal;
+      return {
+        ok: true,
+        status: 200,
+        headers: new Headers({ "content-type": "application/json" }),
+        text: () => new Promise<string>((_resolve, reject) => {
+          signal?.addEventListener("abort", () => {
+            const error = new Error("The operation was aborted");
+            error.name = "AbortError";
+            reject(error);
+          }, { once: true });
+        }),
+      } as unknown as Response;
+    }) as unknown as typeof fetch;
+    globalThis.fetch = fetchImpl;
+    const githubService = createHeadlessGitHubService(
+      "/tmp/ade-project",
+      { debug() {}, info() {}, warn() {}, error() {} } as any,
+      { fetchImpl },
+    );
+    try {
+      githubService.setToken("ghp_body_timeout_token");
+      const pending = githubService.apiRequest({ method: "GET", path: "/user" });
+      const settled = pending.then(() => "resolved").catch((error: unknown) => String(error));
+      await vi.advanceTimersByTimeAsync(120_000);
+      await expect(settled).resolves.toContain("response body timed out");
+    } finally {
+      globalThis.fetch = previousFetch;
+      vi.useRealTimers();
+      environment.restore?.();
+    }
+  });
+
   it("clears only changed PAT health when headless credentials change", () => {
     const environment = isolateHeadlessGithubAuth("ade-headless-github-pat-health-", {
       emptyGhConfig: true,

apps/ade-cli/src/headlessLinearServices.ts

@@ -33,6 +33,7 @@ import type {
   GitHubCredentialVerification,
   GitHubRepoRef,
   GitHubRateLimitState,
+  GitHubRequestBudget,
   GitHubStatus,
   CtoAttentionState,
 } from "../../desktop/src/shared/types";
@@ -89,6 +90,7 @@ import {
   clearGithubCredentialHealth,
   githubBackgroundRequestPauseUntilMs,
   githubCredentialCooldown,
+  githubRequestBudget,
   githubCredentialNonRateLimitCooldown,
   githubCredentialRateLimitCooldown,
   githubCredentialInventoryKey,
@@ -616,6 +618,51 @@ function parseNextGitHubLink(linkHeader: string | null): string | null {
 }
 
 const GITHUB_API_TIMEOUT_MS = 20_000;
+const GITHUB_API_BODY_TIMEOUT_MS = 30_000;
+
+function githubTimeoutError(phase: "request" | "response body"): Error {
+  return new Error(
+    `GitHub API ${phase} timed out. Check network access on this machine.`,
+  );
+}
+
+/**
+ * Bound the body read on the same controller that owns the stream.
+ *
+ * The header timer is cleared the moment headers arrive, so without this a
+ * response whose body stalls mid-stream never settles: the caller's
+ * transport-failure record never runs, and the poller tick that awaited it
+ * never completes either. The desktop owner has always bounded this phase; the
+ * two owners have to agree, because the daemon is the one that actually polls
+ * in a packaged build.
+ */
+function boundGitHubResponseBody(
+  response: Response,
+  controller: AbortController,
+  release: () => void,
+): Response {
+  const readText = response.text.bind(response);
+  let bodyTimedOut = false;
+  Object.defineProperty(response, "text", {
+    configurable: true,
+    value: async (): Promise<string> => {
+      const timer = setTimeout(() => {
+        bodyTimedOut = true;
+        controller.abort();
+      }, GITHUB_API_BODY_TIMEOUT_MS);
+      try {
+        return await readText();
+      } catch (error) {
+        if (bodyTimedOut) throw githubTimeoutError("response body");
+        throw error;
+      } finally {
+        clearTimeout(timer);
+        release();
+      }
+    },
+  });
+  return response;
+}
 
 async function fetchGitHub(
   input: string | URL,
@@ -627,20 +674,25 @@ async function fetchGitHub(
   const abortFromUpstream = (): void => controller.abort(upstreamSignal?.reason);
   if (upstreamSignal?.aborted) abortFromUpstream();
   else upstreamSignal?.addEventListener("abort", abortFromUpstream, { once: true });
+  const release = (): void => {
+    upstreamSignal?.removeEventListener("abort", abortFromUpstream);
+  };
   const timer = setTimeout(() => controller.abort(), GITHUB_API_TIMEOUT_MS);
+  let response: Response;
   try {
-    return await fetchImpl(input, { ...init, signal: controller.signal });
+    response = await fetchImpl(input, { ...init, signal: controller.signal });
   } catch (error) {
+    release();
     if (error instanceof Error && error.name === "AbortError") {
-      throw new Error(
-        "GitHub API request timed out. Check network access on this machine.",
-      );
+      throw githubTimeoutError("request");
     }
     throw error;
   } finally {
     clearTimeout(timer);
-    upstreamSignal?.removeEventListener("abort", abortFromUpstream);
   }
+  // `release` now runs when the body settles, not here — the upstream abort has
+  // to stay wired through the body phase for it to be cancellable at all.
+  return boundGitHubResponseBody(response, controller, release);
 }
 
 export function createHeadlessGitHubService(
@@ -1232,13 +1284,35 @@ export function createHeadlessGitHubService(
           releaseConditionalRequest = conditional.release;
         }
       }
+      // A request that never got an answer from GitHub — a hang, a timeout, a
+      // DNS or TLS failure, a body that stalls mid-stream — used to throw
+      // straight out of here recording nothing. That is the outage shape this
+      // lane targets, and with no record the request budget reported no failure
+      // kind, so the caller's ladder could not climb past its flat unclassified
+      // rung. Recorded with a null rate limit so it cannot clobber the real
+      // quota numbers, and the kinds this produces (`network` / `unknown`)
+      // carry no cooldown, so it can never park a credential the user's next
+      // action needs.
+      const recordTransportFailure: (error: unknown) => never = (error) => {
+        recordGithubOperationFailure(
+          candidate,
+          classifyGitHubAuthFailure({
+            message: error instanceof Error ? error.message : String(error),
+          }).authFailure,
+          null,
+        );
+        throw error;
+      };
+
       let response: Response;
       try {
         response = await requestGitHub(url, {
           method: args.method,
           headers,
           body: args.body == null ? undefined : JSON.stringify(args.body),
         });
+      } catch (error) {
+        recordTransportFailure(error);
       } finally {
         releaseConditionalRequest?.();
       }
@@ -1254,9 +1328,13 @@ export function createHeadlessGitHubService(
           method: args.method,
           headers,
           body: args.body == null ? undefined : JSON.stringify(args.body),
-        });
+        }).catch(recordTransportFailure);
       }
-      const text = await response.text();
+      // The body is read after the header timer is cleared, so a socket error
+      // mid-body surfaces here rather than above — same shape, same record.
+      // (Unlike the desktop owner, this transport arms no body timeout, so a
+      // body that stalls without erroring simply never settles.)
+      const text = await response.text().catch(recordTransportFailure);
       let data: unknown = text;
       try {
         data = text.trim().length ? JSON.parse(text) : {};
@@ -1922,6 +2000,14 @@ export function createHeadlessGitHubService(
         githubOperationCredentialCandidates(inventory.candidates, "read"),
       );
     },
+    /**
+     * Runtime-owned twin of the desktop service's budget read. The daemon owns
+     * GitHub access for runtime-bound windows, so implementing this only on the
+     * desktop side would leave the shipping build's poll governor un-gated.
+     */
+    async getRequestBudget(): Promise<GitHubRequestBudget> {
+      return githubRequestBudget();
+    },
     async getRemoteStatus() {
       const origin = await readGitOriginAsync(projectRoot);
       return {

apps/ade-cli/src/services/sync/syncRemoteCommandService.test.ts

@@ -238,6 +238,7 @@ describe("createSyncRemoteCommandService", () => {
       "agentChat.getEventHistoryPage",
       "github.getStatus",
       "github.getRemoteStatus",
+      "github.getRequestBudget",
       "ai.getStatus",
       "prs.list",
       "prs.listOpenForRepo",
@@ -2088,6 +2089,7 @@ describe("createSyncRemoteCommandService", () => {
       "history.listOperations",
       "github.getStatus",
       "github.getRemoteStatus",
+      "github.getRequestBudget",
       "github.publishCurrentProject",
       "projectConfig.get",
       "projectConfig.save",

apps/ade-cli/src/services/sync/syncRemoteCommandService.ts

@@ -108,6 +108,7 @@ import type {
   GitRevertArgs,
   GitGetUserIdentityArgs,
   GitHubRepoRef,
+  GitHubRequestBudget,
   GitHubStatus,
   GitStashPushArgs,
   GitStashRefArgs,
@@ -5338,6 +5339,11 @@ function registerMiscRemoteCommands({ args, register }: RemoteCommandRegistratio
     }));
   register("github.getRemoteStatus", { viewerAllowed: true, observesAbort: true }, async (): Promise<{ repo: GitHubRepoRef | null; hasOrigin: boolean }> =>
     requireService(args.githubService, "GitHub service not available.").getRemoteStatus());
+  // The web client's PR timers run in the browser but spend THIS machine's
+  // quota. Without this registration its adapter falls back to an all-null
+  // budget and its 5-second checks loop never sees the reserve.
+  register("github.getRequestBudget", { viewerAllowed: true, observesAbort: true }, async (): Promise<GitHubRequestBudget> =>
+    requireService(args.githubService, "GitHub service not available.").getRequestBudget());
   register("github.publishCurrentProject", { viewerAllowed: true }, async (payload): Promise<PublishProjectResult> => {
     const { owner, name, description, isPrivate } = parsePublishCurrentProjectArgs(payload);
     return await requireService(args.githubService, "GitHub service not available.").publishCurrentProject({

apps/ade-cli/src/tuiClient/__tests__/rightPaneFormatters.test.ts

@@ -276,6 +276,51 @@ describe("rightPaneFormatters", () => {
     expect(body).toContain("CI: not run");
   });
 
+  it("shows a failed checks read as a failure, not as an empty suite", () => {
+    // `prService.getChecks` now REJECTS when neither checks source could be
+    // read, and `/pr checks` catches that into `{ error }`. Rendering that as
+    // "No PR checks." would tell the reader the commit ran nothing — the same
+    // false all-clear the host-side change exists to stop.
+    const body = formatPrChecks({ error: "GitHub API rate limit exceeded" });
+
+    expect(body).toContain("could not be read");
+    expect(body).toContain("GitHub API rate limit exceeded");
+    expect(body).not.toContain("No PR checks.");
+  });
+
+  it("shows a failed comments read as a failure", () => {
+    const body = formatPrComments({ error: "fetch failed" });
+
+    expect(body).toContain("could not be read");
+    expect(body).toContain("fetch failed");
+    expect(body).not.toContain("No actionable PR comments.");
+  });
+
+  it("names the sources that failed in a partial PR review read", () => {
+    // `/pr review` fires three reads and catches each independently, so a
+    // partial outage is the common case. The sources that answered still
+    // render; the ones that did not must not report a count of zero.
+    const body = formatPrReview({
+      reviews: [{ state: "approved", reviewer: "octocat" }],
+      threads: { error: "GitHub is unavailable" },
+      comments: { error: "GitHub is unavailable" },
+    });
+
+    expect(body).toContain("1 review");
+    expect(body).toContain("threads unavailable");
+    expect(body).toContain("comments unavailable");
+    expect(body).toContain("GitHub is unavailable");
+    expect(body).not.toContain("0 threads");
+    expect(body).not.toContain("No PR reviews or comments.");
+  });
+
+  it("keeps the empty verdict when every PR review read succeeded", () => {
+    const body = formatPrReview({ reviews: [], threads: [], comments: [] });
+
+    expect(body).toContain("No PR reviews or comments.");
+    expect(body).not.toContain("could not be read");
+  });
+
   it("summarizes PR review comments and threads", () => {
     const body = formatPrComments({
       summary: { checksStatus: "passing", actionableComments: 2 },
@@ -300,6 +345,33 @@ describe("rightPaneFormatters", () => {
     expect(body).not.toContain("\"reviewThreads\"");
   });
 
+  it("names a failed review-thread read instead of reporting nothing to action", () => {
+    // The aggregate preserves a thread-read failure rather than flattening it
+    // to `[]`. Rendering that as "No actionable PR comments." tells an agent a
+    // PR is clean on the strength of a read that never happened.
+    const body = formatPrComments({
+      summary: { checksStatus: "passing", actionableComments: 0 },
+      reviewThreads: [],
+      comments: [],
+      reviewThreadsUnavailable: "GitHub API request failed (HTTP 503)",
+    });
+
+    expect(body).toContain("Could not be read");
+    expect(body).toContain("review threads: GitHub API request failed (HTTP 503)");
+    expect(body).not.toContain("No actionable PR comments.");
+  });
+
+  it("still reports a genuinely empty comment set as empty", () => {
+    const body = formatPrComments({
+      summary: { checksStatus: "passing", actionableComments: 0 },
+      reviewThreads: [],
+      comments: [],
+    });
+
+    expect(body).toContain("No actionable PR comments.");
+    expect(body).not.toContain("Could not be read");
+  });
+
   it("summarizes full PR review data", () => {
     const body = formatPrReview({
       reviews: [{ reviewer: "maintainer", state: "changes_requested", body: "Needs a test." }],