gabriel / muse public
Closed #29 testing
filed by gabriel human · 135 days ago

Comprehensive merge strategy TDD: all --strategy and --history permutations with deliberate conflicts

0 Anchors
— Blast radius
— Churn 30d
0 Proposals

Goal

Build a load-bearing TDD suite that exercises every permutation of `muse merge` strategy and history options with deliberate, engineered conflicts at all three levels Muse tracks: file, directory, and symbol. This suite is the ground truth for "Muse shines at merges" — if it is green, the claim is proved. If it is red, we know exactly what is broken.

Background

Muse tracks conflicts at the symbol level via `.muse/MERGE_STATE.json` and `muse conflicts --json`. Unlike git's line-level conflict markers, Muse can resolve conflicts at the function/class boundary. This suite must exercise all three levels end-to-end and assert the correct resolution mechanics for each.

Agents have been losing work during merge operations because of a flaw in how `muse commit --allow-empty` stores the merge snapshot (it uses the ours-side snapshot, silently discarding theirs-side changes). This suite will catch that regression and others like it.

Strategy matrix

`--strategy` `--history` Description
`overlay` `merge` Feature branch wins on conflicts, full DAG preserved
`overlay` `squash` Feature branch wins, single squashed commit
`overlay` `rebase` Feature branch wins, linear history
`state_merge` `merge` Three-way merge with conflict detection, full DAG
`state_merge` `squash` Three-way merge, single squashed commit
`state_merge` `rebase` Three-way merge, linear history

Conflict level matrix

Level How to engineer What to assert
File-level Both branches edit the same file at non-overlapping regions File contents merged correctly; neither side's changes lost
Directory-level Both branches add/delete/rename files in the same directory All additions present, deletions applied, renames resolved
Symbol-level Both branches modify the same function/class body `muse conflicts --json` lists the symbol; `muse resolve` stages correct merged body

Test phases

Phase 1 — Fixtures and helpers

TDD-first: write a `conftest.py` or `test_merge_matrix.py` with:

  • `make_conflict_repo()` — builds a repo with ours/theirs branches pre-loaded with deliberate conflicts at all three levels
  • `assert_no_work_lost()` — verifies that after merge, every change from both sides is present
  • `assert_symbol_conflict_listed()` — checks `muse conflicts --json` lists expected symbol paths
  • `assert_merge_history()` — verifies parent commit structure matches `--history` option

All helpers must fail before any implementation changes.

Phase 2 — Strategy × History matrix tests

One test per cell in the 6-cell matrix above. Each test:

  1. Creates a fresh repo with engineered conflicts
  2. Runs `muse merge <branch> --strategy <s> --history <h>`
  3. For `state_merge` with conflicts: resolves manually via `muse resolve`
  4. Commits and asserts:
    • All expected files present
    • All expected symbols present with correct bodies
    • Commit DAG structure matches history option
    • `muse verify --json` reports clean

Phase 3 — Regression tests for known failure modes

  • `--allow-empty` snapshot regression: after a merge commit with no tracked changes, the working tree must reflect both sides' content (not just ours)
  • Symbol-level conflict: after `muse resolve`, the staged file must contain the manually-merged body, not a conflict-marker blob
  • Dry-run merge: `muse merge --dry-run` must not mutate any state (working tree, MERGE_STATE, reflog)

Phase 4 — MP GUI smoke tests (optional, separate issue if scope grows)

  • `muse hub proposal read <id>` returns `blockedBy` list when predicted conflicts exist
  • `muse diff --text <from>..<to>` returns the expected diff text for the proposal view

Acceptance criteria

  • All 6 strategy × history combinations have a passing test
  • All 3 conflict levels (file, directory, symbol) are exercised
  • `--allow-empty` snapshot regression test is red before fix, green after
  • `muse verify --json` passes after every merge scenario
  • `muse code test --json` runs only these tests on changed files (no full-suite run needed)
  • Tests are TDD — written first, implementation fixed to make them pass

Out of scope

  • MP GUI conflict surfacing (separate issue)
  • `muse diff --text` in proposal view (separate issue)

Notes

  • Use `muse merge --dry-run --json` to predict conflicts before asserting
  • `muse conflicts --json` returns `[]` when no conflicts are pending
  • `muse resolve path/to/file.py --json` stages the resolution; do NOT use `muse code add`
  • Harmony autoupdate is on by default; verify it learns from each resolved conflict
◎ Activity1
●
gabriel opened this issue 135 days ago
○
gabriel 2 days ago

Implemented and shipped — commit 26088198 on dev/main (pushed to local/staging/production). New file: tests/test_merge_matrix.py.

Corrected strategy matrix + octopus (per your request)

The ticket's strategy matrix named a state_merge strategy that doesn't exist in muse merge --help. The real options are --strategy: recursive (default, three-way + conflict detection — this is what the ticket meant by "state_merge"), overlay, snapshot, replay, ours, theirs; --history: merge, squash, rebase.

Phase 2 exercises recursive and overlay (the two whose behavior meaningfully differs under real conflicts) × all 3 history modes — 6 cells, with file + directory + symbol conflicts engineered simultaneously in one repo per make_conflict_repo(), matching the ticket's Phase 1 spec. snapshot/replay/ours/theirs and the full untouched/convergent/divergent/add-add/delete-modify scenario grid are already covered per-strategy by the pre-existing tests/test_phase3_strategy_matrix.py (SM_01-24) — not duplicated here.

Octopus (N-way) merge (muse#105) — which the ticket predates entirely and doesn't mention — is added as its own section: a structurally different code path that ignores --strategy/--history/--on-conflict completely and refuses outright on any path collision (no resolution, matching git's own octopus behavior exactly). Covers clean 3-way merge (no work lost, 4-parent commit), and refusal-doesn't-move-the-ref on collision. Engine-level depth already existed in tests/test_octopus_merge.py.

Three real bugs found and fixed (TDD — implementation fixed to make the tests pass, per the ticket's own acceptance criterion)

  1. muse/cli/commands/merge.py — Case 3 (the auto-resolve path backing overlay/ours/theirs) always wrote a two-parent commit, completely ignoring --history. muse merge feat --strategy overlay --history squash reported "history": "squash" in its own JSON output while silently writing a two-parent commit anyway. Fixed by adding the same squash/rebase single-parent branch Case 4 (recursive) already had.

  2. muse/core/merge_engine.py + muse/cli/commands/commit.py — completing a conflicted merge via muse resolve + muse commit always produced a two-parent commit, even when --history squash/--history rebase was requested on the original muse merge — the history mode was silently dropped the moment a conflict forced completion through muse commit instead of muse merge's own clean-path commit-writing. Fixed by threading history through MERGE_STATE.json (new field, preserved across muse resolve calls the same way original_conflict_paths already is).

  3. The ticket's own named regression — confirmed reproducible, and worse than described: if a resolved conflict's content happened to exactly match HEAD's current snapshot (e.g. resolving by keeping "ours"), muse commit took the "nothing to commit, working tree clean" early return — exit 0, looks completely benign — but never created the merge commit, never recorded the second parent, and never cleared MERGE_STATE.json. The repo was left permanently stuck: every subsequent muse merge re-surfaced the identical already-resolved conflict forever. This silently discards the entire merge, not just "ours-side only" as described. Fixed by skipping that early return whenever a merge is actually being completed, regardless of --allow-empty — agents no longer need to know to pass --allow-empty for this case at all.

Phase 3 coverage

--dry-run mutates nothing (ref, working tree, MERGE_STATE.json, reflog all verified untouched); muse resolve stages the merged body, not conflict-marker text; Harmony records a resolution after manual resolve; muse verify --json reports all_ok after every scenario including octopus.

Verification

Confirmed genuine RED for all three fixes by reverting them together and re-running: 7 of 13 tests failed, each landing exactly on one of the three bugs. Restored — all 13 green.

Regression (targeted per the ticket's own "no full-suite run needed" note): 1352 tests across test_merge_matrix.py, test_cmd_commit*, test_cmd_merge*, test_phase3_strategy_matrix.py, test_phase6_unified_merge_engine.py, test_core_merge_engine.py, test_merge_data_integrity.py, test_merge_conflict_markers.py, test_merge_supercharge.py, test_phase1_merge_engine.py, test_phase4_harmony_history.py, test_phase7_merge_correctness.py, test_harmony_merge_commit_integrity.py, test_octopus_merge.py, test_addressed_merge_plugin.py, test_comment_only_independence_merge.py, test_merge_base_supercharge.py, test_merge_tree_supercharge.py, test_op_merge.py, test_property_merge_invariants.py, and the full harmony_* suite — all green.

Known gaps, flagged rather than worked around silently

  • tests/test_phase3_merge_state_ordering.py::TestCallOrder::test_apply_manifest_called_before_write_merge_state fails on a clean dev checkout today, unrelated to this change — confirmed by shelving these fixes and re-running (identical failure with or without them). Pre-existing; left for a separate ticket rather than silently fixed as a drive-by here.
  • muse code test --json reported coverage_fraction: 0.0 / zero selected tests for these changed addresses, even after muse code index rebuild. Likely requires a committed baseline the coverage mapper can diff against, which doesn't exist for uncommitted working-tree changes. Worked around by running the specific targeted test files directly rather than relying on automatic selection for this verification pass — the acceptance criterion's actual goal (no full-suite run needed) was still met.
✓
closed this issue 2 days ago