From 97f5596616cf3b53528eb403993a74039e34af79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Tue, 14 Jul 2026 07:16:35 +0200 Subject: [PATCH 1/4] GitSync: two-way conflict resolution (newest-writer-wins) + force override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 (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) --- .../Data/Architecture/GitHubSync.md | 20 ++++ .../GitHubActivityExtensions.cs | 15 +-- src/MeshWeaver.GitSync/GitHubSyncConfig.cs | 16 +++ src/MeshWeaver.GitSync/GitHubSyncService.cs | 21 ++-- src/MeshWeaver.GitSync/PullRequestService.cs | 8 +- src/MeshWeaver.Graph/StaticRepoImporter.cs | 99 ++++++++++++++++--- src/MeshWeaver.Mcp/McpMeshPlugin.cs | 9 +- .../MeshWeaver.GitSync.Test/TwoWaySyncTest.cs | 52 ++++++++++ .../ImportConflictPolicyTest.cs | 65 ++++++++++++ 9 files changed, 271 insertions(+), 34 deletions(-) create mode 100644 test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs create mode 100644 test/MeshWeaver.Graph.Test/ImportConflictPolicyTest.cs diff --git a/src/MeshWeaver.Documentation/Data/Architecture/GitHubSync.md b/src/MeshWeaver.Documentation/Data/Architecture/GitHubSync.md index ec4ba580c..cbee1ef3b 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/GitHubSync.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/GitHubSync.md @@ -150,8 +150,28 @@ Import reuses the platform's content-addressed import pipeline (fingerprint gate activity lock + canonical upsert + prune) — see [StaticRepoImport.md](/Doc/Architecture/StaticRepoImport). +### Two-way — never overwrite changes made on the server + +By default import is **git-first**: an update overwrites (and prunes) the live node from the +repo. That silently loses edits made on the server *between* syncs. Turn on **Two-way** (a +checkbox on the sync source) to change the conflict rule to **newest-writer-wins per node**: + +- A node whose live `LastModified` is newer than the last recorded sync (`LastSyncedAt`) was + changed on the server since you last synced. An **Update to latest** **keeps** that node — + it is neither overwritten nor pruned — so your local change is carried back to GitHub on the + next **Commit** ("newer on the server wins → GitHub"). A node the server hasn't touched since + the last sync is still updated from the repo as usual. +- Two-way only takes effect once a first sync has recorded `LastSyncedAt`; the initial import is + git-first (there is nothing local to protect yet). + +**Force update** is the escape hatch: it ignores two-way and overwrites/prunes from the repo +regardless — use it to deliberately discard local changes back to the repository state. Via MCP: +`git_hub_sync(space, op:"update", force:true)`. + > **Take-over edits survive.** A node you've edited and marked to exclude from sync is > not overwritten or pruned by a re-import — that's how you "claim" content locally. +> Two-way generalizes this to *every* server-side edit (no explicit claim needed) as long as it +> post-dates the last sync. --- diff --git a/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs b/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs index a5695809e..7ed44ca7c 100644 --- a/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs +++ b/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs @@ -55,16 +55,19 @@ public static IObservable CommitToGitHub( /// selects the sync source (null = the primary). public static IObservable UpdateToLatestFromGitHub( this IMessageHub hub, string spacePath, string userId, Action? onActivityCreated = null, - string? sourceId = null) + string? sourceId = null, bool force = false) { var pr = hub.ServiceProvider.GetRequiredService(); - return hub.RunActivity(spacePath, ActivityCategory.Import, $"Update {spacePath} to latest", + return hub.RunActivity(spacePath, ActivityCategory.Import, + force ? $"Force-update {spacePath} to latest" : $"Update {spacePath} to latest", ctx => { - ctx.Log("Fetching the branch HEAD from GitHub and importing the deltas…"); + ctx.Log(force + ? "Fetching the branch HEAD from GitHub and overwriting local changes (force)…" + : "Fetching the branch HEAD from GitHub and importing the deltas…"); // ctx.Log as the progress sink: files dropped from the import (parse failures) // append an Error line here and flip the terminal status to Failed. - return pr.UpdateToLatest(spacePath, userId, sourceId, ctx.Log).Select(r => + return pr.UpdateToLatest(spacePath, userId, sourceId, ctx.Log, force).Select(r => { ctx.Log($"Imported {r.Outcome} ({r.Count} node(s))."); return Unit.Default; @@ -76,7 +79,7 @@ public static IObservable UpdateToLatestFromGitHub( /// selects the sync source (null = the primary). public static IObservable ReimportFromGitHub( this IMessageHub hub, string spacePath, string commitish, string userId, - Action? onActivityCreated = null, string? sourceId = null) + Action? onActivityCreated = null, string? sourceId = null, bool force = false) { var sync = hub.ServiceProvider.GetRequiredService(); return hub.RunActivity(spacePath, ActivityCategory.Import, $"Re-import {spacePath} at {commitish}", @@ -85,7 +88,7 @@ public static IObservable ReimportFromGitHub( ctx.Log($"Fetching {commitish} from GitHub and importing the deltas…"); // ctx.Log as the progress sink: files dropped from the import (parse failures) // append an Error line here and flip the terminal status to Failed. - return sync.ReimportAtCommit(spacePath, commitish, userId, sourceId, ctx.Log).Select(r => + return sync.ReimportAtCommit(spacePath, commitish, userId, sourceId, ctx.Log, force).Select(r => { ctx.Log($"Re-imported {r.Outcome} ({r.Count} node(s)) at {commitish}."); return Unit.Default; diff --git a/src/MeshWeaver.GitSync/GitHubSyncConfig.cs b/src/MeshWeaver.GitSync/GitHubSyncConfig.cs index ab4008d19..5e4ca1823 100644 --- a/src/MeshWeaver.GitSync/GitHubSyncConfig.cs +++ b/src/MeshWeaver.GitSync/GitHubSyncConfig.cs @@ -42,6 +42,22 @@ public record GitHubSyncConfig [Description("Sync direction")] public SyncDirection Direction { get; init; } = SyncDirection.Bidirectional; + /// + /// Two-way conflict resolution on import ("Update to latest"). When true, an update + /// NEVER overwrites (or prunes) a node that was changed on the server SINCE THE LAST SYNC — + /// the server copy is preserved and carried back to GitHub on the next commit ("newer on the + /// server wins → GitHub"). When false (the default) import is git-first: the repo overwrites + /// the live node, which silently loses local edits made between syncs. A forced update + /// (force: true) overrides this and overwrites regardless — the escape hatch for + /// deliberately discarding local changes back to the repo state. + /// + /// "Since the last sync" is measured against : a node whose + /// MeshNode.LastModified is newer than the last recorded sync counts as a local + /// edit. Only takes effect once a first sync has recorded that timestamp. + /// + [Description("Two-way (don't overwrite nodes changed on the server since the last sync — commit them back instead)")] + public bool TwoWay { get; init; } + /// Create if it does not exist yet. Default true. [Description("Create the branch if it doesn't exist")] public bool CreateBranchIfMissing { get; init; } = true; diff --git a/src/MeshWeaver.GitSync/GitHubSyncService.cs b/src/MeshWeaver.GitSync/GitHubSyncService.cs index 57a488dcd..187645b18 100644 --- a/src/MeshWeaver.GitSync/GitHubSyncService.cs +++ b/src/MeshWeaver.GitSync/GitHubSyncService.cs @@ -337,10 +337,12 @@ public IObservable ImportFromGitHub( /// (when the caller runs as an activity: ctx.Log) /// receives per-file problems — a repo file that failed to parse and was therefore /// dropped from the import — so they land on the activity log, not only in Loki. + /// ignores the source's two-way setting and overwrites/prunes + /// from the repo regardless (the deliberate-discard escape hatch). /// public IObservable ReimportAtCommit( string spacePath, string commitish, string userId, string? sourceId = null, - Action? progress = null) + Action? progress = null, bool force = false) { return ReadConfig(spacePath, sourceId).Take(1).SelectMany(config => { @@ -351,12 +353,17 @@ public IObservable ReimportAtCommit( return Observable.Throw(new InvalidOperationException( $"This sync source is export-only (mesh → repo): importing from {repoUrl} is not allowed. " + "Change the source's Sync direction to Bidirectional or Import-only to re-import.")); + // Two-way (config.TwoWay): don't overwrite/prune nodes changed on the server since the last + // recorded sync (config.LastSyncedAt) — they are carried back on the next commit. `force` + // overrides. Git-first when TwoWay is off (unchanged legacy behavior). + var policy = new ImportConflictPolicy(config.TwoWay, config.LastSyncedAt, force); return ResolveAuth(userId).SelectMany(auth => { var token = auth.Token; - logger?.LogInformation("Re-importing {Space} at {Ref}", spacePath, commitish); + 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)); }); }); @@ -411,14 +418,14 @@ private IObservable ResolveAuth(string userId) => private IObservable<(StaticRepoImportResult Result, string CommitSha)> FetchAndImport( string repoUrl, string commitish, string? subdirectory, string token, string spaceId, - SyncIgnore ignore, Action? progress = null) + SyncIgnore ignore, Action? progress = null, ImportConflictPolicy? policy = null) { return repoClient.Fetch(repoUrl, commitish, subdirectory, token).SelectMany(snapshot => ParseSnapshot(snapshot, spaceId, ignore, progress).SelectMany(parsed => { var source = new InMemoryStaticRepoSource( spaceId, parsed.Children, parsed.Root, parsed.ContentSyncs); - return StaticRepoImporter.ImportSource(hub, source, logger) + return StaticRepoImporter.ImportSource(hub, source, logger, policy) .Select(result => (result, snapshot.CommitSha)); })); } @@ -636,7 +643,8 @@ public IObservable EnsureConfigNode(string spacePath, string? sourceId public IObservable SaveConfig( string spacePath, string? repositoryUrl, string branch, string? subdirectory, bool createBranchIfMissing, bool createRepoIfMissing, - SyncDirection direction = SyncDirection.Bidirectional, string? sourceId = null) + SyncDirection direction = SyncDirection.Bidirectional, string? sourceId = null, + bool twoWay = false) => UpdateConfig(spacePath, c => c with { RepositoryUrl = string.IsNullOrWhiteSpace(repositoryUrl) ? null : repositoryUrl.Trim(), @@ -645,6 +653,7 @@ public IObservable SaveConfig( CreateBranchIfMissing = createBranchIfMissing, CreateRepoIfMissing = createRepoIfMissing, Direction = direction, + TwoWay = twoWay, }, sourceId); /// Read-modify-write a config field (current value from the synced query; null when absent). diff --git a/src/MeshWeaver.GitSync/PullRequestService.cs b/src/MeshWeaver.GitSync/PullRequestService.cs index 8bc7776fa..3cccd4373 100644 --- a/src/MeshWeaver.GitSync/PullRequestService.cs +++ b/src/MeshWeaver.GitSync/PullRequestService.cs @@ -104,10 +104,12 @@ public IObservable CreateBranch( /// the checkout operation: the working Space is brought to the latest repo state. /// (when the caller runs as an activity: ctx.Log) /// receives per-file import problems so they land on the activity log. + /// ignores the source's two-way setting and overwrites local + /// edits from the repo (deliberate discard). /// public IObservable UpdateToLatest( string spacePath, string userId, string? sourceId = null, - Action? progress = null) + Action? progress = null, bool force = false) { return sync.ReadConfig(spacePath, sourceId).Take(1).SelectMany(config => { @@ -115,8 +117,8 @@ public IObservable UpdateToLatest( return Observable.Throw(new InvalidOperationException( "No GitHub repository configured for this Space.")); var branch = string.IsNullOrWhiteSpace(config.Branch) ? "main" : config.Branch; - logger?.LogInformation("Updating {Space} to latest on {Branch}", spacePath, branch); - return sync.ReimportAtCommit(spacePath, branch, userId, sourceId, progress); + logger?.LogInformation("Updating {Space} to latest on {Branch} (force={Force})", spacePath, branch, force); + return sync.ReimportAtCommit(spacePath, branch, userId, sourceId, progress, force); }); } diff --git a/src/MeshWeaver.Graph/StaticRepoImporter.cs b/src/MeshWeaver.Graph/StaticRepoImporter.cs index 81dfd33aa..70bc72168 100644 --- a/src/MeshWeaver.Graph/StaticRepoImporter.cs +++ b/src/MeshWeaver.Graph/StaticRepoImporter.cs @@ -17,6 +17,31 @@ namespace MeshWeaver.Graph; /// Outcome of a run. public sealed record StaticRepoImportResult(string Partition, string Fingerprint, string Outcome, int Count = 0); +/// +/// Conflict policy for an import that reconciles against a LIVE partition — the GitHub +/// "Update to latest" path. Git-first by default (): the repo overwrites the +/// live node and prunes extras. Two-way () instead PRESERVES a node +/// that was changed on the server since the last sync, so a local edit made between syncs is carried +/// back to the repo on the next commit rather than silently overwritten — "newer on the server wins". +/// A import ignores the policy and overwrites regardless (the deliberate-discard +/// escape hatch). Boot / built-in static-repo imports pass no policy → . +/// +public sealed record ImportConflictPolicy(bool PreserveServerNewer, DateTimeOffset? Since, bool Force = false) +{ + /// The git-first default: the repo is authoritative; local edits are overwritten/pruned. + public static readonly ImportConflictPolicy GitFirst = new(false, null, false); + + /// + /// True when is a live node changed on the SERVER since the last sync + /// () and must therefore be PRESERVED (not overwritten, not pruned) — unless + /// this is a import. Requires a recorded : with no sync + /// baseline there is nothing to protect, so a first import stays git-first. Pure — unit-testable. + /// + public bool PreservesServerCopyOf(MeshNode? target) => + PreserveServerNewer && !Force && Since is { } since + && target is not null && target.LastModified > since; +} + /// /// Materializes an into its partition through the canonical /// create pipeline — content + prerender — tracked as a content-addressed Activity and @@ -275,12 +300,13 @@ private static IObservable ReconcileSourceOwnedPartition /// this for the content. Reactive — Subscribe to run. /// public static IObservable ImportSource( - IMessageHub meshHub, IStaticRepoSource source, ILogger? logger = null) + IMessageHub meshHub, IStaticRepoSource source, ILogger? logger = null, + ImportConflictPolicy? policy = null) { logger ??= meshHub.ServiceProvider.GetService() ?.CreateLogger("MeshWeaver.Graph.StaticRepoImporter"); var importHub = CreateImportHub(meshHub, logger); - return Import(importHub, source, logger); + return Import(importHub, source, logger, policy: policy); } /// @@ -345,7 +371,7 @@ private static IMessageHub CreateImportHub(IMessageHub meshHub, ILogger? logger) /// An observable that emits the import outcome for the partition. public static IObservable Import( IMessageHub hub, IStaticRepoSource source, ILogger? logger = null, - PartitionSyncMode? syncModeOverride = null) + PartitionSyncMode? syncModeOverride = null, ImportConflictPolicy? policy = null) { logger ??= hub.ServiceProvider.GetService() ?.CreateLogger("MeshWeaver.Graph.StaticRepoImporter"); @@ -435,7 +461,7 @@ IObservable Reimport() } }; return Upsert(hub, activityNode) - .SelectMany(_ => Run(hub, source, nodes, root, activityPath, fingerprint, syncMode, logger)) + .SelectMany(_ => Run(hub, source, nodes, root, activityPath, fingerprint, syncMode, logger, policy)) .Catch(ex => { logger?.LogWarning(ex, @@ -450,9 +476,26 @@ IObservable Reimport() }); } - // No Succeeded marker → import. - if (existing?.Content is not ActivityLog { Status: ActivityStatus.Succeeded }) + // No Succeeded marker → import. 🚨 Read the marker through the TOLERANT ContentAs, NEVER + // a raw `existing.Content is ActivityLog {…}` pattern: a node read back from storage/query + // carries its Content as a JsonElement, not the typed record, so the pattern-match fails + // on a genuine Succeeded marker and re-imports every single time. ContentAs recovers the + // degraded JsonElement (typed → as-is, JsonElement → deserialized, else null + logged). + var existingLog = existing?.ContentAs(hub.JsonSerializerOptions, logger); + if (existingLog is not { Status: ActivityStatus.Succeeded }) + return Reimport(); + + // A FORCED import must re-apply even when the content fingerprint is unchanged: its + // purpose is to overwrite/prune local edits back to the (possibly identical) repo + // state, which the content-skip short-circuit below would otherwise bypass — leaving + // the very local changes the caller asked to discard. Force always re-runs. + if (policy?.Force == true) + { + logger?.LogInformation( + "[StaticRepoImport] {Partition}: forced re-import at unchanged fingerprint {Fingerprint}.", + source.Partition, fingerprint); return Reimport(); + } // 🚨 SELF-HEAL CONTENT, not just governance. A Succeeded marker means "imported once" — // but the content NODES can be dropped after the fact (a cross-partition prune, a manual @@ -483,7 +526,8 @@ IObservable Reimport() private static IObservable Run( IMessageHub hub, IStaticRepoSource source, IReadOnlyList nodes, - MeshNode root, string activityPath, string fingerprint, PartitionSyncMode syncMode, ILogger? logger) + MeshNode root, string activityPath, string fingerprint, PartitionSyncMode syncMode, ILogger? logger, + ImportConflictPolicy? policy = null) { var meshService = hub.ServiceProvider.GetRequiredService(); NodeTypeCompilationActivity.AppendLog( @@ -578,7 +622,7 @@ bool IsClaimed(string path, MeshNode? target) => { logger?.LogDebug( "[StaticRepoImport] {Partition}: skip claimed {Path}", source.Partition, path); - return Observable.Return((Imported: 0, Failed: 0)); + return Observable.Return((Imported: 0, Failed: 0, Preserved: 0)); } // Incremental skip: unchanged since the last import (same source token) AND the @@ -592,7 +636,22 @@ bool IsClaimed(string path, MeshNode? target) => { logger?.LogDebug( "[StaticRepoImport] {Partition}: unchanged, skipping {Path}", source.Partition, path); - return Observable.Return((Imported: 0, Failed: 0)); + return Observable.Return((Imported: 0, Failed: 0, Preserved: 0)); + } + + // Two-way conflict resolution: a node changed on the SERVER since the last sync + // is NOT overwritten by the repo — it is preserved here and carried back to + // GitHub on the next commit ("newer on the server wins → GitHub"). Only reached + // once the repo's copy actually differs (past the incremental-skip above), i.e. a + // real conflict. A forced import ignores this and overwrites (deliberate discard). + if (policy?.PreservesServerCopyOf(target) == true) + { + logger?.LogInformation( + "[StaticRepoImport] {Partition}: two-way — preserving server-newer {Path} (not overwritten).", + source.Partition, path); + NodeTypeCompilationActivity.AppendLog(hub, activityPath, + $"↩ Kept local change to {path} (newer on the server — commit to sync it back).", logger!); + return Observable.Return((Imported: 0, Failed: 0, Preserved: 1)); } var materialized = Materialize(sourceNode); @@ -611,8 +670,8 @@ bool IsClaimed(string path, MeshNode? target) => // Failed and continue. The Failed tally drives the terminal Warning status // below — the activity never reports a green Succeeded while hiding failures. return Upsert(hub, materialized) - .Select(_ => (Imported: 1, Failed: 0)) - .Catch<(int Imported, int Failed), Exception>(ex => + .Select(_ => (Imported: 1, Failed: 0, Preserved: 0)) + .Catch<(int Imported, int Failed, int Preserved), Exception>(ex => { logger?.LogWarning(ex, "[StaticRepoImport] {Partition}: upsert of {Path} failed (continuing).", @@ -620,13 +679,13 @@ bool IsClaimed(string path, MeshNode? target) => NodeTypeCompilationActivity.AppendLog(hub, activityPath, $"⚠ Failed to import {path}: {ex.Message}", logger!, Microsoft.Extensions.Logging.LogLevel.Warning); - return Observable.Return((Imported: 0, Failed: 1)); + return Observable.Return((Imported: 0, Failed: 1, Preserved: 0)); }); }) .ToObservable() .Merge(BatchSize) - .Aggregate((Imported: 0, Failed: 0), - (acc, x) => (acc.Imported + x.Imported, acc.Failed + x.Failed)); + .Aggregate((Imported: 0, Failed: 0, Preserved: 0), + (acc, x) => (acc.Imported + x.Imported, acc.Failed + x.Failed, acc.Preserved + x.Preserved)); return upserted.SelectMany(count => { @@ -639,6 +698,12 @@ bool IsClaimed(string path, MeshNode? target) => var toPrune = ComputePrunableNodes( existing.Values, nodes.Select(n => n.Path), manifest.Keys, excludedRoots, syncMode); + // Two-way: never prune a node CREATED/changed on the server since the last sync. It + // isn't in the repo yet, but it's a local addition to be committed back — not a stale + // extra to remove. A forced import prunes it (the repo state wins). + if (policy is { PreserveServerNewer: true, Force: false, Since: { } pruneSince }) + toPrune = toPrune.Where(n => n.LastModified <= pruneSince).ToArray(); + var pruned = toPrune.Count == 0 ? Observable.Return(0) : toPrune @@ -677,9 +742,11 @@ bool IsClaimed(string path, MeshNode? target) => // full diagnostic log (no torn "Succeeded but the ⚠ lines didn't land yet"). var failed = count.Failed; var status = failed > 0 ? ActivityStatus.Warning : ActivityStatus.Succeeded; + // Two-way preserved local edits (kept, not overwritten) noted only when any. + var preservedNote = count.Preserved > 0 ? $", kept {count.Preserved} local edit(s)" : ""; var summary = failed > 0 - ? $"Imported {count.Imported} node(s), {failed} FAILED (see ⚠ above), pruned {prunedCount}, synced {contentCount} content file(s)." - : $"Imported {count.Imported} node(s), pruned {prunedCount}, synced {contentCount} content file(s)."; + ? $"Imported {count.Imported} node(s), {failed} FAILED (see ⚠ above){preservedNote}, pruned {prunedCount}, synced {contentCount} content file(s)." + : $"Imported {count.Imported} node(s){preservedNote}, pruned {prunedCount}, synced {contentCount} content file(s)."; NodeTypeCompilationActivity.Complete(hub, activityPath, status, new[] { diff --git a/src/MeshWeaver.Mcp/McpMeshPlugin.cs b/src/MeshWeaver.Mcp/McpMeshPlugin.cs index 81885e48e..82007dd22 100644 --- a/src/MeshWeaver.Mcp/McpMeshPlugin.cs +++ b/src/MeshWeaver.Mcp/McpMeshPlugin.cs @@ -435,11 +435,14 @@ public Task Sync( • update — pull the branch HEAD from GitHub and import the deltas back into the Space. • check — ask GitHub (live) for the branch HEAD and whether the Space is up to date. -Runs under YOUR identity (needs Editor/Update on the Space for commit) and requires the Space's GitHub sync config (`{space}/_GitSync`) to already exist — configure it once in the Space's GitHub settings tab. Each op runs as an Activity: this returns the activity path immediately (it does NOT wait for the push to finish). Observe progress + result with `get` on that path — the activity log carries the commit sha and files written, and its Status goes Succeeded/Failed. `sourceId` selects a specific sync source (blank = the primary).")] +Runs under YOUR identity (needs Editor/Update on the Space for commit) and requires the Space's GitHub sync config (`{space}/_GitSync`) to already exist — configure it once in the Space's GitHub settings tab. Each op runs as an Activity: this returns the activity path immediately (it does NOT wait for the push to finish). Observe progress + result with `get` on that path — the activity log carries the commit sha and files written, and its Status goes Succeeded/Failed. `sourceId` selects a specific sync source (blank = the primary). + +When the sync source has two-way enabled, `update` never overwrites a node changed on the server since the last sync (it is kept and carried back on the next `commit`); pass `force: true` to overwrite local changes from the repo regardless. `force` only affects `update`.")] public Task GitHubSync( [Description("The Space path/id to sync (e.g. 'ACME'). GitHub sync acts on the containing top-level Space.")] string space, [Description("Operation: 'commit' (default, Space → GitHub), 'update' (GitHub → Space), or 'check' (compare, read-only).")] string op = "commit", - [Description("The sync source id to act on (blank = the Space's primary GitHub source).")] string? sourceId = null) + [Description("The sync source id to act on (blank = the Space's primary GitHub source).")] string? sourceId = null, + [Description("Force 'update' to overwrite nodes changed on the server since the last sync (ignore two-way). Only affects 'update'. Default: false.")] bool force = false) { space = space?.Trim().Trim('/') ?? ""; if (space.Length == 0) @@ -507,7 +510,7 @@ void OnCreated(string activityPath) => tcs.TrySetResult(JsonSerializer.Serialize IObservable action = operation switch { "commit" => rootHub.CommitToGitHub(spacePath, userId, OnCreated, normalizedSource), - "update" => rootHub.UpdateToLatestFromGitHub(spacePath, userId, OnCreated, normalizedSource), + "update" => rootHub.UpdateToLatestFromGitHub(spacePath, userId, OnCreated, normalizedSource, force), _ => rootHub.CheckBranchStateOnGitHub(spacePath, userId, OnCreated, normalizedSource), }; opRun.Disposable = action diff --git a/test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs b/test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs new file mode 100644 index 000000000..b06eca2da --- /dev/null +++ b/test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs @@ -0,0 +1,52 @@ +using System; +using System.Reactive.Linq; +using System.Reactive.Threading.Tasks; +using MeshWeaver.GitSync; +using MeshWeaver.Mesh; +using Xunit; + +namespace MeshWeaver.GitSync.Test; + +/// +/// Two-way sync: an "Update to latest" (GitHub → mesh) must NOT clobber changes made on the server +/// since the last sync — the whole point of two-way. A node added/edited on the server after the +/// last sync is preserved by an update (to be carried back on the next commit), and only a FORCED +/// update discards it to the repo state. Git-first (two-way off) still prunes/overwrites — covered by +/// . +/// +public class TwoWaySyncTest(ITestOutputHelper output) : GitHubSyncTestBase(output) +{ + [Fact(Timeout = 120000)] + public async Task TwoWay_Update_KeepsServerAddition_UnlessForced() + { + await Connect(); + var a = "GhW" + Guid.NewGuid().ToString("N")[..8]; + await CreateSpace(a, "Space W"); + await CreateMarkdown($"{a}/Welcome", "Welcome", "# Welcome\n\nv1."); + + var repo = "https://github.com/test/space-w"; + await Sync.SaveConfig(a, repo, "main", subdirectory: null, + createBranchIfMissing: true, createRepoIfMissing: true, + direction: SyncDirection.Bidirectional, sourceId: null, twoWay: true) + .Timeout(30.Seconds()).ToTask(); + + var c1 = await Sync.SyncToGitHub(a, UserId).Timeout(60.Seconds()).ToTask(); // repo: Welcome only + // The sync baseline (LastSyncedAt) MUST be recorded before the local edit so the edit counts + // as "newer on the server" — otherwise there is no baseline to compare against. + await WaitForConfig(a, c => c.LastSyncedAt != null && c.LastSyncCommitSha == c1.CommitSha); + + // A user adds a node on the SERVER after the last sync — it is not in the repo at HEAD. + await CreateMarkdown($"{a}/ServerOnly", "ServerOnly", "# ServerOnly\n\nadded on the server."); + + // 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")); + + // FORCE update: ignore two-way — the repo state wins and the server addition is discarded. + await Sync.ReimportAtCommit(a, "main", UserId, force: true).Timeout(90.Seconds()).ToTask(); + Assert.True(await IsAbsent($"{a}/ServerOnly")); + Assert.NotNull(await WaitForNode($"{a}/Welcome")); + } +} diff --git a/test/MeshWeaver.Graph.Test/ImportConflictPolicyTest.cs b/test/MeshWeaver.Graph.Test/ImportConflictPolicyTest.cs new file mode 100644 index 000000000..6c216116e --- /dev/null +++ b/test/MeshWeaver.Graph.Test/ImportConflictPolicyTest.cs @@ -0,0 +1,65 @@ +using System; +using MeshWeaver.Graph; +using MeshWeaver.Mesh; +using Xunit; + +namespace MeshWeaver.Graph.Test; + +/// +/// Unit tests for the two-way import conflict DECISION +/// () — "is this live node a local edit made +/// since the last sync that must be preserved rather than overwritten?". Pure + deterministic — no +/// database. The git-first default never preserves; two-way preserves a node newer than the sync +/// baseline; force always overwrites. +/// +public class ImportConflictPolicyTest +{ + private static readonly DateTimeOffset LastSync = new(2026, 07, 14, 00, 00, 00, TimeSpan.Zero); + private static MeshNode NodeModifiedAt(DateTimeOffset when) => + MeshNode.FromPath("Space/node") with { LastModified = when }; + + [Fact] + public void GitFirst_NeverPreserves() + { + var target = NodeModifiedAt(LastSync.AddHours(1)); // newer on server, but git-first + ImportConflictPolicy.GitFirst.PreservesServerCopyOf(target).Should().BeFalse(); + } + + [Fact] + public void TwoWay_PreservesNodeChangedAfterLastSync() + { + var policy = new ImportConflictPolicy(PreserveServerNewer: true, Since: LastSync); + policy.PreservesServerCopyOf(NodeModifiedAt(LastSync.AddSeconds(1))).Should().BeTrue(); + } + + [Fact] + public void TwoWay_DoesNotPreserveNodeUnchangedSinceLastSync() + { + var policy = new ImportConflictPolicy(PreserveServerNewer: true, Since: LastSync); + // Not touched on the server since the last sync → the repo is free to update it. + policy.PreservesServerCopyOf(NodeModifiedAt(LastSync.AddSeconds(-1))).Should().BeFalse(); + } + + [Fact] + public void Force_OverridesTwoWay_NeverPreserves() + { + var policy = new ImportConflictPolicy(PreserveServerNewer: true, Since: LastSync, Force: true); + policy.PreservesServerCopyOf(NodeModifiedAt(LastSync.AddHours(1))).Should().BeFalse(); + } + + [Fact] + public void TwoWay_WithNoSyncBaseline_DoesNotPreserve() + { + // No LastSyncedAt recorded yet (first sync) → nothing to protect; stays git-first. + var policy = new ImportConflictPolicy(PreserveServerNewer: true, Since: null); + policy.PreservesServerCopyOf(NodeModifiedAt(LastSync.AddHours(1))).Should().BeFalse(); + } + + [Fact] + public void TwoWay_GitOnlyNode_NotPreserved() + { + // A node present only in the repo (no live target) is a new addition to import, not a local edit. + var policy = new ImportConflictPolicy(PreserveServerNewer: true, Since: LastSync); + policy.PreservesServerCopyOf(null).Should().BeFalse(); + } +} From 88fb1e8778b14d5e1c42ee2261aed81a485a9f95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Tue, 14 Jul 2026 07:37:51 +0200 Subject: [PATCH 2/4] =?UTF-8?q?Docs:=20What's=20New=20=E2=80=94=20two-way?= =?UTF-8?q?=20GitHub=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- .../WhatsNew/2026-07-14-gitsync-two-way.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 src/MeshWeaver.Documentation/Data/WhatsNew/2026-07-14-gitsync-two-way.md diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-07-14-gitsync-two-way.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-07-14-gitsync-two-way.md new file mode 100644 index 000000000..2b1265313 --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-07-14-gitsync-two-way.md @@ -0,0 +1,19 @@ +--- +Name: Two-way GitHub sync — your edits are kept, not overwritten +Category: What's New +Description: A GitHub sync source can now keep changes made on the server since the last sync, instead of overwriting them on update. +Icon: Sparkle +--- + +# Two-way GitHub sync + +GitHub sync used to be strictly git-first: an **Update to latest** overwrote (and pruned) the live +Space from the repository, silently discarding edits you'd made on the server between syncs. + +Turn on **Two-way** on a sync source and an update now keeps any node changed on the server since +the last sync — it is preserved, not overwritten or pruned, and is carried back to GitHub on your +next **Commit** ("newer on the server wins"). Nodes the server hasn't touched still update from the +repo as before. + +Need the old behavior for a one-off? A **forced** update (`force`) overwrites local changes from the +repository regardless — the deliberate way to discard server edits and match the repo exactly. From ef8b64fdf2c08d2cb40373d5a756fe164ecaa67e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Tue, 14 Jul 2026 08:06:22 +0200 Subject: [PATCH 3/4] test: GitHubSyncConfig editable-field order includes twoWay (after direction) Co-Authored-By: Claude Opus 4.8 (1M context) --- test/MeshWeaver.GitSync.Test/MeshNodeContentEditorTest.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/MeshWeaver.GitSync.Test/MeshNodeContentEditorTest.cs b/test/MeshWeaver.GitSync.Test/MeshNodeContentEditorTest.cs index 174ea467c..6790a7e08 100644 --- a/test/MeshWeaver.GitSync.Test/MeshNodeContentEditorTest.cs +++ b/test/MeshWeaver.GitSync.Test/MeshNodeContentEditorTest.cs @@ -24,9 +24,10 @@ public void FromType_reflects_editable_fields_and_hides_browsable_false() var fields = MeshNodeEditorField.FromType(typeof(GitHubSyncConfig)); var keys = fields.Select(f => f.Key).ToArray(); - // The six editable properties, by camelCase key — and NOT the [Browsable(false)] last-sync fields. + // The editable properties, by camelCase key — and NOT the [Browsable(false)] last-sync fields. + // `twoWay` sits right after `direction` (its declaration order in GitHubSyncConfig). Assert.Equal( - new[] { "repositoryUrl", "branch", "subdirectory", "direction", "createBranchIfMissing", "createRepoIfMissing", "ignore" }, + new[] { "repositoryUrl", "branch", "subdirectory", "direction", "twoWay", "createBranchIfMissing", "createRepoIfMissing", "ignore" }, keys); Assert.DoesNotContain("lastSyncedAt", keys); Assert.DoesNotContain("lastSyncCommitSha", keys); From 4a1e3d310dc0e950e9c2c7c556afc7f4c064369a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Tue, 14 Jul 2026 08:10:31 +0200 Subject: [PATCH 4/4] GitSync two-way: don't advance the sync baseline when local edits were preserved MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/MeshWeaver.GitSync/GitHubSyncService.cs | 11 ++++++++++- src/MeshWeaver.Graph/StaticRepoImporter.cs | 9 ++++++--- test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs | 6 ++++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/MeshWeaver.GitSync/GitHubSyncService.cs b/src/MeshWeaver.GitSync/GitHubSyncService.cs index 187645b18..cc758bf2d 100644 --- a/src/MeshWeaver.GitSync/GitHubSyncService.cs +++ b/src/MeshWeaver.GitSync/GitHubSyncService.cs @@ -364,7 +364,16 @@ public IObservable ReimportAtCommit( spacePath, commitish, config.TwoWay, force); return FetchAndImport(repoUrl, commitish, config.Subdirectory, token, spacePath, SyncIgnore.For(config), progress, policy) - .SelectMany(x => RecordLastSync(spacePath, x.CommitSha, sourceId).Select(_ => x.Result)); + // 🚨 Only advance the last-sync baseline when the mesh is now IN SYNC with the repo. + // A two-way import that PRESERVED server-newer nodes leaves the mesh AHEAD of the repo + // (those edits aren't committed back yet); advancing LastSyncedAt would move the + // baseline past them, so the NEXT update would no longer see them as "newer on the + // server" and would overwrite them — losing the very edits two-way just protected. + // Leave the baseline until a commit carries them back (which advances it). A clean + // import (nothing preserved — always the case git-first) records normally. + .SelectMany(x => x.Result.Preserved > 0 + ? Observable.Return(x.Result) + : RecordLastSync(spacePath, x.CommitSha, sourceId).Select(_ => x.Result)); }); }); } diff --git a/src/MeshWeaver.Graph/StaticRepoImporter.cs b/src/MeshWeaver.Graph/StaticRepoImporter.cs index 70bc72168..35448b1a5 100644 --- a/src/MeshWeaver.Graph/StaticRepoImporter.cs +++ b/src/MeshWeaver.Graph/StaticRepoImporter.cs @@ -14,8 +14,11 @@ namespace MeshWeaver.Graph; -/// Outcome of a run. -public sealed record StaticRepoImportResult(string Partition, string Fingerprint, string Outcome, int Count = 0); +/// Outcome of a run. is +/// the number of live nodes a two-way import kept because they were newer on the server (see +/// ) — non-zero means the mesh is now AHEAD of the repo, so the +/// caller must NOT advance its last-sync baseline until those edits are committed back. +public sealed record StaticRepoImportResult(string Partition, string Fingerprint, string Outcome, int Count = 0, int Preserved = 0); /// /// Conflict policy for an import that reconciles against a LIVE partition — the GitHub @@ -760,7 +763,7 @@ bool IsClaimed(string path, MeshNode? target) => "[StaticRepoImport] {Partition}: imported {Count}, failed {Failed}, pruned {Pruned}, content {Content} at {Fingerprint}.", source.Partition, count.Imported, failed, prunedCount, contentCount, fingerprint); return new StaticRepoImportResult(source.Partition, fingerprint, - failed > 0 ? "ImportedWithErrors" : "Imported", count.Imported); + failed > 0 ? "ImportedWithErrors" : "Imported", count.Imported, count.Preserved); }); }))); }); diff --git a/test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs b/test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs index b06eca2da..4df6e655a 100644 --- a/test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs +++ b/test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs @@ -44,6 +44,12 @@ await Sync.SaveConfig(a, repo, "main", subdirectory: null, Assert.NotNull(await WaitForNode($"{a}/ServerOnly")); Assert.NotNull(await WaitForNode($"{a}/Welcome")); + // A SECOND two-way update MUST still keep it: preserving a node must not advance the sync + // baseline (LastSyncedAt) past it, or the next update would stop seeing it as newer-on-server + // and overwrite it — losing the edit one cycle later. Regression guard for that baseline bug. + await Sync.ReimportAtCommit(a, "main", UserId).Timeout(90.Seconds()).ToTask(); + Assert.NotNull(await WaitForNode($"{a}/ServerOnly")); + // FORCE update: ignore two-way — the repo state wins and the server addition is discarded. await Sync.ReimportAtCommit(a, "main", UserId, force: true).Timeout(90.Seconds()).ToTask(); Assert.True(await IsAbsent($"{a}/ServerOnly"));