fix: stage agent-resolved conflicts before checking unmerged files - #168
Open
cwbcheng wants to merge 6 commits into
Open
fix: stage agent-resolved conflicts before checking unmerged files#168cwbcheng wants to merge 6 commits into
cwbcheng wants to merge 6 commits into
Conversation
Rejected review-thread feedback previously fell through the GitHub follow-up path silently: needsGitHubFollowUp only considered accepted items, so a rejected review comment was never replied to and its conversation was never resolved, contradicting the agent prompt that says to reply with the rejection reason and resolve the conversation. Now rejected review threads (excluding PatchDeck's own status/audit-trail replies) are collected as follow-up tasks, get a 'Rejected - no code change made' reply with the reason, and the conversation is resolved. Audit-trail verification skips the code-change audit for these items and only requires thread resolution, and the final status update keeps the rejection decision while advancing the thread-resolved flag.
Localize the comments PatchDeck posts to GitHub: - Agent prompt now requires Simplified Chinese for every comment, thread reply, and summary. - Follow-up replies use Chinese headlines (已拒绝/已在提交/已在最新更新) with 原因 for rejected threads, and 正在回复 for quoted references. - Status messages (accepted/in-progress/needs-attention/verifying/resolved) are localized. - APP_STATUS_COMMENT_PATTERN now recognizes both English and Chinese status markers so recovery of existing status replies keeps working. Tests updated to assert the Chinese wording.
The internal-reply exclusion in needsGitHubFollowUp only matched 'PatchDeck status comment' and 'Automation audit trail follow-up', but classifyNonActionableAppFeedback also produces 'PatchDeck audit trail comment' and 'PatchDeck agent command comment'. A rejected item carrying one of those reasons was treated as real reviewer feedback, which made PatchDeck reply to its own follow-up comment, producing a new audit trail, which was then replied to again - an infinite loop. Widen the exclusion to all PatchDeck-authored marker reasons and add a regression test that a rejected audit-trail comment never triggers a follow-up reply.
The code-owner fallback agent is asked to commit and push its own changes, but agents sometimes finish without doing so (e.g. a truncated model response). Previously PatchDeck marked the fallback run completed and removed the worktree, discarding the agent's edits and leaving the PR branch and review threads untouched. After a successful fallback agent run, PatchDeck now: - commits any uncommitted agent edits in the worktree, - pushes the local head to the PR head branch, - verifies the remote head matches the local head. Adds a regression test that a fallback agent leaving uncommitted edits results in git add/commit/push to the PR branch.
The DeepSeek-backed codex agent frequently times out on long review comments: evaluation had a 3-minute cap and apply had a 15-minute cap, which was not enough for multi-file fixes (e.g. 21 generated fixtures). When apply timed out mid-run, the uncommitted worktree edits were discarded. Raise evaluation to 7 minutes and the default apply timeout to 30 minutes (matching the code-owner fallback timeout) so complex review threads can be evaluated and fixed to completion.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a tracked PR has merge conflicts, the conflict-resolution agent often resolves the files in the worktree without staging them (
git add). PatchDeck then checks for unresolved conflicts withgit diff --name-only --diff-filter=U, which reads the index's unmerged state — not the worktree content. So fully-resolved files still show as unmerged and the run is falsely declared failed withcodex left unresolved merge conflicts: <files>, then falls into the code-owner fallback path. The resolution is never committed or pushed, so GitHub keeps showing "This branch has conflicts that must be resolved" forever.Observed in production on PR cwbcheng/OpenAgent#433: the agent reported "All merge conflicts have been resolved … nothing staged/committed", then PatchDeck logged
PR work error: codex left unresolved merge conflicts: tools/novelstudio/embodied-agent-runtime-control/src/contract-shapes.mjs …and the three conflict files stayed unresolved on GitHub.Fix
After the conflict-resolution agent completes successfully, stage everything in the worktree (
git add -A) before the unmerged-index check. If the agent actually left conflict markers behind, the existing marker check (git grepfor<<<<<<<etc.) still catches them and fails with the more accurate "left merge conflict markers" reason, so genuinely-broken resolutions are still escalated.Tests
babysitPR commits agent-resolved conflicts even when the agent does not stage them. It simulates the agent editing the conflicted file without staging: the unmerged index check only clears after PatchDeck runsgit add. Fails without the fix (run fails with "left unresolved merge conflicts").node --import tsx --test server/babysitter.test.ts(111 tests pass),npm run checkpasses.