Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion packages/coding-agent/src/modes/components/status-line/gh.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
import { type RunGh, runGhDefault } from "../../../utils/gh";

const STATUS_LINE_GH_TIMEOUT_MS = 5_000;
const STATUS_LINE_PR_CACHE_TTL_MS = 60_000;
const C0_C1_CONTROL_CHARACTERS = /[\u0000-\u001f\u007f-\u009f]/;
type CurrentPr = { number: number; url: string } | null;

interface PrCacheEntry {
value: CurrentPr;
expiresAt: number;
}

const prCache = new Map<string, PrCacheEntry>();
const prLookupsInFlight = new Map<string, Promise<CurrentPr>>();

function canonicalPrUrl(value: unknown, number: number): string | null {
if (typeof value !== "string" || C0_C1_CONTROL_CHARACTERS.test(value)) return null;
Expand All @@ -22,7 +32,7 @@ function canonicalPrUrl(value: unknown, number: number): string | null {
}
}

export async function lookupCurrentPr(runGh: RunGh = runGhDefault): Promise<{ number: number; url: string } | null> {
export async function lookupCurrentPr(runGh: RunGh = runGhDefault): Promise<CurrentPr> {
try {
const result = await runGh(["pr", "view", "--json", "number,url"], { timeoutMs: STATUS_LINE_GH_TIMEOUT_MS });
if (result.exitCode !== 0 || result.timedOut) return null;
Expand All @@ -35,3 +45,31 @@ export async function lookupCurrentPr(runGh: RunGh = runGhDefault): Promise<{ nu
return null;
}
}

export function lookupCurrentPrCached(
cacheKey: string,
runGh: RunGh = runGhDefault,
now: () => number = Date.now,
): Promise<CurrentPr> {
const cached = prCache.get(cacheKey);
if (cached && cached.expiresAt > now()) return Promise.resolve(cached.value);

const inFlight = prLookupsInFlight.get(cacheKey);
if (inFlight) return inFlight;

const lookup = lookupCurrentPr(runGh)
.then(value => {
prCache.set(cacheKey, { value, expiresAt: now() + STATUS_LINE_PR_CACHE_TTL_MS });
return value;
})
.finally(() => {
if (prLookupsInFlight.get(cacheKey) === lookup) prLookupsInFlight.delete(cacheKey);
});
prLookupsInFlight.set(cacheKey, lookup);
return lookup;
}

export function clearCurrentPrCache(): void {
prCache.clear();
prLookupsInFlight.clear();
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import type { ActionRegistry, FocusDomain } from "../action-registry";
import { EMPTY_JOBS_SNAPSHOT, type JobsSnapshot } from "../jobs-observer";
import { sanitizeStatusText } from "../shared";
import { renderSkillHudBar } from "./skill-hud/render";
import { lookupCurrentPr } from "./status-line/gh";
import { lookupCurrentPrCached } from "./status-line/gh";
import {
canReuseCachedPr,
createPrCacheContext,
Expand Down Expand Up @@ -399,6 +399,10 @@ export class StatusLineComponent implements Component {

this.#prLookupInFlight = true;
const lookupContext = currentContext;
if (!lookupContext) {
this.#prLookupInFlight = false;
return stalePr ?? null;
}

// Fire async lookup, keep stale value visible until resolved
(async () => {
Expand All @@ -415,7 +419,7 @@ export class StatusLineComponent implements Component {
};
try {
// Requires `gh repo set-default` to be configured; fails gracefully if not
const pr = await lookupCurrentPr();
const pr = await lookupCurrentPrCached(`${lookupContext.repoId ?? ""}\0${lookupContext.branch}`);
setCachedPr(pr);
} finally {
this.#prLookupInFlight = false;
Expand Down
42 changes: 40 additions & 2 deletions packages/coding-agent/test/status-line-gh.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { afterEach, describe, expect, it, vi } from "bun:test";
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
import type { Subprocess } from "bun";
import { lookupCurrentPr } from "../src/modes/components/status-line/gh";
import {
clearCurrentPrCache,
lookupCurrentPr,
lookupCurrentPrCached,
} from "../src/modes/components/status-line/gh";
import type { RunGh } from "../src/utils/gh";

function textStream(text: string): ReadableStream<Uint8Array> {
Expand All @@ -14,6 +18,10 @@ afterEach(() => {
});

describe("status-line GitHub PR lookup", () => {
beforeEach(() => {
clearCurrentPrCache();
});

it("detaches gh from TUI stdin", async () => {
const ghPath = "/usr/bin/gh";
vi.spyOn(Bun, "which").mockReturnValue(ghPath);
Expand Down Expand Up @@ -49,6 +57,36 @@ describe("status-line GitHub PR lookup", () => {
expect(timeoutMs).toBe(5_000);
});

it("negative-caches failed lookups across callers", async () => {
let calls = 0;
const runGh: RunGh = async () => {
calls += 1;
return { exitCode: 1, stdout: "", stderr: "no pull requests found", timedOut: false };
};

await expect(lookupCurrentPrCached("/repo/.git/HEAD\0feature", runGh, () => 1_000)).resolves.toBeNull();
await expect(lookupCurrentPrCached("/repo/.git/HEAD\0feature", runGh, () => 2_000)).resolves.toBeNull();
expect(calls).toBe(1);
});

it("deduplicates concurrent lookups across callers", async () => {
let calls = 0;
let resolveLookup!: (value: Awaited<ReturnType<RunGh>>) => void;
const result = new Promise<Awaited<ReturnType<RunGh>>>(resolve => {
resolveLookup = resolve;
});
const runGh: RunGh = async () => {
calls += 1;
return result;
};

const first = lookupCurrentPrCached("/repo/.git/HEAD\0feature", runGh);
const second = lookupCurrentPrCached("/repo/.git/HEAD\0feature", runGh);
expect(calls).toBe(1);
resolveLookup({ exitCode: 1, stdout: "", stderr: "no pull requests found", timedOut: false });
await expect(Promise.all([first, second])).resolves.toEqual([null, null]);
});

it("accepts canonical GitHub Enterprise PR URLs over HTTP(S)", async () => {
for (const url of [
"https://ghe.internal.example.com/teams/cli/pull/3354",
Expand Down
Loading