Skip to content
Merged
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
41 changes: 41 additions & 0 deletions src/utils/__tests__/git-ci-rollup.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import {
describe,
expect,
it
} from 'vitest';

import { computeCiRollup } from '../git-review-cache';

const pass = { status: 'COMPLETED', conclusion: 'SUCCESS' };
const fail = { status: 'COMPLETED', conclusion: 'FAILURE' };
const running = { status: 'IN_PROGRESS', conclusion: '' };
const neutral = { status: 'COMPLETED', conclusion: 'NEUTRAL' };
const skipped = { status: 'COMPLETED', conclusion: 'SKIPPED' };
const statusPass = { state: 'SUCCESS' };
const statusFail = { state: 'FAILURE' };
const statusPending = { state: 'PENDING' };

describe('computeCiRollup', () => {
it.each([
['all passing check runs', [pass, pass, pass], { state: 'passing', failing: 0, pending: 0, success: 3 }],
['a failure makes it failing', [pass, fail, pass], { state: 'failing', failing: 1, pending: 0, success: 2 }],
['a pending run makes it pending', [pass, running], { state: 'pending', failing: 0, pending: 1, success: 1 }],
['failure takes precedence over pending', [fail, running, pass], { state: 'failing', failing: 1, pending: 1, success: 1 }],
['neutral and skipped are ignored (not counted as success)', [pass, neutral, skipped], { state: 'passing', failing: 0, pending: 0, success: 1 }],
['StatusContext success counts as success', [statusPass, statusPass], { state: 'passing', failing: 0, pending: 0, success: 2 }],
['StatusContext failure counts as failing', [statusPass, statusFail], { state: 'failing', failing: 1, pending: 0, success: 1 }],
['StatusContext pending counts as pending', [statusPass, statusPending], { state: 'pending', failing: 0, pending: 1, success: 1 }],
['the screenshot mix (1 fail, 1 neutral, 1 pending, 2 skipped, 4 success)', [fail, neutral, running, skipped, skipped, pass, pass, pass, pass], { state: 'failing', failing: 1, pending: 1, success: 4 }]
])('%s', (_label, rollup, expected) => {
expect(computeCiRollup(rollup)).toEqual(expected);
});

it.each([
['empty array', []],
['non-array', null],
['undefined', undefined],
['string', 'nope']
])('returns null for %s', (_label, input) => {
expect(computeCiRollup(input)).toBeNull();
});
});
95 changes: 92 additions & 3 deletions src/utils/__tests__/git-review-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,19 +160,20 @@ describe('git-review-cache', () => {
it('negative-caches failed gh PR lookups', () => {
const harness = createHarness();
harness.ghResponses.push(new Error('no pull request found'));
harness.ghResponses.push(new Error('no pull request found'));

expect(fetchGitReviewData('/tmp/repo', harness.deps)).toBeNull();

const ghCallsAfterFirstRender = harness.execCalls.filter(call => call.cmd === 'gh');
expect(ghCallsAfterFirstRender).toHaveLength(2);
expect(ghCallsAfterFirstRender).toHaveLength(3);

const cachedMissEntry = [...harness.cacheFiles.values()].at(0);
expect(cachedMissEntry?.content).toBe('');

expect(fetchGitReviewData('/tmp/repo', harness.deps)).toBeNull();

const ghCallsAfterSecondRender = harness.execCalls.filter(call => call.cmd === 'gh');
expect(ghCallsAfterSecondRender).toHaveLength(2);
expect(ghCallsAfterSecondRender).toHaveLength(3);
});

it('uses a different cache entry for each checked-out branch', () => {
Expand Down Expand Up @@ -279,18 +280,28 @@ describe('git-review-cache', () => {
expect(data?.state).toBe('MERGED');
});

it('uses gh\'s default repo resolution when it succeeds (no --repo pin needed)', () => {
it('uses gh\'s default repo resolution and includes CI checks in one query', () => {
const harness = createHarness();
harness.setOriginRemoteUrl('https://github.com/example-owner/example-repo.git');
harness.ghResponses.push(JSON.stringify({
number: 42,
reviewDecision: '',
state: 'OPEN',
statusCheckRollup: [
{ conclusion: 'SUCCESS', status: 'COMPLETED' },
{ conclusion: '', status: 'IN_PROGRESS' }
],
title: 'Standard PR',
url: 'https://github.com/example-owner/example-repo/pull/42'
}));

expect(fetchGitReviewData('/tmp/repo', harness.deps)).toEqual({
checks: {
failing: 0,
pending: 1,
state: 'pending',
success: 1
},
number: 42,
provider: 'gh',
reviewDecision: '',
Expand All @@ -304,6 +315,51 @@ describe('git-review-cache', () => {
);
expect(ghPrCalls).toHaveLength(1);
expect(ghPrCalls[0]?.args).not.toContain('--repo');
expect(ghPrCalls[0]?.args.at(-1)).toBe(
'url,number,title,state,reviewDecision,statusCheckRollup'
);
});

it('retries metadata-only when gh cannot query CI checks', () => {
const harness = createHarness();
harness.setOriginRemoteUrl('https://github.com/example-owner/example-repo.git');
harness.ghResponses.push(new Error('statusCheckRollup is unavailable'));
harness.ghResponses.push(JSON.stringify({
number: 42,
reviewDecision: 'APPROVED',
state: 'OPEN',
title: 'Restricted token PR',
url: 'https://github.com/example-owner/example-repo/pull/42'
}));

const expected = {
number: 42,
provider: 'gh' as const,
reviewDecision: 'APPROVED',
state: 'OPEN',
title: 'Restricted token PR',
url: 'https://github.com/example-owner/example-repo/pull/42'
};
expect(fetchGitReviewData('/tmp/repo', harness.deps)).toEqual(expected);

const ghPrCalls = harness.execCalls.filter(
call => call.cmd === 'gh' && call.args[0] === 'pr'
);
expect(ghPrCalls).toHaveLength(2);
expect(ghPrCalls[0]?.args.slice(0, -2)).toEqual(ghPrCalls[1]?.args.slice(0, -2));
expect(ghPrCalls[0]?.args.at(-1)).toBe(
'url,number,title,state,reviewDecision,statusCheckRollup'
);
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(fetchGitReviewData('/tmp/repo', harness.deps)).toEqual(expected);
const cachedGhPrCalls = harness.execCalls.filter(
call => call.cmd === 'gh' && call.args[0] === 'pr'
);
expect(cachedGhPrCalls).toHaveLength(2);
});

it('falls back to --repo <origin> for forked GitHub repos when gh\'s default resolves elsewhere', () => {
Expand Down Expand Up @@ -337,6 +393,39 @@ describe('git-review-cache', () => {
expect(ghPrCalls[1]?.args).toContain('feature/cache-a');
});

it('reuses the pinned PR target for the metadata-only compatibility retry', () => {
const harness = createHarness();
harness.setOriginRemoteUrl('https://github.com/fork-owner/example-repo.git');
harness.ghResponses.push('');
harness.ghResponses.push(new Error('statusCheckRollup is unavailable'));
harness.ghResponses.push(JSON.stringify({
number: 1,
reviewDecision: '',
state: 'OPEN',
title: 'Forked PR',
url: 'https://github.com/fork-owner/example-repo/pull/1'
}));

expect(fetchGitReviewData('/tmp/repo', harness.deps)).toEqual({
number: 1,
provider: 'gh',
reviewDecision: '',
state: 'OPEN',
title: 'Forked PR',
url: 'https://github.com/fork-owner/example-repo/pull/1'
});

const ghPrCalls = harness.execCalls.filter(
call => call.cmd === 'gh' && call.args[0] === 'pr'
);
expect(ghPrCalls).toHaveLength(3);
expect(ghPrCalls[1]?.args.slice(0, -2)).toEqual(ghPrCalls[2]?.args.slice(0, -2));
expect(ghPrCalls[1]?.args).toContain('feature/cache-a');
expect(ghPrCalls[1]?.args).toContain('--repo');
expect(ghPrCalls[1]?.args).toContain('https://github.com/fork-owner/example-repo');
expect(ghPrCalls[2]?.args.at(-1)).toBe('url,number,title,state,reviewDecision');
});

it('resolves SSH host aliases before selecting GitHub and pinning --repo', () => {
const harness = createHarness();
harness.setOriginRemoteUrl('git@mygit:owner/repo.git');
Expand Down
118 changes: 103 additions & 15 deletions src/utils/git-review-cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,88 @@ import { parseRemoteUrl } from './git-remote';

export type GitReviewProvider = 'gh' | 'glab';

export type GitCiState = 'passing' | 'failing' | 'pending';

export interface GitCiChecks {
state: GitCiState;
failing: number;
pending: number;
success: number;
}

export interface GitReviewData {
number: number;
url: string;
title: string;
state: string;
reviewDecision: string;
provider?: GitReviewProvider;
checks?: GitCiChecks;
}

type CiCheckKind = 'success' | 'failed' | 'pending' | 'ignored';

function readField(entry: Record<string, unknown>, key: string): string {
const value = entry[key];
return typeof value === 'string' ? value.toUpperCase() : '';
}

// Classify a single gh statusCheckRollup entry. CheckRun entries carry a
// `status` (COMPLETED once done) plus a `conclusion`; older StatusContext
// entries carry only a `state`. NEUTRAL/SKIPPED are non-blocking noise and
// map to `ignored` so they drop out of the displayed counts.
function classifyCheck(entry: Record<string, unknown>): CiCheckKind {
if (typeof entry.status === 'string') {
if (entry.status.toUpperCase() !== 'COMPLETED')
return 'pending';
const conclusion = readField(entry, 'conclusion');
if (conclusion === 'SUCCESS')
return 'success';
if (conclusion === 'NEUTRAL' || conclusion === 'SKIPPED')
return 'ignored';
return 'failed';
}
const state = readField(entry, 'state');
if (state === 'SUCCESS')
return 'success';
if (state === 'PENDING' || state === 'EXPECTED')
return 'pending';
return 'failed';
}

export function computeCiRollup(rollup: unknown): GitCiChecks | null {
if (!Array.isArray(rollup) || rollup.length === 0)
return null;

let failing = 0;
let pending = 0;
let success = 0;
let seen = 0;
for (const entry of rollup) {
if (typeof entry !== 'object' || entry === null)
continue;
seen++;
const kind = classifyCheck(entry as Record<string, unknown>);
if (kind === 'failed')
failing++;
else if (kind === 'pending')
pending++;
else if (kind === 'success')
success++;
}

if (seen === 0)
return null;

const state: GitCiState = failing > 0 ? 'failing' : pending > 0 ? 'pending' : 'passing';
return { state, failing, pending, success };
}

const GIT_REVIEW_CACHE_TTL = 30_000;
const CLI_TIMEOUT = 5_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 interface GitReviewCacheDeps {
execFileSync: typeof execFileSync;
Expand Down Expand Up @@ -268,21 +338,15 @@ function mapGlabState(state: string): string {
return state.toUpperCase();
}

function fetchFromGh(cwd: string, repoRef: string | null, deps: GitReviewCacheDeps): GitReviewData | null {
const args = ['pr', 'view'];
if (repoRef) {
// `--repo` disables branch auto-resolution, so pass the branch explicitly.
const branch = getCurrentBranch(cwd, deps);
if (!branch) {
return null;
}
args.push(branch, '--repo', repoRef);
}
args.push('--json', 'url,number,title,state,reviewDecision');

function queryGhPr(
cwd: string,
args: string[],
fields: string,
deps: GitReviewCacheDeps
): Record<string, unknown> | null {
const output = deps.execFileSync(
'gh',
args,
[...args, '--json', fields],
{
encoding: 'utf8',
stdio: ['pipe', 'pipe', 'ignore'],
Expand All @@ -296,7 +360,30 @@ function fetchFromGh(cwd: string, repoRef: string | null, deps: GitReviewCacheDe
return null;
}

const parsed = JSON.parse(output) as Record<string, unknown>;
return JSON.parse(output) as Record<string, unknown>;
}

function fetchFromGh(cwd: string, repoRef: string | null, deps: GitReviewCacheDeps): GitReviewData | null {
const args = ['pr', 'view'];
if (repoRef) {
// `--repo` disables branch auto-resolution, so pass the branch explicitly.
const branch = getCurrentBranch(cwd, deps);
if (!branch) {
return null;
}
args.push(branch, '--repo', repoRef);
}

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 (!parsed) {
return null;
}
if (typeof parsed.number !== 'number' || typeof parsed.url !== 'string') {
return null;
}
Expand All @@ -306,7 +393,8 @@ function fetchFromGh(cwd: string, repoRef: string | null, deps: GitReviewCacheDe
title: typeof parsed.title === 'string' ? parsed.title : '',
state: typeof parsed.state === 'string' ? parsed.state : '',
reviewDecision: typeof parsed.reviewDecision === 'string' ? parsed.reviewDecision : '',
provider: 'gh'
provider: 'gh',
checks: computeCiRollup(parsed.statusCheckRollup) ?? undefined
};
}

Expand Down
1 change: 1 addition & 0 deletions src/utils/widget-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export const WIDGET_MANIFEST: WidgetManifestEntry[] = [
{ type: 'git-clean-status', create: () => new widgets.GitCleanStatusWidget() },
{ type: 'git-root-dir', create: () => new widgets.GitRootDirWidget() },
{ type: 'git-review', create: () => new widgets.GitPrWidget() },
{ type: 'git-ci-status', create: () => new widgets.GitCiStatusWidget() },
{ type: 'git-worktree', create: () => new widgets.GitWorktreeWidget() },
{ type: 'git-status', create: () => new widgets.GitStatusWidget() },
{ type: 'git-staged', create: () => new widgets.GitStagedWidget() },
Expand Down
Loading