fix(autocoder): resolve merge conflicts before escalating, not on first failure - #135
Open
laird wants to merge 1 commit into
Open
fix(autocoder): resolve merge conflicts before escalating, not on first failure#135laird wants to merge 1 commit into
laird wants to merge 1 commit into
Conversation
…st failure merge-to-integration.sh used to abort+escalate (remove 'working', add needs-clarification, post a hard-escalation comment) on the FIRST merge conflict, before the /fix worker that just wrote the fix — sharing this same checkout — ever got a chance to resolve it directly. Conflicts are now left ON DISK (not aborted) and the script exits 3, a code distinct from 1 (push failure) and 2 (test failure). No label change happens at that point; only an advisory comment is posted. The caller (fix.md, at all 3 merge sites) resolves the conflict directly using the context it has from having just written the change, re-runs the merge, and only escalates to needs-clarification — with a proper explicit-release comment — if that attempt also fails. Reported as ey-org/athena2#1766, observed twice in one session where a worker resolved a conflict the script had already escalated. Bumps the autocoder plugin to 4.21.0 across all manifest copies and adds tests/test_merge_conflict_advisory.sh pinning the new behavior.
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
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
merge-to-integration.shaborted the merge and escalated (removedworking,added
needs-clarification, posted a hard-escalation comment) on the firstmerge conflict, before the
/fixworker that had just written the fix — andwas sharing the exact same checkout — ever got a chance to look at it.
Reported as
ey-org/athena2#1766, with two concrete instances 15 minutes apartin one session where the worker read the failure and resolved the conflict by
hand anyway (mechanical conflicts it had full context for), after the label
had already flipped to
needs-clarification.Why it matters:
needs-clarificationis in the blocking set, so between thepremature escalation and the worker's own recovery the issue is invisible to
the claimable queue. If the worker's loop ends before it finishes (context
exhaustion, restart, gate failure), the issue is stranded wearing a
human-blocked label while a complete fix sits unmerged on its branch — and the
label is not self-clearing.
Fix
merge-to-integration.sh: on conflict, leave it ON DISK (nogit merge --abort), don't touch labels, post an advisory-only comment, andexit 3 — a code distinct from 1 (push failure) and 2 (test failure).
merge-poll.sh: documents the new exit code 3 (already passed throughgenerically, no behavior change needed there).
commands/fix.md: at all 3 merge sites, exit 3 now triggers a directself-resolve attempt (the worker resolves the conflict using the context it
has, completes the merge commit, and re-launches). Only if that second
attempt also fails does it escalate to
needs-clarification— via thesame explicit-release-comment-then-label convention already used elsewhere
in the file (fixes a
tests/test_fix_working_lock.pyinvariant that aworking-lock release must have an adjacent terminal-outcome marker).(
tests/test_manifest_versions.shenforces they all agree).tests/test_merge_conflict_advisory.sh: a real git-fixture test thatdrives an actual merge conflict through
merge-to-integration.shandasserts exit 3, the conflict left unresolved on disk, no label-changing
issue_updatecall, and an advisory (not hard-escalation) comment body.Test plan
tests/run-shell-suite.sh— 27/27 passed (includes the new test and theexisting
test_merge_launch_poll.sh, which still passes with the extralaunch/poll call sites at each merge site)
pytest tests/— 116/116 passed (includingtest_fix_working_lock.py, which specifically checks every--remove-label "working"site has an adjacent terminal marker, andtest_gate_md_bash.py, which checks fix.md's bash blocks parse)tests/test_manifest_versions.sh— all manifests agree on 4.21.0