Repository navigation
GitSync: no-op sync never advances the conflict horizon; bidirectional prune protects server additions (#677, #604) - #895
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens GitSync against two remaining server-side data-loss routes by (1) preventing no-op imports from advancing the two-way conflict horizon and (2) making prune behavior horizon-aware for bidirectional Spaces while improving prune auditability.
Changes:
- Split “seen commit” vs “reconciled horizon” semantics: skipped/no-op reimports now update
LastSyncCommitShawithout advancingLastSyncedAt. - Add prune-only protection for bidirectional Spaces (
PreserveServerAdditions) and surface pruned paths prominently (per-path + summaries). - Add/extend deterministic tests covering no-op horizon behavior, bidirectional prune protection, and forced-prune path reporting.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Graph.Test/ImportConflictPolicyTest.cs | Adds unit tests for prune-protection semantics (PreservesFromPruneOf) across modes and force/baseline edge cases. |
| test/MeshWeaver.GitSync.Test/TwoWaySyncTest.cs | Adds regression test ensuring a no-op update records seen commit without moving the conflict horizon, preventing later prune data loss. |
| test/MeshWeaver.GitSync.Test/BidirectionalPruneProtectionTest.cs | New integration tests for bidirectional prune protection, forced-prune auditing, and import-only mirror behavior. |
| src/MeshWeaver.Graph/StaticRepoImporter.cs | Introduces PreserveServerAdditions + PreservesFromPruneOf, counts preserved nodes correctly on skip paths, and logs pruned paths to activity + summary. |
| src/MeshWeaver.GitSync/GitHubSyncService.cs | Applies bidirectional prune-only protection and records seen commits on Skipped outcomes without advancing LastSyncedAt. |
| src/MeshWeaver.GitSync/GitHubSyncConfig.cs | Documents the clarified semantics of LastSyncedAt (conflict horizon) vs LastSyncCommitSha (seen commit marker). |
| src/MeshWeaver.GitSync/GitHubActivityExtensions.cs | Adds user-facing activity lines listing pruned node paths for update/reimport operations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
…l prune protects and names server additions Fixes #677, fixes #604 — the two remaining routes to GitSync server-side data loss after #675. #677: ReimportAtCommit advanced the sync baseline via RecordLastSync (both LastSyncedAt AND LastSyncCommitSha) whenever Preserved == 0 — including the fingerprint no-op ('Skipped'). LastSyncedAt is the conflict horizon behind PreservesServerCopyOf; advancing it without reconciling disarmed the protection, so a LATER push pruned server-side additions. The fields are now split by meaning: a Skipped update records only LastSyncCommitSha (the 'seen this commit' marker — up-to-date display and git-diff base stay correct, a node-free repo commit doesn't leave the space 'behind'); only an import that actually reconciled records the horizon. Diff-scope and unchanged-token skips now count a server-newer node as Preserved (the server-edited-node-whose-file-didn't-change shape). #604: the prune loop logged paths only on FAILURE and the summary carried a bare count — a successful prune was untraceable. And prune protection required TwoWay; a Bidirectional space without TwoWay inherited FullReplace and mirror-pruned uncommitted server additions. ImportConflictPolicy gains PreserveServerAdditions (prune-only protection, on for Bidirectional, reusing #675's kept-from-prune machinery); every prune logs its path and the activity summary names all pruned paths. ImportOnly mirrors keep FullReplace semantics — pinned by a guard test. Fail-without evidence (pre-fix src, new tests): horizon moved on a Skipped update; Bidirectional server addition pruned; no 'Pruned' activity line — 3 failed / mirror-guard passed. With the fix: GitSync.Test 157/157, Graph.Test 820/820, Release -warnaserror clean incl. the Memex.Portal.Monolith umbrella. Closes #677 Closes #604 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rbuergi
force-pushed
the
fix/677-604-gitsync-horizon
branch
from
August 7, 2026 08:14
ae43bee to
6087950
Compare
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.
Summary
Closes #677 and #604 — the two remaining routes to GitSync server-side data loss after #675.
#677 — split the baseline fields by meaning.
ReimportAtCommitadvancedLastSyncedAt(the conflict horizon behindPreservesServerCopyOf) on ANYPreserved == 0outcome — including the fingerprint no-op. That disarmed two-way protection, so a later push pruned server-side additions the sync had verified nothing about. Now: aSkippedupdate records onlyLastSyncCommitSha(seen-commit marker — up-to-date display and diff base stay correct; a node-free repo commit doesn't leave the space "behind"); only a reconciling import advances the horizon. Field semantics documented onGitHubSyncConfig.#604 — prune is loud and horizon-aware for bidirectional spaces. Successful prunes now log every path (server log +
🗑 Pruned {path}activity lines + names in the terminal summary).ImportConflictPolicy.PreserveServerAdditions(on forSyncDirection.Bidirectional, reusing #675's kept-from-prune machinery) stops the mirror-prune of uncommitted server additions;ImportOnlymirrors keep FullReplace semantics, pinned by a guard test.Verification
MeshWeaver.GitSync.Test157/157,MeshWeaver.Graph.Test820/820; independently re-ran the new classes (6/6).dotnet build -c Release -warnaserrorclean on Graph, GitSync, both test projects, and theMemex.Portal.Monolithumbrella.Known inherited residual (documented in the code): with a frozen horizon, import-written nodes are counted preserved on later skips until committed back — #675's documented false-conflict made visible, not created; the per-node import-write-time manifest remains #677's deferred follow-up.
No What's New entry: data-integrity bug fix.
🤖 Generated with Claude Code