Skip to content

feat(git): git merge — model-aware merging with a review step: property-level conflict report and per-unit plan - #1674

Open
engalar wants to merge 4 commits into
mendixlabs:mainfrom
engalar:feat/git-merge-review
Open

engalar wants to merge 4 commits into
mendixlabs:mainfrom
engalar:feat/git-merge-review

Conversation

@engalar

@engalar engalar commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #1670 (closes the merge half of #1669).

What happened

#1670 was merged from an earlier state of its branch: the four commits pushed after 6a5c942fa never landed on main — the mendix.commits-since-gc doctor fix, the staged-files summary fix (F4), and the entire mxcli git merge feature. This PR re-carries exactly those four commits, rebased onto current main (all cherry-picks clean, tests green).

What mxcli git merge <ref> does

git merge cannot merge Mendix models: the .mpr is SQLite and every .mxunit is BSON, so two sides changing the model always meet a binary CONFLICT (content) (measured in a dedicated lab repo with Studio Pro 11.12.6: both files, every time). Studio Pro resolves that with a model-aware merge, and the same engine ships as a command — mx git-merge BASE MINE THEIRS BASE_COMMIT MINE_COMMIT THEIRS_COMMIT.

mxcli git merge <ref> -p app.mpr runs git merge first; a merge git resolves itself (disjoint files, fast-forward) is annotated like mxcli git commit does. On a model conflict the base and theirs revisions are checked out into temporary worktrees and mx git-merge decides (mx resolved for the project's Mendix version — PATH, mxcli cache, Studio Pro installation):

  • auto-resolvable → staged and committed as the normal two-parent merge commit with the mx_metadata note. Measured end-to-end on a two-sided domain-model change: AutoResolved, both sides kept, note identical in shape to Studio Pro's own merge commit's.
  • ManualResolutionRequired → a review step, never a silent choice. The merge aborts (working tree exactly as before), and two things are produced for the human:
    • a property-level conflict report from mx diff, both sides: New type > Length: 200 → 180 (ours) / → 250 (theirs), per conflicted unit;
    • a plan template beside the .mpr (<mpr>.merge-plan.json, one take: ours|theirs entry per conflicted unit, ours by default).
      The human edits the plan and re-runs with --plan <file>: each unit is taken from its chosen side, the .mpr index is repaired from the mixed files (files are the truth), and the merge is committed with the note. Plan validation refuses units that are not modified-on-both-sides (added/deleted/moved conflicts) — those, and any resolution that must mix both sides inside one unit, stay in Studio Pro (Version Control › Merge Changes Here). --ours/--theirs take one side wholesale without consulting mx. Non-model conflicts (text sources) are left like plain git.

Also re-landed (were in the branch but missing from main):

  • git doctor judges only core.autocrlf; the mendix.* keys are backfilled when missing and their values are never judged — measured on a real Studio Pro checkout, mendix.commits-since-gc counts up (was 2, not 0) and a healthy checkout must pass (fix 1);
  • git commit's staged-files summary lists what the commit holds, not the junk just unstaged (fix 2).

Tests

TDD, real git repositories + real SQLite .mpr fixtures (cmd/mxcli/gitmerge_test.go, gitdoctor_test.go): clean merges annotated; model conflicts auto-resolved via an injected mx runner (worktree paths and base/ours/theirs shas asserted); manual conflicts reported and aborted (MERGE_HEAD gone, tree restored); plans applied per unit with hashes repaired; stale plans refused; non-model conflicts left alone; --ours/--theirs deterministic without mx; already-up-to-date. All green locally with the real mx on the lab repository (auto-resolve, report→plan→land, --theirs).

Docs

CHANGELOG (git merge entry under Unreleased), docs-site "Working Outside Studio Pro" › Merging branches, and the teamserver-git skill's model-merge rule.

wenliu49 added 4 commits October 29, 2026 12:01
…e Studio Pro's own (measured on the lab repo, S0)
…th sides, then --ours/--theirs or Studio Pro (lab S5b)
…iff) and a plan file the human edits before landing (lab S5c)
@engalar

engalar commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor Author

@ako

ako commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator

My verdict on #1674 is request changes. Merging from the command line with Mendix’s own merge engine, plus a review step, is a good idea. As it stands, though:

--ours/--theirs silently drop documents.
Several failure paths leave the repository halfway through a merge.
On Linux, the main path can’t run at all.

How I checked it: the permission policy blocked building and running the PR’s own code, because it comes from an external contributor. So I replayed its git steps with my own commands, and called Mendix’s mx (11.13, on the pedapp fixture) exactly as the PR calls it. Its mx git-merge success path on Windows or macOS is therefore not something I could verify.

Must fix
--ours/--theirs drop the other side’s new documents and commit without checking.
They take one whole side of every conflicted model file, and that includes App.mpr, which lists every document in the project.
In the most common case, each side adding a document, I replayed --theirs. It committed a merge in which ours’ new microflow is a file on disk but not part of the project: ORPHAN … ACT_Ours (file, not indexed). Only ACT_Theirs is visible.
Documents the other side deleted stay listed with no file behind them.
The PR’s tests only change units that already exist on both sides, so App.mpr lists the same units on both sides and this never shows up.
These two flags are also exactly what the PR’s error message suggests next.
--plan refuses the normal case, and leaves the merge half-done.
It takes ours’ App.mpr, so any document theirs added becomes an orphan, and its own hash check then refuses to commit. I reproduced that refusal in a scenario where both sides change the same attribute and theirs adds a microflow.
It returns without git merge --abort: MERGE_HEAD remains and App.mpr stays unmerged. That’s the state in which Studio Pro re-runs the merge every time it opens the project, which is how Jonathan’s checkout kept crashing.
The path where mx can’t be found or started doesn’t abort either.
The skill text tells agents that such a merge “aborts the merge cleanly”, which isn’t true in these cases.
A crash is reported as a merge decision.
The engine exits 2 when conflicts need a decision. It prints a JSON list per unit (ManualResolutionRequired versus AutoResolved) and writes conflict markers into App.mpr.
It exits 129 on an exception. One example is the customer’s case: a merge base with access rules an older mxcli wrote incomplete.
The PR treats every non-zero exit as “needs a decision”, writes a plan and suggests --ours/--theirs.
It should handle only exit 2 as a conflict. It could also use the engine’s own JSON list rather than git’s list of conflicted files. Any other exit should be reported as mx failing, followed by a clean abort.
On Linux, mx git-merge doesn’t work.
With the mxbuild that mxcli setup mxbuild installs, it exits 129 with “Git not found”, even though git is on PATH, and also with a git.exe symlink there.
Its git lookup appears to be Windows-oriented: the assembly contains “Could not find a path to git.exe” and Windows registry environment keys.
Linux devcontainers are where agents run mxcli, and there, combined with point 3, every model conflict ends up steered towards point 1.
The tests use a fake mx, so CI can’t catch this.
mx merge (the same engine, taking three project folders) did run here: exit 0 when clean, 2 on conflicts, 129 on an exception. It also wrote the merge result, including theirs’ added document, into MINE. It may be the more portable choice.
Should fix
The PR conflicts with main in CHANGELOG.md, cmd_git_doctor.go and the docs page. Also, cmd_git_commit.go merges without a textual conflict but calls isJunkPath(f), while main has isJunkPath(f, projDir) since my ako/mxcli#1767, so the result wouldn’t compile.
The plan file .merge-plan.json is written into the working tree and isn’t ignored, so the next mxcli git commit would commit it.
When model and non-model files both conflict, it tells the user to resolve the non-model files and then git add and git commit. But the model files are still unmerged, and adding them commits ours’ App.mpr (the same problem as point 1).
Nit: it calls mx directly rather than through docker.PrepareMxCommand, which applies the LD_PRELOAD workaround that 11.10 needs.
What’s good
The doctor change (only core.autocrlf is judged; the mendix.* counters pass whenever they’re present) is sensible and measured.
The staged-files summary fix is correct, once the isJunkPath call is updated.
The stale-plan check on commit SHAs, the property-level report from mx diff, the refusal while Studio Pro is open, and the “modified on both sides” rule for plans are all good ideas.

The common fix for points 1 and 2: never take a whole side of App.mpr. Either merge its list of units three ways, or refuse and abort whenever the hash check finds orphaned or missing units.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants