feat(autofix-pr): 注册 completionChecker 用 gh CLI 探测 PR 完成
Phase 2 of remote-agent completion loop。Phase 1 修了 monitor lock
dangling,但完成信号仍然只能等 CCR session 自然 archive(timing 不可
预测,且不知道 PR 究竟有没有被修好)。Phase 2 加上主动完成探测。
实现:
- 新增 prOutcomeCheck.ts(纯决策矩阵):summariseAutofixOutcome 给定
PR 快照 + 基线 SHA 返回 completed/summary。8 个决策分支单元测试。
- 新增 prFetch.ts(spawn 层):runGhPrView 调 gh CLI,fetchPrHeadSha
在 launch 时捕获基线 SHA,checkPrAutofixOutcome 组合两者。
- AutofixPrRemoteTaskMetadata 加 initialHeadSha?: string 字段,survive
--resume。
- launchAutofixPr.ts 模块顶部 registerCompletionChecker('autofix-pr',
...),5s throttle 防 gh CLI 调用爆。callAutofixPr 启动时调
fetchPrHeadSha 传入 metadata。
决策矩阵:
MERGED → done(merged)
CLOSED 未 merge → done(closed without fix)
OPEN 无 baseline → 继续轮询
OPEN head 未变 → 继续轮询(agent 还没 push)
OPEN head 变 + CI pending → 继续轮询
OPEN head 变 + CI failure → done(surface red,user 决定 retry)
OPEN head 变 + CI success → done(clean fix)
设计:
- gh CLI 而非 Octokit:复用用户已有 auth,不引入 token 管理
- 决策与 spawn 分文件:prOutcomeCheck 纯函数易测,prFetch 单独 mock
避免 Bun mock.module 进程级污染(已在 launchAutofixPr.test 注释说明)
- 5s throttle:framework 每 1s 轮询,gh CLI subprocess 太重不能跟上
- 失败兜底:fetchPrHeadSha/checkPrAutofixOutcome 失败均不抛,returns
null/false,framework 继续走原路径
测试:
- prOutcomeCheck 9 个单测覆盖决策矩阵
- launchAutofixPr 5 个新测试:checker 注册 / fetchPrHeadSha 调用 /
initialHeadSha 传 metadata / SHA 失败仍能 launch / SHA null 处理
完整方案见 docs/features/remote-agent-completion-analysis.md。
This commit is contained in:
parent
cca5102f15
commit
31433ac958
|
|
@ -59,15 +59,34 @@ const getSessionUrlMock = mock(
|
||||||
const registerCompletionHookMock = mock<
|
const registerCompletionHookMock = mock<
|
||||||
(taskType: string, hook: (taskId: string, metadata?: unknown) => void) => void
|
(taskType: string, hook: (taskId: string, metadata?: unknown) => void) => void
|
||||||
>(() => {})
|
>(() => {})
|
||||||
|
const registerCompletionCheckerMock = mock<
|
||||||
|
(
|
||||||
|
taskType: string,
|
||||||
|
checker: (metadata?: unknown) => Promise<string | null>,
|
||||||
|
) => void
|
||||||
|
>(() => {})
|
||||||
|
|
||||||
mock.module('src/tasks/RemoteAgentTask/RemoteAgentTask.js', () => ({
|
mock.module('src/tasks/RemoteAgentTask/RemoteAgentTask.js', () => ({
|
||||||
checkRemoteAgentEligibility: checkEligibilityMock,
|
checkRemoteAgentEligibility: checkEligibilityMock,
|
||||||
registerRemoteAgentTask: registerMock,
|
registerRemoteAgentTask: registerMock,
|
||||||
registerCompletionHook: registerCompletionHookMock,
|
registerCompletionHook: registerCompletionHookMock,
|
||||||
|
registerCompletionChecker: registerCompletionCheckerMock,
|
||||||
getRemoteTaskSessionUrl: getSessionUrlMock,
|
getRemoteTaskSessionUrl: getSessionUrlMock,
|
||||||
formatPreconditionError: (e: { type: string }) => e.type,
|
formatPreconditionError: (e: { type: string }) => e.type,
|
||||||
}))
|
}))
|
||||||
|
|
||||||
|
const fetchPrHeadShaMock = mock<
|
||||||
|
(owner: string, repo: string, prNumber: number) => Promise<string | null>
|
||||||
|
>(() => Promise.resolve('sha-baseline-abc123'))
|
||||||
|
|
||||||
|
// Mock prFetch.ts (gh CLI spawn layer) — keeping the pure decision matrix
|
||||||
|
// in prOutcomeCheck.ts unmocked so its tests are unaffected by this file's
|
||||||
|
// process-global mock.module pollution.
|
||||||
|
mock.module('src/commands/autofix-pr/prFetch.js', () => ({
|
||||||
|
fetchPrHeadSha: fetchPrHeadShaMock,
|
||||||
|
checkPrAutofixOutcome: mock(() => Promise.resolve({ completed: false })),
|
||||||
|
}))
|
||||||
|
|
||||||
const detectRepoMock = mock(() =>
|
const detectRepoMock = mock(() =>
|
||||||
Promise.resolve({ host: 'github.com', owner: 'acme', name: 'myrepo' }),
|
Promise.resolve({ host: 'github.com', owner: 'acme', name: 'myrepo' }),
|
||||||
)
|
)
|
||||||
|
|
@ -436,6 +455,76 @@ describe('callAutofixPr · completion hook wiring (taskId mismatch regression)',
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
// Phase 2: completionChecker wiring + initialHeadSha capture
|
||||||
|
describe('callAutofixPr · Phase 2 completionChecker integration', () => {
|
||||||
|
test('completionChecker is registered at module load with autofix-pr type', () => {
|
||||||
|
// The registration happens during the beforeAll dynamic import; just
|
||||||
|
// verify the mock recorded a call. Filter by task type so any future
|
||||||
|
// additional registrations elsewhere don't break this assertion.
|
||||||
|
const calls = registerCompletionCheckerMock.mock.calls.filter(
|
||||||
|
c => c[0] === 'autofix-pr',
|
||||||
|
)
|
||||||
|
expect(calls.length).toBeGreaterThan(0)
|
||||||
|
const hook = calls[calls.length - 1]?.[1]
|
||||||
|
expect(typeof hook).toBe('function')
|
||||||
|
})
|
||||||
|
|
||||||
|
test('callAutofixPr captures initialHeadSha via fetchPrHeadSha', async () => {
|
||||||
|
fetchPrHeadShaMock.mockClear()
|
||||||
|
await callAutofixPr(onDone, makeContext(), '42')
|
||||||
|
expect(fetchPrHeadShaMock).toHaveBeenCalledWith('acme', 'myrepo', 42)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('initialHeadSha is passed into remoteTaskMetadata on register', async () => {
|
||||||
|
fetchPrHeadShaMock.mockImplementationOnce(() =>
|
||||||
|
Promise.resolve('sha-from-launch'),
|
||||||
|
)
|
||||||
|
await callAutofixPr(onDone, makeContext(), '42')
|
||||||
|
expect(registerMock).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
remoteTaskMetadata: expect.objectContaining({
|
||||||
|
owner: 'acme',
|
||||||
|
repo: 'myrepo',
|
||||||
|
prNumber: 42,
|
||||||
|
initialHeadSha: 'sha-from-launch',
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('fetchPrHeadSha failure → metadata initialHeadSha undefined, launch still succeeds', async () => {
|
||||||
|
fetchPrHeadShaMock.mockImplementationOnce(() =>
|
||||||
|
Promise.reject(new Error('gh not installed')),
|
||||||
|
)
|
||||||
|
await callAutofixPr(onDone, makeContext(), '42')
|
||||||
|
expect(registerMock).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
remoteTaskMetadata: expect.objectContaining({
|
||||||
|
owner: 'acme',
|
||||||
|
repo: 'myrepo',
|
||||||
|
prNumber: 42,
|
||||||
|
initialHeadSha: undefined,
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
// Launch must NOT fail just because SHA capture failed
|
||||||
|
const firstArg = onDone.mock.calls[0]?.[0] as string
|
||||||
|
expect(firstArg).toMatch(/Autofix launched/)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('fetchPrHeadSha returning null → metadata initialHeadSha undefined', async () => {
|
||||||
|
fetchPrHeadShaMock.mockImplementationOnce(() => Promise.resolve(null))
|
||||||
|
await callAutofixPr(onDone, makeContext(), '42')
|
||||||
|
expect(registerMock).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
remoteTaskMetadata: expect.objectContaining({
|
||||||
|
initialHeadSha: undefined,
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
// Cover ../index.ts load() — placed in this test file so all the heavy mocks
|
// Cover ../index.ts load() — placed in this test file so all the heavy mocks
|
||||||
// (teleport / detectRepository / RemoteAgentTask / bootstrap-state / analytics /
|
// (teleport / detectRepository / RemoteAgentTask / bootstrap-state / analytics /
|
||||||
// skillDetect) are already registered when load() dynamically imports
|
// skillDetect) are already registered when load() dynamically imports
|
||||||
|
|
|
||||||
158
src/commands/autofix-pr/__tests__/prOutcomeCheck.test.ts
Normal file
158
src/commands/autofix-pr/__tests__/prOutcomeCheck.test.ts
Normal file
|
|
@ -0,0 +1,158 @@
|
||||||
|
import { describe, expect, test } from 'bun:test'
|
||||||
|
import {
|
||||||
|
type PrViewPayload,
|
||||||
|
summariseAutofixOutcome,
|
||||||
|
} from '../prOutcomeCheck.js'
|
||||||
|
|
||||||
|
function basePayload(overrides: Partial<PrViewPayload> = {}): PrViewPayload {
|
||||||
|
return {
|
||||||
|
headRefOid: 'sha-baseline',
|
||||||
|
state: 'OPEN',
|
||||||
|
statusCheckRollup: [],
|
||||||
|
...overrides,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const identity = (overrides: Partial<{ initialHeadSha: string }> = {}) => ({
|
||||||
|
owner: 'acme',
|
||||||
|
repo: 'myrepo',
|
||||||
|
prNumber: 42,
|
||||||
|
initialHeadSha: 'sha-baseline',
|
||||||
|
...overrides,
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('summariseAutofixOutcome · terminal PR states', () => {
|
||||||
|
test('MERGED → completed regardless of head SHA / CI', () => {
|
||||||
|
const result = summariseAutofixOutcome(
|
||||||
|
basePayload({ state: 'MERGED', headRefOid: 'sha-baseline' }),
|
||||||
|
identity(),
|
||||||
|
)
|
||||||
|
expect(result).toEqual({
|
||||||
|
completed: true,
|
||||||
|
summary: 'acme/myrepo#42 merged. Autofix monitoring complete.',
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
test('CLOSED → completed regardless of head SHA / CI', () => {
|
||||||
|
const result = summariseAutofixOutcome(
|
||||||
|
basePayload({ state: 'CLOSED' }),
|
||||||
|
identity(),
|
||||||
|
)
|
||||||
|
expect(result).toEqual({
|
||||||
|
completed: true,
|
||||||
|
summary:
|
||||||
|
'acme/myrepo#42 closed without merge. Autofix monitoring complete.',
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('summariseAutofixOutcome · OPEN PR without push', () => {
|
||||||
|
test('no initialHeadSha baseline → not completed (cannot detect push)', () => {
|
||||||
|
const result = summariseAutofixOutcome(
|
||||||
|
basePayload({ state: 'OPEN' }),
|
||||||
|
identity({ initialHeadSha: undefined as unknown as string }),
|
||||||
|
)
|
||||||
|
expect(result).toEqual({ completed: false })
|
||||||
|
})
|
||||||
|
|
||||||
|
test('headRefOid unchanged → not completed (autofix has not pushed yet)', () => {
|
||||||
|
const result = summariseAutofixOutcome(
|
||||||
|
basePayload({ state: 'OPEN', headRefOid: 'sha-baseline' }),
|
||||||
|
identity(),
|
||||||
|
)
|
||||||
|
expect(result).toEqual({ completed: false })
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
describe('summariseAutofixOutcome · OPEN PR with push, CI variations', () => {
|
||||||
|
test('push detected + no checks configured → completed (success)', () => {
|
||||||
|
const result = summariseAutofixOutcome(
|
||||||
|
basePayload({
|
||||||
|
state: 'OPEN',
|
||||||
|
headRefOid: 'sha-new',
|
||||||
|
statusCheckRollup: [],
|
||||||
|
}),
|
||||||
|
identity(),
|
||||||
|
)
|
||||||
|
expect(result).toEqual({
|
||||||
|
completed: true,
|
||||||
|
summary: 'Autofix pushed commits to acme/myrepo#42, CI green.',
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
test('push detected + CI pending → not completed (wait for CI)', () => {
|
||||||
|
const result = summariseAutofixOutcome(
|
||||||
|
basePayload({
|
||||||
|
state: 'OPEN',
|
||||||
|
headRefOid: 'sha-new',
|
||||||
|
statusCheckRollup: [
|
||||||
|
{ status: 'IN_PROGRESS', conclusion: null, name: 'ci' },
|
||||||
|
{ status: 'COMPLETED', conclusion: 'SUCCESS', name: 'lint' },
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
identity(),
|
||||||
|
)
|
||||||
|
expect(result).toEqual({ completed: false })
|
||||||
|
})
|
||||||
|
|
||||||
|
test('push detected + CI all green → completed (success summary)', () => {
|
||||||
|
const result = summariseAutofixOutcome(
|
||||||
|
basePayload({
|
||||||
|
state: 'OPEN',
|
||||||
|
headRefOid: 'sha-new',
|
||||||
|
statusCheckRollup: [
|
||||||
|
{ status: 'COMPLETED', conclusion: 'SUCCESS', name: 'ci' },
|
||||||
|
{ status: 'COMPLETED', conclusion: 'SUCCESS', name: 'lint' },
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
identity(),
|
||||||
|
)
|
||||||
|
expect(result.completed).toBe(true)
|
||||||
|
if (result.completed) {
|
||||||
|
expect(result.summary).toContain('CI green')
|
||||||
|
expect(result.summary).toContain('acme/myrepo#42')
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
test('push detected + CI red → completed (failure summary surfaces the red)', () => {
|
||||||
|
const result = summariseAutofixOutcome(
|
||||||
|
basePayload({
|
||||||
|
state: 'OPEN',
|
||||||
|
headRefOid: 'sha-new',
|
||||||
|
statusCheckRollup: [
|
||||||
|
{ status: 'COMPLETED', conclusion: 'FAILURE', name: 'ci' },
|
||||||
|
{ status: 'COMPLETED', conclusion: 'SUCCESS', name: 'lint' },
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
identity(),
|
||||||
|
)
|
||||||
|
expect(result.completed).toBe(true)
|
||||||
|
if (result.completed) {
|
||||||
|
expect(result.summary).toContain('CI is failing')
|
||||||
|
expect(result.summary).toContain('1/2 checks failing')
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
test('neutral / skipped conclusions count as success (not failure)', () => {
|
||||||
|
const result = summariseAutofixOutcome(
|
||||||
|
basePayload({
|
||||||
|
state: 'OPEN',
|
||||||
|
headRefOid: 'sha-new',
|
||||||
|
statusCheckRollup: [
|
||||||
|
{
|
||||||
|
status: 'COMPLETED',
|
||||||
|
conclusion: 'NEUTRAL',
|
||||||
|
name: 'optional-check',
|
||||||
|
},
|
||||||
|
{ status: 'COMPLETED', conclusion: 'SKIPPED', name: 'docs-check' },
|
||||||
|
{ status: 'COMPLETED', conclusion: 'SUCCESS', name: 'ci' },
|
||||||
|
],
|
||||||
|
}),
|
||||||
|
identity(),
|
||||||
|
)
|
||||||
|
expect(result.completed).toBe(true)
|
||||||
|
if (result.completed) {
|
||||||
|
expect(result.summary).toContain('CI green')
|
||||||
|
}
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
@ -13,8 +13,10 @@ import {
|
||||||
checkRemoteAgentEligibility,
|
checkRemoteAgentEligibility,
|
||||||
formatPreconditionError,
|
formatPreconditionError,
|
||||||
getRemoteTaskSessionUrl,
|
getRemoteTaskSessionUrl,
|
||||||
|
registerCompletionChecker,
|
||||||
registerCompletionHook,
|
registerCompletionHook,
|
||||||
registerRemoteAgentTask,
|
registerRemoteAgentTask,
|
||||||
|
type AutofixPrRemoteTaskMetadata,
|
||||||
type BackgroundRemoteSessionPrecondition,
|
type BackgroundRemoteSessionPrecondition,
|
||||||
} from '../../tasks/RemoteAgentTask/RemoteAgentTask.js'
|
} from '../../tasks/RemoteAgentTask/RemoteAgentTask.js'
|
||||||
import type { LocalJSXCommandCall } from '../../types/command.js'
|
import type { LocalJSXCommandCall } from '../../types/command.js'
|
||||||
|
|
@ -30,16 +32,53 @@ import {
|
||||||
updateActiveMonitor,
|
updateActiveMonitor,
|
||||||
} from './monitorState.js'
|
} from './monitorState.js'
|
||||||
import { parseAutofixArgs } from './parseArgs.js'
|
import { parseAutofixArgs } from './parseArgs.js'
|
||||||
|
import { checkPrAutofixOutcome, fetchPrHeadSha } from './prFetch.js'
|
||||||
import { detectAutofixSkills, formatSkillsHint } from './skillDetect.js'
|
import { detectAutofixSkills, formatSkillsHint } from './skillDetect.js'
|
||||||
|
|
||||||
|
// Throttle map for the completionChecker: gh CLI is called at most once per
|
||||||
|
// PR per CHECK_INTERVAL_MS, regardless of the framework's 1s poll cadence.
|
||||||
|
// Key is `${owner}/${repo}#${prNumber}`. Cleared when the completion hook
|
||||||
|
// fires so a re-launched monitor starts with a fresh budget.
|
||||||
|
const lastCheckAt = new Map<string, number>()
|
||||||
|
const CHECK_INTERVAL_MS = 5_000
|
||||||
|
|
||||||
|
function throttleKey(meta: AutofixPrRemoteTaskMetadata): string {
|
||||||
|
return `${meta.owner}/${meta.repo}#${meta.prNumber}`
|
||||||
|
}
|
||||||
|
|
||||||
|
// Register the completionChecker once at module load. The framework calls it
|
||||||
|
// on every poll tick for tasks with remoteTaskType==='autofix-pr'; throttle
|
||||||
|
// inside so we don't fire gh CLI 60×/min. Returns the summary string on
|
||||||
|
// completion (becomes the task-notification body) or null to keep polling.
|
||||||
|
registerCompletionChecker('autofix-pr', async metadata => {
|
||||||
|
const meta = metadata as AutofixPrRemoteTaskMetadata | undefined
|
||||||
|
if (!meta) return null
|
||||||
|
|
||||||
|
const key = throttleKey(meta)
|
||||||
|
const now = Date.now()
|
||||||
|
if (now - (lastCheckAt.get(key) ?? 0) < CHECK_INTERVAL_MS) return null
|
||||||
|
lastCheckAt.set(key, now)
|
||||||
|
|
||||||
|
const result = await checkPrAutofixOutcome({
|
||||||
|
owner: meta.owner,
|
||||||
|
repo: meta.repo,
|
||||||
|
prNumber: meta.prNumber,
|
||||||
|
initialHeadSha: meta.initialHeadSha,
|
||||||
|
})
|
||||||
|
return result.completed ? result.summary : null
|
||||||
|
})
|
||||||
|
|
||||||
// Release the singleton monitor lock when the framework transitions the
|
// Release the singleton monitor lock when the framework transitions the
|
||||||
// autofix task to a terminal state. Without this, the lock — keyed by the
|
// autofix task to a terminal state. Without this, the lock — keyed by the
|
||||||
// framework-assigned taskId (after callAutofixPr's updateActiveMonitor swap)
|
// framework-assigned taskId (after callAutofixPr's updateActiveMonitor swap)
|
||||||
// — would dangle past natural completion, blocking subsequent /autofix-pr
|
// — would dangle past natural completion, blocking subsequent /autofix-pr
|
||||||
// invocations until the process restarts. Registered at module load; the
|
// invocations until the process restarts. Registered at module load; the
|
||||||
// framework's runCompletionHook invokes it once per terminal transition.
|
// framework's runCompletionHook invokes it once per terminal transition.
|
||||||
registerCompletionHook('autofix-pr', taskId => {
|
// Also clear the per-PR throttle entry so a re-launch starts fresh.
|
||||||
|
registerCompletionHook('autofix-pr', (taskId, metadata) => {
|
||||||
clearActiveMonitor(taskId)
|
clearActiveMonitor(taskId)
|
||||||
|
const meta = metadata as AutofixPrRemoteTaskMetadata | undefined
|
||||||
|
if (meta) lastCheckAt.delete(throttleKey(meta))
|
||||||
})
|
})
|
||||||
|
|
||||||
function makeErrorText(message: string, code: string): string {
|
function makeErrorText(message: string, code: string): string {
|
||||||
|
|
@ -286,6 +325,15 @@ export const callAutofixPr: LocalJSXCommandCall = async (
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 4.8b capture PR head SHA before registering so the completionChecker
|
||||||
|
// can detect when the agent has pushed new commits. Best-effort — if gh
|
||||||
|
// is unavailable or the call fails, leave initialHeadSha undefined and
|
||||||
|
// the checker falls back to terminal-state-only completion (closed /
|
||||||
|
// merged). Don't block on this; teleport succeeded already.
|
||||||
|
const initialHeadSha =
|
||||||
|
(await fetchPrHeadSha(owner, repo, prNumber).catch(() => null)) ??
|
||||||
|
undefined
|
||||||
|
|
||||||
// 4.9 register task. If this throws, release the lock so the user can
|
// 4.9 register task. If this throws, release the lock so the user can
|
||||||
// retry — the remote CCR session is already created so we surface a
|
// retry — the remote CCR session is already created so we surface a
|
||||||
// dedicated error code.
|
// dedicated error code.
|
||||||
|
|
@ -303,7 +351,7 @@ export const callAutofixPr: LocalJSXCommandCall = async (
|
||||||
command: `/autofix-pr ${prNumber}`,
|
command: `/autofix-pr ${prNumber}`,
|
||||||
context,
|
context,
|
||||||
isLongRunning: true,
|
isLongRunning: true,
|
||||||
remoteTaskMetadata: { owner, repo, prNumber },
|
remoteTaskMetadata: { owner, repo, prNumber, initialHeadSha },
|
||||||
})
|
})
|
||||||
updateActiveMonitor({ taskId: frameworkTaskId })
|
updateActiveMonitor({ taskId: frameworkTaskId })
|
||||||
} catch (regErr: unknown) {
|
} catch (regErr: unknown) {
|
||||||
|
|
|
||||||
155
src/commands/autofix-pr/prFetch.ts
Normal file
155
src/commands/autofix-pr/prFetch.ts
Normal file
|
|
@ -0,0 +1,155 @@
|
||||||
|
// gh CLI integration for autofix-pr: fetches PR snapshots and feeds them
|
||||||
|
// through the pure decision matrix in prOutcomeCheck.ts. Kept separate so
|
||||||
|
// tests of the decision matrix never have to mock node:child_process — and
|
||||||
|
// tests of callAutofixPr can mock this module without polluting the pure
|
||||||
|
// decision matrix module (Bun mock.module is process-global).
|
||||||
|
|
||||||
|
import { spawn } from 'node:child_process'
|
||||||
|
import {
|
||||||
|
type AutofixOutcomeProbeResult,
|
||||||
|
type PrViewPayload,
|
||||||
|
summariseAutofixOutcome,
|
||||||
|
} from './prOutcomeCheck.js'
|
||||||
|
|
||||||
|
export interface AutofixOutcomeProbeInput {
|
||||||
|
owner: string
|
||||||
|
repo: string
|
||||||
|
prNumber: number
|
||||||
|
/**
|
||||||
|
* Head commit SHA captured at /autofix-pr launch. When this differs from
|
||||||
|
* the current head, autofix has pushed at least one commit.
|
||||||
|
*/
|
||||||
|
initialHeadSha?: string
|
||||||
|
/**
|
||||||
|
* Timeout for the gh CLI invocation. Caller is the framework's per-tick
|
||||||
|
* poller, so failures must be bounded — a hung gh process would stall
|
||||||
|
* the entire poll loop.
|
||||||
|
*/
|
||||||
|
timeoutMs?: number
|
||||||
|
}
|
||||||
|
|
||||||
|
const DEFAULT_TIMEOUT_MS = 5_000
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Fetch the PR's current head SHA, state, and CI rollup, and decide whether
|
||||||
|
* autofix has finished. Returns `{ completed: true, summary }` if so;
|
||||||
|
* otherwise `{ completed: false }`. Never throws.
|
||||||
|
*/
|
||||||
|
export async function checkPrAutofixOutcome(
|
||||||
|
input: AutofixOutcomeProbeInput,
|
||||||
|
): Promise<AutofixOutcomeProbeResult> {
|
||||||
|
const { owner, repo, prNumber, initialHeadSha, timeoutMs } = input
|
||||||
|
|
||||||
|
let payload: PrViewPayload
|
||||||
|
try {
|
||||||
|
payload = await runGhPrView(
|
||||||
|
owner,
|
||||||
|
repo,
|
||||||
|
prNumber,
|
||||||
|
timeoutMs ?? DEFAULT_TIMEOUT_MS,
|
||||||
|
)
|
||||||
|
} catch {
|
||||||
|
return { completed: false }
|
||||||
|
}
|
||||||
|
|
||||||
|
return summariseAutofixOutcome(payload, {
|
||||||
|
owner,
|
||||||
|
repo,
|
||||||
|
prNumber,
|
||||||
|
initialHeadSha,
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolve the PR's current head commit SHA. Used at /autofix-pr launch to
|
||||||
|
* capture a baseline; later compared against the live SHA to detect pushes.
|
||||||
|
* Returns null on any failure (network, missing gh, permissions) — the
|
||||||
|
* caller treats null as "no baseline" and falls back to terminal-state-only
|
||||||
|
* completion detection.
|
||||||
|
*/
|
||||||
|
export async function fetchPrHeadSha(
|
||||||
|
owner: string,
|
||||||
|
repo: string,
|
||||||
|
prNumber: number,
|
||||||
|
timeoutMs = DEFAULT_TIMEOUT_MS,
|
||||||
|
): Promise<string | null> {
|
||||||
|
try {
|
||||||
|
const payload = await runGhPrView(owner, repo, prNumber, timeoutMs)
|
||||||
|
return payload.headRefOid || null
|
||||||
|
} catch {
|
||||||
|
return null
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
interface SpawnError extends Error {
|
||||||
|
code?: string
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Spawn `gh pr view {n} --repo {owner}/{repo} --json ...` and parse the
|
||||||
|
* result. Rejects on non-zero exit, timeout, or JSON parse failure.
|
||||||
|
*/
|
||||||
|
function runGhPrView(
|
||||||
|
owner: string,
|
||||||
|
repo: string,
|
||||||
|
prNumber: number,
|
||||||
|
timeoutMs: number,
|
||||||
|
): Promise<PrViewPayload> {
|
||||||
|
return new Promise((resolve, reject) => {
|
||||||
|
const proc = spawn(
|
||||||
|
'gh',
|
||||||
|
[
|
||||||
|
'pr',
|
||||||
|
'view',
|
||||||
|
String(prNumber),
|
||||||
|
'--repo',
|
||||||
|
`${owner}/${repo}`,
|
||||||
|
'--json',
|
||||||
|
'headRefOid,state,statusCheckRollup',
|
||||||
|
],
|
||||||
|
{ stdio: ['ignore', 'pipe', 'pipe'] },
|
||||||
|
)
|
||||||
|
const stdoutChunks: Buffer[] = []
|
||||||
|
const stderrChunks: Buffer[] = []
|
||||||
|
let settled = false
|
||||||
|
|
||||||
|
const timer = setTimeout(() => {
|
||||||
|
if (settled) return
|
||||||
|
settled = true
|
||||||
|
proc.kill('SIGKILL')
|
||||||
|
reject(new Error(`gh pr view timed out after ${timeoutMs}ms`))
|
||||||
|
}, timeoutMs)
|
||||||
|
|
||||||
|
proc.stdout.on('data', chunk => stdoutChunks.push(chunk as Buffer))
|
||||||
|
proc.stderr.on('data', chunk => stderrChunks.push(chunk as Buffer))
|
||||||
|
|
||||||
|
proc.on('error', (err: SpawnError) => {
|
||||||
|
if (settled) return
|
||||||
|
settled = true
|
||||||
|
clearTimeout(timer)
|
||||||
|
reject(err)
|
||||||
|
})
|
||||||
|
|
||||||
|
proc.on('close', code => {
|
||||||
|
if (settled) return
|
||||||
|
settled = true
|
||||||
|
clearTimeout(timer)
|
||||||
|
if (code !== 0) {
|
||||||
|
const stderr = Buffer.concat(stderrChunks).toString('utf8').trim()
|
||||||
|
reject(
|
||||||
|
new Error(`gh pr view exited ${code}: ${stderr || '<no stderr>'}`),
|
||||||
|
)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
const stdout = Buffer.concat(stdoutChunks).toString('utf8').trim()
|
||||||
|
try {
|
||||||
|
const parsed = JSON.parse(stdout) as PrViewPayload
|
||||||
|
resolve(parsed)
|
||||||
|
} catch (e) {
|
||||||
|
reject(
|
||||||
|
new Error(`gh pr view JSON parse failed: ${(e as Error).message}`),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
})
|
||||||
|
}
|
||||||
123
src/commands/autofix-pr/prOutcomeCheck.ts
Normal file
123
src/commands/autofix-pr/prOutcomeCheck.ts
Normal file
|
|
@ -0,0 +1,123 @@
|
||||||
|
// Pure decision matrix for autofix-pr completion detection.
|
||||||
|
//
|
||||||
|
// Given a snapshot of the PR (state, head SHA, CI rollup) and a baseline
|
||||||
|
// head SHA captured at /autofix-pr launch, decide whether autofix has
|
||||||
|
// finished. No side effects — extracted from the gh CLI invocation in
|
||||||
|
// prFetch.ts so unit tests can exercise every branch without spawning
|
||||||
|
// subprocesses.
|
||||||
|
|
||||||
|
export type AutofixOutcomeProbeResult =
|
||||||
|
| { completed: true; summary: string }
|
||||||
|
| { completed: false }
|
||||||
|
|
||||||
|
export interface PrViewPayload {
|
||||||
|
headRefOid: string
|
||||||
|
state: 'OPEN' | 'CLOSED' | 'MERGED'
|
||||||
|
statusCheckRollup?: Array<{
|
||||||
|
conclusion?: string | null
|
||||||
|
status?: string | null
|
||||||
|
name?: string
|
||||||
|
}>
|
||||||
|
}
|
||||||
|
|
||||||
|
export interface AutofixOutcomeIdentity {
|
||||||
|
owner: string
|
||||||
|
repo: string
|
||||||
|
prNumber: number
|
||||||
|
/**
|
||||||
|
* Head commit SHA captured at /autofix-pr launch. When this differs from
|
||||||
|
* the current head, autofix has pushed at least one commit. Optional —
|
||||||
|
* absence means we can only finish on terminal PR states (merged/closed).
|
||||||
|
*/
|
||||||
|
initialHeadSha?: string
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Pure judgement of whether autofix has finished, given a PR snapshot and
|
||||||
|
* the baseline head SHA. Decision matrix:
|
||||||
|
* - MERGED → done (merged)
|
||||||
|
* - CLOSED (not merged) → done (closed without fix)
|
||||||
|
* - OPEN, no baseline → keep polling
|
||||||
|
* - OPEN, head unchanged → keep polling (agent hasn't pushed)
|
||||||
|
* - OPEN, head changed, CI pending → keep polling (wait for CI)
|
||||||
|
* - OPEN, head changed, CI failure → done (surface red so user can retry)
|
||||||
|
* - OPEN, head changed, CI success → done (clean fix)
|
||||||
|
*/
|
||||||
|
export function summariseAutofixOutcome(
|
||||||
|
payload: PrViewPayload,
|
||||||
|
identity: AutofixOutcomeIdentity,
|
||||||
|
): AutofixOutcomeProbeResult {
|
||||||
|
const { owner, repo, prNumber, initialHeadSha } = identity
|
||||||
|
|
||||||
|
if (payload.state === 'MERGED') {
|
||||||
|
return {
|
||||||
|
completed: true,
|
||||||
|
summary: `${owner}/${repo}#${prNumber} merged. Autofix monitoring complete.`,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (payload.state === 'CLOSED') {
|
||||||
|
return {
|
||||||
|
completed: true,
|
||||||
|
summary: `${owner}/${repo}#${prNumber} closed without merge. Autofix monitoring complete.`,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (!initialHeadSha) return { completed: false }
|
||||||
|
if (payload.headRefOid === initialHeadSha) return { completed: false }
|
||||||
|
|
||||||
|
const ciState = summariseCiRollup(payload.statusCheckRollup)
|
||||||
|
if (ciState.state === 'pending') return { completed: false }
|
||||||
|
if (ciState.state === 'failure') {
|
||||||
|
return {
|
||||||
|
completed: true,
|
||||||
|
summary: `Autofix pushed commits to ${owner}/${repo}#${prNumber} but CI is failing (${ciState.detail}).`,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
completed: true,
|
||||||
|
summary: `Autofix pushed commits to ${owner}/${repo}#${prNumber}, CI green.`,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
interface CiSummary {
|
||||||
|
state: 'success' | 'pending' | 'failure'
|
||||||
|
detail: string
|
||||||
|
}
|
||||||
|
|
||||||
|
function summariseCiRollup(
|
||||||
|
rollup: PrViewPayload['statusCheckRollup'],
|
||||||
|
): CiSummary {
|
||||||
|
if (!rollup || rollup.length === 0) {
|
||||||
|
// No checks configured on this repo — treat as success so completion
|
||||||
|
// can fire on push alone. PRs without CI are perfectly valid.
|
||||||
|
return { state: 'success', detail: 'no checks configured' }
|
||||||
|
}
|
||||||
|
let pending = 0
|
||||||
|
let failed = 0
|
||||||
|
const total = rollup.length
|
||||||
|
for (const check of rollup) {
|
||||||
|
const status = (check.status ?? '').toUpperCase()
|
||||||
|
const conclusion = (check.conclusion ?? '').toUpperCase()
|
||||||
|
if (status && status !== 'COMPLETED') {
|
||||||
|
pending++
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if (
|
||||||
|
conclusion === 'SUCCESS' ||
|
||||||
|
conclusion === 'NEUTRAL' ||
|
||||||
|
conclusion === 'SKIPPED'
|
||||||
|
) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if (conclusion === '') {
|
||||||
|
pending++
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
failed++
|
||||||
|
}
|
||||||
|
if (pending > 0)
|
||||||
|
return { state: 'pending', detail: `${pending}/${total} checks pending` }
|
||||||
|
if (failed > 0)
|
||||||
|
return { state: 'failure', detail: `${failed}/${total} checks failing` }
|
||||||
|
return { state: 'success', detail: `${total}/${total} checks passing` }
|
||||||
|
}
|
||||||
|
|
@ -91,6 +91,14 @@ export type AutofixPrRemoteTaskMetadata = {
|
||||||
owner: string;
|
owner: string;
|
||||||
repo: string;
|
repo: string;
|
||||||
prNumber: number;
|
prNumber: number;
|
||||||
|
/**
|
||||||
|
* PR head commit SHA captured at /autofix-pr launch. The completionChecker
|
||||||
|
* compares this against the live head to detect when the agent has pushed
|
||||||
|
* new commits. Optional because gh CLI may be unavailable at launch — in
|
||||||
|
* that case the checker falls back to terminal-state-only completion.
|
||||||
|
* Survives --resume via the session sidecar.
|
||||||
|
*/
|
||||||
|
initialHeadSha?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
export type RemoteTaskMetadata = AutofixPrRemoteTaskMetadata;
|
export type RemoteTaskMetadata = AutofixPrRemoteTaskMetadata;
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user