Repository navigation
Fix agent: conflict resolution should reconcile behavioural overlap with incoming main changes, not defer it as out of scope #1666
Copy link
Copy link
Open
Description
What happened
On PR #6980, a fix for #6977, a human asked /fs-fix rebase and resolve conflicts on 2026-09-08. While the PR waited, main had gained shadowedProfiles from #6975 for #6971. It warns when a profiles/ directory copy shadows a harness-resolved profile. The PR's filterProfilesByDirIDs skips importing exactly those profiles.
- Rebase run run 34201845382 (fix agent from ec2f8a5, opus, 53 turns, $1.46):
- The textual conflict was two adjacent function definitions. The caller had auto-merged and now ran both the filter and the warning.
- The transcript shows the agent noticed the problem: the warning would still be computed on the unfiltered list, so it would fire for profiles the filter had already skipped, with different wording.
- It then decided that "the task is just to rebase and resolve conflicts rather than fix semantics", kept both, and described them as "complementary".
- It did not inspect the incoming commit (089e0ef1). It did not grep
docs/for the warning text, whichdocs/guides/user/customizing-agents.mdandrunning-agents-locally.mddescribe. It ran only targeted unit tests for each function separately.
- Five review→fix cycles followed, unwinding the interaction one piece at a time:
- Review 1 found the call on the unfiltered list and stale docs (run 34203017200).
- Review 2 found the call now dead after fix 1 (run 34205804692).
- Review 3 found the same dead call plus more stale docs (run 34208563254).
- Review 4 found the function and
generatedProfileIDsorphaned (run 34211327213). - Review 5 found doc-comment wording (run 34213808088).
- The cycles took 08:11–10:36, about $23 in review runs plus about $5 in fix runs, and the coder added
needs-human.
- Final state: the right end result was simply "the incoming warning is redundant once the filter exists: remove it, its helper map and its tests, and update the two doc pages". That was all knowable at rebase time from the information the agent already had.
- Why the agent stopped short: the fix agent's scope rules at ec2f8a5 push against this reconciliation:
agents/fix.md: "scope is strictly limited to addressing the review feedback", and every diff line must be traceable to a finding or human instruction.skills/fix-review/SKILL.mdstep 6: no unmentioned refactors, bug fixes or doc changes.- Neither file had conflict-resolution guidance.
- The
fix-history-rewriteskill and the "Reconcile forge-reported merge conflicts" section added since (2a18bf7, 2026-09-29) are mechanical only ("resolve them, git add, rebase --continue").
What could go better
- Confidence: high that better conflict-resolution guidance would have removed most of the 4–5 follow-up rounds on this PR. The agent explicitly saw the contradiction and cited scope as its reason to leave it, so this is an instruction gap rather than a capability gap.
- What a resolution must deliver: when a human asks to resolve conflicts, the implied deliverable is a coherent merged result. Two features that now overlap behaviourally (duplicate or contradictory warnings, a check made unreachable by a filter, docs describing the superseded behaviour) are part of the conflict, even when git only marks a few lines.
- How this differs from agents#486 (open): Fix agent should verify import and convention consistency with target branch after merge conflict resolution #486 asks for a post-merge check of imports and conventions against the target branch. This case is about behavioural overlap between the PR's change and an incoming feature. It is also about the scope rules actively stopping the agent from fixing an interaction it had already identified. A consistency-check step alone might not override "every line must trace to a finding". The scope carve-out is the key addition. Implementers may reasonably fold both into one change.
- Uncertainty: some semantic reconciliations are genuine design decisions. For example, keep the warning, or make the directory copy win? Here the PR's issue (#6977) had already decided that the directory copy wins, so the reconciliation was mechanical in hindsight. The guidance should let the agent escalate instead of improvising when the PR's intent and the incoming change truly disagree. The 09-24 sonnet run did exactly that and aborted on a real architectural collision, so the skill should keep that path.
- Contributing factor: the first 09-08 fix run, cancelled by concurrency when the human re-posted the command, had started reading the incoming commit's diff. The run that replaced it skipped that step. Making the step explicit would remove that luck dependence.
Proposed change
In fullsend-ai/agents:
agents/fix.md, "Reconcile forge-reported merge conflicts" section, andskills/fix-history-rewrite/references/procedure.md: add a "semantic reconciliation" step after textual conflict resolution and before committing.- For each conflicted file, identify the incoming main commits that touched the conflicting hunks (
git log --oneline <merge-base>..origin/main -- <file>, thengit showthe relevant ones). - For each symbol on either side of a conflict hunk, and for each auto-merged caller that now invokes both sides, ask: does the PR's change make the incoming behaviour redundant, unreachable or contradictory, or the reverse?
- Grep for user-visible strings (warning/log messages, flag names) from both sides across
docs/and tests. Update or remove any that now describe superseded behaviour. - After resolving, check whether any symbol from either side has lost all its production callers. If so, remove it and its tests in the same pass.
- For each conflicted file, identify the incoming main commits that touched the conflicting hunks (
- Scope carve-out: in
agents/fix.md(scope section) andskills/fix-review/SKILL.mdstep 6, state explicitly that when the human instruction or the forge state requires conflict resolution, changes needed to make the merged result coherent are in scope. That covers redundant or contradictory code from the incoming side, docs describing the incoming behaviour, and newly orphaned code. Each such change must be recorded as a decision point naming the incoming commit/PR. - Escalation path: if reconciliation would require choosing between the PR's stated intent and the incoming feature's intent, and the PR's linked issue doesn't already settle it, do not improvise. Record the tension in
agent-result.jsonand stop for human input, as the 09-24 run on #6980 did. - Regression fixture: add a skill test or eval fixture modelled on PR #6980. The feature branch adds a filter. Main adds a warning over the same set, plus docs. The expected resolution removes the redundant warning and updates the docs in one commit.
Validation criteria
- Fixture: an eval or skill test modelled on PR #6980 (filter on the branch, overlapping warning plus docs on main) produces a single resolution commit that leaves no unreachable warning call, no orphaned helper and no stale doc paragraph.
- Production sample: over the next 10 human-triggered
/fs-fixrebase/conflict runs across fullsend-ai repos, the follow-up review should raise no findings whose root cause is an unreconciled interaction with the incoming main change (stale-reference, newly dead code, stale docs about the incoming feature). At least 8 of 10 rebase runs should be followed by at most 1 review→fix cycle, versus 5 on #6980. - Decision points: in those runs,
agent-result.jsondecision points name the incoming commit/PR whenever semantic reconciliation changed code beyond the conflict markers. - Escalation: genuine design conflicts still escalate to a human (needs-human or an abort) rather than being silently resolved.
Generated by retro agent from fullsend-ai/fullsend#6980
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels
Type
Projects
- StatusShow more project fieldsTodo
Activity