diff --git a/AGENTS.md b/AGENTS.md index 92be25e..df0b6fc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -63,6 +63,7 @@ Every reproduced race requires a failing-before and passing-after regression. As - Validate every field used to classify an external record as clear, including enum values and required nullable fields. Partial records and malformed policy objects must fail closed. - Validate coupled lifecycle fields as allowed combinations. A terminal-looking conclusion must not override an active or unknown status. - Treat a successful external command as the transition it actually performed. If it can enqueue or schedule work, model and verify that lifecycle before reporting the final action as complete. +- For safety-critical API responses, require and validate every requested field before any early return, including terminal-success paths. Treat an omitted field differently from an explicit `null` allowed by the API contract. - Once an irreversible external command succeeds, do not convert later refresh or rendering failures into action failure. Return the committed result, keep repeat controls disabled, and require fresh confirmation of terminal state. - Track an in-flight irreversible subprocess as part of server shutdown. Abort it, await its settlement, and only then close the state it depends on. - Set the shutdown admission flag before snapshotting active work, and enforce it again at the irreversible action boundary for requests admitted before shutdown began. diff --git a/github/merge.ts b/github/merge.ts index 7cc3549..deab86c 100644 --- a/github/merge.ts +++ b/github/merge.ts @@ -22,6 +22,15 @@ export interface RemoteMergeState { } export interface MergeResult { url: string; } +export type MergeQueueEntryPhase = 'AWAITING_CHECKS' | 'LOCKED' | 'MERGEABLE' | 'QUEUED'; +export type MergeQueueObservation = + | { state: 'queued'; reviewedHead: string; entryId: string; phase: MergeQueueEntryPhase; position: number; enqueuedAt: string; queueHead: string } + | { state: 'removed'; reviewedHead: string; removedAt: string; reason: string } + | { state: 'failed'; reviewedHead: string; entryId: string; reason: string } + | { state: 'merged'; reviewedHead: string; mergedAt: string }; +export interface MergeQueueGateway { + inspectQueue(expectedHead: string, options?: { signal?: AbortSignal; timeoutMs?: number }): Promise; +} export interface MergeGateway { inspect(options?: { fresh?: boolean; timeoutMs?: number }): Promise; merge(expectedHead: string, options?: { signal?: AbortSignal }): Promise; @@ -46,8 +55,13 @@ function flattenPages(value: unknown): unknown[] { return value.flat(); } +function timestamp(value: unknown, label: string): string { + if (typeof value !== 'string' || !/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:\d{2})$/.test(value) || !Number.isFinite(Date.parse(value))) throw new Error(`GitHub returned an invalid ${label}.`); + return value; +} + /** GitHub CLI adapter. All arguments are literal argv; no shell is involved. */ -export class GhMergeGateway implements MergeGateway { +export class GhMergeGateway implements MergeGateway, MergeQueueGateway { readonly config: GhMergeConfig; readonly run: RunGh; #cache: { expiresAt: number; state: RemoteMergeState } | null = null; @@ -228,6 +242,74 @@ export class GhMergeGateway implements MergeGateway { return attempt; } + async inspectQueue(expectedHead: string, options: { signal?: AbortSignal; timeoutMs?: number } = {}): Promise { + fullSha(expectedHead, 'expected head SHA'); + const timeoutMs = options.timeoutMs ?? 12_000; + if (!Number.isSafeInteger(timeoutMs) || timeoutMs < 1 || timeoutMs > 12_000) throw new Error('Invalid GitHub queue inspection timeout.'); + const [owner, name] = this.config.repository.split('/') as [string, string]; + const query = `query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){number headRefOid state mergedAt mergeQueueEntry{id state position enqueuedAt headCommit{oid} pullRequest{number headRefOid}} timelineItems(last:20,itemTypes:[ADDED_TO_MERGE_QUEUE_EVENT,REMOVED_FROM_MERGE_QUEUE_EVENT]){nodes{__typename ... on AddedToMergeQueueEvent{createdAt} ... on RemovedFromMergeQueueEvent{createdAt reason beforeCommit{oid}}}}}}`; + const timeout = new AbortController(); + const timer = setTimeout(() => timeout.abort(new Error('GitHub merge-queue inspection timed out.')), timeoutMs); + const signal = options.signal ? AbortSignal.any([options.signal, timeout.signal]) : timeout.signal; + try { + if (options.signal?.aborted) throw options.signal.reason; + const response = await this.#json(['api','graphql','-f',`query=${query}`,'-f',`owner=${owner}`,'-f',`name=${name}`,'-F',`number=${this.config.pullRequest}`], signal) as { + data?: { repository?: { pullRequest?: Record | null } | null }; + errors?: unknown; + }; + if (signal.aborted) throw signal.reason; + if (Object.hasOwn(response, 'errors') && (!Array.isArray(response.errors) || response.errors.length > 0)) throw new Error('GitHub returned merge-queue data with errors.'); + const pull = response.data?.repository?.pullRequest; + if (!pull || pull.number !== this.config.pullRequest) throw new Error('GitHub returned an incomplete merge-queue pull request.'); + const reviewedHead = fullSha(pull.headRefOid, 'queue pull request head SHA'); + if (reviewedHead !== expectedHead) throw new Error('The pull request head changed after review.'); + if (!['OPEN','CLOSED','MERGED'].includes(String(pull.state))) throw new Error('GitHub returned an invalid queue pull request state.'); + if (!Object.hasOwn(pull, 'mergedAt') || (pull.mergedAt !== null && typeof pull.mergedAt !== 'string')) throw new Error('GitHub returned invalid merge completion data.'); + if (!Object.hasOwn(pull, 'mergeQueueEntry') || !Object.hasOwn(pull, 'timelineItems')) throw new Error('GitHub returned incomplete merge-queue data.'); + const entry = pull.mergeQueueEntry; + const timeline = pull.timelineItems; + if (!timeline || typeof timeline !== 'object' || Array.isArray(timeline) || !Array.isArray((timeline as { nodes?: unknown }).nodes)) throw new Error('GitHub returned incomplete merge-queue history.'); + const events = (timeline as { nodes: unknown[] }).nodes.map(event => { + if (!event || typeof event !== 'object' || Array.isArray(event)) throw new Error('GitHub returned a malformed merge-queue event.'); + const value = event as { __typename?: unknown; createdAt?: unknown; reason?: unknown; beforeCommit?: { oid?: unknown } | null }; + if (!['AddedToMergeQueueEvent','RemovedFromMergeQueueEvent'].includes(String(value.__typename))) throw new Error('GitHub returned an unknown merge-queue event.'); + const createdAt = timestamp(value.createdAt, 'merge-queue event time'); + if (value.__typename === 'RemovedFromMergeQueueEvent' && (typeof value.reason !== 'string' || !value.reason.trim())) throw new Error('GitHub returned a merge-queue removal without a reason.'); + const beforeHead = value.__typename === 'RemovedFromMergeQueueEvent' ? fullSha(value.beforeCommit?.oid, 'removed merge-queue head SHA') : undefined; + return { type: value.__typename, createdAt, reason: value.reason as string | undefined, beforeHead }; + }); + if (pull.state === 'MERGED') { + if (entry !== null) throw new Error('GitHub returned an active queue entry for a merged pull request.'); + return { state: 'merged', reviewedHead, mergedAt: timestamp(pull.mergedAt, 'merge completion time') }; + } + if (pull.mergedAt !== null) throw new Error('GitHub returned inconsistent merge completion data.'); + + if (entry !== null) { + if (pull.state !== 'OPEN' || !entry || typeof entry !== 'object' || Array.isArray(entry)) throw new Error('GitHub returned an invalid merge-queue entry.'); + const value = entry as { id?: unknown; state?: unknown; position?: unknown; enqueuedAt?: unknown; headCommit?: { oid?: unknown } | null; pullRequest?: { number?: unknown; headRefOid?: unknown } | null }; + if (typeof value.id !== 'string' || !value.id || !['AWAITING_CHECKS','LOCKED','MERGEABLE','QUEUED','UNMERGEABLE'].includes(String(value.state)) || !Number.isSafeInteger(value.position) || (value.position as number) < 0) + throw new Error('GitHub returned an invalid merge-queue entry.'); + timestamp(value.enqueuedAt, 'merge-queue entry time'); + const queueHead = fullSha(value.headCommit?.oid, 'merge-queue head SHA'); + const entryHead = fullSha(value.pullRequest?.headRefOid, 'merge-queue entry pull request head SHA'); + if (value.pullRequest?.number !== this.config.pullRequest || queueHead !== expectedHead || entryHead !== expectedHead) throw new Error('The merge-queue entry does not match the reviewed pull request head.'); + if (value.state === 'UNMERGEABLE') return { state: 'failed', reviewedHead, entryId: value.id, reason: 'GitHub reported the merge queue entry as unmergeable.' }; + return { + state: 'queued', reviewedHead, entryId: value.id, phase: value.state as MergeQueueEntryPhase, + position: value.position as number, enqueuedAt: value.enqueuedAt as string, queueHead, + }; + } + const last = events.at(-1); + if (!last || last.type !== 'RemovedFromMergeQueueEvent') throw new Error('GitHub did not confirm a queued, removed, failed, or merged state.'); + if (last.beforeHead !== expectedHead) throw new Error('The merge-queue removal does not match the reviewed pull request head.'); + return { state: 'removed', reviewedHead, removedAt: last.createdAt, reason: last.reason! }; + } catch (error) { + if (options.signal?.aborted) throw options.signal.reason; + if (timeout.signal.aborted && !options.signal?.aborted) throw new Error('GitHub merge-queue inspection timed out.'); + throw error; + } finally { clearTimeout(timer); } + } + async merge(expectedHead: string, options: { signal?: AbortSignal } = {}): Promise { fullSha(expectedHead, 'expected head SHA'); const flag = this.config.method === 'squash' ? '--squash' : this.config.method === 'rebase' ? '--rebase' : '--merge'; diff --git a/test/merge.test.ts b/test/merge.test.ts index 6dc23d9..908a573 100644 --- a/test/merge.test.ts +++ b/test/merge.test.ts @@ -162,6 +162,113 @@ it('rejects an unsupported runtime merge method', () => { expect(() => new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 21, method: 'typo' as 'merge' })).toThrow(/merge method/i); }); +const queueFixture = (pullRequest: Record) => JSON.stringify({ + data: { repository: { pullRequest: { number: 7, headRefOid: sha('b'), ...pullRequest } } }, +}); + +it.each([ + ['QUEUED', 'queued'], + ['AWAITING_CHECKS', 'queued'], + ['LOCKED', 'queued'], + ['MERGEABLE', 'queued'], +] as const)('maps the recorded %s merge-queue entry to %s', async (entryState, expectedState) => { + const run = async () => queueFixture({ + state: 'OPEN', mergedAt: null, timelineItems: { nodes: [{ __typename: 'AddedToMergeQueueEvent', createdAt: '2026-09-24T08:00:00Z' }] }, + mergeQueueEntry: { id: 'MQE_1', state: entryState, position: 2, enqueuedAt: '2026-09-24T08:00:00Z', headCommit: { oid: sha('b') }, pullRequest: { number: 7, headRefOid: sha('b') } }, + }); + const observation = await new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 24 }, run).inspectQueue(sha('b')); + expect(observation).toEqual({ state: expectedState, reviewedHead: sha('b'), entryId: 'MQE_1', phase: entryState, position: 2, enqueuedAt: '2026-09-24T08:00:00Z', queueHead: sha('b') }); +}); + +it('maps a recorded unmergeable queue entry to a failed terminal state', async () => { + const run = async () => queueFixture({ + state: 'OPEN', mergedAt: null, timelineItems: { nodes: [{ __typename: 'AddedToMergeQueueEvent', createdAt: '2026-09-24T08:00:00Z' }] }, + mergeQueueEntry: { id: 'MQE_1', state: 'UNMERGEABLE', position: 1, enqueuedAt: '2026-09-24T08:00:00Z', headCommit: { oid: sha('b') }, pullRequest: { number: 7, headRefOid: sha('b') } }, + }); + await expect(new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 24 }, run).inspectQueue(sha('b'))).resolves.toEqual({ + state: 'failed', reviewedHead: sha('b'), entryId: 'MQE_1', reason: 'GitHub reported the merge queue entry as unmergeable.', + }); +}); + +it('preserves the recorded reason when GitHub removes a pull request from the merge queue', async () => { + const run = async () => queueFixture({ + state: 'OPEN', mergedAt: null, mergeQueueEntry: null, timelineItems: { nodes: [ + { __typename: 'AddedToMergeQueueEvent', createdAt: '2026-09-24T08:00:00Z' }, + { __typename: 'RemovedFromMergeQueueEvent', createdAt: '2026-09-24T08:05:00Z', reason: 'Checks failed', beforeCommit: { oid: sha('b') } }, + ] }, + }); + await expect(new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 24 }, run).inspectQueue(sha('b'))).resolves.toEqual({ + state: 'removed', reviewedHead: sha('b'), removedAt: '2026-09-24T08:05:00Z', reason: 'Checks failed', + }); +}); + +it('reports merged only when GitHub confirms the reviewed head was merged', async () => { + let call: readonly string[] = []; + const run = async (args: readonly string[]) => { + call = args; + return queueFixture({ state: 'MERGED', mergedAt: '2026-09-24T08:10:00Z', mergeQueueEntry: null, timelineItems: { nodes: [] } }); + }; + await expect(new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 24 }, run).inspectQueue(sha('b'))).resolves.toEqual({ + state: 'merged', reviewedHead: sha('b'), mergedAt: '2026-09-24T08:10:00Z', + }); + expect(call).toEqual([ + 'api', 'graphql', '-f', + 'query=query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){number headRefOid state mergedAt mergeQueueEntry{id state position enqueuedAt headCommit{oid} pullRequest{number headRefOid}} timelineItems(last:20,itemTypes:[ADDED_TO_MERGE_QUEUE_EVENT,REMOVED_FROM_MERGE_QUEUE_EVENT]){nodes{__typename ... on AddedToMergeQueueEvent{createdAt} ... on RemovedFromMergeQueueEvent{createdAt reason beforeCommit{oid}}}}}}', + '-f', 'owner=owner', '-f', 'name=repo', '-F', 'number=7', + ]); +}); + +it.each([ + ['a replaced reviewed head', queueFixture({ state: 'OPEN', mergedAt: null, headRefOid: sha('d'), mergeQueueEntry: { id: 'MQE_1', state: 'QUEUED', position: 1, enqueuedAt: '2026-09-24T08:00:00Z', headCommit: { oid: sha('d') }, pullRequest: { number: 7, headRefOid: sha('d') } }, timelineItems: { nodes: [] } })], + ['a queue entry for another head', queueFixture({ state: 'OPEN', mergedAt: null, mergeQueueEntry: { id: 'MQE_1', state: 'QUEUED', position: 1, enqueuedAt: '2026-09-24T08:00:00Z', headCommit: { oid: sha('d') }, pullRequest: { number: 7, headRefOid: sha('b') } }, timelineItems: { nodes: [] } })], + ['a stale removal from another head', queueFixture({ state: 'OPEN', mergedAt: null, mergeQueueEntry: null, timelineItems: { nodes: [{ __typename: 'RemovedFromMergeQueueEvent', createdAt: '2026-09-24T08:05:00Z', reason: 'Checks failed', beforeCommit: { oid: sha('d') } }] } })], + ['an absent queue entry without a removal event', queueFixture({ state: 'OPEN', mergedAt: null, mergeQueueEntry: null, timelineItems: { nodes: [] } })], + ['a removal without a reason', queueFixture({ state: 'OPEN', mergedAt: null, mergeQueueEntry: null, timelineItems: { nodes: [{ __typename: 'RemovedFromMergeQueueEvent', createdAt: '2026-09-24T08:05:00Z', reason: null, beforeCommit: { oid: sha('b') } }] } })], + ['an omitted mergeQueueEntry field', queueFixture({ state: 'MERGED', mergedAt: '2026-09-24T08:10:00Z', timelineItems: { nodes: [] } })], + ['an omitted timelineItems field', queueFixture({ state: 'MERGED', mergedAt: '2026-09-24T08:10:00Z', mergeQueueEntry: null })], + ['a malformed timeline node on a merged response', queueFixture({ state: 'MERGED', mergedAt: '2026-09-24T08:10:00Z', mergeQueueEntry: null, timelineItems: { nodes: [null] } })], + ['GraphQL errors alongside data', JSON.stringify({ data: { repository: { pullRequest: { number: 7, headRefOid: sha('b'), state: 'MERGED', mergedAt: '2026-09-24T08:10:00Z', mergeQueueEntry: null, timelineItems: { nodes: [] } } } }, errors: [{ message: 'partial' }] })], +] as const)('fails closed for %s', async (_case, fixture) => { + const client = new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 24 }, async () => fixture); + await expect(client.inspectQueue(sha('b'))).rejects.toThrow(); +}); + +it('bounds merge-queue reads with one overall deadline', async () => { + vi.useFakeTimers(); + try { + let aborts = 0; + const run = async (_args: readonly string[], options?: { signal?: AbortSignal }) => new Promise((_resolve, reject) => { + options?.signal?.addEventListener('abort', () => { aborts++; reject(options.signal?.reason); }, { once: true }); + }); + const pending = new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 24 }, run).inspectQueue(sha('b'), { timeoutMs: 25 }); + const outcome = pending.catch(error => error); + await vi.advanceTimersByTimeAsync(25); + await expect(outcome).resolves.toMatchObject({ message: 'GitHub merge-queue inspection timed out.' }); + expect(aborts).toBe(1); + } finally { vi.useRealTimers(); } +}); + +it('preserves caller cancellation while reading merge-queue state', async () => { + const controller = new AbortController(); + const run = async (_args: readonly string[], options?: { signal?: AbortSignal }) => new Promise((_resolve, reject) => { + options?.signal?.addEventListener('abort', () => reject(new Error('generic runner abort')), { once: true }); + }); + const pending = new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 24 }, run).inspectQueue(sha('b'), { signal: controller.signal }); + controller.abort(new Error('closing queue watcher')); + await expect(pending).rejects.toThrow('closing queue watcher'); +}); + +it('discards a GraphQL response that resolves after caller cancellation', async () => { + let release!: (value: string) => void; + const response = new Promise(resolve => { release = resolve; }); + const controller = new AbortController(); + const client = new GhMergeGateway({ repository: 'owner/repo', pullRequest: 7, issue: 24 }, async () => response); + const pending = client.inspectQueue(sha('b'), { signal: controller.signal }); + controller.abort(new Error('closing queue watcher')); + release(queueFixture({ state: 'MERGED', mergedAt: '2026-09-24T08:10:00Z', mergeQueueEntry: null, timelineItems: { nodes: [] } })); + await expect(pending).rejects.toThrow('closing queue watcher'); +}); + it.each([ ['ruleset', [{ type: 'required_status_checks', parameters: { strict_required_status_checks_policy: true, required_status_checks: [{ context: 'test', integration_id: '10' }] } }], { strict: false, checks: [] }], ['classic', [], { strict: true, checks: [{ context: 'test' }] }],