Stop a non-converging sync source re-cloning its repository on every delivery (#3945) by rbuergi · Pull Request #3974 · Systemorph/MeshWeaver

rbuergi · 2026-09-10T19:56:36Z

The defect, in one sentence

GitHubWebhookProcessor.SkipReason made a green-build delivery free for exactly one reason —
lastSyncCommitSha == headSha — and GitHubSyncService.MayAdvanceBaseline deliberately holds
that same field whenever an import did not converge (Preserved > 0 #675/#677, Failed > 0 #2229
item C, outcome Failed). The cheapness gate and the convergence guard were the same field, so
a source that could never converge paid a full git fetch --depth 1 of its entire repository
plus a full parse on every delivery — at the source repository's CI cadence, not at any rate of
its own.

Measured 2026-09-10: Essentials/_GitSync on memex.meshweaver.cloud had been doing that against
MeshWeaver.Plugins since 2026-08-10 — 31 days, 436 commits — while its 33 siblings on the same
repository, the same webhook and the same schedule were skipped for free. On core the delivery rate
is ~10/h (254 green publish-signal runs in 24 h).

The fix — a second, weaker pointer

Question	Field
Is the mesh known to hold this commit's content?	`lastSyncCommitSha` — unchanged, still held by `MayAdvanceBaseline`
Have we already looked at exactly these bytes?	`lastAttemptedCommitSha` + `lastAttemptWasFinal` — new

StaticRepoImportResult.VerdictIsFinal — would running again at the same content change the
answer? Deliberately a different question from Converged (is the partition now equal to the
source?). Final: Preserved > 0 alone; ImportedWithContentErrors (every failure a content
verdict). Not final: ImportedWithErrors, outcome Failed, PruneRefused.
The pair is written on every import conclusion and CLEARED by an export (it advances the
conflict horizon, which decides which nodes get preserved) and by a hold (no attempt ran, and a
seal arriving later is the archetype of a condition that clears without the commit moving).
A fifth SkipReason arm fires only when both halves match the built commit.

lastSyncCommitSha and lastSyncedAt keep their present semantics exactly, so #675 / #677 /
#2229 item C are untouched: the next NEW commit still brings a full unscoped import, and the GUI's
own Update to latest never consults SkipReason at all.

Why the flag, and not the sha alone

Today's re-clone-per-delivery is also, accidentally, the retry loop for exactly the failures
StaticRepoImporter.IsContentVerdict refuses to call final. Unknown ("Persistence read failed…",
"Inner CreateNode faulted…") stays retryable because getting that backwards is #3101. Skipping on
"same commit, already attempted" alone would make a transient store fault permanent on any
repository quiet enough not to produce a new commit — a fix that strands every self-healing source.

Controls, both end-to-end on the CLONE itself

The transfer is the cost (FetchAndImport fetches first and diffs second — it has to; a
subdirectory scopes the parse, never the transfer), so an assertion on a decision function or a
log line would not be measuring it. GreenBuildSkipsASettledSourceTest substitutes the GitHub
transport and one marked storage path; the webhook processor, the sync service, the importer and
the node streams are real.

AContentVerdictAtThisCommit_CostsNoSecondClone_AndANewCommitStillDoesImport — a nested Space
refused with InvalidPath → the same green build again emits no fetch → a new sha does
fetch, at the commit that build proved. Also pins lastSyncCommitSha still null.
AFailureThatMightNotRecur_IsStillAttemptedAtTheSameCommit — one node whose store faults with
the production connect-timeout shape, so the pass is ImportedWithErrors with
VerdictIsFinal == false; the same green build then must reach GitHub. This is the control
that could falsify the first.

The cron trigger — measured, and deliberately NOT narrowed

87 of core's 254 daily publish signals are Prod synthetic probe, a */15 cron whose entire body
is curl (no checkout, no compile). Excluding it, or dropping schedule, was measured and is not
the win it looks like:

its 87 runs cover 34 distinct shas and every one was already published by a green push run —
so excluding it saves nothing this PR's skip does not already save;
no repository in the fleet gets its green default-branch signal only from schedule (all six
satellites' content CI fires on push and repository_dispatch and schedule); over the whole
of September, dropping it would have widened exactly one repository's worst quiet stretch, by 6 h;
and it would not close the class: on core 192a60d84a36, Chart Gate and Hosting Operator
both went green on push in the same second that push MeshWeaver Build and Test failed.

The real discriminator is workflow_run.name/.path — did this repository's own content CI prove
this tree — which is a separate change with its own risk. §8 of the doc page carries the evidence
and a correctness finding that goes with it (a */10 PR-updater cron recorded 20+ green build
completions for commits whose content CI had failed).

Documentation

Doc/Architecture/GitSyncTriggerCost §7 (the fix) and §8 (the trigger evidence);
Doc/Architecture/GitHubSync's three clocks are now four facts. What's New entry
(Category: Fix — a Space admin sees the repeated import activity records stop, and the settings
tab now states why a settled source has stopped attempting).

Pairs-with: none — nothing public is removed; the diff only ADDS members (StaticRepoImportResult.VerdictIsFinal, .ContentErrorsOutcome, two GitHubSyncConfig properties, one optional-parameter pair on a private method).

Implementers: none — no member is added to a public interface or abstract class. The two new members land on StaticRepoImportResult, a sealed record, and on GitHubSyncConfig, a plain record with no implementers.

Mirror-sync: npm run sync:i18n -- --ref <this PR's merged core sha> — one key added (ui.gitSync.settled, en + de). Tracked on #3596, which is the open issue for the pinned-mirror staleness this key joins.

🤖 Generated with Claude Code

https://claude.ai/code/session_01S25PJZbVeCbkWhCfMub9P7

…ery (#3945) `GitHubWebhookProcessor.SkipReason` made a green-build delivery free for exactly one reason — `lastSyncCommitSha == headSha` — and `MayAdvanceBaseline` deliberately HOLDS that same field whenever an import did not converge (`Preserved > 0` #675/#677, `Failed > 0` #2229 item C, outcome `Failed`). The cheapness gate and the convergence guard were one field, so a source that could never converge paid a full `fetch --depth 1` of its ENTIRE repository plus a full parse on EVERY delivery, at the source repository's CI cadence. Measured 2026-09-10: `Essentials/_GitSync` on memex.meshweaver.cloud had been doing that against MeshWeaver.Plugins since 2026-08-10 — 31 days, 436 commits — while its 33 siblings on the same repository, the same webhook and the same schedule were skipped for free. On core the delivery rate is ~10/h (254 green publish-signal runs in 24 h). The fix is a SECOND, WEAKER pointer that never touches the conservative baseline: * `StaticRepoImportResult.VerdictIsFinal` — would running again at the same content change the answer? Distinct from `Converged` (is the partition now equal to the source?). Final: `Preserved > 0` alone, and `ImportedWithContentErrors` (every failure a content verdict). Not final: `ImportedWithErrors`, outcome `Failed`, `PruneRefused`. * `GitHubSyncConfig.LastAttemptedCommitSha` + `LastAttemptWasFinal`, written on every import conclusion and CLEARED by an export (it moves the conflict horizon) and by a hold (no attempt ran). * a fifth `SkipReason` arm that fires only when BOTH halves match the built commit. The flag is what stops this becoming a worse bug. Today's re-clone-per-delivery is also, accidentally, the retry loop for the failures `IsContentVerdict` refuses to call final (#3101); skipping on the sha alone would make a transient store fault permanent on any repository quiet enough not to produce a new commit. `lastSyncCommitSha` and `lastSyncedAt` keep their present semantics exactly, so #675 / #677 / #2229 item C are untouched: the next NEW commit still brings a full unscoped import, and the GUI's "Update to latest" never consults `SkipReason` at all. The trigger allow-list is deliberately NOT narrowed. 87 of core's 254 daily publish signals are a `*/15` cron probe that compiles nothing — but it publishes no sha a green push run had not already published, so excluding it saves nothing the skip above does not; no repository in the fleet gets its green signal only from `schedule`; and narrowing would leave the class open (`Chart Gate` and `Hosting Operator` went green on push in the same second `MeshWeaver Build and Test` failed, on core `192a60d84a36`). The evidence and the real discriminator are written up in §8. Controls, both end-to-end on the CLONE itself (the transfer is the cost): `AContentVerdictAtThisCommit_CostsNoSecondClone_AndANewCommitStillDoesImport` and `AFailureThatMightNotRecur_IsStillAttemptedAtTheSameCommit`. Doc: `Doc/Architecture/GitSyncTriggerCost` §7 + §8; `GitHubSync`'s three clocks are now four. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S25PJZbVeCbkWhCfMub9P7

§8 concluded that the trigger allow-list is the wrong discriminator and said the real one 'is filed as such' without naming it. #3978 carries the correctness half — a */10 cron that builds nothing recorded 20+ green build completions for commits whose content CI had failed — and the design questions the workflow-name discriminator needs answered first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S25PJZbVeCbkWhCfMub9P7

The fix's whole benefit for the two measured sources rests on Preserved > 0 being FINAL, so "which arm are they on" had better be measured rather than inferred from twoWay. Re-read read-only 2026-09-10T19:41Z: Essentials/_GitSync is at version 378, lastSyncAttemptAt 30 minutes old, lastSyncOutcome Imported — which means Failed == 0 and the outcome is not Failed, so of MayAdvanceBaseline's three clauses only Preserved > 0 can be holding the baseline. By elimination, on live data. It also does NOT carry twoWay, so its preservation is the bidirectional prune sparing server-side additions (#604), not two-way overwrite protection. The previous wording named Deployments as "the only source carrying twoWay: true, i.e. the Preserved > 0 arm", which reads as if Essentials were on some other arm — two mechanisms feed that one counter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S25PJZbVeCbkWhCfMub9P7

Copilot

🟡 Changes recommended

VerdictIsFinal is currently too broad and would incorrectly mark retryable/self-healing outcomes as “final”, and the settings tab “settled” message renders outside the settled-but-not-converged state.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR addresses GitSync delivery churn where a non-converging sync source would re-clone (fetch + parse) the entire repository on every green-build webhook delivery by separating “mesh has this commit” from “we already attempted these exact bytes”.

Changes:

Add an “attempt pointer” (LastAttemptedCommitSha + LastAttemptWasFinal) to GitHubSyncConfig and a new webhook skip arm keyed on that pair.
Introduce StaticRepoImportResult.VerdictIsFinal to distinguish “retrying can’t help” outcomes from transient failures.
Add an end-to-end integration test that measures the clone itself (transport is substituted; webhook processor + sync service + importer + streams are real), plus docs/i18n/UI text explaining the settled state.

File summaries

File	Description
test/MeshWeaver.Hosting.Test/GreenBuildSkipsASettledSourceTest.cs	New integration test asserting “no second clone” for final content verdicts and “still retries” for transient failures.
src/MeshWeaver.Messaging.Hub/Localization/strings.en.json	Adds `ui.gitSync.settled` string (English).
src/MeshWeaver.Messaging.Hub/Localization/strings.de.json	Adds `ui.gitSync.settled` string (German).
src/MeshWeaver.Graph/StaticRepoImporter.cs	Adds `VerdictIsFinal` and centralizes `ImportedWithContentErrors` literal.
src/MeshWeaver.GitSync/GitHubWebhookProcessor.cs	Adds a new skip reason arm based on the attempt pointer + final verdict.
src/MeshWeaver.GitSync/GitHubSyncSettingsTab.cs	Displays a “settled” hint on the settings tab.
src/MeshWeaver.GitSync/GitHubSyncService.cs	Stamps/clears the attempt pointer via `RecordSyncResult`, and documents the semantics split.
src/MeshWeaver.GitSync/GitHubSyncConfig.cs	Adds `LastAttemptedCommitSha` and `LastAttemptWasFinal` fields.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-10-a-stuck-sync-stops-re-downloading-its-repository.md	What’s New entry for the user-visible change.
src/MeshWeaver.Documentation/Data/Architecture/GitSyncTriggerCost.md	Architecture doc updates describing the cost model and the new skip mechanism.
src/MeshWeaver.Documentation/Data/Architecture/GitHubSync.md	Updates “three clocks” → “four facts” including the new attempt pointer.
src/MeshWeaver.Documentation/Data/Architecture.md	Updates the Architecture index entry summary for GitSyncTriggerCost.

Review details

Files reviewed: 12/12 changed files
Comments generated: 2
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    public bool VerdictIsFinal =>
+        !PruneRefused
+        && !string.Equals(Outcome, "Failed", StringComparison.OrdinalIgnoreCase)
+        && (Failed == 0
+            || string.Equals(Outcome, ContentErrorsOutcome, StringComparison.OrdinalIgnoreCase));


+            // 🚨 #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,


github-actions · 2026-09-10T20:06:54Z