feat(autofix-pr): 完整完成回流机制 (latent bug fix + completionChecker + 内容回流)#1240
feat(autofix-pr): 完整完成回流机制 (latent bug fix + completionChecker + 内容回流)#1240amDosion wants to merge 6 commits into
Conversation
问题:createAutofixTeammate 生成 teammate UUID 作为 monitor lock 的 key, 但 registerRemoteAgentTask 内部生成的 framework taskId 是另一个 UUID。 CCR session 自然完成时框架调 clearActiveMonitor(frameworkTaskId) guard 失败,lock 永不释放,导致后续 /autofix-pr 报 "already monitoring"。 修复(Phase 1 of remote-agent completion loop): - monitorState 新增 updateActiveMonitor(partial) 原子更新 - callAutofixPr 在 register 后 swap lock 的 taskId 到 framework 分配的 id - RemoteAgentTask 引入 registerCompletionHook 注册式 API(参考已有的 registerCompletionChecker 模式),在 5 个完成路径调 runCompletionHook - autofix-pr 命令模块自己注册 cleanup hook,避免 framework 反向依赖 command 模块 测试: - monitorState 新增 4 个测试(updateActiveMonitor 行为 + bug 复现/修复) - launchAutofixPr 新增 3 个端到端回归测试(taskId swap + hook 触发 + subsequent launch 不报 already monitoring) 完整分析与 Phase 2/3 改造方案见 docs/features/remote-agent-completion-analysis.md。
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。
Phase 3 of remote-agent completion loop。Phase 2 注册了 completionChecker
让框架能在 PR 合并/关闭/有 push+CI 绿时主动完成 task,但 task-notification
仍然只携带 generic 文本(""${owner}/${repo}claude-code-best#42 merged"")。Phase 3 让本地
模型读到远端 agent 自己产出的结构化结果(commits 列表、files 列表、CI
状态、人类可读 summary)。
实现:
- 新增 extractAutofixResultFromLog (src/commands/autofix-pr/
extractAutofixResult.ts):从 SDKMessage[] 中扫 <autofix-result> tag,
优先 hook stdout 后 fallback assistant text,latest-wins。10 个单测。
- RemoteAgentTask 新增 registerContentExtractor 注册式 API + 私有
enqueueRichRemoteNotification(参考 enqueueRemoteReviewNotification),
在 3 个 generic 完成路径(archived / completionChecker / result-driven)
先尝试 tryExtractRichContent,有内容用 rich 变体,没有走 generic。
isRemoteReview 路径不变(它走自己的 enqueueRemoteReviewNotification)。
- launchAutofixPr.ts 模块顶部 registerContentExtractor('autofix-pr',
extractAutofixResultFromLog)。initialMessage 加 <autofix-result> 输出
指令(pr-number / commits-pushed / files-changed / ci-status / summary)。
设计:
- 注册式 API(同 Phase 1 hook + Phase 2 checker):framework 不反向依赖
命令模块,所有 PR-specific 逻辑在 autofix-pr/
- latest-wins:agent 重试时只取最新 tag,旧 tag 不会污染
- truncated tag → null:开 tag 无对应闭 tag 视为不完整,走 generic
fallback
- 跨 message 不拼接:开 tag 和闭 tag 在不同 message 视为不完整(避免
误拼字符串)
- 字符串 content 不解析:assistant.message.content 为 string(非 block
array)的少见路径直接 skip,不 crash
测试:
- extractAutofixResultFromLog 10 个单测(空 log / 无 tag / hook stdout /
assistant text / hook_response subtype / 多 tag latest-wins / 截断 /
hook 后于 assistant 的优先级 / 跨 message 不拼接 / 字符串 content
graceful)
- launchAutofixPr 3 个新测试(extractor 注册 / initialMessage 含 tag
schema / extractor 真实行为)
完整方案见 docs/features/remote-agent-completion-analysis.md 第 5.3 节。
如果远端 agent 重试时写了完整 <autofix-result> 后又开了一个被截断的 第二个 tag, 旧实现只看 lastIndexOf(open) 然后找不到 close 就返回 null, 导致前面那个完整结果被丢弃。改为从尾向首遍历所有 open tag, 返回第一个 能配对的 open/close 对。 附带: - docs/features/remote-agent-completion-analysis.md: 9 处裸 fenced block 补 language tag (text/http), 修复 markdownlint MD040 警告 - 同文件: 两处"三选项" → "三个选项" 符合中文量词习惯
针对 codecov patch coverage gap, 补足三块此前未走到的代码路径: prOutcomeCheck.ts (原 96.92%, 2 lines missing): - statusCheckRollup === undefined 路径 (与空数组分支不同, GitHub 在无 checks 配置的 PR 上直接省略字段) - COMPLETED 状态但 conclusion 为 null/空 的 in-flight 检查归为 pending launchAutofixPr.ts (原 58.33%, 15 lines missing): - registerCompletionChecker arrow body: metadata 缺失早返回 / 节流窗口内 返回 null / completed=false 返回 null / completed=true 返回 summary / initialHeadSha 透传到 checkPrAutofixOutcome - registerCompletionHook 的 if(meta) 短路两侧: 有 metadata 时清空节流条目, 无 metadata 时仍释放 active monitor lock 所有新测试沿用现有 mock.module 与 registerXxxMock.mock.calls 拉取注册 回调的模式, 无新增依赖。prOutcomeCheck 11/11 本地通过。
📝 WalkthroughWalkthroughThe PR extends autofix-pr remote task monitoring by adding completion detection, structured result extraction, and completion hook infrastructure. It introduces pure decision logic for PR state/CI analysis, gh CLI integration for live PR fetching, XML tag extraction from logs, monitor taskId swapping, and framework-level registration APIs for custom completion handlers and content extractors. ChangesAutofix Completion Monitoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/tasks/RemoteAgentTask/RemoteAgentTask.tsx (2)
1052-1055:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
runCompletionHookcall in timeout fallback path.When the review times out during an API error (lines 1047-1055), the task transitions to
failedand the notification is enqueued, butrunCompletionHookis not called. This is inconsistent with the other remote-review failure path at line 1008, and could leave command-module state (e.g., singleton locks) dangling.Proposed fix
enqueueRemoteReviewFailureNotification(taskId, 'remote session exceeded 30 minutes', context.setAppState); void evictTaskOutput(taskId); void removeRemoteAgentMetadata(taskId); + // Fetch task for hook — it was retrieved above but may have been + // updated; re-fetch is safer for the metadata reference. + const taskForHook = context.getAppState().tasks?.[taskId] as RemoteAgentTaskState | undefined; + if (taskForHook) runCompletionHook(taskId, taskForHook); return; // Stop polling🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tasks/RemoteAgentTask/RemoteAgentTask.tsx` around lines 1052 - 1055, The timeout fallback path currently enqueues a failure notification and cleans up outputs/metadata (enqueueRemoteReviewFailureNotification, evictTaskOutput, removeRemoteAgentMetadata) but fails to call runCompletionHook for the task, leaving command-module state (e.g., locks) dangling; update that branch to invoke runCompletionHook(taskId, taskResult) (using the same signature/arguments used at the other failure path around line 1008) before returning, ensuring any completion logic and cleanup run; keep the notification and eviction calls in place and call runCompletionHook prior to the final return.
1087-1126:⚠️ Potential issue | 🟠 MajorCall
runCompletionHookon user-initiated kill.The
killmethod transitions the task to the'killed'terminal state but does not callrunCompletionHook. Since the hook is documented for "releasing command-module state (e.g. singleton locks)" after terminal transitions, and the autofix-pr hook'sclearActiveMonitoris essential for releasing locks, a killed task will leave those resources dangling. Either callrunCompletionHook(taskId, task)before theif (killed)block, or clarify in theregisterCompletionHookJSDoc that hooks are only invoked on natural completion, not user-initiated termination.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tasks/RemoteAgentTask/RemoteAgentTask.tsx` around lines 1087 - 1126, The kill method sets a task to the 'killed' terminal state but never invokes runCompletionHook, which leaves command-module state (e.g. locks) dangling; modify RemoteAgentTask.kill so that immediately after updateTaskState sets killed=true (and before emitting terminations/archiving) you call runCompletionHook(taskId, theTask) with the task object (use the same captured fields: toolUseId, description, sessionId or obtain the updated task from updateTaskState) so registered completion hooks (like clearActiveMonitor) always run on user-initiated kills; keep the existing emitTaskTerminatedSdk, archiveRemoteSession and evict/remove calls unchanged and ensure any errors from runCompletionHook are caught/logged to avoid breaking the kill flow.
🧹 Nitpick comments (1)
src/commands/autofix-pr/launchAutofixPr.ts (1)
55-70: 💤 Low valueConsider wrapping
checkPrAutofixOutcomein try-catch.If
checkPrAutofixOutcomethrows (e.g., gh CLI spawn failure, JSON parse error), the exception propagates to the framework's poll loop. Depending on how the framework handles rejected promises from completion checkers, this could cause repeated error logs or stop polling entirely. A defensive try-catch returningnullon failure would make the checker more resilient.💡 Optional defensive handling
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 + try { + const result = await checkPrAutofixOutcome({ + owner: meta.owner, + repo: meta.repo, + prNumber: meta.prNumber, + initialHeadSha: meta.initialHeadSha, + }) + return result.completed ? result.summary : null + } catch { + return null + } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/autofix-pr/launchAutofixPr.ts` around lines 55 - 70, The completion checker registered via registerCompletionChecker('autofix-pr') currently calls checkPrAutofixOutcome directly; wrap the await checkPrAutofixOutcome(...) call in a try-catch so any thrown errors (e.g., gh CLI spawn or JSON parse) are caught, optionally log the error using the same logger/context and return null on failure to avoid propagating exceptions into the poll loop; keep the throttleKey/lastCheckAt/CHECK_INTERVAL_MS logic unchanged and only catch around the call that produces result so you still return result.summary when completed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/autofix-pr/extractAutofixResult.ts`:
- Around line 63-68: The current loop over content in extractAutofixResult.ts
scans messages newest→oldest but iterates blocks oldest→newest, allowing an
earlier tag in the same assistant message to win; change the inner iteration
over the blocks (the array named content and loop variable block) to traverse in
reverse (newest block first) before calling extractBetween with TAG_OPEN and
TAG_CLOSE so that the latest tag in the same message wins; keep the same
early-continue guards (block.type !== 'text' and typeof block.text !== 'string')
and return the first extracted value found.
- Around line 61-63: In extractAutofixResult, the loop currently skips when (msg
as SDKAssistantMessage).message?.content is a string, which drops valid
<autofix-result> payloads; update the logic in the function that iterates
messages (look for the variables content and block iteration) to also handle
string content by treating the string as a single block (or parsing it into
blocks) before the for..of loop — e.g., if typeof content === 'string' convert
it to an array like [{ type: 'text', text: content }] or run a small parser to
extract tag blocks so the subsequent processing of blocks (including detection
of <autofix-result>) works for both string and array forms.
In `@src/commands/autofix-pr/prFetch.ts`:
- Around line 145-147: The parsed JSON from running `gh pr view` is currently
cast to `PrViewPayload` without validation in the `prFetch.ts` promise (the
`parsed` variable); add runtime validation to ensure required fields (at minimum
`state`, `headRefOid`, and any fields used by
`summariseAutofixOutcome`/prOutcomeCheck logic) are present and of the expected
types before resolving; if validation fails, reject/throw a descriptive error
(including the raw `stdout`) so callers won’t receive malformed
objects—implement this in the same code path where `JSON.parse(stdout)` is used
and return/resolve only a validated `PrViewPayload`-shaped object.
---
Outside diff comments:
In `@src/tasks/RemoteAgentTask/RemoteAgentTask.tsx`:
- Around line 1052-1055: The timeout fallback path currently enqueues a failure
notification and cleans up outputs/metadata
(enqueueRemoteReviewFailureNotification, evictTaskOutput,
removeRemoteAgentMetadata) but fails to call runCompletionHook for the task,
leaving command-module state (e.g., locks) dangling; update that branch to
invoke runCompletionHook(taskId, taskResult) (using the same signature/arguments
used at the other failure path around line 1008) before returning, ensuring any
completion logic and cleanup run; keep the notification and eviction calls in
place and call runCompletionHook prior to the final return.
- Around line 1087-1126: The kill method sets a task to the 'killed' terminal
state but never invokes runCompletionHook, which leaves command-module state
(e.g. locks) dangling; modify RemoteAgentTask.kill so that immediately after
updateTaskState sets killed=true (and before emitting terminations/archiving)
you call runCompletionHook(taskId, theTask) with the task object (use the same
captured fields: toolUseId, description, sessionId or obtain the updated task
from updateTaskState) so registered completion hooks (like clearActiveMonitor)
always run on user-initiated kills; keep the existing emitTaskTerminatedSdk,
archiveRemoteSession and evict/remove calls unchanged and ensure any errors from
runCompletionHook are caught/logged to avoid breaking the kill flow.
---
Nitpick comments:
In `@src/commands/autofix-pr/launchAutofixPr.ts`:
- Around line 55-70: The completion checker registered via
registerCompletionChecker('autofix-pr') currently calls checkPrAutofixOutcome
directly; wrap the await checkPrAutofixOutcome(...) call in a try-catch so any
thrown errors (e.g., gh CLI spawn or JSON parse) are caught, optionally log the
error using the same logger/context and return null on failure to avoid
propagating exceptions into the poll loop; keep the
throttleKey/lastCheckAt/CHECK_INTERVAL_MS logic unchanged and only catch around
the call that produces result so you still return result.summary when completed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 08ead1b8-5e51-45ae-910e-19890648a3fe
📒 Files selected for processing (10)
src/commands/autofix-pr/__tests__/extractAutofixResult.test.tssrc/commands/autofix-pr/__tests__/launchAutofixPr.test.tssrc/commands/autofix-pr/__tests__/monitorState.test.tssrc/commands/autofix-pr/__tests__/prOutcomeCheck.test.tssrc/commands/autofix-pr/extractAutofixResult.tssrc/commands/autofix-pr/launchAutofixPr.tssrc/commands/autofix-pr/monitorState.tssrc/commands/autofix-pr/prFetch.tssrc/commands/autofix-pr/prOutcomeCheck.tssrc/tasks/RemoteAgentTask/RemoteAgentTask.tsx
| const content = (msg as SDKAssistantMessage).message?.content | ||
| if (!content || typeof content === 'string') continue | ||
| for (const block of content as Array<{ type: string; text?: string }>) { |
There was a problem hiding this comment.
Don’t drop valid tags when assistant content is raw string.
Current logic explicitly skips string message.content, so valid <autofix-result> payloads in that format are never surfaced.
🔧 Proposed fix
- const content = (msg as SDKAssistantMessage).message?.content
- if (!content || typeof content === 'string') continue
- for (const block of content as Array<{ type: string; text?: string }>) {
+ const content = (msg as SDKAssistantMessage).message?.content
+ if (!content) continue
+ if (typeof content === 'string') {
+ const extracted = extractBetween(content, TAG_OPEN, TAG_CLOSE)
+ if (extracted) return extracted
+ continue
+ }
+ for (const block of content as Array<{ type: string; text?: string }>) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/autofix-pr/extractAutofixResult.ts` around lines 61 - 63, In
extractAutofixResult, the loop currently skips when (msg as
SDKAssistantMessage).message?.content is a string, which drops valid
<autofix-result> payloads; update the logic in the function that iterates
messages (look for the variables content and block iteration) to also handle
string content by treating the string as a single block (or parsing it into
blocks) before the for..of loop — e.g., if typeof content === 'string' convert
it to an array like [{ type: 'text', text: content }] or run a small parser to
extract tag blocks so the subsequent processing of blocks (including detection
of <autofix-result>) works for both string and array forms.
| for (const block of content as Array<{ type: string; text?: string }>) { | ||
| if (block.type !== 'text' || typeof block.text !== 'string') continue | ||
| if (!block.text.includes(TAG_OPEN)) continue | ||
| const extracted = extractBetween(block.text, TAG_OPEN, TAG_CLOSE) | ||
| if (extracted) return extracted | ||
| } |
There was a problem hiding this comment.
Preserve “latest-wins” within a single assistant message.
You scan messages newest→oldest, but blocks inside one message are scanned oldest→newest; that can return a stale earlier tag from the same message.
🔧 Proposed fix
- for (const block of content as Array<{ type: string; text?: string }>) {
+ for (
+ let j = (content as Array<{ type: string; text?: string }>).length - 1;
+ j >= 0;
+ j--
+ ) {
+ const block = (content as Array<{ type: string; text?: string }>)[j]
if (block.type !== 'text' || typeof block.text !== 'string') continue
if (!block.text.includes(TAG_OPEN)) continue
const extracted = extractBetween(block.text, TAG_OPEN, TAG_CLOSE)
if (extracted) return extracted
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for (const block of content as Array<{ type: string; text?: string }>) { | |
| if (block.type !== 'text' || typeof block.text !== 'string') continue | |
| if (!block.text.includes(TAG_OPEN)) continue | |
| const extracted = extractBetween(block.text, TAG_OPEN, TAG_CLOSE) | |
| if (extracted) return extracted | |
| } | |
| for ( | |
| let j = (content as Array<{ type: string; text?: string }>).length - 1; | |
| j >= 0; | |
| j-- | |
| ) { | |
| const block = (content as Array<{ type: string; text?: string }>)[j] | |
| if (block.type !== 'text' || typeof block.text !== 'string') continue | |
| if (!block.text.includes(TAG_OPEN)) continue | |
| const extracted = extractBetween(block.text, TAG_OPEN, TAG_CLOSE) | |
| if (extracted) return extracted | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/autofix-pr/extractAutofixResult.ts` around lines 63 - 68, The
current loop over content in extractAutofixResult.ts scans messages
newest→oldest but iterates blocks oldest→newest, allowing an earlier tag in the
same assistant message to win; change the inner iteration over the blocks (the
array named content and loop variable block) to traverse in reverse (newest
block first) before calling extractBetween with TAG_OPEN and TAG_CLOSE so that
the latest tag in the same message wins; keep the same early-continue guards
(block.type !== 'text' and typeof block.text !== 'string') and return the first
extracted value found.
| try { | ||
| const parsed = JSON.parse(stdout) as PrViewPayload | ||
| resolve(parsed) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the documented JSON schema and field nullability for gh pr view --json headRefOid,state,statusCheckRollup?
💡 Result:
The GitHub CLI (gh) does not provide a formal, standalone JSON schema document for the output of the gh pr view --json command [1][2]. Instead, the fields available for the --json flag are derived directly from the GitHub GraphQL API's PullRequest object [2]. Because the output is a direct mapping from the GraphQL API, there is no single "documented JSON schema" file provided by the GitHub CLI project [2]. To understand the structure, types, and nullability of the fields headRefOid, state, and statusCheckRollup, you must refer to the official GitHub GraphQL API documentation for the PullRequest object [2]. Regarding the specific fields requested: 1. headRefOid: This field represents the Git object ID (SHA) of the head branch. In the GraphQL schema, it is a GitObjectID (a custom scalar type, typically a string) and is non-nullable [2]. 2. state: This field represents the state of the pull request (e.g., OPEN, CLOSED, MERGED). It is an enum type (PullRequestState) and is non-nullable [2]. 3. statusCheckRollup: This field provides a summary of the status checks for the pull request. It is a StatusCheckRollup object and is nullable, as a pull request may not have any status checks associated with it [2]. For any field, if you are unsure of its structure or nullability, you can inspect the raw output by running the command with the desired fields and piping it to a tool like jq, or by consulting the GitHub GraphQL API reference for the PullRequest object [2]. Note that some fields in the GitHub CLI may have pagination limits or specific behaviors that differ from the raw GraphQL API [3].
Citations:
- 1: https://cli.github.com/manual/gh_pr_view
- 2: Where is documentation for json fields? cli/cli#5902
- 3: The
pr viewcommand with the--json filesflag omits some of the files changed cli/cli#9916
🏁 Script executed:
cat -n src/commands/autofix-pr/prFetch.ts | head -200Repository: claude-code-best/claude-code
Length of output: 5426
🏁 Script executed:
cat -n src/commands/autofix-pr/prOutcomeCheck.ts | head -150Repository: claude-code-best/claude-code
Length of output: 4865
Add runtime validation for gh pr view JSON before using fields in decision logic.
JSON.parse(stdout) is cast directly to PrViewPayload without runtime checks. If gh pr view returns missing or malformed fields (e.g., null state or missing headRefOid), the undefined values will flow into summariseAutofixOutcome, causing incorrect completion detection or comparison failures (lines 52, 58, 66 of prOutcomeCheck.ts).
🔧 Proposed fix
- try {
- const parsed = JSON.parse(stdout) as PrViewPayload
- resolve(parsed)
+ try {
+ const raw = JSON.parse(stdout) as Record<string, unknown>
+ const headRefOid = raw.headRefOid
+ const state = raw.state
+ const statusCheckRollup = raw.statusCheckRollup
+ if (
+ typeof headRefOid !== 'string' ||
+ (state !== 'OPEN' && state !== 'CLOSED' && state !== 'MERGED') ||
+ (statusCheckRollup !== undefined && !Array.isArray(statusCheckRollup))
+ ) {
+ reject(new Error('gh pr view JSON shape invalid'))
+ return
+ }
+ resolve({
+ headRefOid,
+ state,
+ statusCheckRollup: statusCheckRollup as PrViewPayload['statusCheckRollup'],
+ })
} catch (e) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/autofix-pr/prFetch.ts` around lines 145 - 147, The parsed JSON
from running `gh pr view` is currently cast to `PrViewPayload` without
validation in the `prFetch.ts` promise (the `parsed` variable); add runtime
validation to ensure required fields (at minimum `state`, `headRefOid`, and any
fields used by `summariseAutofixOutcome`/prOutcomeCheck logic) are present and
of the expected types before resolving; if validation fails, reject/throw a
descriptive error (including the raw `stdout`) so callers won’t receive
malformed objects—implement this in the same code path where
`JSON.parse(stdout)` is used and return/resolve only a validated
`PrViewPayload`-shaped object.
2 participants
Summary
为 `/autofix-pr` 命令实现完整的完成回流闭环。
总和: 6 commits, ~1339 行净改动。本地全套测试 107/107 pass。
Why
之前 `/autofix-pr` 启动后两个问题:
Architecture
framework 新增 3 个注册式 API (同既有 `registerCompletionChecker` 风格), framework 不反向依赖命令模块:
```
registerCompletionChecker // 主动判定何时完成
registerContentExtractor // 从 log 抽 rich content 注入 notification
registerCompletionHook // 完成后命令模块自己 cleanup
```
autofix-pr 完成闭环:
```
PR 有新 commit + CI 绿
→ completionChecker 检测 (5s throttle 限速 gh CLI)
→ contentExtractor 抽 tag
→ enqueueRichRemoteNotification 注入含结构化结果的 task-notification
→ completionHook 释放 monitor lock + 清 throttle entry
→ 本地模型下一回合汇报 "修了 X, push 了 Y, CI 绿了"
```
Self-validation
本 PR 在 fork 里(amDosion#13) 用改造后的 `/autofix-pr` 命令跑自己的 PR, agent 真实检测到 `extractBetween` 的边界 case bug(latest open tag 截断时丢弃前面完整结果),并自动 push 修复 + 补测试 + lint 整形 3 个 commits。CI 全绿、mergeable: YES。
Test plan
Notes
Summary by CodeRabbit
Release Notes