Compare commits

..

1 Commits

Author SHA1 Message Date
Matthew Breedlove e7411d6ee6 fix(git): move review and CI refreshes off render path
CI / Build (push) Blocked by required conditions
Publish / Publish to npm (push) Waiting to run
CI / Lint & Type Check (push) Waiting to run
CI / Test (push) Waiting to run
Serve PR and CI widgets from a versioned disk cache and refresh stale or missing data in a detached helper so slow GitHub CLI requests cannot block Claude Code's status line.

Request statusCheckRollup only when a CI widget needs it, deduplicate background refreshes with recoverable lock files, and share a single deadline across GitHub fallback attempts. Restrict metadata retries to CI-field compatibility failures.

Preserve upgrade compatibility with legacy raw JSON and empty negative-cache files, while marking new entries with whether CI checks were queried. Add regression coverage for migration, stale reads, cache upgrades, lock recovery, and timeout behavior.

Bump the package version to 2.2.25.
2026-07-20 18:37:53 -04:00
9 changed files with 602 additions and 78 deletions
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "ccstatusline",
"version": "2.2.24",
"version": "2.2.25",
"bugs": {
"url": "https://github.com/sirmalloc/ccstatusline/issues"
},
+29 -1
View File
@@ -22,6 +22,10 @@ import {
loadSettings,
saveSettings
} from './utils/config';
import {
GIT_REVIEW_REFRESH_FLAG,
refreshGitReviewCacheFromCli
} from './utils/git-review-cache';
import { handleHookInput } from './utils/hook-handler';
import {
getSessionDuration,
@@ -170,7 +174,8 @@ async function renderMultipleLines(data: StatusJSON) {
terminalWidth: getTerminalWidth(),
isPreview: false,
minimalist: settings.minimalistMode,
gitCacheTtlSeconds: settings.gitCacheTtlSeconds
gitCacheTtlSeconds: settings.gitCacheTtlSeconds,
gitReviewNeedsChecks: lines.some(line => line.some(item => item.type === 'git-ci-status'))
};
// Always pre-render all widgets once (for efficiency)
@@ -276,7 +281,30 @@ async function handleHook(): Promise<void> {
handleHookInput(input);
}
function handleGitReviewRefresh(): boolean {
const flagIndex = process.argv.indexOf(GIT_REVIEW_REFRESH_FLAG);
if (flagIndex === -1) {
return false;
}
const cwd = process.argv[flagIndex + 1];
const mode = process.argv[flagIndex + 2];
const lockPath = process.argv[flagIndex + 3];
if (!cwd || (mode !== 'metadata' && mode !== 'checks') || !lockPath) {
return true;
}
refreshGitReviewCacheFromCli(cwd, { includeChecks: mode === 'checks' }, lockPath);
return true;
}
async function main() {
// Detached cache refreshes re-enter this executable without reading stdin
// or loading user settings. This mode intentionally emits no output.
if (handleGitReviewRefresh()) {
return;
}
// Print version and exit (#461). Standard CLI behavior, runs before any other mode.
if (process.argv.includes('--version')) {
console.log(getPackageVersion());
+1
View File
@@ -44,6 +44,7 @@ export interface RenderContext {
isPreview?: boolean;
minimalist?: boolean;
gitCacheTtlSeconds?: number;
gitReviewNeedsChecks?: boolean;
lineIndex?: number; // Index of the current line being rendered (for theme cycling)
globalSeparatorIndex?: number; // Global separator index that continues across lines
+222 -8
View File
@@ -6,6 +6,8 @@ import {
import {
fetchGitReviewData,
getCachedGitReviewData,
refreshGitReviewCacheFromCli,
type GitReviewCacheDeps
} from '../git-review-cache';
@@ -18,8 +20,11 @@ interface PrCacheHarness {
cacheFiles: Map<string, FakeCacheFile>;
deps: GitReviewCacheDeps;
execCalls: { args: string[]; cmd: string; cwd?: string }[];
ghDurations: number[];
ghResponses: (Error | string)[];
glabResponses: (Error | string)[];
spawnCalls: { args: string[]; command: string }[];
advanceNow: (milliseconds: number) => void;
setCurrentRef: (ref: string) => void;
setOriginRemoteUrl: (url: string) => void;
setGlabAvailable: (available: boolean) => void;
@@ -30,9 +35,11 @@ interface PrCacheHarness {
function createHarness(): PrCacheHarness {
const cacheFiles = new Map<string, FakeCacheFile>();
const execCalls: { args: string[]; cmd: string; cwd?: string }[] = [];
const ghDurations: number[] = [];
const ghResponses: (Error | string)[] = [];
const glabResponses: (Error | string)[] = [];
const now = 1_700_000_000_000;
const spawnCalls: { args: string[]; command: string }[] = [];
let now = 1_700_000_000_000;
let currentRef = 'feature/cache-a';
let originRemoteUrl: string | null = null;
let glabAvailable = false;
@@ -43,6 +50,7 @@ function createHarness(): PrCacheHarness {
const sshHostAliases = new Map<string, string>();
const deps: GitReviewCacheDeps = {
closeSync: () => undefined,
execFileSync: ((cmd, args, options) => {
const commandArgs = Array.isArray(args)
? args.map(arg => String(arg))
@@ -81,6 +89,7 @@ function createHarness(): PrCacheHarness {
return '';
}
if (cmd === 'gh' && commandArgs[0] === 'pr') {
now += ghDurations.shift() ?? 0;
const response = ghResponses.shift();
if (response instanceof Error)
throw response;
@@ -110,11 +119,33 @@ function createHarness(): PrCacheHarness {
throw new Error(`Unexpected command: ${cmd} ${commandArgs.join(' ')}`);
}) as GitReviewCacheDeps['execFileSync'],
existsSync: filePath => cacheFiles.has(String(filePath)),
getExecPath: () => '/usr/bin/node',
getHomedir: () => '/tmp/home',
mkdirSync: () => undefined,
openSync: (filePath) => {
const normalizedPath = String(filePath);
if (cacheFiles.has(normalizedPath)) {
throw new Error('EEXIST');
}
cacheFiles.set(normalizedPath, { content: '', mtimeMs: now });
return 42;
},
now: () => now,
readFileSync: (filePath => cacheFiles.get(String(filePath))?.content ?? '') as GitReviewCacheDeps['readFileSync'],
getScriptPath: () => '/app/ccstatusline.js',
spawn: ((command, args) => {
spawnCalls.push({
args: Array.isArray(args) ? args.map(arg => String(arg)) : [],
command
});
return { unref: () => undefined };
}) as GitReviewCacheDeps['spawn'],
statSync: (filePath => ({ mtimeMs: cacheFiles.get(String(filePath))?.mtimeMs ?? now })) as GitReviewCacheDeps['statSync'],
unlinkSync: (filePath) => {
if (!cacheFiles.delete(String(filePath))) {
throw new Error('ENOENT');
}
},
writeFileSync: (filePath, content) => {
const normalizedContent = typeof content === 'string'
? content
@@ -132,8 +163,13 @@ function createHarness(): PrCacheHarness {
cacheFiles,
deps,
execCalls,
ghDurations,
ghResponses,
glabResponses,
spawnCalls,
advanceNow: (milliseconds) => {
now += milliseconds;
},
setCurrentRef: (ref: string) => {
currentRef = ref;
},
@@ -156,9 +192,21 @@ function createHarness(): PrCacheHarness {
};
}
function prepareCachePath(harness: PrCacheHarness): string {
getCachedGitReviewData('/tmp/repo', {}, harness.deps);
const lockPath = [...harness.cacheFiles.keys()].find(filePath => filePath.endsWith('.lock'));
if (!lockPath) {
throw new Error('Expected a refresh lock');
}
harness.cacheFiles.delete(lockPath);
harness.spawnCalls.length = 0;
return lockPath.slice(0, -'.lock'.length);
}
describe('git-review-cache', () => {
it('negative-caches failed gh PR lookups', () => {
const harness = createHarness();
harness.setOriginRemoteUrl('https://github.com/example-owner/example-repo.git');
harness.ghResponses.push(new Error('no pull request found'));
harness.ghResponses.push(new Error('no pull request found'));
@@ -166,9 +214,18 @@ describe('git-review-cache', () => {
const ghCallsAfterFirstRender = harness.execCalls.filter(call => call.cmd === 'gh');
expect(ghCallsAfterFirstRender).toHaveLength(3);
const ghPrCalls = ghCallsAfterFirstRender.filter(call => call.args[0] === 'pr');
expect(ghPrCalls).toHaveLength(2);
expect(ghPrCalls[0]?.args).not.toContain('--repo');
expect(ghPrCalls[1]?.args).toContain('--repo');
expect(ghPrCalls.every(call => call.args.at(-1) === 'url,number,title,state,reviewDecision')).toBe(true);
const cachedMissEntry = [...harness.cacheFiles.values()].at(0);
expect(cachedMissEntry?.content).toBe('');
expect(JSON.parse(cachedMissEntry?.content ?? '')).toEqual({
checksQueried: true,
data: null,
version: 1
});
expect(fetchGitReviewData('/tmp/repo', harness.deps)).toBeNull();
@@ -176,6 +233,155 @@ describe('git-review-cache', () => {
expect(ghCallsAfterSecondRender).toHaveLength(3);
});
it('does not retry metadata-only for ordinary CI lookup failures', () => {
const harness = createHarness();
harness.setOriginRemoteUrl('https://github.com/example-owner/example-repo.git');
harness.ghResponses.push(new Error('no pull request found'));
harness.ghResponses.push(new Error('no pull request found'));
expect(fetchGitReviewData('/tmp/repo', harness.deps, { includeChecks: true })).toBeNull();
const ghPrCalls = harness.execCalls.filter(
call => call.cmd === 'gh' && call.args[0] === 'pr'
);
expect(ghPrCalls).toHaveLength(2);
expect(ghPrCalls[0]?.args).not.toContain('--repo');
expect(ghPrCalls[1]?.args).toContain('--repo');
expect(ghPrCalls.every(call => call.args.at(-1)?.includes('statusCheckRollup'))).toBe(true);
});
it('shares one deadline across unpinned and pinned CI lookups', () => {
const harness = createHarness();
harness.setOriginRemoteUrl('https://github.com/example-owner/example-repo.git');
harness.ghDurations.push(5_000);
harness.ghResponses.push(new Error('timed out'));
harness.ghResponses.push(JSON.stringify({
number: 42,
reviewDecision: '',
state: 'OPEN',
title: 'Too late',
url: 'https://github.com/example-owner/example-repo/pull/42'
}));
expect(fetchGitReviewData('/tmp/repo', harness.deps, { includeChecks: true })).toBeNull();
const ghPrCalls = harness.execCalls.filter(
call => call.cmd === 'gh' && call.args[0] === 'pr'
);
expect(ghPrCalls).toHaveLength(1);
});
it('returns immediately on a cache miss and schedules one metadata refresh', () => {
const harness = createHarness();
expect(getCachedGitReviewData('/tmp/repo', {}, harness.deps)).toBeNull();
expect(getCachedGitReviewData('/tmp/repo', {}, harness.deps)).toBeNull();
expect(harness.execCalls.filter(call => call.cmd === 'gh')).toHaveLength(0);
expect(harness.spawnCalls).toHaveLength(1);
expect(harness.spawnCalls[0]?.command).toBe('/usr/bin/node');
expect(harness.spawnCalls[0]?.args.slice(0, 4)).toEqual([
'/app/ccstatusline.js',
'--internal-refresh-git-review-cache',
'/tmp/repo',
'metadata'
]);
});
it('refreshes through the detached CLI mode and releases its lock', () => {
const harness = createHarness();
harness.setOriginRemoteUrl('https://github.com/example-owner/example-repo.git');
harness.ghResponses.push(JSON.stringify({
number: 42,
reviewDecision: '',
state: 'OPEN',
title: 'Background result',
url: 'https://github.com/example-owner/example-repo/pull/42'
}));
expect(getCachedGitReviewData('/tmp/repo', {}, harness.deps)).toBeNull();
const lockPath = [...harness.cacheFiles.keys()].find(filePath => filePath.endsWith('.lock'));
expect(lockPath).toBeDefined();
refreshGitReviewCacheFromCli('/tmp/repo', {}, lockPath ?? '', harness.deps);
expect([...harness.cacheFiles.keys()].some(filePath => filePath.endsWith('.lock'))).toBe(false);
expect(getCachedGitReviewData('/tmp/repo', {}, harness.deps)?.title).toBe('Background result');
expect(harness.spawnCalls).toHaveLength(1);
});
it('returns stale data while scheduling a refresh', () => {
const harness = createHarness();
harness.ghResponses.push(JSON.stringify({
number: 42,
reviewDecision: '',
state: 'OPEN',
title: 'Stale but useful',
url: 'https://github.com/example-owner/example-repo/pull/42'
}));
expect(fetchGitReviewData('/tmp/repo', harness.deps)?.title).toBe('Stale but useful');
harness.advanceNow(30_001);
expect(getCachedGitReviewData('/tmp/repo', {}, harness.deps)?.title).toBe('Stale but useful');
expect(harness.spawnCalls).toHaveLength(1);
});
it('recovers a stale refresh lock', () => {
const harness = createHarness();
expect(getCachedGitReviewData('/tmp/repo', {}, harness.deps)).toBeNull();
expect(harness.spawnCalls).toHaveLength(1);
harness.advanceNow(30_001);
expect(getCachedGitReviewData('/tmp/repo', {}, harness.deps)).toBeNull();
expect(harness.spawnCalls).toHaveLength(2);
});
it('reads legacy metadata cache files and upgrades them when CI is requested', () => {
const harness = createHarness();
const cachePath = prepareCachePath(harness);
const legacyData = {
number: 42,
reviewDecision: '',
state: 'OPEN',
title: 'Legacy cache',
url: 'https://github.com/example-owner/example-repo/pull/42'
};
harness.cacheFiles.set(cachePath, {
content: JSON.stringify(legacyData),
mtimeMs: harness.deps.now()
});
expect(getCachedGitReviewData('/tmp/repo', {}, harness.deps)).toEqual(legacyData);
expect(harness.spawnCalls).toHaveLength(0);
expect(getCachedGitReviewData('/tmp/repo', { includeChecks: true }, harness.deps)).toEqual(legacyData);
expect(harness.spawnCalls).toHaveLength(1);
expect(harness.spawnCalls[0]?.args[3]).toBe('checks');
});
it('accepts legacy empty negative-cache files without refreshing them while fresh', () => {
const harness = createHarness();
const cachePath = prepareCachePath(harness);
harness.cacheFiles.set(cachePath, { content: '', mtimeMs: harness.deps.now() });
expect(getCachedGitReviewData('/tmp/repo', { includeChecks: true }, harness.deps)).toBeNull();
expect(harness.spawnCalls).toHaveLength(0);
});
it('records an empty CI rollup as queried so it is not fetched repeatedly', () => {
const harness = createHarness();
harness.ghResponses.push(JSON.stringify({
number: 42,
reviewDecision: '',
state: 'OPEN',
statusCheckRollup: [],
title: 'No checks configured',
url: 'https://github.com/example-owner/example-repo/pull/42'
}));
expect(fetchGitReviewData('/tmp/repo', harness.deps, { includeChecks: true })?.checks).toBeUndefined();
expect(getCachedGitReviewData('/tmp/repo', { includeChecks: true }, harness.deps)?.title).toBe('No checks configured');
expect(harness.spawnCalls).toHaveLength(0);
});
it('uses a different cache entry for each checked-out branch', () => {
const harness = createHarness();
harness.ghResponses.push(JSON.stringify({
@@ -295,7 +501,7 @@ describe('git-review-cache', () => {
url: 'https://github.com/example-owner/example-repo/pull/42'
}));
expect(fetchGitReviewData('/tmp/repo', harness.deps)).toEqual({
expect(fetchGitReviewData('/tmp/repo', harness.deps, { includeChecks: true })).toEqual({
checks: {
failing: 0,
pending: 1,
@@ -340,7 +546,7 @@ describe('git-review-cache', () => {
title: 'Restricted token PR',
url: 'https://github.com/example-owner/example-repo/pull/42'
};
expect(fetchGitReviewData('/tmp/repo', harness.deps)).toEqual(expected);
expect(fetchGitReviewData('/tmp/repo', harness.deps, { includeChecks: true })).toEqual(expected);
const ghPrCalls = harness.execCalls.filter(
call => call.cmd === 'gh' && call.args[0] === 'pr'
@@ -353,9 +559,13 @@ describe('git-review-cache', () => {
expect(ghPrCalls[1]?.args.at(-1)).toBe('url,number,title,state,reviewDecision');
const cachedEntry = [...harness.cacheFiles.values()].at(0);
expect(JSON.parse(cachedEntry?.content ?? '')).toEqual(expected);
expect(JSON.parse(cachedEntry?.content ?? '')).toEqual({
checksQueried: true,
data: expected,
version: 1
});
expect(fetchGitReviewData('/tmp/repo', harness.deps)).toEqual(expected);
expect(fetchGitReviewData('/tmp/repo', harness.deps, { includeChecks: true })).toEqual(expected);
const cachedGhPrCalls = harness.execCalls.filter(
call => call.cmd === 'gh' && call.args[0] === 'pr'
);
@@ -406,7 +616,7 @@ describe('git-review-cache', () => {
url: 'https://github.com/fork-owner/example-repo/pull/1'
}));
expect(fetchGitReviewData('/tmp/repo', harness.deps)).toEqual({
expect(fetchGitReviewData('/tmp/repo', harness.deps, { includeChecks: true })).toEqual({
number: 1,
provider: 'gh',
reviewDecision: '',
@@ -650,7 +860,11 @@ describe('git-review-cache', () => {
);
expect(ghPrCalls).toHaveLength(0);
const cachedMissEntry = [...harness.cacheFiles.values()].at(0);
expect(cachedMissEntry?.content).toBe('');
expect(JSON.parse(cachedMissEntry?.content ?? '')).toEqual({
checksQueried: true,
data: null,
version: 1
});
});
it('prefers glab over gh for unknown host when both CLIs are authed', () => {
+307 -36
View File
@@ -1,9 +1,15 @@
import { execFileSync } from 'child_process';
import {
execFileSync,
spawn
} from 'child_process';
import {
closeSync,
existsSync,
mkdirSync,
openSync,
readFileSync,
statSync,
unlinkSync,
writeFileSync
} from 'fs';
import { createHash } from 'node:crypto';
@@ -33,6 +39,20 @@ export interface GitReviewData {
checks?: GitCiChecks;
}
export interface GitReviewFetchOptions { includeChecks?: boolean }
interface StoredGitReviewCache {
version: 1;
data: GitReviewData | null;
checksQueried: boolean;
}
interface CachedGitReviewData {
data: GitReviewData | null;
checksQueried: boolean;
stale: boolean;
}
type CiCheckKind = 'success' | 'failed' | 'pending' | 'ignored';
function readField(entry: Record<string, unknown>, key: string): string {
@@ -93,27 +113,41 @@ export function computeCiRollup(rollup: unknown): GitCiChecks | null {
const GIT_REVIEW_CACHE_TTL = 30_000;
const CLI_TIMEOUT = 5_000;
const REFRESH_LOCK_STALE_MS = 30_000;
const DEFAULT_TITLE_MAX_WIDTH = 30;
const GH_PR_METADATA_FIELDS = 'url,number,title,state,reviewDecision';
const GH_PR_WITH_CHECKS_FIELDS = `${GH_PR_METADATA_FIELDS},statusCheckRollup`;
export const GIT_REVIEW_REFRESH_FLAG = '--internal-refresh-git-review-cache';
export interface GitReviewCacheDeps {
closeSync: typeof closeSync;
execFileSync: typeof execFileSync;
existsSync: typeof existsSync;
getExecPath: () => string;
mkdirSync: typeof mkdirSync;
openSync: typeof openSync;
readFileSync: typeof readFileSync;
getScriptPath: () => string | undefined;
spawn: typeof spawn;
statSync: typeof statSync;
unlinkSync: typeof unlinkSync;
writeFileSync: typeof writeFileSync;
getHomedir: typeof os.homedir;
now: typeof Date.now;
}
const DEFAULT_GIT_REVIEW_CACHE_DEPS: GitReviewCacheDeps = {
closeSync,
execFileSync,
existsSync,
getExecPath: () => process.execPath,
mkdirSync,
openSync,
readFileSync,
getScriptPath: () => process.argv[1],
spawn,
statSync,
unlinkSync,
writeFileSync,
getHomedir: os.homedir,
now: Date.now
@@ -170,36 +204,84 @@ function getCachePath(cwd: string, ref: string, deps: GitReviewCacheDeps): strin
return path.join(getGitReviewCacheDir(deps), `git-review-${hash}.json`);
}
function readCache(cachePath: string, deps: GitReviewCacheDeps): GitReviewData | null | 'miss' {
function isGitReviewData(value: unknown): value is GitReviewData {
if (typeof value !== 'object' || value === null) {
return false;
}
const candidate = value as Partial<GitReviewData>;
return typeof candidate.number === 'number' && typeof candidate.url === 'string';
}
function decodeCache(content: string): Omit<CachedGitReviewData, 'stale'> | 'miss' {
if (content.length === 0) {
// v2.2.24 and earlier represented a cached "no PR" result as an
// empty file. A missing PR also implies that no CI checks can exist.
return { data: null, checksQueried: true };
}
const parsed = JSON.parse(content) as unknown;
if (typeof parsed === 'object' && parsed !== null) {
const stored = parsed as Partial<StoredGitReviewCache>;
if (stored.version === 1
&& typeof stored.checksQueried === 'boolean'
&& (stored.data === null || isGitReviewData(stored.data))) {
return {
data: stored.data,
checksQueried: stored.data === null || stored.checksQueried
};
}
}
if (isGitReviewData(parsed)) {
// Legacy cache files stored GitReviewData directly. The presence of
// checks proves the old lookup included CI data; absence is treated
// as metadata-only because an empty rollup was previously omitted.
return {
data: parsed,
checksQueried: parsed.checks !== undefined
};
}
return 'miss';
}
function readCache(cachePath: string, deps: GitReviewCacheDeps): CachedGitReviewData | 'miss' {
try {
if (!deps.existsSync(cachePath)) {
return 'miss';
}
const age = deps.now() - deps.statSync(cachePath).mtimeMs;
if (age > GIT_REVIEW_CACHE_TTL) {
return 'miss';
}
const content = deps.readFileSync(cachePath, 'utf-8').trim();
if (content.length === 0) {
return null;
}
const data = JSON.parse(content) as GitReviewData;
if (typeof data.number !== 'number' || typeof data.url !== 'string') {
const decoded = decodeCache(content);
if (decoded === 'miss') {
return 'miss';
}
return data;
return {
...decoded,
stale: age > GIT_REVIEW_CACHE_TTL
};
} catch {
return 'miss';
}
}
function writeCache(cachePath: string, data: GitReviewData | null, deps: GitReviewCacheDeps): void {
function writeCache(
cachePath: string,
data: GitReviewData | null,
checksQueried: boolean,
deps: GitReviewCacheDeps
): void {
try {
const cacheDir = getGitReviewCacheDir(deps);
if (!deps.existsSync(cacheDir)) {
deps.mkdirSync(cacheDir, { recursive: true });
}
deps.writeFileSync(cachePath, data ? JSON.stringify(data) : '', 'utf-8');
const stored: StoredGitReviewCache = {
version: 1,
data,
checksQueried: data === null || checksQueried
};
deps.writeFileSync(cachePath, JSON.stringify(stored), 'utf-8');
} catch {
// Best-effort caching
}
@@ -300,11 +382,21 @@ function getProviderCandidates(cwd: string, deps: GitReviewCacheDeps): GitReview
return authed;
}
function isCliAvailable(cli: GitReviewProvider, deps: GitReviewCacheDeps): boolean {
class GitReviewDeadlineError extends Error {}
function getRemainingTimeout(deadline: number, deps: GitReviewCacheDeps): number {
const remaining = deadline - deps.now();
if (remaining <= 0) {
throw new GitReviewDeadlineError('Git review lookup deadline exceeded');
}
return Math.max(1, Math.min(CLI_TIMEOUT, remaining));
}
function isCliAvailable(cli: GitReviewProvider, deadline: number, deps: GitReviewCacheDeps): boolean {
try {
deps.execFileSync(cli, ['--version'], {
stdio: ['pipe', 'pipe', 'ignore'],
timeout: CLI_TIMEOUT,
timeout: getRemainingTimeout(deadline, deps),
windowsHide: true
});
return true;
@@ -338,10 +430,29 @@ function mapGlabState(state: string): string {
return state.toUpperCase();
}
function errorText(error: unknown): string {
if (!(error instanceof Error)) {
return '';
}
const stderr = 'stderr' in error
? (error as Error & { stderr?: Buffer | string }).stderr
: undefined;
const stderrText = Buffer.isBuffer(stderr) ? stderr.toString('utf8') : (stderr ?? '');
return `${error.message}\n${stderrText}`.toLowerCase();
}
function isCiFieldUnavailableError(error: unknown): boolean {
const text = errorText(error);
return text.includes('statuscheckrollup')
|| text.includes('resource not accessible by integration');
}
function queryGhPr(
cwd: string,
args: string[],
fields: string,
deadline: number,
deps: GitReviewCacheDeps
): Record<string, unknown> | null {
const output = deps.execFileSync(
@@ -349,9 +460,9 @@ function queryGhPr(
[...args, '--json', fields],
{
encoding: 'utf8',
stdio: ['pipe', 'pipe', 'ignore'],
stdio: ['pipe', 'pipe', 'pipe'],
cwd,
timeout: CLI_TIMEOUT,
timeout: getRemainingTimeout(deadline, deps),
windowsHide: true
}
).trim();
@@ -363,7 +474,13 @@ function queryGhPr(
return JSON.parse(output) as Record<string, unknown>;
}
function fetchFromGh(cwd: string, repoRef: string | null, deps: GitReviewCacheDeps): GitReviewData | null {
function fetchFromGh(
cwd: string,
repoRef: string | null,
includeChecks: boolean,
deadline: number,
deps: GitReviewCacheDeps
): GitReviewData | null {
const args = ['pr', 'view'];
if (repoRef) {
// `--repo` disables branch auto-resolution, so pass the branch explicitly.
@@ -375,10 +492,17 @@ function fetchFromGh(cwd: string, repoRef: string | null, deps: GitReviewCacheDe
}
let parsed: Record<string, unknown> | null;
try {
parsed = queryGhPr(cwd, args, GH_PR_WITH_CHECKS_FIELDS, deps);
} catch {
parsed = queryGhPr(cwd, args, GH_PR_METADATA_FIELDS, deps);
if (includeChecks) {
try {
parsed = queryGhPr(cwd, args, GH_PR_WITH_CHECKS_FIELDS, deadline, deps);
} catch (error) {
if (!isCiFieldUnavailableError(error)) {
throw error;
}
parsed = queryGhPr(cwd, args, GH_PR_METADATA_FIELDS, deadline, deps);
}
} else {
parsed = queryGhPr(cwd, args, GH_PR_METADATA_FIELDS, deadline, deps);
}
if (!parsed) {
@@ -398,7 +522,12 @@ function fetchFromGh(cwd: string, repoRef: string | null, deps: GitReviewCacheDe
};
}
function fetchFromGlab(cwd: string, repoRef: string | null, deps: GitReviewCacheDeps): GitReviewData | null {
function fetchFromGlab(
cwd: string,
repoRef: string | null,
deadline: number,
deps: GitReviewCacheDeps
): GitReviewData | null {
const args = ['mr', 'view'];
if (repoRef) {
// `--repo` disables branch auto-resolution, so pass the branch explicitly.
@@ -417,7 +546,7 @@ function fetchFromGlab(cwd: string, repoRef: string | null, deps: GitReviewCache
encoding: 'utf8',
stdio: ['pipe', 'pipe', 'ignore'],
cwd,
timeout: CLI_TIMEOUT,
timeout: getRemainingTimeout(deadline, deps),
windowsHide: true
}
).trim();
@@ -442,48 +571,190 @@ function fetchFromGlab(cwd: string, repoRef: string | null, deps: GitReviewCache
// First try the CLI's own repo resolution, then fall back to pinning `--repo`
// to origin. The pinned pass catches forks where the CLI resolves to upstream.
function fetchFromProvider(provider: GitReviewProvider, cwd: string, repoRef: string | null, deps: GitReviewCacheDeps): GitReviewData | null {
const fetchFn = provider === 'gh' ? fetchFromGh : fetchFromGlab;
function fetchFromProvider(
provider: GitReviewProvider,
cwd: string,
repoRef: string | null,
includeChecks: boolean,
deadline: number,
deps: GitReviewCacheDeps
): GitReviewData | null {
const fetch = (targetRepoRef: string | null): GitReviewData | null => provider === 'gh'
? fetchFromGh(cwd, targetRepoRef, includeChecks, deadline, deps)
: fetchFromGlab(cwd, targetRepoRef, deadline, deps);
try {
const unpinned = fetchFn(cwd, null, deps);
const unpinned = fetch(null);
if (unpinned) {
return unpinned;
}
} catch { /* fall through */ }
if (repoRef) {
return fetchFn(cwd, repoRef, deps);
return fetch(repoRef);
}
return null;
}
export function fetchGitReviewData(cwd: string, deps: GitReviewCacheDeps = DEFAULT_GIT_REVIEW_CACHE_DEPS): GitReviewData | null {
export function fetchGitReviewData(
cwd: string,
deps: GitReviewCacheDeps = DEFAULT_GIT_REVIEW_CACHE_DEPS,
options: GitReviewFetchOptions = {}
): GitReviewData | null {
const includeChecks = options.includeChecks ?? false;
const cachePath = getCachePath(cwd, getCacheRef(cwd, deps), deps);
const cached = readCache(cachePath, deps);
if (cached !== 'miss') {
return cached;
if (cached !== 'miss'
&& !cached.stale
&& (!includeChecks || cached.checksQueried)) {
return cached.data;
}
const repoRef = getOriginRepoRef(cwd, deps);
const deadline = deps.now() + CLI_TIMEOUT;
for (const provider of getProviderCandidates(cwd, deps)) {
if (!isCliAvailable(provider, deps)) {
if (!isCliAvailable(provider, deadline, deps)) {
continue;
}
try {
const data = fetchFromProvider(provider, cwd, repoRef, deps);
const data = fetchFromProvider(provider, cwd, repoRef, includeChecks, deadline, deps);
if (data) {
writeCache(cachePath, data, deps);
writeCache(cachePath, data, includeChecks, deps);
return data;
}
} catch { /* try next provider */ }
}
writeCache(cachePath, null, deps);
// Keep useful stale data on transient refresh failures. A later statusline
// invocation will schedule another refresh because its mtime stays stale.
if (cached !== 'miss' && cached.data !== null) {
return cached.data;
}
writeCache(cachePath, null, true, deps);
return null;
}
function getRefreshLockPath(cachePath: string): string {
return `${cachePath}.lock`;
}
function releaseRefreshLock(lockPath: string, deps: GitReviewCacheDeps): void {
try {
deps.unlinkSync(lockPath);
} catch {
// Another process may already have cleaned up a stale lock.
}
}
function createRefreshLock(cachePath: string, deps: GitReviewCacheDeps): string | null {
const cacheDir = getGitReviewCacheDir(deps);
try {
if (!deps.existsSync(cacheDir)) {
deps.mkdirSync(cacheDir, { recursive: true });
}
} catch {
return null;
}
const lockPath = getRefreshLockPath(cachePath);
for (let attempt = 0; attempt < 2; attempt++) {
try {
const descriptor = deps.openSync(lockPath, 'wx');
deps.closeSync(descriptor);
return lockPath;
} catch {
try {
const age = deps.now() - deps.statSync(lockPath).mtimeMs;
if (age <= REFRESH_LOCK_STALE_MS) {
return null;
}
deps.unlinkSync(lockPath);
} catch {
return null;
}
}
}
return null;
}
function scheduleRefresh(
cwd: string,
cachePath: string,
includeChecks: boolean,
deps: GitReviewCacheDeps
): void {
const scriptPath = deps.getScriptPath();
if (!scriptPath) {
return;
}
const lockPath = createRefreshLock(cachePath, deps);
if (!lockPath) {
return;
}
try {
const child = deps.spawn(
deps.getExecPath(),
[
scriptPath,
GIT_REVIEW_REFRESH_FLAG,
cwd,
includeChecks ? 'checks' : 'metadata',
lockPath
],
{
detached: true,
stdio: 'ignore',
windowsHide: true
}
);
child.unref();
} catch {
releaseRefreshLock(lockPath, deps);
}
}
export function getCachedGitReviewData(
cwd: string,
options: GitReviewFetchOptions = {},
deps: GitReviewCacheDeps = DEFAULT_GIT_REVIEW_CACHE_DEPS
): GitReviewData | null {
const includeChecks = options.includeChecks ?? false;
const cachePath = getCachePath(cwd, getCacheRef(cwd, deps), deps);
const cached = readCache(cachePath, deps);
const needsRefresh = cached === 'miss'
|| cached.stale
|| (includeChecks && !cached.checksQueried);
if (needsRefresh) {
scheduleRefresh(cwd, cachePath, includeChecks, deps);
}
return cached === 'miss' ? null : cached.data;
}
export function refreshGitReviewCacheFromCli(
cwd: string,
options: GitReviewFetchOptions,
lockPath: string,
deps: GitReviewCacheDeps = DEFAULT_GIT_REVIEW_CACHE_DEPS
): void {
const expectedLockPath = getRefreshLockPath(
getCachePath(cwd, getCacheRef(cwd, deps), deps)
);
try {
fetchGitReviewData(cwd, deps, options);
} finally {
// Only unlink the path derived from the supplied repository. This
// keeps the internal CLI mode from becoming an arbitrary file delete.
if (lockPath === expectedLockPath) {
releaseRefreshLock(lockPath, deps);
}
}
}
export function getGitReviewStatusLabel(state: string, reviewDecision: string): string {
if (state === 'MERGED')
return 'MERGED';
+4 -4
View File
@@ -11,7 +11,7 @@ import {
resolveGitCwd
} from '../utils/git';
import type { GitCiChecks } from '../utils/git-review-cache';
import { fetchGitReviewData } from '../utils/git-review-cache';
import { getCachedGitReviewData } from '../utils/git-review-cache';
import {
getHideNoGitKeybinds,
@@ -29,14 +29,14 @@ const SYMBOLS = {
} as const;
export interface GitCiStatusWidgetDeps {
fetchGitReviewData: typeof fetchGitReviewData;
getCachedGitReviewData: typeof getCachedGitReviewData;
getProcessCwd: typeof process.cwd;
isInsideGitWorkTree: typeof isInsideGitWorkTree;
resolveGitCwd: typeof resolveGitCwd;
}
const DEFAULT_DEPS: GitCiStatusWidgetDeps = {
fetchGitReviewData,
getCachedGitReviewData,
getProcessCwd: () => process.cwd(),
isInsideGitWorkTree,
resolveGitCwd
@@ -110,7 +110,7 @@ export class GitCiStatusWidget implements Widget {
}
const cwd = this.deps.resolveGitCwd(context) ?? this.deps.getProcessCwd();
const checks = this.deps.fetchGitReviewData(cwd)?.checks;
const checks = this.deps.getCachedGitReviewData(cwd, { includeChecks: true })?.checks;
if (!checks) {
return NO_CHECKS;
}
+4 -4
View File
@@ -13,7 +13,7 @@ import {
import { getRemoteInfo } from '../utils/git-remote';
import type { GitReviewData } from '../utils/git-review-cache';
import {
fetchGitReviewData,
getCachedGitReviewData,
getGitReviewStatusLabel,
truncateTitle
} from '../utils/git-review-cache';
@@ -37,7 +37,7 @@ const TOGGLE_STATUS_ACTION = 'toggle-status';
const TOGGLE_TITLE_ACTION = 'toggle-title';
export interface GitPrWidgetDeps {
fetchGitReviewData: typeof fetchGitReviewData;
getCachedGitReviewData: typeof getCachedGitReviewData;
getProcessCwd: typeof process.cwd;
getRemoteInfo: typeof getRemoteInfo;
isInsideGitWorkTree: typeof isInsideGitWorkTree;
@@ -45,7 +45,7 @@ export interface GitPrWidgetDeps {
}
const DEFAULT_GIT_PR_WIDGET_DEPS: GitPrWidgetDeps = {
fetchGitReviewData,
getCachedGitReviewData,
getProcessCwd: () => process.cwd(),
getRemoteInfo,
isInsideGitWorkTree,
@@ -153,7 +153,7 @@ export class GitPrWidget implements Widget {
}
const cwd = this.deps.resolveGitCwd(context) ?? this.deps.getProcessCwd();
const prData = this.deps.fetchGitReviewData(cwd);
const prData = this.deps.getCachedGitReviewData(cwd, { includeChecks: context.gitReviewNeedsChecks ?? false });
if (!prData) {
return hideNoGit ? null : `(no ${resolvePrNoun(null, context, this.deps)})`;
}
+9 -9
View File
@@ -29,7 +29,7 @@ const PASSING_PR = prWithChecks('passing', 0, 0, 5);
function createDeps(overrides: Partial<GitCiStatusWidgetDeps> = {}): GitCiStatusWidgetDeps {
return {
fetchGitReviewData: () => PASSING_PR,
getCachedGitReviewData: () => PASSING_PR,
getProcessCwd: () => '/tmp/process-cwd',
isInsideGitWorkTree: () => true,
resolveGitCwd: context => context.data?.cwd,
@@ -71,12 +71,12 @@ describe('GitCiStatusWidget', () => {
['mixed', prWithChecks('failing', 1, 1, 97), '✗1 ●1 ✓97'],
['zeros are hidden', prWithChecks('failing', 2, 0, 0), '✗2']
])('renders %s as non-zero glyph + count', (_label, pr, expected) => {
expect(render({ cwd: '/tmp/repo' }, { fetchGitReviewData: () => pr })).toBe(expected);
expect(render({ cwd: '/tmp/repo' }, { getCachedGitReviewData: () => pr })).toBe(expected);
});
it('falls back to ✓0 when only skipped/neutral checks exist', () => {
const allIgnored = prWithChecks('passing', 0, 0, 0);
expect(render({ cwd: '/tmp/repo' }, { fetchGitReviewData: () => allIgnored })).toBe('✓0');
expect(render({ cwd: '/tmp/repo' }, { getCachedGitReviewData: () => allIgnored })).toBe('✓0');
});
it.each([
@@ -84,16 +84,16 @@ describe('GitCiStatusWidget', () => {
['failing', prWithChecks('failing', 1, 0, 4), 'failing'],
['pending', prWithChecks('pending', 0, 3, 2), 'pending']
])('renders rawValue %s as the state word', (_label, pr, expected) => {
expect(render({ cwd: '/tmp/repo', rawValue: true }, { fetchGitReviewData: () => pr })).toBe(expected);
expect(render({ cwd: '/tmp/repo', rawValue: true }, { getCachedGitReviewData: () => pr })).toBe(expected);
});
it('renders "-" when no PR exists', () => {
expect(render({ cwd: '/tmp/repo' }, { fetchGitReviewData: () => null })).toBe('-');
expect(render({ cwd: '/tmp/repo' }, { getCachedGitReviewData: () => null })).toBe('-');
});
it('renders "-" when the PR has no checks', () => {
const noChecks = { ...PASSING_PR, checks: undefined };
expect(render({ cwd: '/tmp/repo' }, { fetchGitReviewData: () => noChecks })).toBe('-');
expect(render({ cwd: '/tmp/repo' }, { getCachedGitReviewData: () => noChecks })).toBe('-');
});
it('returns (no git) when not in a git repo', () => {
@@ -105,8 +105,8 @@ describe('GitCiStatusWidget', () => {
});
it('uses process cwd when repo path is omitted', () => {
const fetchGitReviewData = vi.fn(() => PASSING_PR);
render({}, { fetchGitReviewData, resolveGitCwd: () => undefined });
expect(fetchGitReviewData).toHaveBeenCalledWith('/tmp/process-cwd');
const getCachedGitReviewData = vi.fn(() => PASSING_PR);
render({}, { getCachedGitReviewData, resolveGitCwd: () => undefined });
expect(getCachedGitReviewData).toHaveBeenCalledWith('/tmp/process-cwd', { includeChecks: true });
});
});
+25 -15
View File
@@ -24,7 +24,7 @@ const SAMPLE_PR = {
function createDeps(overrides: Partial<GitPrWidgetDeps> = {}): GitPrWidgetDeps {
return {
fetchGitReviewData: () => SAMPLE_PR,
getCachedGitReviewData: () => SAMPLE_PR,
getProcessCwd: () => '/tmp/process-cwd',
getRemoteInfo: () => null,
isInsideGitWorkTree: () => true,
@@ -40,6 +40,7 @@ function render(
hideStatus?: boolean;
hideTitle?: boolean;
isPreview?: boolean;
needsChecks?: boolean;
rawValue?: boolean;
} = {},
depOverrides: Partial<GitPrWidgetDeps> = {}
@@ -47,6 +48,7 @@ function render(
const widget = new GitPrWidget(createDeps(depOverrides));
const context: RenderContext = {
data: options.cwd ? { cwd: options.cwd } : undefined,
gitReviewNeedsChecks: options.needsChecks,
isPreview: options.isPreview
};
const metadata: Record<string, string> = {};
@@ -113,16 +115,16 @@ describe('GitPrWidget', () => {
it('should return (no PR) when PR lookup returns null', () => {
expect(render({}, {
fetchGitReviewData: () => null,
getCachedGitReviewData: () => null,
resolveGitCwd: () => undefined
})).toBe('(no PR)');
});
it('should use process cwd when repo paths are omitted', () => {
const fetchGitReviewData = vi.fn(() => SAMPLE_PR);
const getCachedGitReviewData = vi.fn(() => SAMPLE_PR);
const result = render({}, {
fetchGitReviewData,
getCachedGitReviewData,
getProcessCwd: () => '/tmp/process-cwd',
resolveGitCwd: () => undefined
});
@@ -130,7 +132,15 @@ describe('GitPrWidget', () => {
expect(result).toBe(
`${renderOsc8Link('https://github.com/owner/repo/pull/123', 'PR #123')} OPEN Fix authentication bug`
);
expect(fetchGitReviewData).toHaveBeenCalledWith('/tmp/process-cwd');
expect(getCachedGitReviewData).toHaveBeenCalledWith('/tmp/process-cwd', { includeChecks: false });
});
it('should request checks when a CI widget shares the render context', () => {
const getCachedGitReviewData = vi.fn(() => SAMPLE_PR);
render({ cwd: '/tmp/repo', needsChecks: true }, { getCachedGitReviewData });
expect(getCachedGitReviewData).toHaveBeenCalledWith('/tmp/repo', { includeChecks: true });
});
it('should truncate long titles', () => {
@@ -139,17 +149,17 @@ describe('GitPrWidget', () => {
title: 'This is a very long pull request title that exceeds the default limit'
};
const result = render({ cwd: '/tmp/repo' }, { fetchGitReviewData: () => longPr });
const result = render({ cwd: '/tmp/repo' }, { getCachedGitReviewData: () => longPr });
expect(result).toContain('This is a very long pull requ\u2026');
});
it('should render MERGED status', () => {
expect(render({ cwd: '/tmp/repo' }, { fetchGitReviewData: () => ({ ...SAMPLE_PR, state: 'MERGED' }) })).toContain('MERGED');
expect(render({ cwd: '/tmp/repo' }, { getCachedGitReviewData: () => ({ ...SAMPLE_PR, state: 'MERGED' }) })).toContain('MERGED');
});
it('should render APPROVED status', () => {
expect(render({ cwd: '/tmp/repo' }, {
fetchGitReviewData: () => ({
getCachedGitReviewData: () => ({
...SAMPLE_PR,
reviewDecision: 'APPROVED',
state: 'OPEN'
@@ -159,7 +169,7 @@ describe('GitPrWidget', () => {
it('should render CHANGES_REQ status', () => {
expect(render({ cwd: '/tmp/repo' }, {
fetchGitReviewData: () => ({
getCachedGitReviewData: () => ({
...SAMPLE_PR,
reviewDecision: 'CHANGES_REQUESTED',
state: 'OPEN'
@@ -172,7 +182,7 @@ describe('GitPrWidget', () => {
...SAMPLE_PR,
url: 'https://gitlab.com/owner/repo/-/merge_requests/123'
};
expect(render({ cwd: '/tmp/repo' }, { fetchGitReviewData: () => gitlabPr })).toBe(
expect(render({ cwd: '/tmp/repo' }, { getCachedGitReviewData: () => gitlabPr })).toBe(
`${renderOsc8Link('https://gitlab.com/owner/repo/-/merge_requests/123', 'MR #123')} OPEN Fix authentication bug`
);
});
@@ -182,14 +192,14 @@ describe('GitPrWidget', () => {
...SAMPLE_PR,
url: 'https://gitlab.com/owner/repo/-/merge_requests/123'
};
expect(render({ cwd: '/tmp/repo', rawValue: true }, { fetchGitReviewData: () => gitlabPr })).toBe(
expect(render({ cwd: '/tmp/repo', rawValue: true }, { getCachedGitReviewData: () => gitlabPr })).toBe(
`${renderOsc8Link('https://gitlab.com/owner/repo/-/merge_requests/123', '#123')} OPEN Fix authentication bug`
);
});
it('should return (no MR) when origin is GitLab and no MR exists', () => {
expect(render({ cwd: '/tmp/repo' }, {
fetchGitReviewData: () => null,
getCachedGitReviewData: () => null,
getRemoteInfo: () => ({
name: 'origin',
url: 'git@gitlab.com:owner/repo.git',
@@ -214,7 +224,7 @@ describe('GitPrWidget', () => {
title: 'Add optional wallet type field',
url: 'https://git.example.com/group/project/-/merge_requests/1626'
};
expect(render({ cwd: '/tmp/repo' }, { fetchGitReviewData: () => legacyCacheEntry })).toBe(
expect(render({ cwd: '/tmp/repo' }, { getCachedGitReviewData: () => legacyCacheEntry })).toBe(
`${renderOsc8Link('https://git.example.com/group/project/-/merge_requests/1626', 'MR #1626')} OPEN Add optional wallet type field`
);
});
@@ -226,14 +236,14 @@ describe('GitPrWidget', () => {
url: 'https://git.example.com/team/repo/-/merge_requests/7',
provider: 'glab' as const
};
expect(render({ cwd: '/tmp/repo' }, { fetchGitReviewData: () => selfHostedMr })).toBe(
expect(render({ cwd: '/tmp/repo' }, { getCachedGitReviewData: () => selfHostedMr })).toBe(
`${renderOsc8Link('https://git.example.com/team/repo/-/merge_requests/7', 'MR #7')} OPEN Fix authentication bug`
);
});
it('should fall back to (no PR) when the origin host name does not identify the forge', () => {
expect(render({ cwd: '/tmp/repo' }, {
fetchGitReviewData: () => null,
getCachedGitReviewData: () => null,
getRemoteInfo: () => ({
name: 'origin',
url: 'git@git.example.com:team/repo.git',