Repository navigation
fix(merge-policy): read a conflicting merge into the tip, wait on a conflict in a check's file; doctor checks git (#1077) - #1090
Conversation
… a conflict in a check's file (#1077) A merge into the default branch's tip that git conflicts on got no second reading, so a PR GitHub merges cleanly where git conflicts (a file added under a folder the tip renamed) could land unread from the tip when the combined check didn't run. Now the tree merge-tree still writes for a conflict is measured from the tip like a clean one, and a conflict in a test, lint, guard, allowlist or table file waits (fail closed). A conflict elsewhere is still left to the conflict paths. mergedTree reads the conflicted paths (-z --name-only) and refuses output that names no tree, a conflict with no path or an exit other than 0 or 1: before, any exit 1 counted as a conflict. When the second measurement can't run or passes its bound, the first one's reasons are kept beside its reason instead of being replaced. Tests kill the surviving mutants listed in #1077: the three-place cap across measurements, merge-tree's argument order, the guard against an option-like revision, the tree-id check, and an older git that must wait. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…1077) The merge decision measures a merge with git merge-tree --write-tree (2.38) and reads a tree's attributes with check-attr --source (2.40). With an older git every PR waits with "could not run". doctor now says so up front: its git line fails below 2.40, naming the version needed. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…he git it needs (#1077) docs/design.md still named entries a PR merges in and takes out as not covered; it now describes the reading from the default branch's tip (#1069) and how a conflict is read. The #1069 changelog fragment's residual sentence pointed at a case #1077 now holds. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
|
Independent final check (R90a), posted by the coordinator. VERDICT: PASS @9acf107447c6dcfc903eb901b171cc978a4dccef (final check) Reviewer R90a. Detached worktrees: head at Findings
Weakening flagsScan:
Follow-ups (not FAIL)
|
|
Independent adversarial review (R90b), posted by the coordinator. VERDICT: FAIL @9acf107447c6dcfc903eb901b171cc978a4dccef (adversarial) FindingsF1 (FAIL, bars c and d: regression versus main, measured). An ordinary PR that conflicts with the tip in a test file or
F2 (FAIL, bar a: overclaim). For git 2.38 and 2.39, doctor's message (pinned by F3 (follow-up, measured). A directory rename split on the tip (base No fail-open found (measured). Each of these holds with Mutants:
What I ran (measured)
Weakening flags
|
…y is unknown (#1077) While GitHub is still computing whether a PR can merge (the first poll after every merge), a PR held only by the tip-conflict reason was handed to the owner (needs-owner label, comment, review request) before GitHub could say dirty and route it to the conflict fix, as main does. It now waits, up to MERGEABLE_CAP polls; a clean answer, or none at the cap, still holds it for the owner. Also discloses the directory-rename split that merge-tree reports as a conflict with no path. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…tributes waits (#1077) Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…#1077) Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
|
Independent adversarial review (R90d), posted by the coordinator. VERDICT: FAIL @3fe63751caf448dc947285c5624bbaf9cc2c620a (adversarial) FindingsF1 (FAIL, bar d: regression versus main, measured). R90b's F1 is still there for a conflict inside a registered table. Shape:
Wait checks (measured, coordinator harness probe, head and main):
Mutants of the wait (dist, restored and checked with
What I ran (measured)
Weakening flags
|
|
Independent final check (R90c), posted by the coordinator. VERDICT: FAIL @3fe63751caf448dc947285c5624bbaf9cc2c620a (final check) Reviewer R90c. Detached worktrees under BlockingRequired CI is red because of the change (measured).
That control is from c2a8bd8 (#1074, now on main). CI tested Reproduced: The second reason, the mark "removed or changed", is a false reason that comes from reading the conflict tree (inferred). It names something the PR didn't do. Also, the body's "the results carry over to 3fe6375 (inferred)" no longer holds against current main. Confirmed (measured)
Weakening flags
Follow-ups (not FAIL)Surviving mutants of the deferral, all against
|
…cy-followups Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
… paths, and its markers unread (#1077) Reading the conflicted tree from the tip read conflict markers as lines: in a registered table they gave a second reason ("line N is at its indent") beside the conflict hold, and in a test file a false "mark removed". So a PR whose merge conflicts on a table's or test's lines was handed to the owner while GitHub was still computing, and the adversarial control control-default-branch-marked-table-conflicting-pr went red. The tip reading now leaves the conflicted files out. Only a conflict on where a check's file goes or whether it stays (any merge-tree conflict type but CONFLICT (contents), or a conflicted path no message names) holds, since GitHub's merge may complete there; a conflict on lines holds nothing, as on main. merge-tree's messages are parsed for that, and a conflict only on a directory (a split) names no path and is read as a clean merge. Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
… cap and lone-hold check (#1077) Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
|
Independent adversarial review (R90f), posted by the coordinator. VERDICT: PASS @db43bf9895e04b5df011d9c3b2850118340effd2 (adversarial) Reviewer R90f. My worktrees: (a) Conflict shapes and message parsing (measured:
|
|
Independent final check (R90e), posted by the coordinator. VERDICT: PASS @db43bf9895e04b5df011d9c3b2850118340effd2 (final check) Reviewer R90e. Detached worktrees (1) CI and merge (measured)
(2) R90c and R90d fixed (measured)
(3) Against #1077
(4) No regression versus main (measured)
(5) Weak-scan (
|
|
Weakening scan decision for head The owner's instruction (verbatim): "Stp asking me quesgtions and just take recommended option each time". This PR is in the critical set. Both independent reviews of this exact head passed: the final check (R90e) and the adversarial review (R90f). Both classified the scan's flags the same way. The weakening scan reports 2 reasons (measured, R90e and R90f):
This PR is "Part of #1077"; #1077 stays open for the parts not delivered (R90e). Decision: proceed (recommended option). This covers only this head; any new push needs a fresh scan and review. |
What and why
Part of #1077 (the clear items; skipped ones listed below). Follow-ups from the reviews of #1073.
git merge-tree --write-tree(tip, head) conflicts, the second reading adds nothing and the PR goes to the conflict paths. If GitHub's own merge completes where git conflicts and the combined check doesn't run, the PR lands unread from the tip. Now:merge-treestill writes for a conflict is measured from the tip like a clean one, its conflicted files left out (round 3: their markers gave false reasons);CONFLICT (contents)) waits with a new reason:conflicts with the default branch's current tip on where a test, lint, guard, allowlist or table file goes or whether it stays, so what merging it makes there can't be read. A conflict on a file's lines holds nothing, as on main (round 3);p1-6-unbounded-loops(a conflict insrc/price.js) is unchanged.mergedTreereads the conflicted paths (-z --name-only). It refuses output with no tree id, a conflict with no path, or an exit other than 0 or 1. On main, any exit 1 counted as a conflict.doctorgets agitline that fails below 2.40 and names the version needed.merge-tree --write-tree(2.38) is used for every PR, andcheck-attr --source(2.40) only for a PR that changes a.gitattributes. So below 2.38 every PR waits, and on 2.38 or 2.39 only a PR that changes a.gitattributeswaits (reworded in round 2)..gitattributes merge=unionprobe (R19c/R19f): added as a test. Git merges by the attributes of the checkout, not the PR's. With the checkout at the default branch, the union PR conflicts on the test file's lines, which is left to the conflict paths (round 3). With the checkout at the PR head, union keeps both sides and nothing is taken out (measured in the scratch repos).docs/design.mddrops a "not covered" clause merge policy: a PR that merges in a tip mark and reverts the file to base text removes the mark from main (net-zero diff) #1069 closed and describes the tip reading. The merge policy: a PR that merges in a tip mark and reverts the file to base text removes the mark from main (net-zero diff) #1069 fragment's residual sentence now points here. New fragmentchangelog.d/1077-tip-conflict-read.md.Critical set: yes (merge policy). Two independent reviews are needed on the same head.
Fail-before / pass-after (measured)
test/merge-policy-tip-conflict.test.ts, built against main'ssrc(scratch detached worktree at origin/main d5650da with only this test copied in): 3 pass, 8 fail. The 3 that pass are the app-code control, the D5 pin (a stale conflicting PR shrinking a tip-marked table) and the count pin. The mergedTree tests fail there partly because the return shape changed. Command:node --import ./dist/test/isolate.js --test dist/test/merge-policy-tip-conflict.test.js.test/doctor-git-version.test.ts: 2/2 pass (new feature, not a bug fix).Mutants (measured; dist patched in place, checksum verified restored)
Suites: merge-policy-tip-conflict, merge-policy-tip-merged-in, merge-policy-tip, merge-policy, policy-only-registration (92 tests).
merge-treearguments swapped (M14);Math.maxwithoutinto.first.length): "and N more" shows only whenn > first.length. Dropping a term that is never above the finalfirst.lengthcan't change that test or its value.fromis already the tip): the tip's marks equalfrom's, and no tip-only table or lint branch fires. No test can tell them apart.Tests run (measured; TMPDIR a 0700 directory directly under /var/tmp)
npm run typecheckexit 0;check:denylist1663 files clean;origin/main..HEAD: 3 commits, 1 tip clean.merge-vacuity-r10-shutdown(2 tests): fails in the sandbox on main's build too, and passes outside it on both builds;coordinator"dependency cache": the knownPIP_CACHE_DIRartifact (R19e).npm teston 9acf107 in the builder sandbox (measured): 5877 tests, 5584 pass, 134 fail, 119 skipped, 40 todo./dev, Homebrew node paths, the shutdown-fetch tests above,PIP_CACHE_DIR, the symlinked-node_modulesbackstop, and init's systemctl queries.gh pr view 1090 --json statusCheckRollup): every check is green or skipped, including all 8 test shards on Node 22 and 24, push-gate, denylist and dco.The engine's own weakening scan of this diff (measured,
weakeningOffrom this head's dist)removes or changes a line of a guard or lint: refactors insrc/coordinator/change-files.ts(mergedTree,gitVersion) andsrc/merge-policy-weakening.ts(findings' bound,weakeningOf). No reason is dropped: the mutant and fail-before runs above show each path still holds.adds a line with a skip, only or lint or type-check suppression mark: theeslint-disable-next-linestrings intest/merge-policy-tip-conflict.test.tsare fixture text written into a scratch repo (the suppression test), not a real suppression.Skipped, and why
merge-treeconflict as a light-check conflict, or merge with main's attributes) live insrc/coordinator/merge-flow.ts, which fix(vacuity): pair moved code git doesn't pair, only ever taking measure away (#1014) #1068 owns. Reading the conflicted tree (R7) was done here in round 1 and narrowed in round 3: conflicted files are left out.changelog.d/1020-adversarial-tip-cases.mdand "Open on main" indocs/guardrails.md: both describe the adversarial suite's todo lines, which belong to the adversarial-suite work.Inferred / guess
merge-treereads its attributes.merge=unionormerge.directoryRenames.lib/a.js lib/b.js, the tip moves them toone/andtwo/, the PR addslib/c.js).merge-treeexits 1 with no conflicted path, so the second reading "could not run" and the PR waits for the owner, where main gives no reason. It fails closed. GitHub probably reports it dirty anyway (guess). Not changed here.Round 2 (R90a PASS, R90b FAIL at 9acf107)
Head: 3fe6375, which is 9acf107 plus three commits (a fast-forward, not rebased).
The runs below were made on the same commits rebased onto origin/main a4b16ac (8a5e5358 → 0e01bd7c).
git diff 009ec24e 3fe63751andgit diff a4b16ac7 0e01bd7care identical, andgit merge-tree --write-tree origin/main HEADexits 0 (measured). That the results carry over to 3fe6375 was inferred, and was wrong against newer main: CI on 3fe6375 merged with fba8946 failed the adversarial control (round 3)."Before" below means 8a5e5358 (9acf107 rebased).
F1 (regression vs main): fixed.
unknown/null (the first poll after every merge), an ordinary PR conflicting with the tip in a test file gotmerge.decided auto:false, aneeds-ownerlabel, a comment and a review request. Main defers there.mergeOrWait(src/coordinator/merge-flow.ts): a decision held only bytip-conflictreasons (or the still-to-come coverage run) is deferred while mergeability is unknown. It uses the samemergeable:poll counter andMERGEABLE_CAPas the auto branch. GitHub'sdirtyon a later poll sends the PR to the conflict fix, as on main. A clean answer, or none at the cap, still holds it for the owner. A PR with any other hold is decided as before.isTipConflictis exported fromsrc/merge-policy-weakening.ts.merge-flow.ts, which it owns. This edit is 8 lines inmergeOrWaitplus one import, far from fix(vacuity): pair moved code git doesn't pair, only ever taking measure away (#1014) #1068's hunks.git merge-tree --write-tree origin/issue-1014-vacuity-moved-code HEADexits 0 (measured).F2: fixed.
gitTooOld(doctor's message) now says the right thing per version:.gitattributeswaits;The 2.40 threshold is kept. The changelog, the test header and the doc comment are reworded to match.
isNonUtf8Pathmutant: killed. New unit test: a conflict inweb/x\uFFFD.bingivestip-conflict, andweb/x.bingives nothing. Measured by deletingisNonUtf8Path(p) ||in dist: 11 pass, 1 fail (the new test). The file was restored and checked withcmp.F3: disclosed (above, in the changelog and in
docs/design.md), not changed.Fail-before / pass-after (measured)
Each command was run from the worktree after
npm run build, with TMPDIR a 0700 directory under /var/tmp.node --import ./dist/test/isolate.js --test dist/test/merge-tip-conflict-unknown.test.js(new file, 3 tests).src/changes reverted to 8a5e5358: 2 pass, 1 fail. The failing test is the unknown-mergeability one: "no merge decision while GitHub is still computing".node --import ./dist/test/isolate.js --test dist/test/doctor-git-version.test.js.change-files.ts: 1 pass, 1 fail (gitTooOld: 2.38 and 2.39 said "every PR waits").dist/test/merge-policy-tip-conflict.test.jsgives 12/12 pass. The mutant result is above.Tests run in round 2 (measured)
npm run typecheck: exit 0.npm run check:denylist: 1674 files clean.check-dco.mjs origin/main HEAD: all commits signed off.coordinator"dependency cache".PIP_CACHE_DIR. It passes outside the sandbox with that and the other*_CACHE_DIRvariables unset (measured).flock /var/tmp/wl-coord/fulltest.lock nice -n 15 ionice -c3 npm test, run outside the sandbox with the*_CACHE_DIRvariables unset. Result: 5913 tests, 5787 pass, 17 fail.machine-sudoers-closed,machine-sudoers-narrow,machine-sudoers-removal);desktop-removed's symlinked-node_modulesbackstop.browser-run"a browser that can't start", hit a 30 s timeout under the full run's load. Run alone on this head it passed 12/12, twice (measured).Round 3 (R90d FAIL, R90c FAIL at 3fe6375)
Head: db43bf9 = 3fe6375 + a merge of origin/main c41f37c (signed off; a fast-forward, since a force push is blocked here) + 2 commits.
control-default-branch-marked-table-conflicting-pr(from adversarial: tip-table cases for use outside the list, hidden tables and the rule-name floor #1074, on main since c2a8bd8) failed: a comment added to a test file whose table the tip marked conflicts on its lines, and it got the tip-conflict hold plus a false "mark removed or changed" reason. Round 2 tested against a4b16ac, which lacks c2a8bd8.mergedTreenow parses merge-tree's messages. Thetip-conflicthold applies only to a conflict on where a strict file goes or whether it stays: any type butCONFLICT (contents), or a conflicted path no message names (fail closed). A conflict on a file's lines holds nothing in the decision, as on main. Every hold main has is kept: main held nothing on any conflict (inferred from the code; the control passes on main, measured by R90c).dirty(inferred; whether its merge matches git's on renames or attributes is a guess). The case Merge policy: follow-ups from #1073's reviews (conflict residual, mutants, git version check) #1077 targets, a file added under a folder the tip renamed, is afile locationconflict and still holds.dirtyroutes both to the conflict fix, whose head is read again.cmp):every(...): killed by a new test, a tip-conflict hold beside a design hold is decided at once while unknown (5 pass, 1 fail);<to<=at the cap: killed, the cap test now pinsafter 10 polls(5 pass, 1 fail).wait.cappedis emitted at the cap withGitHub was still computing whether it can merge after N polls, as the auto branch does.dirty; a PR with another hold is decided at once; content conflicts hold nothing.Fail-before / pass-after (measured)
Commands, from the worktree after
npm run build:node --import ./dist/test/isolate.js --test dist/test/merge-tip-conflict-unknown.test.js dist/test/merge-policy-tip-conflict.test.jsnode --import ./dist/test/isolate.js --test --test-name-pattern=control-default-branch-marked-table-conflicting-pr dist/test/adversarial/adversarial.test.jsResults:
BAD_WORDS: line 4 is at its indent ...and the conflict hold).wait.cappedpin.Tests run in round 3 (measured; outside the sandbox,
*_CACHE_DIRunset)npm run typecheck: exit 0.check:denylist: 1680 files clean.