GitSync: two-way conflict resolution (newest-writer-wins) + force override#468
Merged
Conversation
…rride
By default an 'Update to latest' is git-first: it overwrites and prunes the live node from the
repo, silently losing edits made on the server between syncs (this is what clobbered a Space's
locally-edited nodes on the next auto-sync). Add a per-source Two-way setting: on import, a node
changed on the server since the last sync (LastModified > LastSyncedAt) is PRESERVED — neither
overwritten nor pruned — so it is carried back to GitHub on the next commit ('newer on the server
wins'). A forced update overrides this and re-applies the repo state regardless (and bypasses the
content-fingerprint short-circuit, so it discards local edits even when the repo is unchanged).
- GitHubSyncConfig.TwoWay (auto-renders in the settings editor); SaveConfig gains twoWay.
- StaticRepoImporter: ImportConflictPolicy threaded through ImportSource/Import/Run; preserve on
overwrite + on prune; force bypasses the Succeeded-fingerprint skip. Boot/built-in imports pass
no policy -> unchanged git-first behavior. Also read the import marker via the tolerant
ContentAs<ActivityLog> (a queried node's Content is a JsonElement, not the typed record — a raw
'is ActivityLog' pattern silently mis-fires and re-imports every time).
- GitHubSyncService.ReimportAtCommit / PullRequestService.UpdateToLatest /
GitHubActivityExtensions / MCP git_hub_sync gain a force flag.
- Doc: Two-way section in GitHubSync.md. Tests: ImportConflictPolicyTest (unit), TwoWaySyncTest.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the GitSync “Update to latest” (GitHub → mesh) workflow to support optional two-way conflict resolution (“newest-writer-wins” per node) so server-side edits made between syncs are preserved instead of being overwritten/pruned, and adds a force override to deliberately re-apply repo state even when unchanged.
Changes:
- Introduces
GitHubSyncConfig.TwoWayand threads a newImportConflictPolicythrough the GitHub import path to preserve server-newer nodes on overwrite and prune. - Adds a
forceflag to update/reimport APIs and the MCPgit_hub_synctool, including bypassing the fingerprint short-circuit when forced. - Adds unit + integration tests for the decision policy and the two-way/force behavior, plus documentation updates (Architecture + What’s New).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Graph.Test/ImportConflictPolicyTest.cs | Unit tests for the pure two-way/force preserve decision. |
| test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs | Integration test covering two-way update preserving server additions and force discarding them. |
| src/MeshWeaver.Mcp/McpMeshPlugin.cs | Adds force parameter to the MCP GitHub sync tool and passes it to update. |
| src/MeshWeaver.Graph/StaticRepoImporter.cs | Implements per-node preserve-on-overwrite and preserve-on-prune via ImportConflictPolicy; hardens activity-marker typing; force bypasses fingerprint skip. |
| src/MeshWeaver.GitSync/PullRequestService.cs | Threads force into “Update to latest” execution and logging. |
| src/MeshWeaver.GitSync/GitHubSyncService.cs | Builds import conflict policy from config (TwoWay, LastSyncedAt) and threads force through reimport/import pipeline. |
| src/MeshWeaver.GitSync/GitHubSyncConfig.cs | Adds TwoWay config flag and documents behavior. |
| src/MeshWeaver.GitSync/GitHubActivityExtensions.cs | Adds force plumbing and user-facing activity messaging/title changes for force-update. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-07-14-gitsync-two-way.md | What’s New entry describing two-way + force behavior. |
| src/MeshWeaver.Documentation/Data/Architecture/GitHubSync.md | Architecture documentation for two-way conflict rule and force update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
363
to
367
| logger?.LogInformation("Re-importing {Space} at {Ref} (twoWay={TwoWay}, force={Force})", | ||
| spacePath, commitish, config.TwoWay, force); | ||
| return FetchAndImport(repoUrl, commitish, config.Subdirectory, token, spacePath, | ||
| SyncIgnore.For(config), progress) | ||
| SyncIgnore.For(config), progress, policy) | ||
| .SelectMany(x => RecordLastSync(spacePath, x.CommitSha, sourceId).Select(_ => x.Result)); |
Contributor
Author
There was a problem hiding this comment.
Good catch — this was a real bug, fixed in 4a1e3d3. StaticRepoImportResult now carries a Preserved count and ReimportAtCommit skips advancing the last-sync baseline (RecordLastSync) when Preserved > 0: the mesh is ahead of the repo until those edits are committed back, so the baseline stays put and the next update still sees them as newer-on-server.
Comment on lines
+41
to
+46
| // Two-way update to the branch HEAD: the server-added node is KEPT — not pruned as a | ||
| // repo-absent extra ("newer on the server wins; commit to carry it back"). | ||
| await Sync.ReimportAtCommit(a, "main", UserId).Timeout(90.Seconds()).ToTask(); | ||
| Assert.NotNull(await WaitForNode($"{a}/ServerOnly")); | ||
| Assert.NotNull(await WaitForNode($"{a}/Welcome")); | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Done in 4a1e3d3 — TwoWaySyncTest now runs a second non-forced update before the forced one and asserts the server-only node still survives, which is exactly the regression guard for the baseline-advance bug above.
…rection) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e preserved Addresses the Copilot review: ReimportAtCommit advanced LastSyncedAt after every import, so a two-way update that preserved a server-newer node moved the baseline past it — the NEXT update no longer saw it as newer-on-server and overwrote it (preservation lasted a single cycle). Surface a Preserved count on StaticRepoImportResult and skip the last-sync record when Preserved > 0 (the mesh is ahead of the repo until those edits are committed back, which advances the baseline). Extend TwoWaySyncTest with a second non-forced update asserting the server node still survives. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Two-way GitHub sync (newest-writer-wins) + force override
Why
Update to latest(GitHub → mesh) was strictly git-first: it overwrote and pruned the liveSpace from the repo, silently discarding edits made on the server between syncs. This is exactly
what clobbered a Space's locally-edited nodes on the next automatic sync — a node edited on the
server was overwritten by the older repo copy instead of being carried back to GitHub.
What
A per-source Two-way setting changes the import conflict rule to newest-writer-wins per node:
LastModifiedis newer than the last recorded sync (LastSyncedAt) ispreserved on update — neither overwritten nor pruned — so it flows back to GitHub on the next
Commit ("newer on the server wins").
Force is the escape hatch:
git_hub_sync(op:"update", force:true)ignores two-way and re-appliesthe repo state regardless — and now also bypasses the content-fingerprint short-circuit, so it
discards local edits even when the repo itself is unchanged.
Changes
GitHubSyncConfig.TwoWay(auto-renders in the GitHub Sync settings editor);SaveConfiggainstwoWay.StaticRepoImporter: newImportConflictPolicythreaded throughImportSource/Import/Run;preserve on overwrite and on prune;
forcebypasses the Succeeded-fingerprint skip.Boot / built-in static-repo imports pass no policy → git-first behavior is unchanged.
ContentAs<ActivityLog>— a queriednode's
Contentis aJsonElement, so the previousis ActivityLog {…}pattern silentlymis-fired and re-imported every time.
ReimportAtCommit/PullRequestService.UpdateToLatest/GitHubActivityExtensions/ MCPgit_hub_syncgain aforceflag.Tests
ImportConflictPolicyTest(6 unit tests — the pure decision).TwoWaySyncTest(integration — two-way keeps a server addition on update;forcediscards it).GitHubExportImportTeststill 4/4 (git-first still prunes — no regression).🤖 Generated with Claude Code