diff --git a/docs/tools.md b/docs/tools.md index 8c9c519..5687845 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -4,6 +4,73 @@ Tool reference for the `sdlc-server` MCP. The authoritative list is the register Tools below are listed in alphabetic order. To add a tool: drop a file in `handlers/.ts`; the next CI run regenerates the registry and exposes it. +## pr_wait_ci + +**Purpose.** Block until a PR/MR's status checks settle. Server-side polling with configurable interval (default `30s`, hard floor `5s`) and timeout (default `1800s`). Used by `/scpmmr`, the wave-pattern Flight finalizer, and any caller that needs a deterministic "wait for CI to finish" with a typed terminal. + +**Inputs.** + +| Field | Type | Default | Description | +|-------|------|---------|-------------| +| `number` | number | (required) | PR (GitHub) or MR (GitLab) iid. | +| `poll_interval_sec` | number | `30` | Seconds between snapshots. Hard floor `5`. | +| `timeout_sec` | number | `1800` | Maximum wall-clock wait. | +| `repo` | string | (cwd remote) | `owner/repo` slug for cross-repo dispatch. GitLab nested groups (`group/subgroup/repo`) are accepted. | + +**Returns — polling-loop path** (one or more checks configured on the head ref). + +```json +{ + "ok": true, + "number": 42, + "final_state": "passed" | "failed" | "timed_out", + "checks": { "total": 3, "passed": 3, "failed": 0, "pending": 0, "summary": "3/3 passed" }, + "waited_sec": 8, + "url": "https://github.com/org/repo/pull/42" +} +``` + +`final_state: 'passed'` requires `total > 0 && pending === 0 && failed === 0`. All-skipped checks (every workflow's `if:` guard didn't match) count as `passed` — see #221. + +**Returns — empty-rollup short-circuit** (#416). When the head ref has no required status checks, the handler returns immediately at t=0 instead of spinning to timeout. The semantics is "wait until CI is settled" — if there are no checks to settle, that condition is satisfied at t=0. + +```json +{ + "ok": true, + "number": 42, + "status": "no_checks_required", + "elapsed_sec": 0, + "mergeable": true, + "url": "https://github.com/org/repo/pull/42" +} +``` + +When the rollup is empty AND the PR/MR is obstructed (draft, closed, conflicts, …), `mergeable` is `false` and a `blocker` field names the obstruction: + +```json +{ + "ok": true, + "number": 42, + "status": "no_checks_required", + "elapsed_sec": 0, + "mergeable": false, + "blocker": "draft" | "closed" | "merged" | "conflicts" | "not_mergeable" | "locked", + "url": "https://github.com/org/repo/pull/42" +} +``` + +**Discriminator.** Callers should branch on the response shape via either field: + +- `status === 'no_checks_required'` → empty-rollup short-circuit (`final_state` absent). +- `final_state` present → polling-loop result (`status` absent). + +**Platform notes.** + +- GitHub: probe is `gh pr view --json statusCheckRollup,url,state,isDraft,mergeable,mergeStateStatus`. Polling-loop snapshot uses the slimmer `--json statusCheckRollup,url` (per-iteration cost stays minimal). `gh pr checks --json` is NOT used — it broke on Ubuntu 24.04's gh 2.45 (#220). +- GitLab: probe is `glab api projects//merge_requests/`. Empty-rollup means `head_pipeline === null && pipeline === null`. Polling-loop status mapping: `success → passed`, `failed/canceled → failed`, `running/pending/created/preparing/waiting_for_resource/scheduled/manual → pending`, anything else → uncounted (loop times out). + +**See also.** `pattern_decorative_ac_and_stub_orphan.md` for the failure mode this short-circuit closes (autopilot callers losing 30-minute timeout windows on docs-only PRs with no CI). + ## wave_wait_for_signal **Purpose.** Sanctioned idle-wait for wave-pattern Orchestrators (and Primes) that have dispatched work and need to block until filesystem-bus artifacts appear. Replaces ad-hoc polling loops, `Bash(sleep)` invocations, and the agent-anxiety failure mode where idle loops are exited prematurely with non-canonical lines like `"Sleep is still running. Let me wait for the notification."` See `pattern_sanctioned_fidget_tool.md` (cc-workflow memory) for the design rationale. diff --git a/lib/adapters/pr-wait-ci-github.test.ts b/lib/adapters/pr-wait-ci-github.test.ts index fbd7c62..faeccde 100644 --- a/lib/adapters/pr-wait-ci-github.test.ts +++ b/lib/adapters/pr-wait-ci-github.test.ts @@ -35,6 +35,8 @@ const { prWaitCiGithub, classifyRollupItem, snapshotGithub, + emptyRollupBlocker, + probeGithub, } = await import('./pr-wait-ci-github.ts'); function on(match: string, respond: string | (() => string)): void { @@ -243,6 +245,9 @@ describe('prWaitCiGithub — #221 all-skipped regression', () => { if (!('ok' in result) || !result.ok) { throw new Error(`expected ok result, got ${JSON.stringify(result)}`); } + if (!('final_state' in result.data)) { + throw new Error(`expected polled-response shape, got ${JSON.stringify(result.data)}`); + } expect(result.data.final_state).toBe('passed'); expect(result.data.checks.passed).toBe(0); expect(result.data.checks.total).toBe(3); // total counts SKIPPED @@ -251,6 +256,192 @@ describe('prWaitCiGithub — #221 all-skipped regression', () => { }); }); +// --- #416: empty-rollup short-circuit at the adapter boundary ----------- +describe('prWaitCiGithub — empty-rollup short-circuit (#416)', () => { + test('empty rollup + mergeable → no_checks_required immediately', async () => { + on( + 'gh pr view', + JSON.stringify({ + url: 'https://github.com/org/repo/pull/42', + statusCheckRollup: [], + state: 'OPEN', + isDraft: false, + mergeable: 'MERGEABLE', + mergeStateStatus: 'CLEAN', + }), + ); + + const result = await prWaitCiGithub({ + number: 42, + poll_interval_sec: 5, + timeout_sec: 1800, // huge — proves we don't enter the poll loop + }); + if (!('ok' in result) || !result.ok) { + throw new Error(`expected ok result, got ${JSON.stringify(result)}`); + } + const data = result.data as { status?: string; mergeable?: boolean; blocker?: string; elapsed_sec?: number; url?: string }; + expect(data.status).toBe('no_checks_required'); + expect(data.mergeable).toBe(true); + expect(data.blocker).toBeUndefined(); + expect(data.elapsed_sec).toBeLessThan(5); + expect(data.url).toBe('https://github.com/org/repo/pull/42'); + // Probe argv must include the new fields the short-circuit needs. + const viewCall = execCalls.find((c) => c.startsWith('gh pr view')) ?? ''; + expect(viewCall).toContain('mergeable'); + expect(viewCall).toContain('isDraft'); + expect(viewCall).toContain('state'); + // #220 regression — never use the broken `gh pr checks --json` form. + expect(execCalls.some((c) => c.startsWith('gh pr checks'))).toBe(false); + }); + + test('empty rollup + CONFLICTING → no_checks_required + conflicts blocker', async () => { + on( + 'gh pr view', + JSON.stringify({ + url: 'https://github.com/org/repo/pull/43', + statusCheckRollup: [], + state: 'OPEN', + isDraft: false, + mergeable: 'CONFLICTING', + mergeStateStatus: 'DIRTY', + }), + ); + const result = await prWaitCiGithub({ + number: 43, + poll_interval_sec: 5, + timeout_sec: 1800, + }); + if (!('ok' in result) || !result.ok) throw new Error('expected ok'); + const data = result.data as { status?: string; mergeable?: boolean; blocker?: string }; + expect(data.status).toBe('no_checks_required'); + expect(data.mergeable).toBe(false); + expect(data.blocker).toBe('conflicts'); + }); + + test('empty rollup + draft → no_checks_required + draft blocker', async () => { + on( + 'gh pr view', + JSON.stringify({ + url: 'https://github.com/org/repo/pull/44', + statusCheckRollup: [], + state: 'OPEN', + isDraft: true, + mergeable: 'MERGEABLE', + mergeStateStatus: 'CLEAN', + }), + ); + const result = await prWaitCiGithub({ number: 44, poll_interval_sec: 5, timeout_sec: 60 }); + if (!('ok' in result) || !result.ok) throw new Error('expected ok'); + const data = result.data as { status?: string; mergeable?: boolean; blocker?: string }; + expect(data.status).toBe('no_checks_required'); + expect(data.mergeable).toBe(false); + expect(data.blocker).toBe('draft'); + }); + + test('empty rollup + closed PR → no_checks_required + closed blocker', async () => { + on( + 'gh pr view', + JSON.stringify({ + url: 'https://github.com/org/repo/pull/45', + statusCheckRollup: [], + state: 'CLOSED', + isDraft: false, + mergeable: 'UNKNOWN', + mergeStateStatus: 'UNKNOWN', + }), + ); + const result = await prWaitCiGithub({ number: 45, poll_interval_sec: 5, timeout_sec: 60 }); + if (!('ok' in result) || !result.ok) throw new Error('expected ok'); + const data = result.data as { status?: string; mergeable?: boolean; blocker?: string }; + expect(data.status).toBe('no_checks_required'); + expect(data.mergeable).toBe(false); + expect(data.blocker).toBe('closed'); + }); + + test('non-empty rollup does NOT short-circuit — polled-response shape returned', async () => { + // Regression — proves the addition is conditional on rollup.length === 0. + on( + 'gh pr view', + JSON.stringify({ + url: 'https://github.com/org/repo/pull/46', + statusCheckRollup: [ + { __typename: 'CheckRun', name: 'ci', status: 'COMPLETED', conclusion: 'SUCCESS' }, + ], + state: 'OPEN', + isDraft: false, + mergeable: 'MERGEABLE', + mergeStateStatus: 'CLEAN', + }), + ); + const result = await prWaitCiGithub({ number: 46, poll_interval_sec: 5, timeout_sec: 10 }); + if (!('ok' in result) || !result.ok) throw new Error('expected ok'); + const data = result.data as { status?: string; final_state?: string }; + expect(data.final_state).toBe('passed'); + expect(data.status).toBeUndefined(); + }); +}); + +// --- pure mapper tests for the empty-rollup blocker classifier (#416) ------ +describe('emptyRollupBlocker — pure mapping table', () => { + test('OPEN + mergeable + not draft → null (no blocker)', () => { + expect(emptyRollupBlocker({ state: 'OPEN', isDraft: false, mergeable: 'MERGEABLE', mergeStateStatus: 'CLEAN' })).toBeNull(); + }); + + test('OPEN + mergeable=true (boolean) → null', () => { + expect(emptyRollupBlocker({ state: 'OPEN', isDraft: false, mergeable: true })).toBeNull(); + }); + + test('CLOSED → closed', () => { + expect(emptyRollupBlocker({ state: 'CLOSED', mergeable: 'MERGEABLE' })).toBe('closed'); + }); + + test('MERGED → merged', () => { + expect(emptyRollupBlocker({ state: 'MERGED', mergeable: 'MERGEABLE' })).toBe('merged'); + }); + + test('isDraft=true → draft (even when mergeable)', () => { + expect(emptyRollupBlocker({ state: 'OPEN', isDraft: true, mergeable: 'MERGEABLE' })).toBe('draft'); + }); + + test('CONFLICTING → conflicts', () => { + expect(emptyRollupBlocker({ state: 'OPEN', isDraft: false, mergeable: 'CONFLICTING', mergeStateStatus: 'DIRTY' })).toBe('conflicts'); + }); + + test('mergeable=UNKNOWN → not_mergeable (defensive)', () => { + // GitHub returns UNKNOWN while it's still computing mergeability. We don't + // promise the caller "yes mergeable" until GitHub has actually decided. + expect(emptyRollupBlocker({ state: 'OPEN', isDraft: false, mergeable: 'UNKNOWN' })).toBe('not_mergeable'); + }); + + test('mergeable=false (boolean) → not_mergeable', () => { + expect(emptyRollupBlocker({ state: 'OPEN', isDraft: false, mergeable: false })).toBe('not_mergeable'); + }); +}); + +describe('probeGithub — argv shape', () => { + test('asks for statusCheckRollup,url,state,isDraft,mergeable,mergeStateStatus', () => { + on( + 'gh pr view', + JSON.stringify({ + url: 'https://github.com/org/repo/pull/1', + statusCheckRollup: [], + state: 'OPEN', + isDraft: false, + mergeable: 'MERGEABLE', + mergeStateStatus: 'CLEAN', + }), + ); + probeGithub(1); + const viewCall = execCalls.find((c) => c.startsWith('gh pr view')) ?? ''; + expect(viewCall).toContain('statusCheckRollup'); + expect(viewCall).toContain('url'); + expect(viewCall).toContain('state'); + expect(viewCall).toContain('isDraft'); + expect(viewCall).toContain('mergeable'); + expect(viewCall).toContain('mergeStateStatus'); + }); +}); + describe('prWaitCiGithub — failure surfaces as AdapterResult', () => { test('gh failure → ok:false, code unexpected_error', async () => { on('gh pr view', () => { diff --git a/lib/adapters/pr-wait-ci-github.ts b/lib/adapters/pr-wait-ci-github.ts index eb8854f..ec6fb96 100644 --- a/lib/adapters/pr-wait-ci-github.ts +++ b/lib/adapters/pr-wait-ci-github.ts @@ -30,6 +30,7 @@ import { import type { AdapterResult, PrWaitCiArgs, + PrWaitCiNoChecksResponse, PrWaitCiResponse, } from './types.js'; @@ -61,6 +62,15 @@ interface PrViewResponse { statusCheckRollup?: RollupItem[]; } +interface PrProbeResponse { + url?: string; + statusCheckRollup?: RollupItem[]; + state?: string; // OPEN | CLOSED | MERGED + isDraft?: boolean; + mergeable?: string | boolean; // MERGEABLE | CONFLICTING | UNKNOWN | bool + mergeStateStatus?: string; // CLEAN | DIRTY | BLOCKED | UNSTABLE | UNKNOWN +} + type Bucket = 'pass' | 'fail' | 'pending' | 'skipping'; /** @@ -139,12 +149,73 @@ export function snapshotGithub(number: number, repo?: string): ChecksSnapshot { }; } +/** + * Initial probe — single `gh pr view` that pulls rollup + mergeability fields + * in one shot (#416). Used to detect the empty-rollup short-circuit case + * before the polling loop even starts. Separate from `snapshotGithub` because + * the polling loop's per-iteration query stays minimal (no need to repeat + * mergeability on every poll). + */ +export function probeGithub(number: number, repo?: string): PrProbeResponse { + const raw = exec( + `gh pr view ${number} --json statusCheckRollup,url,state,isDraft,mergeable,mergeStateStatus${repoFlag(repo)}`, + ); + return JSON.parse(raw) as PrProbeResponse; +} + +/** + * Resolve the empty-rollup short-circuit blocker (#416). Returns `null` when + * the PR is mergeable today (no checks AND no obstructions). Otherwise returns + * a short string naming the obstruction — `draft`, `closed`, `merged`, + * `conflicts`, or `not_mergeable` — for inclusion in the typed response. + * + * `mergeable` is a tri-state on GitHub: `MERGEABLE | CONFLICTING | UNKNOWN` + * (or boolean on older REST shapes). We treat anything that isn't an explicit + * "yes, mergeable" as a blocker so callers never get a false-positive on a PR + * that GitHub is still computing. + */ +export function emptyRollupBlocker(probe: PrProbeResponse): string | null { + const state = (probe.state ?? '').toUpperCase(); + if (state === 'CLOSED') return 'closed'; + if (state === 'MERGED') return 'merged'; + if (probe.isDraft === true) return 'draft'; + + const mergeableRaw = + typeof probe.mergeable === 'string' ? probe.mergeable.toUpperCase() : probe.mergeable; + const mergeable = mergeableRaw === true || mergeableRaw === 'MERGEABLE'; + if (!mergeable) { + const mergeState = (probe.mergeStateStatus ?? '').toUpperCase(); + if (mergeState === 'DIRTY' || mergeableRaw === 'CONFLICTING') return 'conflicts'; + return 'not_mergeable'; + } + return null; +} + export async function prWaitCiGithub( args: PrWaitCiArgs, ): Promise> { // Bound any exception that escapes the snapshot helper into a typed result — // adapter callers must not have to try/catch. try { + // #416 short-circuit. One probe BEFORE entering the polling loop: if the + // PR's rollup is empty there is nothing to settle and we return at t=0. + const probeStart = Date.now(); + const probe = probeGithub(args.number, args.repo); + const rollup = probe.statusCheckRollup ?? []; + if (rollup.length === 0) { + const blocker = emptyRollupBlocker(probe); + const elapsedSec = Math.max(0, Math.floor((Date.now() - probeStart) / 1000)); + const data: PrWaitCiNoChecksResponse = { + number: args.number, + status: 'no_checks_required', + elapsed_sec: elapsedSec, + mergeable: blocker === null, + url: probe.url ?? '', + ...(blocker !== null ? { blocker } : {}), + }; + return { ok: true, data }; + } + const pollArgs: PollArgs = { number: args.number, poll_interval_sec: args.poll_interval_sec, diff --git a/lib/adapters/pr-wait-ci-gitlab.test.ts b/lib/adapters/pr-wait-ci-gitlab.test.ts index ab1e5ce..5ad88b9 100644 --- a/lib/adapters/pr-wait-ci-gitlab.test.ts +++ b/lib/adapters/pr-wait-ci-gitlab.test.ts @@ -185,6 +185,9 @@ describe('prWaitCiGitlab — full poll path', () => { if (!('ok' in result) || !result.ok) { throw new Error(`expected ok result, got ${JSON.stringify(result)}`); } + if (!('final_state' in result.data)) { + throw new Error(`expected polled-response shape, got ${JSON.stringify(result.data)}`); + } expect(result.data.final_state).toBe('passed'); expect(result.data.url).toBe('https://gitlab.com/org/repo/-/merge_requests/3'); expect(result.data.number).toBe(3); @@ -202,10 +205,118 @@ describe('prWaitCiGitlab — full poll path', () => { if (!('ok' in result) || !result.ok) { throw new Error(`expected ok result, got ${JSON.stringify(result)}`); } + if (!('final_state' in result.data)) { + throw new Error(`expected polled-response shape, got ${JSON.stringify(result.data)}`); + } expect(result.data.final_state).toBe('failed'); expect(result.data.checks.failed).toBe(1); }); + // --- #416: empty-pipeline short-circuit --- + test('empty pipeline + mergeable MR → no_checks_required immediately', async () => { + on('git remote get-url origin', 'https://gitlab.com/org/repo.git'); + on( + 'glab api projects/org%2Frepo/merge_requests/3', + JSON.stringify({ + iid: 3, + state: 'opened', + web_url: 'https://gitlab.com/org/repo/-/merge_requests/3', + // No head_pipeline / pipeline → empty pipeline. + draft: false, + has_conflicts: false, + merge_status: 'can_be_merged', + detailed_merge_status: 'mergeable', + }), + ); + + const result = await prWaitCiGitlab({ + number: 3, + // Generous timeout — proves we never enter the poll loop. + poll_interval_sec: 5, + timeout_sec: 1800, + }); + if (!('ok' in result) || !result.ok) { + throw new Error(`expected ok result, got ${JSON.stringify(result)}`); + } + const data = result.data as { status?: string; mergeable?: boolean; blocker?: string; elapsed_sec?: number }; + expect(data.status).toBe('no_checks_required'); + expect(data.mergeable).toBe(true); + expect(data.blocker).toBeUndefined(); + expect(data.elapsed_sec).toBeLessThan(5); + }); + + test('empty pipeline + draft MR → no_checks_required + draft blocker', async () => { + on('git remote get-url origin', 'https://gitlab.com/org/repo.git'); + on( + 'glab api projects/org%2Frepo/merge_requests/4', + JSON.stringify({ + iid: 4, + state: 'opened', + web_url: 'https://gitlab.com/org/repo/-/merge_requests/4', + draft: true, + }), + ); + + const result = await prWaitCiGitlab({ + number: 4, + poll_interval_sec: 5, + timeout_sec: 1800, + }); + if (!('ok' in result) || !result.ok) { + throw new Error(`expected ok result, got ${JSON.stringify(result)}`); + } + const data = result.data as { status?: string; mergeable?: boolean; blocker?: string }; + expect(data.status).toBe('no_checks_required'); + expect(data.mergeable).toBe(false); + expect(data.blocker).toBe('draft'); + }); + + test('empty pipeline + conflicts → no_checks_required + conflicts blocker', async () => { + on('git remote get-url origin', 'https://gitlab.com/org/repo.git'); + on( + 'glab api projects/org%2Frepo/merge_requests/5', + JSON.stringify({ + iid: 5, + state: 'opened', + web_url: 'https://gitlab.com/org/repo/-/merge_requests/5', + has_conflicts: true, + merge_status: 'cannot_be_merged', + }), + ); + + const result = await prWaitCiGitlab({ + number: 5, + poll_interval_sec: 5, + timeout_sec: 1800, + }); + if (!('ok' in result) || !result.ok) { + throw new Error(`expected ok result, got ${JSON.stringify(result)}`); + } + const data = result.data as { status?: string; mergeable?: boolean; blocker?: string }; + expect(data.status).toBe('no_checks_required'); + expect(data.mergeable).toBe(false); + expect(data.blocker).toBe('conflicts'); + }); + + test('non-empty pipeline (success) does NOT short-circuit — polled-response shape returned', async () => { + // Regression — when head_pipeline is present we hit the poll path and the + // returned envelope must carry `final_state`, NOT `status: no_checks_required`. + on('git remote get-url origin', 'https://gitlab.com/org/repo.git'); + on('glab api projects/org%2Frepo/merge_requests/6', mrJson('success', 'https://gitlab.com/org/repo/-/merge_requests/6')); + + const result = await prWaitCiGitlab({ + number: 6, + poll_interval_sec: 5, + timeout_sec: 10, + }); + if (!('ok' in result) || !result.ok) { + throw new Error(`expected ok result, got ${JSON.stringify(result)}`); + } + const data = result.data as { status?: string; final_state?: string }; + expect(data.final_state).toBe('passed'); + expect(data.status).toBeUndefined(); + }); + test('glab failure → AdapterResult ok:false with unexpected_error code', async () => { on('git remote get-url origin', 'https://gitlab.com/org/repo.git'); on('glab api projects/org%2Frepo/merge_requests/77', () => { diff --git a/lib/adapters/pr-wait-ci-gitlab.ts b/lib/adapters/pr-wait-ci-gitlab.ts index d890cc4..b955651 100644 --- a/lib/adapters/pr-wait-ci-gitlab.ts +++ b/lib/adapters/pr-wait-ci-gitlab.ts @@ -29,6 +29,7 @@ import { import type { AdapterResult, PrWaitCiArgs, + PrWaitCiNoChecksResponse, PrWaitCiResponse, } from './types.js'; @@ -99,10 +100,61 @@ export function snapshotGitlab(number: number, repo?: string): ChecksSnapshot { }; } +/** + * Resolve the empty-pipeline short-circuit blocker (#416). Mirrors the GitHub + * adapter's `emptyRollupBlocker` but reads from a `GitlabMr` shape: an MR with + * no pipeline AND no obstructions returns `null`. Anything else returns a + * short string naming the obstruction — `draft`, `closed`, `merged`, + * `locked`, `conflicts`, or `not_mergeable`. + */ +function emptyPipelineBlockerGitlab( + mr: { state?: string; draft?: boolean; work_in_progress?: boolean; has_conflicts?: boolean; merge_status?: string; detailed_merge_status?: string }, +): string | null { + const state = (mr.state ?? '').toLowerCase(); + if (state === 'closed') return 'closed'; + if (state === 'merged') return 'merged'; + if (state === 'locked') return 'locked'; + if (mr.draft === true || mr.work_in_progress === true) return 'draft'; + if (mr.has_conflicts === true) return 'conflicts'; + // GitLab's `merge_status: 'cannot_be_merged'` (or the newer + // `detailed_merge_status` non-`mergeable` value) is the catch-all for any + // obstruction we haven't named explicitly above. `can_be_merged` / + // `mergeable` is the only happy path. + const ms = (mr.merge_status ?? '').toLowerCase(); + const dms = (mr.detailed_merge_status ?? '').toLowerCase(); + if (ms === 'cannot_be_merged') return 'not_mergeable'; + if (dms !== '' && dms !== 'mergeable' && dms !== 'unchecked' && dms !== 'checking') { + return 'not_mergeable'; + } + return null; +} + export async function prWaitCiGitlab( args: PrWaitCiArgs, ): Promise> { try { + // #416 short-circuit. One probe BEFORE entering the polling loop: if the + // MR has no pipeline data at all (neither `head_pipeline` nor `pipeline`), + // there is nothing to settle and we return at t=0. + const probeStart = Date.now(); + const mr = gitlabApiMr(args.number, parseSlugOpts(args.repo)); + const hasPipeline = + (mr.head_pipeline !== undefined && mr.head_pipeline !== null) || + (mr.pipeline !== undefined && mr.pipeline !== null); + if (!hasPipeline) { + const blocker = emptyPipelineBlockerGitlab(mr); + const elapsedSec = Math.max(0, Math.floor((Date.now() - probeStart) / 1000)); + const data: PrWaitCiNoChecksResponse = { + number: args.number, + status: 'no_checks_required', + elapsed_sec: elapsedSec, + mergeable: blocker === null, + url: mr.web_url ?? '', + ...(blocker !== null ? { blocker } : {}), + }; + return { ok: true, data }; + } + const pollArgs: PollArgs = { number: args.number, poll_interval_sec: args.poll_interval_sec, diff --git a/lib/adapters/types.ts b/lib/adapters/types.ts index 4ee9310..1dcf3c0 100644 --- a/lib/adapters/types.ts +++ b/lib/adapters/types.ts @@ -276,7 +276,12 @@ export interface PrWaitCiChecks { summary: string; } -export interface PrWaitCiResponse { +/** + * Polling-loop return shape — the original `pr_wait_ci` response from before + * #416. Reached when the PR has at least one configured check; the adapter + * polls until every check is settled (or timeout). + */ +export interface PrWaitCiPolledResponse { number: number; final_state: PrWaitCiFinalState; checks: PrWaitCiChecks; @@ -284,6 +289,28 @@ export interface PrWaitCiResponse { url: string; } +/** + * Short-circuit return shape (#416). Reached on the very first probe when the + * PR's status-check rollup is empty — there is nothing to settle, so the + * "wait until CI is settled" semantics are satisfied at t=0. `mergeable` and + * an optional `blocker` distinguish the happy path (PR is ready to merge) + * from the obstructed path (draft / conflicts / closed PR). + * + * `elapsed_sec` is intentionally a small integer (the wall-clock cost of the + * single probe), not a polling-loop duration. + */ +export interface PrWaitCiNoChecksResponse { + number: number; + status: 'no_checks_required'; + elapsed_sec: number; + mergeable: boolean; + /** Reason the PR is not mergeable today (e.g. `draft`, `conflicts`, `closed`). Omitted when mergeable. */ + blocker?: string; + url: string; +} + +export type PrWaitCiResponse = PrWaitCiPolledResponse | PrWaitCiNoChecksResponse; + export type CiWaitRunArgs = unknown; export type CiWaitRunResponse = unknown; diff --git a/tests/pr_wait_ci.test.ts b/tests/pr_wait_ci.test.ts index 84dda94..3bd22b4 100644 --- a/tests/pr_wait_ci.test.ts +++ b/tests/pr_wait_ci.test.ts @@ -595,6 +595,139 @@ describe('pr_wait_ci handler', () => { expect(data.final_state).toBe('passed'); }); + // --- #416: empty-rollup short-circuit --- + // When `pr_wait_ci` is called against a PR whose head branch has no required + // status checks (or whose check rollup is empty for any reason), the handler + // must return immediately rather than spinning to timeout. The semantics is: + // "wait until CI is settled" — if there are no checks to settle, that + // condition is satisfied at t=0. + + test('test_pr_wait_ci_empty_rollup_mergeable — empty rollup + mergeable returns no_checks_required immediately', async () => { + execMockFn = (cmd: string) => { + if (cmd.startsWith('git remote')) + return 'https://github.com/org/repo.git\n'; + if (cmd.startsWith('gh pr view')) + return JSON.stringify({ + url: 'https://github.com/org/repo/pull/42', + statusCheckRollup: [], // empty — nothing to settle + state: 'OPEN', + isDraft: false, + mergeable: 'MERGEABLE', + mergeStateStatus: 'CLEAN', + }); + throw new Error(`unexpected exec: ${cmd}`); + }; + + const result = await handler.execute({ + number: 42, + poll_interval_sec: 5, + // Generous timeout — proves the short-circuit returns *before* any sleep. + timeout_sec: 1800, + }); + const data = parseResult(result); + expect(data.ok).toBe(true); + expect(data.status).toBe('no_checks_required'); + expect(data.mergeable).toBe(true); + expect(typeof data.elapsed_sec).toBe('number'); + expect(data.elapsed_sec).toBeLessThan(5); // far less than poll_interval + expect(data.blocker).toBeUndefined(); + expect(data.url).toBe('https://github.com/org/repo/pull/42'); + // `final_state` is the polling-loop shape — must NOT appear on the + // short-circuit envelope (would confuse callers reading the discriminator). + expect(data.final_state).toBeUndefined(); + }); + + test('test_pr_wait_ci_empty_rollup_with_conflict — empty rollup + conflict returns no_checks_required + blocker', async () => { + execMockFn = (cmd: string) => { + if (cmd.startsWith('git remote')) + return 'https://github.com/org/repo.git\n'; + if (cmd.startsWith('gh pr view')) + return JSON.stringify({ + url: 'https://github.com/org/repo/pull/43', + statusCheckRollup: [], + state: 'OPEN', + isDraft: false, + mergeable: 'CONFLICTING', + mergeStateStatus: 'DIRTY', + }); + throw new Error(`unexpected exec: ${cmd}`); + }; + + const result = await handler.execute({ + number: 43, + poll_interval_sec: 5, + timeout_sec: 1800, + }); + const data = parseResult(result); + expect(data.ok).toBe(true); + expect(data.status).toBe('no_checks_required'); + expect(data.mergeable).toBe(false); + expect(data.blocker).toBe('conflicts'); + expect(data.elapsed_sec).toBeLessThan(5); + }); + + test('test_pr_wait_ci_empty_rollup_draft — empty rollup + draft PR returns no_checks_required + draft blocker', async () => { + execMockFn = (cmd: string) => { + if (cmd.startsWith('git remote')) + return 'https://github.com/org/repo.git\n'; + if (cmd.startsWith('gh pr view')) + return JSON.stringify({ + url: 'https://github.com/org/repo/pull/44', + statusCheckRollup: [], + state: 'OPEN', + isDraft: true, + mergeable: 'MERGEABLE', + mergeStateStatus: 'CLEAN', + }); + throw new Error(`unexpected exec: ${cmd}`); + }; + + const result = await handler.execute({ number: 44, poll_interval_sec: 5, timeout_sec: 60 }); + const data = parseResult(result); + expect(data.ok).toBe(true); + expect(data.status).toBe('no_checks_required'); + expect(data.mergeable).toBe(false); + expect(data.blocker).toBe('draft'); + }); + + test('test_pr_wait_ci_normal_path_still_works — non-empty rollup behaves as today', async () => { + // Regression for #416 — the polling-loop path must NOT change shape when + // the rollup is non-empty. We expect `final_state: passed` and the full + // polled-response envelope, NOT the short-circuit `status` discriminator. + execMockFn = (cmd: string) => { + if (cmd.startsWith('git remote')) + return 'https://github.com/org/repo.git\n'; + if (cmd.startsWith('gh pr view')) + return JSON.stringify({ + url: 'https://github.com/org/repo/pull/45', + statusCheckRollup: [ + { __typename: 'CheckRun', name: 'build', status: 'COMPLETED', conclusion: 'SUCCESS' }, + { __typename: 'CheckRun', name: 'test', status: 'COMPLETED', conclusion: 'SUCCESS' }, + ], + state: 'OPEN', + isDraft: false, + mergeable: 'MERGEABLE', + mergeStateStatus: 'CLEAN', + }); + throw new Error(`unexpected exec: ${cmd}`); + }; + + const result = await handler.execute({ + number: 45, + poll_interval_sec: 5, + timeout_sec: 10, + }); + const data = parseResult(result); + expect(data.ok).toBe(true); + // Polled-response shape — `final_state` present, `status` absent. + expect(data.final_state).toBe('passed'); + expect(data.status).toBeUndefined(); + expect(data.url).toBe('https://github.com/org/repo/pull/45'); + const checks = data.checks as Record; + expect(checks.passed).toBe(2); + expect(checks.total).toBe(2); + }); + test('strict_schema_accepts_repo — .strict() schema does not reject new field', async () => { // Proof that adding repo to a .strict() schema doesn't trigger InvalidParams // via the real MCP dispatch surface (handler.execute), not just the test seam.