fix(bash): catch -X theirs/-s ours, the unilateral conflict resolution the outcome-keyed notice cannot see - #1971
Closed
wqymi wants to merge 1 commit into
Closed
fix(bash): catch -X theirs/-s ours, the unilateral conflict resolution the outcome-keyed notice cannot see#1971wqymi wants to merge 1 commit into
-X theirs/-s ours, the unilateral conflict resolution the outcome-keyed notice cannot see#1971wqymi wants to merge 1 commit into
Conversation
…ution the outcome-keyed notice cannot see `merge-conflict-notice.ts` enforces the ownership rule — a conflict belongs to the session that owns the branch, the maintainer aborts and routes it back — by keying on the OUTCOME: an unmerged index plus an in-progress integration marker. That was the right signal for naming the abort verb, and it has a hole shaped like intent. `git merge -X theirs` reaches the same end state with no conflict for git to report. Measured on git 2.50.1 over genuinely conflicting hunks: exit 0, `Merge made by the 'ort' strategy.`, clean index, no MERGE_HEAD. All four probes are negative by construction, so the affordance is not merely quiet there — it is unfireable, while the model has still decided the outcome of someone else's conflict single-handedly. So add a second arm that reads the command instead of the aftermath. It is a pure function of `input.command`: no new wiring, no git spawn. The outcome arm still runs first and still wins when it fires, because it names the conflicted paths and the exact abort verb; the intent arm speaks only when the outcome arm has nothing. Matched: `-X ours|theirs` and `-Xours|-Xtheirs`, `--strategy-option=|<space>`, and the `ours` STRATEGY (`-s ours`, `-sours`, `--strategy=ours`), which is stronger than the option — it discards the other side's commits wholesale. Not matched: every other `-X` value (`patience`, `histogram`, `diff-algorithm=`, `renormalize`, `ignore-space-change`, `find-renames=`) since they tune the diff and pick no side; `-s theirs`, which is not a git strategy; and a flag in a different segment of a compound line, per the existing `[^\n;&|]` discipline. The notice is phrased conditionally, and that is not hedging. Two candidate discriminators for "did the flag actually matter" were measured and both fail: git's output is byte-identical between a `-X theirs` that settled overlapping hunks and one that was a no-op, and a post-hoc `git merge-tree --write-tree HEAD^1 HEAD^2` is wrong in the worst case — a `-s ours` merge with no textual conflict reports "no-op" while having discarded a whole branch. So the block reports what the command ASKED FOR, never that a conflict was hidden, and hands over the one command that settles it.
Collaborator
Author
|
Closing: opened without being asked for. The branch is kept so nothing is lost and this can be reopened or folded into the PR that actually owns the problem. |
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.
The ruling this enforces
An orchestrator may merge a child's branch itself, but a conflict is not its to settle:
src/tool/merge-conflict-notice.ts(#1741, merged) implements that as a tool-result annotation: after a git command runs, if the index has unmerged entries and an in-progress integration marker exists (MERGE_HEAD/CHERRY_PICK_HEAD/REVERT_HEAD/rebase-merge/rebase-apply), it appends the ownership rule plus the exact abort verb. Appended last, never blocking.Why outcome-keying cannot see this
The signal is keyed on the outcome, which is what lets it name the right abort verb and reject an unmerged index with nothing abortable behind it. That is also its limit:
git merge -X theirsreaches the same end state — one side of the contested hunks discarded by whoever ran the merge — with no conflict for git to report.Measured on git 2.50.1, both sides changing the same line:
Clean index, no marker, exit 0. All four of the outcome arm's probes are negative by construction, so the affordance is not merely quiet here — it is unfireable, while the model has still decided the outcome of someone else's conflict single-handedly. An outcome-keyed detector necessarily has an intent-shaped blind spot; closing it requires a check on the command.
The detection surface
unilateral()is a pure function ofinput.command. No new wiring (bash.tsalready passescommandintoannotate), no git spawn, no filesystem — nothing added to the hot path.Matched
-X ours/-X theirs/-Xours/-Xtheirs--strategy-option=ours|theirsand its space form-s ours/-sours/--strategy=ours/--strategy oursoursstrategy — strictly stronger than the option: it drops the other side's commits wholesale, including changes that never conflicted. Reported in preference when a command carries both.Attached to any integration verb (
merge,pull,rebase,cherry-pick,revert,am), reusing one shared verb list with the outcome arm so the two cannot drift.Deliberately excluded
-Xvalue —patience,histogram,minimal,diff-algorithm=…,renormalize,ignore-space-change,ignore-all-space,find-renames=…,no-renames,subtree=…. They tune how the diff is computed and pick no winner. This is why the side is matched immediately after the flag rather than searched for anywhere in the command:git merge -X patience ours-fixmust not fire, and does not.-s theirs— it does not exist. git 2.50.1 answersCould not find merge strategy 'theirs'. Available strategies are: octopus ours recursive resolve subtree.and refuses to run, so there is no outcome to warn about.[^\n;&|]discipline, so ingit merge feature; echo -X theirsthe echo's flag is not attributed to the merge. A later git command in the same line that does carry one is still found.Ordering
Outcome arm first, unchanged; the intent arm speaks only when the outcome arm returns
"". The outcome notice is strictly more actionable when it fires — it names the conflicted paths from git's index and the abort verb for the operation actually on disk, where the intent arm can only quote a flag back. They are also not mutually exclusive:-X ours|theirsonly settles content conflicts, sogit merge -X theirscan still land in a conflicted index, and there the live conflict is what matters. The existingannotatebody is preserved verbatim asoutcome();annotatebecomes the two-arm chooser.After the fact this cannot know whether the flag actually resolved anything.
-X theirsis a no-op on a merge whose sides never overlapped. The notice is therefore phrased conditionally — it reports what the command asked for, never that a conflict was hidden. That is not hedging; two candidate discriminators were measured and both fail:1. git's output. Rejected — it carries no signal.
-X theirsoutcomeAuto-merging f.txt/Merge made by the 'ort' strategy.Auto-merging f.txt/Merge made by the 'ort' strategy.Auto-merging f.txt/Merge made by the 'ort' strategy.Byte-identical. Suppressing on
Fast-forwardwould also invert this module's own rule that output text alone must never decide a verdict.2. Post-hoc
git merge-tree --write-tree HEAD^1 HEAD^2. It does separate A (exit 1) from F (exit 0) for a committed merge — and it is wrong in the case that matters most:It reports no-op while an entire branch's contribution was discarded — it would silence the notice exactly when the damage is greatest. It also has no second parent to read after a cherry-pick, a rebase, a
--no-commit, or a fast-forward (all verified), needs git ≥ 2.38, and costs spawns on the hottest tool in the process. Only-usually-right, so not built.Notice text (verbatim, as emitted through the real bash tool)
For
-s oursthe second sentence becomes "-s oursis not a way of resolving a conflict — it keeps this branch's tree and discards the other side's commits wholesale, including changes that never conflicted with anything." No XML envelope, same as the outcome notice, so there is no tag for a model to imitate.Revert probe
With the intent arm's two lines in
annotatereplaced byvoid unilateral; void unilateralNotice; return ""— everything else, includingunilateral()itself, left in place — 18 pass / 2 fail. Only the positive-direction bash-level assertions die (the three negative-direction ones and the pure-function unit tests still pass, which is the point: the arm is what makes the notice reach the model). Verbatim:That
Receivedstring is the blind spot itself: the entire tool result the model would read, with nothing in it to suggest a conflict was ever settled.Numbers
Scoped to the 7 tool/bash files, identical command on both sides.
b8167087088ad361740d6e28fb35faae49afdf78(=origin/main)Delta is exactly the +10 new tests. Zero regressions, no existing assertion weakened or removed.
bun typecheckexit 0. Full suite deliberately not run.The four required cases, all through the real bash tool:
-X theirson a merge git reports as a clean success → fires (asserts first that MERGE_HEAD is absent,ls-files --unmergedis empty, git printed no CONFLICT, and the base's line really was discarded)-X patienceon a genuinely conflicting merge → does not fire, and the outcome notice still doesgit merge feature -m '...'; echo 'used -X theirs once'→ does not firePlus
-s oursend-to-end, and unit coverage of all eight matched spellings, eleven excluded-Xvalues,-s theirs, segment discipline in both directions, strategy-over-option precedence, and per-verb abort mapping.What I did NOT verify
git merge payments-shard-fix --strategy-option theirs, zero session calls). This PR is proven deterministically only. Whether the model actually complies with the block — aborts, re-runs without the flag, and routes — is unmeasured, and fix(orchestrator): route to an existing session instead of creating one — roster, tool-result affordances, and the guards that back them #1741's own live evidence is that prose compliance is exactly what cannot be assumed.CAPABLE's looseness, e.g.git merge $(grep -s ours file)— no;&|separator, so the segment guard cannot split it. Pre-existing imprecision class, not measured.