From efb8c8f229b00fd4eb7f118a50b97cedbd2e8413 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Sun, 29 Mar 2026 16:42:39 +0700 Subject: [PATCH] feat(server): cache getPullRequestStatus with TTL and in-flight dedup Adds @isaacs/ttlcache to avoid repeated gh CLI calls for the same working directory. Concurrent lookups for the same cwd share a single in-flight promise. Cache expires after 30s by default. --- package-lock.json | 10 + packages/server/package.json | 1 + .../server/src/utils/checkout-git.test.ts | 179 +++++++++++++++++- packages/server/src/utils/checkout-git.ts | 60 ++++++ 4 files changed, 249 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index f2b1aa353..941cf9f36 100644 --- a/package-lock.json +++ b/package-lock.json @@ -35318,6 +35318,7 @@ "@deepgram/sdk": "^3.4.0", "@getpaseo/highlight": "0.1.37", "@getpaseo/relay": "0.1.37", + "@isaacs/ttlcache": "^2.1.4", "@modelcontextprotocol/sdk": "^1.20.1", "@opencode-ai/sdk": "1.2.6", "@sctg/sentencepiece-js": "^1.1.0", @@ -35360,6 +35361,15 @@ "vitest": "^3.2.4" } }, + "packages/server/node_modules/@isaacs/ttlcache": { + "version": "2.1.4", + "resolved": "https://registry.npmjs.org/@isaacs/ttlcache/-/ttlcache-2.1.4.tgz", + "integrity": "sha512-7kMz0BJpMvgAMkyglums7B2vtrn5g0a0am77JY0GjkZZNetOBCFn7AG7gKCwT0QPiXyxW7YIQSgtARknUEOcxQ==", + "license": "BlueOak-1.0.0", + "engines": { + "node": ">=12" + } + }, "packages/server/node_modules/@modelcontextprotocol/sdk": { "version": "1.20.1", "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.20.1.tgz", diff --git a/packages/server/package.json b/packages/server/package.json index 3db464ef7..a99d632a6 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -65,6 +65,7 @@ "@deepgram/sdk": "^3.4.0", "@getpaseo/highlight": "0.1.37", "@getpaseo/relay": "0.1.37", + "@isaacs/ttlcache": "^2.1.4", "@modelcontextprotocol/sdk": "^1.20.1", "@opencode-ai/sdk": "1.2.6", "@sctg/sentencepiece-js": "^1.1.0", diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index 2edeef819..26904e40a 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -1,9 +1,19 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { execSync } from "child_process"; -import { mkdtempSync, rmSync, writeFileSync, realpathSync, mkdirSync, symlinkSync } from "fs"; +import { + mkdtempSync, + rmSync, + writeFileSync, + readFileSync, + realpathSync, + mkdirSync, + symlinkSync, +} from "fs"; import { join } from "path"; import { tmpdir } from "os"; import { + __resetPullRequestStatusCacheForTests, + __setPullRequestStatusCacheTtlForTests, commitAll, getCheckoutDiff, getCheckoutShortstat, @@ -38,6 +48,10 @@ function initRepo(): { tempDir: string; repoDir: string } { return { tempDir, repoDir }; } +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + describe("checkout git utilities", () => { let tempDir: string; let repoDir: string; @@ -48,9 +62,11 @@ describe("checkout git utilities", () => { tempDir = setup.tempDir; repoDir = setup.repoDir; paseoHome = join(tempDir, "paseo-home"); + __resetPullRequestStatusCacheForTests(); }); afterEach(() => { + __resetPullRequestStatusCacheForTests(); rmSync(tempDir, { recursive: true, force: true }); }); @@ -681,6 +697,167 @@ const x = 1; } }); + it("caches PR status results for duplicate lookups", async () => { + execSync("git checkout -b feature", { cwd: repoDir }); + execSync("git remote add origin https://github.com/getpaseo/paseo.git", { cwd: repoDir }); + + const fakeBinDir = join(tempDir, "fake-bin-gh-cache-hit"); + const callCountPath = join(tempDir, "gh-call-count.txt"); + mkdirSync(fakeBinDir); + const gitPath = execSync("command -v git", { stdio: "pipe" }).toString().trim(); + symlinkSync(gitPath, join(fakeBinDir, "git")); + writeFileSync(callCountPath, "0\n"); + writeFileSync( + join(fakeBinDir, "gh"), + [ + "#!/usr/bin/env bash", + "set -euo pipefail", + `count_file=${JSON.stringify(callCountPath)}`, + 'if [[ "${1-}" == "--version" ]]; then', + ' echo "gh version 2.0.0"', + " exit 0", + "fi", + 'args="$*"', + 'if [[ "$args" == "pr view --json url,title,state,baseRefName,headRefName,mergedAt" ]]; then', + ' count="$(cat "$count_file")"', + ' printf "%s\\n" "$((count + 1))" > "$count_file"', + ' echo \'{"url":"https://github.com/getpaseo/paseo/pull/123","title":"Ship feature","state":"OPEN","baseRefName":"main","headRefName":"feature","mergedAt":null}\'', + " exit 0", + "fi", + 'echo "unexpected gh args: $args" >&2', + "exit 1", + "", + ].join("\n"), + "utf8", + ); + execSync(`chmod +x ${join(fakeBinDir, "gh")}`); + + const originalPath = process.env.PATH; + process.env.PATH = `${fakeBinDir}:${originalPath ?? ""}`; + try { + const first = await getPullRequestStatus(repoDir); + const second = await getPullRequestStatus(repoDir); + expect(first).toEqual(second); + expect(first.status?.url).toContain("/pull/123"); + expect(readFileSync(callCountPath, "utf8").trim()).toBe("1"); + } finally { + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + } + }); + + it("expires cached PR status after the TTL", async () => { + execSync("git checkout -b feature", { cwd: repoDir }); + execSync("git remote add origin https://github.com/getpaseo/paseo.git", { cwd: repoDir }); + + const fakeBinDir = join(tempDir, "fake-bin-gh-cache-expiry"); + const callCountPath = join(tempDir, "gh-call-count-expiry.txt"); + mkdirSync(fakeBinDir); + const gitPath = execSync("command -v git", { stdio: "pipe" }).toString().trim(); + symlinkSync(gitPath, join(fakeBinDir, "git")); + writeFileSync(callCountPath, "0\n"); + writeFileSync( + join(fakeBinDir, "gh"), + [ + "#!/usr/bin/env bash", + "set -euo pipefail", + `count_file=${JSON.stringify(callCountPath)}`, + 'if [[ "${1-}" == "--version" ]]; then', + ' echo "gh version 2.0.0"', + " exit 0", + "fi", + 'args="$*"', + 'if [[ "$args" == "pr view --json url,title,state,baseRefName,headRefName,mergedAt" ]]; then', + ' count="$(cat "$count_file")"', + ' next="$((count + 1))"', + ' printf "%s\\n" "$next" > "$count_file"', + ' printf \'{"url":"https://github.com/getpaseo/paseo/pull/%s","title":"Ship feature","state":"OPEN","baseRefName":"main","headRefName":"feature","mergedAt":null}\\n\' "$next"', + " exit 0", + "fi", + 'echo "unexpected gh args: $args" >&2', + "exit 1", + "", + ].join("\n"), + "utf8", + ); + execSync(`chmod +x ${join(fakeBinDir, "gh")}`); + + const originalPath = process.env.PATH; + process.env.PATH = `${fakeBinDir}:${originalPath ?? ""}`; + __setPullRequestStatusCacheTtlForTests(50); + try { + const first = await getPullRequestStatus(repoDir); + await sleep(80); + const second = await getPullRequestStatus(repoDir); + expect(first.status?.url).toContain("/pull/1"); + expect(second.status?.url).toContain("/pull/2"); + expect(readFileSync(callCountPath, "utf8").trim()).toBe("2"); + } finally { + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + } + }); + + it("dedupes concurrent PR status lookups for the same cwd", async () => { + execSync("git checkout -b feature", { cwd: repoDir }); + execSync("git remote add origin https://github.com/getpaseo/paseo.git", { cwd: repoDir }); + + const fakeBinDir = join(tempDir, "fake-bin-gh-cache-concurrent"); + const callCountPath = join(tempDir, "gh-call-count-concurrent.txt"); + mkdirSync(fakeBinDir); + const gitPath = execSync("command -v git", { stdio: "pipe" }).toString().trim(); + symlinkSync(gitPath, join(fakeBinDir, "git")); + writeFileSync(callCountPath, "0\n"); + writeFileSync( + join(fakeBinDir, "gh"), + [ + "#!/usr/bin/env bash", + "set -euo pipefail", + `count_file=${JSON.stringify(callCountPath)}`, + 'if [[ "${1-}" == "--version" ]]; then', + ' echo "gh version 2.0.0"', + " exit 0", + "fi", + 'args="$*"', + 'if [[ "$args" == "pr view --json url,title,state,baseRefName,headRefName,mergedAt" ]]; then', + ' count="$(cat "$count_file")"', + ' printf "%s\\n" "$((count + 1))" > "$count_file"', + " sleep 0.2", + ' echo \'{"url":"https://github.com/getpaseo/paseo/pull/123","title":"Ship feature","state":"OPEN","baseRefName":"main","headRefName":"feature","mergedAt":null}\'', + " exit 0", + "fi", + 'echo "unexpected gh args: $args" >&2', + "exit 1", + "", + ].join("\n"), + "utf8", + ); + execSync(`chmod +x ${join(fakeBinDir, "gh")}`); + + const originalPath = process.env.PATH; + process.env.PATH = `${fakeBinDir}:${originalPath ?? ""}`; + try { + const [first, second] = await Promise.all([ + getPullRequestStatus(repoDir), + getPullRequestStatus(repoDir), + ]); + expect(first).toEqual(second); + expect(readFileSync(callCountPath, "utf8").trim()).toBe("1"); + } finally { + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + } + }); + it("returns typed MergeConflictError on merge conflicts", async () => { const conflictFile = join(repoDir, "conflict.txt"); writeFileSync(conflictFile, "base\n"); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index ded5c96cd..1fbda4393 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -3,6 +3,7 @@ import { promisify } from "util"; import { resolve, dirname, basename } from "path"; import { realpathSync } from "fs"; import { open as openFile, stat as statFile } from "fs/promises"; +import { TTLCache } from "@isaacs/ttlcache"; import type { ParsedDiffFile } from "../server/utils/diff-highlighter.js"; import { parseAndHighlightDiff } from "../server/utils/diff-highlighter.js"; import { isPaseoOwnedWorktreeCwd } from "./worktree.js"; @@ -16,6 +17,40 @@ const READ_ONLY_GIT_ENV: NodeJS.ProcessEnv = { }; const SMALL_OUTPUT_MAX_BUFFER = 20 * 1024 * 1024; // 20MB +const DEFAULT_PULL_REQUEST_STATUS_CACHE_TTL_MS = 30_000; +const PULL_REQUEST_STATUS_CACHE_MAX = 1_000; + +let pullRequestStatusCacheTtlMs = DEFAULT_PULL_REQUEST_STATUS_CACHE_TTL_MS; +let pullRequestStatusCache = createPullRequestStatusCache(pullRequestStatusCacheTtlMs); +const pullRequestStatusInFlight = new Map>(); + +function createPullRequestStatusCache(ttlMs: number) { + return new TTLCache({ + ttl: ttlMs, + max: PULL_REQUEST_STATUS_CACHE_MAX, + checkAgeOnGet: true, + }); +} + +function getPullRequestStatusCacheKey(cwd: string): string { + return resolve(cwd); +} + +export function __resetPullRequestStatusCacheForTests(): void { + pullRequestStatusCache.clear(); + pullRequestStatusCache.cancelTimer(); + pullRequestStatusCacheTtlMs = DEFAULT_PULL_REQUEST_STATUS_CACHE_TTL_MS; + pullRequestStatusCache = createPullRequestStatusCache(pullRequestStatusCacheTtlMs); + pullRequestStatusInFlight.clear(); +} + +export function __setPullRequestStatusCacheTtlForTests(ttlMs: number): void { + pullRequestStatusCache.clear(); + pullRequestStatusCache.cancelTimer(); + pullRequestStatusCacheTtlMs = ttlMs; + pullRequestStatusCache = createPullRequestStatusCache(ttlMs); + pullRequestStatusInFlight.clear(); +} async function execGit( command: string, @@ -1807,6 +1842,31 @@ export async function createPullRequest( } export async function getPullRequestStatus(cwd: string): Promise { + const cacheKey = getPullRequestStatusCacheKey(cwd); + const cached = pullRequestStatusCache.get(cacheKey); + if (cached) { + return cached; + } + + const existing = pullRequestStatusInFlight.get(cacheKey); + if (existing) { + return existing; + } + + const lookup = getPullRequestStatusUncached(cwd) + .then((status) => { + pullRequestStatusCache.set(cacheKey, status); + return status; + }) + .finally(() => { + pullRequestStatusInFlight.delete(cacheKey); + }); + + pullRequestStatusInFlight.set(cacheKey, lookup); + return lookup; +} + +async function getPullRequestStatusUncached(cwd: string): Promise { await requireGitRepo(cwd); const head = await getCurrentBranch(cwd); if (!head) {