Stop a non-converging sync source re-cloning its repository on every delivery (#3945)

src/MeshWeaver.Documentation/Data/Architecture.md

@@ -160,7 +160,7 @@ Each theme starts with its introductory page, followed by related architecture t
 - [DatabaseBackups](DatabaseBackups)
 - [Declarative export and import](DeclarativeImportExport)
 - [Syncing a Space with GitHub](GitHubSync)
-- [What a Green Build Costs a Synced Space](GitSyncTriggerCost) — one field decides whether a delivery is free or a full clone, and it is deliberately frozen while an import does not converge; so a source that cannot converge re-clones on every green build, at the source repository's CI cadence
+- [What a Green Build Costs a Synced Space](GitSyncTriggerCost) — one field decided whether a delivery was free or a full clone, and it is deliberately frozen while an import does not converge; the second, weaker pointer that makes a settled source free again, and why the skip needs the verdict to be FINAL and not merely recorded
 - [The Import Marker Records Convergence](ImportMarkerRecordsConvergence)
 - [Instance Sync — bi-directional space replication between MeshWeaver instances](InstanceSync)
 - [Managing Partition Sync (Admin Guide)](PartitionSyncGuide)

src/MeshWeaver.Documentation/Data/Architecture/GitHubSync.md

@@ -173,16 +173,17 @@ regardless — use it to deliberately discard local changes back to the reposito
 > Two-way generalizes this to *every* server-side edit (no explicit claim needed) as long as it
 > post-dates the last sync.
 
-### The three clocks on a sync source — and why they disagree on purpose
+### The four facts on a sync source — and why they disagree on purpose
 
-A sync source records **three separate facts**, and reading any one of them as the answer to
+A sync source records **four separate facts**, and reading any one of them as the answer to
 another's question is how an investigation goes wrong. They are deliberately independent:
 
 | Field | The question it answers | When it moves |
 |---|---|---|
 | `lastSyncAttemptAt` + `lastSyncOutcome` | *When did a sync last RUN here, and what did it conclude?* | **Every** conclusion — an import, a no-op, one that preserved server-side edits, one that landed nothing. |
 | `lastSyncCommitSha` | *Which repo commit has this Space already got?* | Whenever the mesh genuinely reached that commit — **including** a no-op update, so a repo commit touching no node files does not leave the Space forever "behind". |
 | `lastSyncedAt` | *When were mesh and repo last RECONCILED?* — the two-way **conflict horizon** | Only on an import that really reconciled: **not** on a fingerprint-matched no-op, **not** when server-newer nodes were preserved, **not** when something failed to land. |
+| `lastAttemptedCommitSha` + `lastAttemptWasFinal` | *Have we already LOOKED at exactly these bytes, and could looking again change the answer?* | On every import conclusion; **cleared** by an export and by a hold. This is the pair that makes a green build free for a source that cannot converge — see [What a Green Build Costs a Synced Space](/Doc/Architecture/GitSyncTriggerCost). |
 
 The horizon is the one with teeth. Everything newer than it counts as a pending server-side change
 and is protected from overwrite and from the prune, so advancing it past uncommitted work disarms
@@ -199,6 +200,12 @@ sync"**.
 > registry. `lastSyncAttemptAt` is that fact, and the settings tab now shows the three separately
 > instead of printing the horizon under the words *"Last synced"*.
 
+> 🚨 **And a frozen `lastSyncAttemptAt` is not necessarily a dead webhook.** A source whose last
+> attempt reached a FINAL verdict at a commit is deliberately skipped for every later delivery of
+> that same commit, so its recency stamp stops moving until the repository produces a new one.
+> The settings tab says so in as many words — *"this commit has a final verdict — the next new
+> commit re-attempts"* — precisely so the stopped clock cannot be read as a stopped delivery.
+
 Note that a node's own `lastModified` is **not** a substitute: `stream.Update` does not re-stamp
 it, so a node can be rewritten without its modification time moving.
 

src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-10-a-stuck-sync-stops-re-downloading-its-repository.md

@@ -0,0 +1,33 @@
+---
+Name: A stuck sync stops re-downloading its repository
+Category: Fix
+Description: A GitHub-synced Space that cannot finish an import no longer re-downloads the whole repository on every green build of that repository — and the settings tab now says why it has stopped trying.
+Icon: ArrowSync
+Order: -20260910
+---
+
+A synced Space imports whenever the repository it tracks builds green. That delivery was free only
+when the Space had already fully reached the built commit — and a Space that *cannot* fully reach
+it (a node kept back because it was edited here, a file the mesh refuses) was, by exactly that
+rule, the one that never qualified. So it downloaded the entire repository again on every green
+build, at the repository's build cadence rather than at any rate of its own. On one live Space that
+had been running for a month; on a busy repository it is roughly ten downloads an hour, most of
+them for a commit that had already been looked at.
+
+Now the source remembers **which commit it last looked at** separately from **which commit it has
+actually got**. When the last attempt ended in a verdict that reading the same files again cannot
+change, later builds of that same commit are skipped outright — no download, no re-import. The next
+new commit still brings a full import, and **Update to latest** still works exactly as before.
+
+Sync sources that are healthy are unaffected: they were already skipping those deliveries.
+
+Two things a Space admin will notice on the **GitHub Sync** settings tab. Repeated import activity
+records stop piling up for a Space that is stuck. And when a source has settled, the line under the
+repository now reads *"this commit has a final verdict — the next new commit re-attempts"*, so a
+last-run time that has stopped moving reads as a deliberate skip rather than as a delivery that
+never arrived.
+
+Nothing was made less careful. A failure that might not happen again — a store that was briefly
+unreachable, an import that faulted outright — is still retried on the very next delivery, because
+freezing a Space out over a passing problem is the more expensive mistake. Only a verdict about the
+files themselves settles.

src/MeshWeaver.GitSync/GitHubSyncConfig.cs

@@ -169,6 +169,52 @@ public record GitHubSyncConfig
     [Browsable(false)]
     public string? LastSyncOutcome { get; init; }
 
+    /// <summary>
+    /// 🚨 <b>The commit the last IMPORT ATTEMPT targeted — which is NOT
+    /// <see cref="LastSyncCommitSha"/></b> (issue #3945). That field means "the mesh is known to
+    /// hold this commit's content" and is deliberately held back whenever an import did not fully
+    /// converge (<c>GitHubSyncService.MayAdvanceBaseline</c>); this one means only "we have already
+    /// looked at exactly these bytes". One field could not be both, which is why a source that can
+    /// never converge re-cloned its whole repository on every green build of the source repository
+    /// — at that repository's CI cadence, ~10/h on core.
+    ///
+    /// <para>Written on EVERY import conclusion, alongside
+    /// <see cref="LastAttemptWasFinal"/>. CLEARED (null) by an export/commit and by a hold, because
+    /// both change what a re-import would do: an export moves the conflict horizon, and a hold means
+    /// no attempt ran at all. Absent on a source that has never imported, and on one whose last
+    /// import predates this field — in both cases the next delivery attempts, which is the safe
+    /// direction.</para>
+    ///
+    /// <para>Set by the sync operation; not user-editable.</para>
+    /// </summary>
+    [Browsable(false)]
+    public string? LastAttemptedCommitSha { get; init; }
+
+    /// <summary>
+    /// 🚨 <b>Whether the attempt at <see cref="LastAttemptedCommitSha"/> reached a verdict that
+    /// re-attempting the SAME commit provably cannot change</b>
+    /// (<c>StaticRepoImportResult.VerdictIsFinal</c>) — the second half of the pair, and the half
+    /// that stops issue #3945's fix becoming a worse bug.
+    ///
+    /// <para>🚨 <b>Without it, the fix would strand every self-healing source.</b> Today's
+    /// re-clone-on-every-delivery is also, accidentally, the retry loop for the failures
+    /// <c>StaticRepoImporter.IsContentVerdict</c> deliberately refuses to call final — an
+    /// unreachable store, an owner that did not answer, a hub that went down mid-import. Skipping on
+    /// "same commit, already attempted" alone would make those wait for the next commit, which on an
+    /// idle repository is never. So the skip requires this flag too: a preserved node or an
+    /// all-content-verdict refusal settles (skip — the same bytes meet the same rules), while
+    /// <c>ImportedWithErrors</c>, a whole-import <c>Failed</c> and a truncated listing do not
+    /// (attempt — the next delivery may land it).</para>
+    ///
+    /// <para>🚨 <b>Never true without <see cref="LastAttemptedCommitSha"/>.</b> The two are written
+    /// in one <c>stream.Update</c> and mean nothing apart; <c>false</c> is the default and the safe
+    /// reading of an absent value.</para>
+    ///
+    /// <para>Set by the sync operation; not user-editable.</para>
+    /// </summary>
+    [Browsable(false)]
+    public bool LastAttemptWasFinal { get; init; }
+
     /// <summary>
     /// WHY the last attempt did not move the source — the hold reason of the sealed-publication
     /// gate ("built at X, not sealed for this instance …"), or the reconciler's finding — so an

src/MeshWeaver.GitSync/GitHubSyncService.cs

@@ -142,6 +142,11 @@ public IObservable<GitHubPushResult> SyncToGitHub(
                         // Record the commit by MERGING only the last-sync fields atop the latest
                         // node content (stream.Update read-modify-write) — never a full-content write,
                         // so a concurrent repo-field edit in the GUI editor is not clobbered.
+                        // 🚨 The attempt pair (#3945) is left to its default — i.e. CLEARED. An
+                        // export advances the conflict horizon, and the horizon is what decides
+                        // which live nodes an import preserves, so any "final at commit X" verdict
+                        // an earlier import recorded is stale the moment this lands: the next green
+                        // build must attempt again rather than skip on it.
                         return repoClient.Push(request).SelectMany(result =>
                             RecordSyncResult(spacePath, CommittedOutcome, result.CommitSha,
                                     advanceHorizon: true, sourceId)
@@ -482,7 +487,16 @@ private IObservable<StaticRepoImportResult> ReimportAtCommitCore(
                                 // while the mesh still has instances) is drift the sync cannot close
                                 // by itself — it is stated on the config, where the settings tab and
                                 // the status surface read it, not only in the activity log.
-                                note: HeldNote(x.Result))
+                                note: HeldNote(x.Result),
+                                // 🚨 #3945 — the SECOND, weaker pointer, and the whole reason it is
+                                // a second one. "We have already looked at exactly these bytes" is
+                                // not "the mesh holds this commit", and one field answering both is
+                                // what made a source that cannot converge re-clone its entire
+                                // repository on every green build of the source repository. This one
+                                // is stamped whatever the outcome; whether it may LICENCE a skip is
+                                // the flag beside it, never this sha alone.
+                                attemptedCommitSha: x.CommitSha,
+                                attemptWasFinal: x.Result.VerdictIsFinal)
                             .Select(_ => x.Result);
                     });
             });
@@ -515,6 +529,17 @@ private IObservable<StaticRepoImportResult> ReimportAtCommitCore(
     /// <para>A "Skipped" (fingerprint-matched no-op) outcome is NOT judged here — it is allowed
     /// through to <c>RecordSyncResult</c> with <c>advanceHorizon: false</c>, which advances the SEEN commit only and deliberately
     /// leaves the conflict horizon alone.</para>
+    ///
+    /// <para>🚨 <b>This is NOT the question "would attempting again change anything" (#3945), and it
+    /// must never be relaxed into it.</b> Holding the baseline is what keeps un-landed content
+    /// reachable; but because <c>GitHubWebhookProcessor.SkipReason</c> also read this same field to
+    /// decide whether a delivery was FREE, holding it made every later green build of the source
+    /// repository pay a full clone — 10/h on core, for as long as the source did not converge. That
+    /// second question now has its own pair of fields
+    /// (<see cref="GitHubSyncConfig.LastAttemptedCommitSha"/> +
+    /// <see cref="GitHubSyncConfig.LastAttemptWasFinal"/>), fed by
+    /// <see cref="StaticRepoImportResult.VerdictIsFinal"/>, so this one can stay exactly as
+    /// conservative as #675 / #677 / #2229 item C need it to be.</para>
     /// </summary>
     /// <param name="result">The import outcome to judge.</param>
     /// <returns><c>true</c> when the mesh is genuinely in sync with the repo at this commit.</returns>
@@ -984,7 +1009,7 @@ private IObservable<MeshNode> UpdateConfig(
     /// concurrent GUI edit of the repository fields is never clobbered, and re-writing an unchanged
     /// value costs nothing: the write travels as an RFC 7396 merge patch of what actually changed.
     ///
-    /// <para>🚨 <b>The three clocks are separate ON PURPOSE and must stay so.</b> Folding the horizon
+    /// <para>🚨 <b>The clocks are separate ON PURPOSE and must stay so.</b> Folding the horizon
     /// into the recency stamp is the one change that must never be made — it would advance the
     /// horizon on exactly the outcomes that suppress it (a fingerprint-matched no-op, an import that
     /// preserved server-newer nodes, one that landed nothing), moving it past pending uncommitted
@@ -1000,9 +1025,18 @@ private IObservable<MeshNode> UpdateConfig(
     /// never landed.</param>
     /// <param name="advanceHorizon">Whether this outcome RECONCILED mesh and repo.</param>
     /// <param name="sourceId">The sync source (null = the primary).</param>
+    /// <param name="note">The hold reason / finding to state on the config, or null to clear it.</param>
+    /// <param name="attemptedCommitSha">The commit an IMPORT attempt actually read, or <c>null</c>
+    /// to CLEAR the attempt pair — which every non-import conclusion does, because an export moves
+    /// the conflict horizon (so an earlier verdict at that commit is stale) and a hold means no
+    /// attempt ran at all (#3945).</param>
+    /// <param name="attemptWasFinal">Whether that attempt's verdict is final at that commit
+    /// (<see cref="StaticRepoImportResult.VerdictIsFinal"/>). Meaningless without
+    /// <paramref name="attemptedCommitSha"/>, and written in the same patch as it.</param>
     private IObservable<MeshNode> RecordSyncResult(
         string spacePath, string outcome, string? seenCommitSha, bool advanceHorizon,
-        string? sourceId = null, string? note = null)
+        string? sourceId = null, string? note = null,
+        string? attemptedCommitSha = null, bool attemptWasFinal = false)
     {
         var now = DateTimeOffset.UtcNow;
         return hub.GetWorkspace().GetMeshNodeStream(ConfigPath(spacePath, sourceId)).Update(node =>
@@ -1019,6 +1053,17 @@ private IObservable<MeshNode> RecordSyncResult(
                     // A hold's reason, or cleared by an attempt that ran: the note describes the
                     // LAST attempt only, never an older one.
                     LastSyncNote = note,
+                    // 🚨 #3945 — ALWAYS written, never merged with what was already there. Unlike
+                    // the baseline above (`?? cur.…`, a HIGH-WATER MARK that only ever advances)
+                    // this pair describes the LAST ATTEMPT, so an older value surviving a conclusion
+                    // that set none would licence a skip for an attempt that never happened. The
+                    // RFC 7396 diff a cross-hub `stream.Update` ships emits a key present in the
+                    // stored node and absent from the new content as `null` — an RFC 7396 REMOVE —
+                    // so passing null here genuinely clears the stored value.
+                    LastAttemptedCommitSha = attemptedCommitSha,
+                    // Never true on its own: the flag is only ever read beside the sha, and a true
+                    // with no sha would be a licence attached to no commit.
+                    LastAttemptWasFinal = attemptedCommitSha is { Length: > 0 } && attemptWasFinal,
                 },
             };
         });
@@ -1032,6 +1077,11 @@ private IObservable<MeshNode> RecordSyncResult(
     /// Records that this source was HELD from advancing, and why — onto the config, so the reason
     /// is visible where the outcome is (2026-09-08: a source held for hours showed only
     /// <c>Skipped</c>). Moves nothing else: not the commit, not the horizon. Cold.
+    ///
+    /// <para>🚨 It also CLEARS the attempt pair (#3945), by leaving it at its default. A hold means
+    /// no import ran, and a seal that has not arrived yet is the archetype of a condition that
+    /// clears without the commit moving — so a hold must never leave a "final at this commit"
+    /// licence standing for the delivery that follows it.</para>
     /// </summary>
     public IObservable<MeshNode> RecordHold(string spacePath, string? sourceId, string reason)
         => RecordSyncResult(spacePath, HeldOutcome, seenCommitSha: null, advanceHorizon: false,

src/MeshWeaver.GitSync/GitHubSyncSettingsTab.cs

@@ -594,6 +594,13 @@ cfg.LastSyncAttemptAt is { } at
                 ? Esc(LocalizationCatalog.Get("ui.gitSync.atCommit", locale))
                   + $" <span style=\"font-family:monospace;\">{Esc(sha)}</span>"
                 : null,
+            // 🚨 #3945 — a SETTLED source stops attempting, so without this line its recency stamp
+            // simply freezes and the reader has no way to tell "the webhook stopped arriving" from
+            // "the webhook arrives and is deliberately skipped". Rendered only in the settled state,
+            // which is exactly when the frozen dates need explaining.
+            cfg.LastAttemptWasFinal && cfg.LastAttemptedCommitSha is { Length: > 0 }
+                ? Esc(LocalizationCatalog.Get("ui.gitSync.settled", locale))
+                : null,
         };
         return $"<p style=\"{Style}\">{string.Join(" — ", parts.Where(x => x is not null))}</p>";
     }