diff --git a/src/MeshWeaver.Documentation/Data/Architecture.md b/src/MeshWeaver.Documentation/Data/Architecture.md index f12d5ead01..e8bf031d04 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture.md +++ b/src/MeshWeaver.Documentation/Data/Architecture.md @@ -171,6 +171,7 @@ Each theme starts with its introductory page, followed by related architecture t - [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 - [When a Publication Seal Stops Advancing](PublicationSealStarvation) — a Space converges on a green build only while the publication sealed for THIS instance's framework identity keeps reaching the built commit; when the instance's identity and the lane that publishes for it drift apart that condition stops being satisfiable, and until the hold was recorded on the node a held source was byte-identical to a settled one - [The Import Marker Records Convergence](ImportMarkerRecordsConvergence) +- [A Content Verdict Is Per Node](AContentVerdictIsPerNode) — one unstorable byte in one file earned a partition-wide "these bytes cannot import" verdict and took every instance action offline for an evening; where the refusal is remembered now, and what an import must name when it loses a node - [Instance Sync — bi-directional space replication between MeshWeaver instances](InstanceSync) - [Managing Partition Sync (Admin Guide)](PartitionSyncGuide) - [Static Node Providers](StaticNodeProviders) diff --git a/src/MeshWeaver.Documentation/Data/Architecture/AContentVerdictIsPerNode.md b/src/MeshWeaver.Documentation/Data/Architecture/AContentVerdictIsPerNode.md new file mode 100644 index 0000000000..9d999b75da --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/Architecture/AContentVerdictIsPerNode.md @@ -0,0 +1,188 @@ +--- +Name: A Content Verdict Is Per Node +Category: Architecture +Description: A single unstorable byte in one source file took every instance action on the control instance offline for an evening. The import refused that one row, earned a partition-wide "these bytes cannot import" verdict on the evidence of it, and every later import answered Skipped with a sentence claiming the content had been recorded. Why the rule was right and its granularity was not, where the refusal is remembered now, and what an import must name when it loses a node. +Icon: +--- + +# A Content Verdict Is Per Node + +A [static-repo import](/Doc/Architecture/StaticRepoImport) writes each source node through the +canonical upsert, and a node whose write the owner refuses is **isolated**: the failure is logged +against that file, every other node still lands, and the pass reports how many did not. + +Some of those refusals are about the **moment** — a store briefly unreachable, an owner that did not +answer — and some are about the **bytes**: a validator rule, an invalid path, a NodeType the mesh +does not know, an RLS denial. The second kind is deterministic. Re-reading the same bytes at the +same fingerprint re-derives the identical refusal, so re-issuing that write can accomplish nothing, +and doing it anyway cost `memex-cloud` **19 full import passes in three hours** — roughly 425 +identical failing upserts plus a NodeType compile each, on a portal already at 8/8 replicas. That is +issue #3146, and its rule is right. + +**What was wrong is where the rule was recorded.** + +## The shape of the defect + +The refusal was recorded on the **content-addressed marker** — the activity at +`{Partition}/_Activity/import-{fingerprint}` whose whole purpose is to say something about the +*partition*. A pass whose every failure was a content verdict stamped that marker `Failed` with the +outcome `ImportedWithContentErrors`, and the next import read it and answered `Skipped` for the +whole partition without reading anything. + +A content verdict, though, is earned by **one node**. Forty files can land and one be refused, and +the pass still earns that word — it means *"every failure was deterministic"*, not *"everything +failed"*. Recording a per-node fact as a per-partition verdict is the entire defect: + +> The marker said *"an earlier FULL import already recorded this exact content"* about content the +> import demonstrably lost. + +## What it cost + +**Measured on `memex.systemorph.com`, 2026-09-15.** + +One file — `Hosting/Deployment/Source/SelfUpdateRouting.cs` — contained a **literal NUL byte +(0x00)** inside a char literal. PostgreSQL cannot store a NUL in a text column, so the write of that +one row was refused while the rest of the `Hosting` tree imported normally. The pass reported: + +``` +Re-imported ImportedWithErrors (41 node(s)) at 85515f62. +``` + +— a count, no paths, at `Information`, on an activity whose terminal status was `Succeeded`. Then: + +- **Five NodeTypes parked**, each on the symbol whose file had not landed: `Hosting/Backup`, + `Hosting/Deployment`, `Hosting/InstanceAction`, `Hosting/InstanceRequest`, + `Hosting/PlatformBuildInbox`, with `CS0246: The type or namespace name 'SelfUpdateRouting' could + not be found`. +- **`Hosting/InstanceAction` is the control instance's entire action surface**, so while it was + parked *no instance action could run at all* — no `Sample`, no `Logs`, no `HelmRelease`, no + `Roll`. Operations on the fleet were offline. +- The readiness gate on the new replica correctly refused (`5 NodeType(s) regressed on this image`), + so the rollout could not complete either. +- The operator's one repair — re-import — was **refused**: + +``` +Imported Skipped — an earlier FULL import already recorded this exact content at +fingerprint bb9801101e859a21 (Hosting/_Activity/import-bb9801101e859a21), so the +partition was not re-read. +``` + +The visible symptom was a compile error on a symbol whose source file is plainly in git, with +nothing anywhere naming the node that had not landed. The issues are +[#4459](https://github.com/Systemorph/MeshWeaver/issues/4459) and +[#4456](https://github.com/Systemorph/MeshWeaver/issues/4456); #4456 reads the same evidence as +*"applies modifications but not additions"*, which is what one dropped file looks like from outside +when nothing names it. + +## The fix: the memory moves to the node + +The per-node **import manifest** (`{Partition}/_Activity/import-manifest`, a `{path → token}` map) +already exists and is already the thing that decides, per node, whether a write is worth issuing. +Two changes make it carry the refusal too. + +**1. A run may only claim what it WROTE.** The manifest used to record a node's source token even +when its write had been refused — the ledger asserting the partition held content that is provably +not in it. It is the same false claim the marker made, one layer down, and it had to become honest +first. Now: + +| what happened to the node | what the manifest records | +|---|---| +| written | its source token | +| refused, deterministically | `!` + its source token — *evaluated, and these bytes were refused* | +| failed, retryably | nothing at all — which is what makes the next pass look again | + +The `!` prefix is a prefix rather than a schema change on purpose: a reader that does not know it +compares `!abc` against the token it computes (`abc`), sees a mismatch, and re-evaluates the node — +the safe direction, and exactly the behaviour before the field existed. + +**2. The skip moves with it.** The import loop skips a node whose manifest entry is a refusal *of +this exact token* — reported, with no write attempted — and `ImportedWithContentErrors` stops being +a fingerprint-recording outcome. The marker keeps it as a **record** of what the pass found; it is +no longer a licence to answer `Skipped` for the partition without reading it. + +Both properties then hold at once: + +- the write that provably fails is **not re-issued** — #3146's storm stays prevented, and its test + now pins the quantity that storm was actually made of (`WriteRequests == 0`) rather than the word + `Skipped`; +- every **other** node is still evaluated, so a later import can create, repair and prune — and a + partition that has drifted from its marker (a cross-partition prune, a manual delete, a botched + migration) can heal, which is the repair the whole-partition skip refused. + +A `Force` or `Reconcile` import bypasses the refusal memory, exactly as it bypasses the marker: +re-applying regardless is what those are for. + +### What it costs + +A partition holding a refused node now re-evaluates on every trigger instead of short-circuiting. +That pass issues **no node writes and no recompiles** — every landed node is manifest-skipped, the +refused one is refusal-skipped — so what it costs is one subtree read, the prune evaluation and two +bookkeeping writes. That is the same cost the marker already accepts whenever a verdict is absent or +records a non-converged run; see +[The Import Marker Records Convergence](/Doc/Architecture/ImportMarkerRecordsConvergence). + +## An import that loses a node has to NAME it + +The second half of both issues is reporting, and it is the half that decides how long an outage +lasts. Every other thing an import can leave behind already named its paths — written, pruned, +blocked creates, held NodeTypes, refused content. **A failure was the one category reported as a +bare number.** + +- `StaticRepoImportResult.FailedPaths` carries each failed node with the write path's own reason and + whether it was deterministic — the same argument `RefusedContent` won for content files in #3101: + *"refused"* alone cannot separate a byte the store will not take from a validator rule from an RLS + denial, and those have three different fixes. +- The import's terminal summary names them, bounded. +- The GitSync activity logs them as their own line, **and the outcome line's level now follows the + outcome**: `Failed` is an error, any `ImportedWith…` a warning. Every line here used to be written + at `Information` whatever the import did, and `RunActivity` derives the activity's severity and + terminal status from those levels — which is why an import that dropped a source node showed up as + `maxSeverity: Information`, `status: Succeeded`. + +## What this does not cover + +🚨 **An operator who starts from the COMPILE ERROR still has nothing pointing at the import.** Both +issues list this as their third item, and it is deliberately not fixed here — it is +[#4469](https://github.com/Systemorph/MeshWeaver/issues/4469). + +A partition left incomplete still hands the compiler source files that reference a symbol whose own +file did not land, so the NodeType fails on `CS0246` / `CS0103` for a symbol that is plainly in git, +and nothing in that message connects it to the import. What changed is the other direction: an +operator reading the **sync activity** now learns which file is missing and why, and the activity is +no longer green. The facts a fix needs are all recorded — the manifest's `!` entries say which +declared nodes the partition is missing — so #4469 is a wiring problem, not a measurement one. + +Note the granularity trap in it: refusing to compile a whole partition because one unrelated node was +refused would re-create exactly the over-broad reading this page is about, so any fix has to be +scoped to the types that actually reference the missing node. + +🚨 **A manifest written BEFORE this change still carries the old lie, for one case.** Entries predate +the `!` sigil, so a node whose *update* was refused back then sits under its plain token while the +mesh holds the older content; the incremental skip matches that token and passes over it. That is +not new — it is precisely the behaviour this change removes going forward — and it is self-limiting +in the same way the marker's absent-verdict residue is: any edit to that file moves its token and the +node is evaluated again. It is named here rather than migrated because a migration would have to +decide, for every pre-existing entry, a fact no pre-existing entry records. The *create* case needs +nothing: a refused create leaves no node, and the incremental skip already requires the node to be +present. + +## What this does not change + +- **`MayAdvanceBaseline` is untouched.** It already requires `Failed == 0`, so a partial import never + advanced the git baseline; that guard was working and is not what broke. A remembered refusal is + still reported as a failure, so the baseline is still held — the mesh genuinely is not at that + commit. `VerdictIsFinal` is also unchanged, so the delivery stays cheap + ([What a Green Build Costs a Synced Space](/Doc/Architecture/GitSyncTriggerCost)). +- **A refusal is still a refusal.** If the bytes genuinely cannot be stored, no import creates the + node — the repair is to fix the source file, which moves the fingerprint and the token. What + changes is that the partition is no longer frozen around it, and that the file is *named*. +- **The green short-circuit is unchanged.** A clean import still stamps a `Succeeded` marker and the + next run still skips on it. + +## Related + +- [The Import Marker Records Convergence](/Doc/Architecture/ImportMarkerRecordsConvergence) — what the content-addressed marker may claim, and why an absent verdict reads as UNKNOWN +- [Static Repo Import](/Doc/Architecture/StaticRepoImport) — the pipeline the marker and the manifest sit in +- [The Prune Requires a Complete Listing](/Doc/Architecture/PruneRequiresACompleteListing) — the other way a pass leaves the partition unequal to the source +- [What a Green Build Costs a Synced Space](/Doc/Architecture/GitSyncTriggerCost) — the trigger rate this cost is paid at +- [Syncing a Space with GitHub](/Doc/Architecture/GitHubSync) — the route that fetches, imports and reports diff --git a/src/MeshWeaver.Documentation/Data/Architecture/ImportMarkerRecordsConvergence.md b/src/MeshWeaver.Documentation/Data/Architecture/ImportMarkerRecordsConvergence.md index 2f881b38ff..6d9d64b8aa 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/ImportMarkerRecordsConvergence.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/ImportMarkerRecordsConvergence.md @@ -177,6 +177,7 @@ about who writes nodes, not a law of nature, so it is stated here rather than le ## Related +- [A Content Verdict Is Per Node](/Doc/Architecture/AContentVerdictIsPerNode) — the other claim the marker used to make on too little evidence: a refusal earned by ONE node, recorded as a verdict about the partition - [The Prune Requires a Complete Listing](/Doc/Architecture/PruneRequiresACompleteListing) — the two reads the prune's inference rests on - [Static Repo Import](/Doc/Architecture/StaticRepoImport) — the pipeline, and the marker's place in it - [Retiring a NodeType](/Doc/Architecture/RetiringANodeType) — what a retirement has to remove beyond the source files diff --git a/src/MeshWeaver.Documentation/Data/Architecture/StaticRepoImport.md b/src/MeshWeaver.Documentation/Data/Architecture/StaticRepoImport.md index 612ee8d52a..a870e674df 100644 --- a/src/MeshWeaver.Documentation/Data/Architecture/StaticRepoImport.md +++ b/src/MeshWeaver.Documentation/Data/Architecture/StaticRepoImport.md @@ -383,10 +383,22 @@ So the outcome now distinguishes **why** a pass failed: | outcome | lock status | next trigger at the SAME fingerprint | |---|---|---| | `Imported` | `Succeeded` | skips | -| `ImportedWithContentErrors` | `Failed` | **skips**, logging the recorded verdict | +| `ImportedWithContentErrors` | `Failed` | **re-imports** — but issues no write for the refused node | | `ImportedWithErrors` | `Warning` | re-imports | | `ImportedWithRefusedContent` / `ImportedWithBlockedCreates` | `Warning` | re-imports | +🚨 **That `ImportedWithContentErrors` row changed, and the argument above is the reason it had to.** +Until 2026-09-16 it read *"**skips**, logging the recorded verdict"* — the whole PARTITION skipped, +on a marker written because a content verdict was earned. But a content verdict is earned by a +**node**, and `ImportedWithContentErrors` means *"every failure was deterministic"*, not *"everything +failed"*: forty files can land and one be refused. Recording that as a verdict about the partition is +what froze `Hosting` out of `memex.systemorph.com` over a single unstorable byte, with the next +import answering *"an earlier FULL import already recorded this exact content"* about content it had +lost. The refusal is now remembered **per node**, in the import manifest, so the failing write is +still never re-issued — #3146's measurement is unchanged — while the rest of the partition is +evaluated again. Full account: +[A Content Verdict Is Per Node](/Doc/Architecture/AContentVerdictIsPerNode). + `ImportedWithContentErrors` is reached only when **every** failure in the pass was a content verdict (`StaticRepoImporter.IsContentVerdict`), and that is decided on the owner's **structured** `NodeUpsertRejectionReason`, never on the exception type: diff --git a/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-16-one-file-the-import-cannot-write-no-longer-freezes-the-whole-space.md b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-16-one-file-the-import-cannot-write-no-longer-freezes-the-whole-space.md new file mode 100644 index 0000000000..d246d61a0b --- /dev/null +++ b/src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-16-one-file-the-import-cannot-write-no-longer-freezes-the-whole-space.md @@ -0,0 +1,59 @@ +--- +Name: One file the import cannot write no longer freezes the whole Space +Category: Fix +Description: A single source file the store refused made every later import of that Space answer "already up to date" without looking — so a Space could be left missing a file, with no way to repair it and nothing anywhere naming which file was missing. Imports now remember the refusal against that one file, keep importing everything else, and say which node did not land and why. +Icon: RefreshCircle +Order: -20260916 +--- + +# One file the import cannot write no longer freezes the whole Space + +When a Space is synced from a repository, the import writes each file into the mesh one node at a +time. Nearly always they all land. Occasionally one cannot — a file that breaks a rule the mesh +enforces, or that the database genuinely will not store. That case has always been isolated: the +offending file is reported and every other file still imports. + +What was wrong is what happened **next time**. The import concluded, from that one file, that *this +Space's content cannot be imported* — and recorded it that way. Every later import of the same +content then answered: + +> Skipped — an earlier FULL import already recorded this exact content … so the partition was not +> re-read. + +Which was not true. The import had lost a file, and the sentence said the content was recorded. From +then on the Space was frozen: nothing was re-read, nothing could be repaired, and pressing **Update** +did nothing but repeat the same sentence. + +## What that looked like when it happened + +A source file in an internal Space contained a single invisible character that PostgreSQL cannot +store in a text column. One node out of forty-one did not land. Five node types that referenced it +stopped compiling, among them the one that runs every operational action, so for an evening no +action on that portal would run at all. + +The only thing anyone could see was a compile error on a symbol whose file is plainly in the +repository. The import had reported *"41 node(s)"* with a count of failures and **no path at all** — +and it had done so in the same ordinary, green-looking line a completely successful import writes. + +## What changes + +**The refusal is remembered against the file, not the Space.** The next import skips exactly that +one node — so it is not retried pointlessly, which is what the old behaviour was protecting against +— and evaluates everything else normally. A Space that has drifted from what the repository says can +be repaired again, including by the obvious route of pressing Update. + +**An import that loses a file says which one, and why.** The path and the reason now appear in the +sync activity, in the import summary, and in the result the platform passes on. *"Refused"* on its +own could not distinguish a character the database will not accept from a rule your content breaks +from a permissions problem — three different problems with three different fixes. + +**And it no longer looks green.** An import that could not write everything is now reported as a +warning, and one that failed outright as an error, so the sync activity's own status reflects what +happened rather than reading as a success. + +If the file really cannot be stored, the fix is still to correct it in the repository — the platform +will not invent content it was refused. The difference is that the rest of the Space keeps working +while you do, and that you can see which file to correct. + +Background, with the measurements: [A Content Verdict Is Per +Node](/Doc/Architecture/AContentVerdictIsPerNode). diff --git a/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs b/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs index feaf452b3c..c6ced35446 100644 --- a/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs +++ b/src/MeshWeaver.GitSync/GitHubActivityExtensions.cs @@ -240,7 +240,7 @@ public static IObservable UpdateToLatestFromGitHub( // deletes user-visible data, and "pruned N" alone left no record of WHAT. if (r.PrunedPaths.Count > 0) ctx.Log(PrunedLine(r)); - ctx.Log(ImportedLine(r, commitish: null)); + LogImportOutcome(ctx, r, commitish: null); return Unit.Default; }); }, onActivityCreated))); @@ -326,7 +326,7 @@ public static IObservable UpdateToProvenCommitFromGitHub( { if (r.PrunedPaths.Count > 0) ctx.Log(PrunedLine(r)); - ctx.Log(ImportedLine(r, commitish: shortSha)); + LogImportOutcome(ctx, r, commitish: shortSha); return Unit.Default; }); }, onActivityCreated))); @@ -377,7 +377,7 @@ public static IObservable ReconcileAtProvenCommitFromGitHub( { if (r.PrunedPaths.Count > 0) ctx.Log(PrunedLine(r)); - ctx.Log(ImportedLine(r, commitish: shortSha)); + LogImportOutcome(ctx, r, commitish: shortSha); return Unit.Default; }); }, onActivityCreated))); @@ -419,7 +419,7 @@ public static IObservable ReimportFromGitHub( // deletes user-visible data, and "pruned N" alone left no record of WHAT. if (r.PrunedPaths.Count > 0) ctx.Log(PrunedLine(r)); - ctx.Log(ImportedLine(r, commitish)); + LogImportOutcome(ctx, r, commitish); return Unit.Default; }); }, onActivityCreated)); @@ -444,6 +444,71 @@ private static string DescribeOutcome(StaticRepoImportResult result) => private static bool IsSkipped(StaticRepoImportResult result) => string.Equals(result.Outcome, "Skipped", StringComparison.OrdinalIgnoreCase); + /// + /// 🚨 Issue #4456 — an import that LOST nodes must not render as a green activity. + /// + /// Every outcome line here was written at , whatever the + /// import actually did. RunActivity derives the activity's maxSeverity and terminal + /// status from the levels of the lines it collects, so a GitSync import that dropped a + /// source node reported maxSeverity: Information and status: Succeeded — measured + /// on memex.systemorph.com, 2026-09-15, on the very import that lost + /// Hosting/Deployment/Source/SelfUpdateRouting. The outcome word sat in the middle of a + /// green sentence, the failure count was not in it at all, and nothing anywhere said WHICH node. + /// Five NodeTypes then parked on that symbol, and the first sign anyone had was a compile error + /// on a file that is plainly in git. + /// + /// So the LEVEL follows the outcome: Failed is an error, any ImportedWith… is + /// a warning (nodes did not land, creates were blocked, or assets were refused — every one of + /// them a state the partition is left INCOMPLETE in), and everything else stays + /// informational. + /// + private static LogLevel LevelFor(StaticRepoImportResult result) => + string.Equals(result.Outcome, "Failed", StringComparison.OrdinalIgnoreCase) + ? LogLevel.Error + : result.Outcome.StartsWith("ImportedWith", StringComparison.OrdinalIgnoreCase) + ? LogLevel.Warning + : LogLevel.Information; + + /// + /// 🚨 Issue #4459 / #4456 — the ONE place a sync activity reports an import's outcome. + /// The nodes that did not land are named FIRST (and at Warning, which is what colours the + /// activity), then the outcome line. Every call site goes through this rather than logging + /// directly, so a fifth one cannot quietly re-acquire the + /// green-on-a-partial-import shape the issues were filed on. + /// + private static void LogImportOutcome( + ActivityContext ctx, StaticRepoImportResult result, string? commitish) + { + if (FailedNodesLine(result) is { } failedNodes) + ctx.Log(failedNodes); + ctx.Log(ImportedLine(result, commitish)); + } + + /// + /// 🚨 Issue #4459 / #4456 — the sentence that NAMES what did not land. The importer + /// reports its failures as paths now (); this is + /// where an operator reading the sync activity sees them. Bounded, so a pathological source + /// cannot write an unbounded activity line. + /// + private static LogMessage? FailedNodesLine(StaticRepoImportResult result) + { + if (result.FailedPaths.Count == 0) + return null; + const int Named = 10; + var paths = string.Join("; ", result.FailedPaths + .Take(Named) + .Select(f => $"{f.NodePath} ({f.Reason})")) + + (result.FailedPaths.Count > Named + ? $", … (+{result.FailedPaths.Count - Named} more)" + : ""); + return new LogMessage( + $"⚠ {result.FailedPaths.Count} node(s) did NOT land — the mesh does NOT hold this " + + $"content, so anything referencing them will not compile: {paths}", + LogLevel.Warning) + .WithKey("activity.gitsync.failedNodes", + ("count", result.FailedPaths.Count), ("paths", paths)); + } + /// /// The terminal import line, keyed so a German viewer reads it in German (#3281). Four keys /// rather than a single {outcome} argument over : the @@ -469,12 +534,16 @@ private static LogMessage ImportedLine(StaticRepoImportResult result, string? co ("fingerprint", result.Fingerprint), ("markerPath", markerPath), ("commitish", commitish)); + // 🚨 #4459/#4456 — the LEVEL follows the outcome (see LevelFor); the nodes that did not land + // get their OWN keyed line (see FailedNodesLine), so a German reader gets a German sentence + // around the paths rather than an English one smuggled in as an argument. + var level = LevelFor(result); return commitish is null - ? new LogMessage($"Imported {DescribeOutcome(result)}.", LogLevel.Information) + ? new LogMessage($"Imported {DescribeOutcome(result)}.", level) .WithKey("activity.gitsync.import.done", ("outcome", result.Outcome), ("count", result.Count)) : new LogMessage( - $"Re-imported {DescribeOutcome(result)} at {commitish}.", LogLevel.Information) + $"Re-imported {DescribeOutcome(result)} at {commitish}.", level) .WithKey("activity.gitsync.reimport.done", ("outcome", result.Outcome), ("count", result.Count), ("commitish", commitish)); } diff --git a/src/MeshWeaver.Graph/StaticRepoImporter.cs b/src/MeshWeaver.Graph/StaticRepoImporter.cs index fdf082e7b1..f790027214 100644 --- a/src/MeshWeaver.Graph/StaticRepoImporter.cs +++ b/src/MeshWeaver.Graph/StaticRepoImporter.cs @@ -73,6 +73,22 @@ public sealed record StaticRepoImportResult(string Partition, string Fingerprint /// public int Failed { get; init; } + /// + /// 🚨 Issue #4459 / #4456 — WHICH nodes did not land, and why. The per-path twin of + /// , carried to the CALLER rather than only to the attempt activity's ⚠ + /// lines — the same argument already makes for content files. + /// + /// Every other category the import can leave behind names its paths: + /// , , , + /// , . Failures were the one reported + /// as a bare NUMBER, and that is how one dropped node cost memex.systemorph.com an evening: a + /// literal NUL byte in Hosting/Deployment/Source/SelfUpdateRouting.cs made Postgres refuse + /// that one row, the import reported ImportedWithContentErrors (41 node(s)) — naming + /// neither the path nor even how many —, five Hosting NodeTypes parked on CS0246, and the + /// operator's only visible symptom was a compile error on a symbol whose file is in git. + /// + public ImmutableList FailedPaths { get; init; } = ImmutableList.Empty; + /// /// The node paths this import could NOT create, for a reason no retry of the same pass can close, /// while the mesh has no node there at all. Two causes, one meaning: @@ -213,6 +229,16 @@ public sealed record StaticRepoImportResult(string Partition, string Fingerprint /// bytes re-derives this same refusal". Named here, beside which /// reads it and the fold in StaticRepoImporter.Run which writes it, so a re-wording /// cannot silently split the two. + /// + /// 🚨 It is NOT a fingerprint-recording outcome (issue #4459). The marker keeps it + /// as a RECORD of what the pass found; it is no longer a licence to answer Skipped for + /// the partition without reading it. A content verdict is earned by a NODE — one file that + /// Postgres, a validator or RLS refuses — and a pass earns this word on the evidence of that one + /// node even when forty others landed, so reading it as "this whole partition cannot import" + /// froze a partition out of the mesh over a single unstorable byte. The refusal is remembered + /// per node instead, in the import manifest (see StaticRepoImporter.RefusedTokenPrefix), + /// which keeps #3146's storm protection — the failing write is not re-issued — without the + /// freeze. /// public const string ContentErrorsOutcome = "ImportedWithContentErrors"; } @@ -232,6 +258,24 @@ public sealed record StaticRepoImportResult(string Partition, string Fingerprint /// Why it did not succeed, with sizes and limits named where they apply. public sealed record RefusedContentSync(string NodePath, string Reason); +/// +/// 🚨 Issue #4459 / #4456. ONE source node whose write the import could not complete, the +/// reason the write path gave, and whether that reason was a verdict about the BYTES +/// ( — see StaticRepoImporter.IsContentVerdict) rather than +/// about the moment. +/// +/// The reason is the half that was missing, exactly as it was for +/// : ImportedWithContentErrors (41 node(s)) is +/// indistinguishable between a file Postgres cannot store, a validator refusing a rule, and RLS +/// declining the write — three problems with three different fixes, reported identically and with +/// no path at all. +/// +/// The source node whose write did not land. +/// The write path's own message, so the fix is visible without the attempt log. +/// True when the same bytes re-read at the same fingerprint break the +/// same rule — the fact the per-node refusal memory records so the next pass does not re-attempt it. +public sealed record FailedImport(string NodePath, string Reason, bool Deterministic); + /// /// 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 @@ -1104,31 +1148,48 @@ IObservable Reimport() => // degraded JsonElement (typed → as-is, JsonElement → deserialized, else null + logged). var existingLog = existing?.ContentAs(hub.JsonSerializerOptions, logger); - // 🚨 #3146 — a recorded CONTENT verdict at this fingerprint is as final as a green - // one, and for the same reason: the marker is content-addressed, so re-running reads - // the same bytes and re-derives the same refusal. memex-cloud paid 19 full passes in - // 3 h — ≈425 identical validation failures plus a NodeType compile storm each — on a - // portal already at 8/8 replicas, because "not Succeeded" was read as "try again". + // 🚨 #4459/#4456 — A CONTENT VERDICT IS FINAL FOR THE NODE THAT EARNED IT, NEVER FOR + // THE PARTITION. This arm used to answer "Skipped" for the whole partition whenever + // the marker recorded ContentErrorsOutcome, on #3146's argument: the marker is + // content-addressed, so re-running re-reads the same bytes and re-derives the same + // refusal (memex-cloud paid 19 full passes in 3 h — ≈425 identical validation + // failures plus a NodeType compile storm each — for not making it). // - // Only an ALL-deterministic pass lands here (see the outcome fold); a single - // retryable failure among them keeps Warning and keeps re-importing, and a - // fingerprint change re-runs this from scratch because the marker id IS the - // fingerprint. Force still re-runs — that is its purpose. - if (policy?.Force != true - && existingLog is { Status: ActivityStatus.Failed } - && MarkerOutcome(existingLog) == StaticRepoImportResult.ContentErrorsOutcome) - { - logger?.LogWarning( - "[StaticRepoImport] {Partition} already FAILED at {Fingerprint} on content " - + "rules — skipping. These same bytes cannot import; re-running compiles and " - + "re-writes for nothing. Fix the source (or force) — see {Path}.", - source.Partition, fingerprint, activityPath); - return Observable.Return( - new StaticRepoImportResult(source.Partition, fingerprint, "Skipped")); - } - + // The argument is right and the GRANULARITY was wrong, and the granularity is what + // cost memex.systemorph.com an evening on 2026-09-15. A literal NUL byte in ONE + // file — Hosting/Deployment/Source/SelfUpdateRouting.cs — is unstorable in a + // Postgres text column, so that one row was refused while 40 others landed. The pass + // earned ContentErrorsOutcome on the evidence of that single node, the marker + // recorded it for the partition, and every later import answered: + // + // Skipped — an earlier FULL import already recorded this exact content at + // fingerprint bb9801101e859a21 … so the partition was not re-read + // + // which is not true: the import lost a node, and the sentence claims the content was + // recorded. Five Hosting NodeTypes stayed parked on CS0246 SelfUpdateRouting, among + // them Hosting/InstanceAction — so NO instance action could run on the control + // instance at all (no Sample, no Logs, no HelmRelease, no Roll) — and the operator's + // one repair, a re-import, was refused by this arm with that same sentence. + // + // The memory now lives PER NODE, in the partition's import manifest (see + // RefusedTokenPrefix and the refusal skip in Run's stage loop): the write that + // provably fails is not re-issued — no storm, and #3146's measurement is pinned on + // WRITE REQUESTS rather than on the word "Skipped" — while every other node in the + // partition is evaluated again, so a re-import can still repair, create and prune. + // ImportedWithContentErrors is therefore no longer a fingerprint-recording outcome: + // the marker keeps it as a RECORD of what happened, never as a licence to skip. if (existingLog is not { Status: ActivityStatus.Succeeded }) + { + if (existingLog is { Status: ActivityStatus.Failed } + && MarkerOutcome(existingLog) == StaticRepoImportResult.ContentErrorsOutcome) + logger?.LogInformation( + "[StaticRepoImport] {Partition}: the marker at {Fingerprint} records a " + + "CONTENT verdict. Re-importing anyway — the refusal is remembered per " + + "node, so the refused write is not re-issued while the rest of the " + + "partition is evaluated. See {Path}.", + source.Partition, fingerprint, activityPath); 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 @@ -1562,6 +1623,57 @@ target is null // expensive cross-hub upsert + owner re-render. Token is over the RAW source node, // matching what the manifest stored. var token = PartitionSourceFingerprint.ComputeNodeToken(sourceNode, hub.JsonSerializerOptions); + + // 🚨 #4459/#4456 — THE REFUSAL IS REMEMBERED PER NODE, NOT PER PARTITION. + // + // A previous pass evaluated exactly these bytes at exactly this path and + // the write path refused them for a reason that is a verdict about the + // bytes (IsContentVerdict). Re-issuing that write re-derives the identical + // refusal — the cost #3146 measured on memex-cloud: 19 full passes in 3 h, + // ≈425 identical failing upserts plus a NodeType compile each. + // + // That memory USED to live on the content-addressed marker, as a verdict + // about the WHOLE partition, and answering "Skipped" on it is what turned + // one dropped node into an outage: on 2026-09-15 a literal NUL byte in + // Hosting/Deployment/Source/SelfUpdateRouting.cs made Postgres refuse that + // ONE row, the marker recorded a content verdict for the partition, and + // every later import answered "an earlier FULL import already recorded + // this exact content … the partition was not re-read" — a claim about 40 + // nodes made on the evidence of 1. Five Hosting NodeTypes stayed parked on + // CS0246 and no instance action could run on the control instance. + // + // Per node, both hold: the write that provably fails is not re-issued (no + // storm), and every other node in the partition is still evaluated, so a + // re-import can still repair, create and prune (no freeze). Force and + // Reconcile bypass it — re-applying regardless is their whole purpose. + // + // 🚨 The sentence says "the mesh does not hold this content", NOT "the node + // is not in the mesh" — deliberately, because a refused write is not always + // a refused CREATE. An UPDATE refused on a node that already exists leaves + // that node PRESENT and STALE, and telling an operator it is absent would + // send them looking for the wrong thing. + if (policy?.Force != true && policy?.Reconcile != true + && manifest.TryGetValue(path, out var priorEntry) + && IsRefusalOf(priorEntry, token)) + { + logger?.LogDebug( + "[StaticRepoImport] {Partition}: {Path} was REFUSED at this exact " + + "content by an earlier pass — reported, not re-attempted.", + source.Partition, path); + settled.Add(new ImportItem( + Failed: 1, + FailedDeterministic: 1, + Failure: new FailedImport(path, RememberedRefusalReason, true), + Log: new LogMessage( + $"⚠ {path} was REFUSED at this exact content by an earlier import, " + + "so the mesh does NOT hold it — the same bytes break the same " + + "rule, and no write was attempted. Fix the source file, or " + + "re-import with force to see the refusal again in full.", + Microsoft.Extensions.Logging.LogLevel.Warning) + .WithKey("activity.import.itemRefusedBefore", ("path", path)))); + continue; + } + // Reconcile was requested because this ledger disagrees with the live // sources; evaluate the write with conflict protection instead of trusting it. if (policy?.Reconcile != true && target is not null @@ -1726,6 +1838,13 @@ target is null .Where(x => x.Blocked is not null) .Select(x => x.Blocked!) .ToImmutableList(), + // 🚨 #4459/#4456 — the failures NAMED, collected in the same single + // materialization pass as every other path list. A bare count is what let one + // refused node sit unnoticed until five NodeTypes parked on it. + Failures: results + .Where(x => x.Failure is not null) + .Select(x => x.Failure!) + .ToImmutableList(), Written: results .Where(x => x.Written is not null) .Select(x => x.Written!) @@ -1926,7 +2045,7 @@ target is null // diff sees exactly what's now in the partition. One write; survives prune (_Activity). return WriteContentSyncLedgers(hub, source.Partition, content, logger) .SelectMany(_ => WriteManifest(hub, source.Partition, nodes.Append(root).ToArray(), manifest, changedNodePaths, - heldPaths, hub.JsonSerializerOptions, logger)).Select(_ => + heldPaths, count.Failures, count.Written, hub.JsonSerializerOptions, logger)).Select(_ => { // 🚨 Terminal status reflects per-file outcomes: ANY failed upsert → // Warning (the ⚠ lines above pinpoint which files), all-clear → @@ -2001,6 +2120,22 @@ target is null // very different problems — an asset tree the transport cannot carry, a // content collection that is not configured, a path escaping the root — // read identically without it, and only one of them is about size. + // 🚨 #4459/#4456 — NAMED, not counted. Every other thing an import can + // leave behind names its paths; a failure was the one reported as a bare + // number, and "ImportedWithContentErrors (41 node(s))" is what an operator + // was given while a NodeType parked on the one file that did not land. + // Bounded exactly like blockedNote/refusedNote so a pathological source + // cannot write an unbounded log line. + var failedNote = count.Failures.Count > 0 + ? $" ⚠ {count.Failures.Count} node(s) did NOT land — the mesh does NOT hold " + + "this content, so anything referencing them will not compile: " + + string.Join("; ", count.Failures.Take(BlockedPathsNamed) + .Select(f => $"{f.NodePath} ({f.Reason})")) + + (count.Failures.Count > BlockedPathsNamed + ? $", … (+{count.Failures.Count - BlockedPathsNamed} more)" + : "") + + "." + : ""; var refusedNote = refusedContent.Count > 0 ? $" 📦 {refusedContent.Count} node(s) whose CONTENT SYNC WAS REFUSED — their " + "assets are NOT in the mesh: " @@ -2030,7 +2165,7 @@ target is null // routine housekeeping unless it says what the prune refused and why. var heldNote = heldReport is null ? "" : " " + heldReport.Message; var summary = failed > 0 - ? $"Imported {count.Imported} node(s), {failed} FAILED (see ⚠ above){preservedNote}{claimedNote}, {prunedNote}, synced {contentCount} content file(s).{blockedNote}{refusedNote}{heldNote}" + ? $"Imported {count.Imported} node(s), {failed} FAILED{preservedNote}{claimedNote}, {prunedNote}, synced {contentCount} content file(s).{failedNote}{blockedNote}{refusedNote}{heldNote}" : $"Imported {count.Imported} node(s){preservedNote}{claimedNote}, {prunedNote}, synced {contentCount} content file(s).{blockedNote}{refusedNote}{heldNote}"; NodeTypeCompilationActivity.Complete(hub, activityPath, status, new[] @@ -2085,6 +2220,11 @@ status is ActivityStatus.Warning // last-sync guard has to know a node did not land, or it advances // the baseline past it and the miss is permanent (#2229 item C). Failed = failed, + // 🚨 #4459/#4456 — and the PATHS with it, for the same reason + // RefusedContent carries its reasons: a caller (the GitSync activity, + // a webhook, a test) that has to read the attempt node to learn WHICH + // node is missing will not learn it at all. + FailedPaths = count.Failures, BlockedCreatePaths = blockedCreates, WriteRequests = tally.Requests, // 🚨 #3101 — carried to the CALLER with the reason, not just to the @@ -2652,6 +2792,53 @@ private static void NotifyStartupFailure( partition, activityPath)); } + /// + /// 🚨 Issue #4459 / #4456 — the sigil that turns a manifest entry from "the partition holds + /// this" into "this was REFUSED at this token". + /// + /// The per-node manifest is the {path → token} ledger of what the partition holds. + /// Before this, a node whose write the owner REFUSED was recorded under its plain source token + /// anyway — the same false claim the content-addressed marker used to make, one layer down: the + /// ledger said "already at this content" about content that is not there. Recording the refusal + /// INSTEAD keeps the ledger honest AND gives the next pass the one fact it needs to avoid + /// re-attempting a write that provably fails (#3146's storm), WITHOUT freezing the whole + /// partition on a per-node fact (#4459's outage). + /// + /// A prefix rather than a schema change, deliberately: an OLDER reader compares + /// "!abc" to the token it computes ("abc"), sees a mismatch, and re-evaluates the + /// node — the safe direction, and exactly what it did before this field existed. A NEWER reader + /// on an older manifest sees no sigils and behaves as it always did. '!' cannot occur in a + /// token: ends in + /// Convert.ToHexString, so every token is 0-9A-F and nothing else. + /// + private const string RefusedTokenPrefix = "!"; + + /// + /// The reason carried by a failure the run did not re-derive because a previous pass already + /// recorded it (see the refusal-memory skip in the stage loop). It is a statement about the + /// LEDGER, not about the bytes — the original message is on the attempt that first measured it — + /// so it says which instrument answered rather than pretending to a diagnosis it did not run. + /// + private const string RememberedRefusalReason = + "Refused at this exact content by an earlier import (recorded in the partition's import " + + "manifest); no write was attempted. Re-import with force to re-derive the original reason."; + + /// + /// The manifest value that records " was REFUSED here" — see + /// . Pure. + /// + private static string RefusedEntry(string token) => RefusedTokenPrefix + token; + + /// + /// True when is a recorded refusal of exactly — + /// i.e. a previous pass evaluated THESE bytes at THIS path and the write path refused them for a + /// reason that is a verdict about the bytes. Any other token re-opens the question. Pure. + /// + private static bool IsRefusalOf(string entry, string token) => + entry.Length > RefusedTokenPrefix.Length + && entry.StartsWith(RefusedTokenPrefix, StringComparison.Ordinal) + && string.Equals(entry[RefusedTokenPrefix.Length..], token, StringComparison.Ordinal); + private const string ManifestId = "import-manifest"; /// Path of a partition's per-node import manifest (an _Activity node; survives prune). @@ -2941,10 +3128,18 @@ private static ImmutableDictionary ParseManifest(MeshNode? manif /// only evidence about nodes this run did not evaluate. /// The git-diff scope; means the run evaluated /// every source node and may claim the whole map. + /// 🚨 The nodes this run could NOT write (issue #4459/#4456). A run may + /// only claim what it WROTE — the same rule as , for the other + /// way a node can fail to land. A DETERMINISTIC refusal is recorded as a refusal + /// () so the next pass skips the write instead of re-deriving it; a + /// retryable one is dropped from the map entirely, which is what makes the next pass look again. + /// The paths that DID land. A source may carry duplicate entries for + /// one path, so a path can appear in both lists; what landed wins (see the failures clause). private static IObservable WriteManifest( IMessageHub hub, string partition, IReadOnlyList nodes, IReadOnlyDictionary previous, IReadOnlySet? evaluatedPaths, - IReadOnlyCollection heldNodeTypePaths, + IReadOnlyCollection heldNodeTypePaths, IReadOnlyCollection failures, + IReadOnlyCollection writtenPaths, JsonSerializerOptions opts, ILogger? logger) { var map = nodes @@ -2997,6 +3192,54 @@ private static IObservable WriteManifest( map = builder.ToImmutable(); } + // 🚨 #4459/#4456 — A RUN MAY ONLY CLAIM WHAT IT WROTE, and this is the LAST word because it + // is the only clause stating something this run MEASURED. Until now a node whose upsert the + // owner REFUSED was recorded under its plain source token, so the ledger asserted the + // partition held content that is provably not in it — the same false claim the + // content-addressed marker made about the whole partition, one layer down, and the claim + // that has to become honest before that whole-partition skip can be retired. + // + // 🚨 The token comes from the SOURCE node, never from `map`: the git-diff carve-out above + // may just have written the PREVIOUS run's token into this entry (a node absent from the DB + // is attempted even when it is outside the diff scope), and a refusal recorded against the + // wrong token would be re-opened on the next pass — or, worse, honoured for bytes nobody + // refused. + if (failures.Count > 0) + { + var written = writtenPaths.ToImmutableHashSet(StringComparer.OrdinalIgnoreCase); + var sourceTokens = nodes + .Where(n => !string.IsNullOrEmpty(n.Path)) + .GroupBy(n => n.Path, StringComparer.OrdinalIgnoreCase) + .ToImmutableDictionary( + g => g.Key, + g => PartitionSourceFingerprint.ComputeNodeToken(g.First(), opts), + StringComparer.OrdinalIgnoreCase); + var builder = map.ToBuilder(); + foreach (var failure in failures) + { + if (!sourceTokens.TryGetValue(failure.NodePath, out var token)) + continue; + // 🚨 A path that ALSO landed is not a refused path (Copilot review). A source may + // carry two entries at one path — ImportWriteOrder deliberately preserves duplicates + // — so one attempt can fail while another succeeds. Recording a refusal for a node + // that IS in the mesh would make every later pass re-report a refusal that no longer + // describes anything, and `map`'s own token comes from `g.First()`, which need not be + // the entry that won. The written set is the authority on what landed. + if (written.Contains(failure.NodePath)) + continue; + if (failure.Deterministic) + // These bytes break a rule, and they break it again at the same token. Recorded + // so the next pass reports the refusal WITHOUT re-attempting the write (#3146's + // storm) while still evaluating every other node (#4459's freeze). + builder[failure.NodePath] = RefusedEntry(token); + else + // A store blip, an owner that did not answer. Unknown means retryable: leave no + // entry at all, which is exactly the state that makes the next pass look again. + builder.Remove(failure.NodePath); + } + map = builder.ToImmutable(); + } + var node = new MeshNode(ManifestId, $"{partition}/_Activity") { Name = $"Import manifest ({partition})", @@ -3088,6 +3331,9 @@ private sealed record ImportMarkerVerdict( /// 1 when a claim declined the node, else 0. /// The path that landed, or null. /// The path that can never be created, or null. + /// The path that did NOT land and why, or null — always present + /// when Failed: 1, so the caller can NAME what is missing instead of counting it + /// (#4459/#4456). /// The activity line this item contributes, or null. private readonly record struct ImportItem( int Imported = 0, @@ -3097,6 +3343,7 @@ private readonly record struct ImportItem( int Claimed = 0, string? Written = null, string? Blocked = null, + FailedImport? Failure = null, LogMessage? Log = null); /// @@ -3256,9 +3503,11 @@ private static IObservable WriteOne( logger?.LogWarning(ex, "[StaticRepoImport] {Partition}: upsert of {Path} failed (continuing).", partition, node.Path); + var deterministic = IsContentVerdict(ex); return Observable.Return(new ImportItem( Failed: 1, - FailedDeterministic: IsContentVerdict(ex) ? 1 : 0, + FailedDeterministic: deterministic ? 1 : 0, + Failure: new FailedImport(node.Path!, ex.Message, deterministic), Log: new LogMessage( $"⚠ Failed to import {node.Path}: {ex.Message}", Microsoft.Extensions.Logging.LogLevel.Warning) diff --git a/src/MeshWeaver.Messaging.Hub/Localization/strings.de.json b/src/MeshWeaver.Messaging.Hub/Localization/strings.de.json index 2aeb663b60..96f78c7de3 100644 --- a/src/MeshWeaver.Messaging.Hub/Localization/strings.de.json +++ b/src/MeshWeaver.Messaging.Hub/Localization/strings.de.json @@ -1375,6 +1375,7 @@ "activity.import.pruned": "🗑 {path} entfernt (nicht mehr im Repository vorhanden).", "activity.import.pruneRefusedIncompleteListing": "⛔ In {partition} wurde nichts entfernt: Die QUELL-Auflistung kam UNVOLLSTÄNDIG zurück (der Repository-Baum wurde abgeschnitten) — ein dort fehlender Node ist eine ungelesene Datei, keine gelöschte. Alles Gelesene wurde importiert; es wurde nichts entfernt.", "activity.import.itemFailed": "⚠ Import von {path} fehlgeschlagen: {error}", + "activity.import.itemRefusedBefore": "⚠ {path} wurde bei exakt diesem Inhalt bereits von einem früheren Import ABGELEHNT, das Mesh enthält diesen Inhalt also NICHT — dieselben Bytes verletzen dieselbe Regel, und es wurde kein Schreibvorgang versucht. Korrigieren Sie die Quelldatei oder importieren Sie erneut mit „force“, um die Ablehnung wieder vollständig zu sehen.", "activity.prune.heldNodeTypes": "⏸ {count} NodeType(s) zurückgehalten, die die Quelle nicht mehr führt, weil das Mesh noch Instanzen davon enthält — NICHT entfernt (das Löschen des Typs liesse diese Instanzen ohne eigenen Node-Hub zurück; sie würden als Unavailable gelesen und leer gerendert). Ändern Sie den Typ der Instanzen oder löschen Sie sie, dann schliesst der nächste Sync die Ausmusterung ab: {detail}.", "activity.writeConflict.merged": "Ein Schreibvorgang mit Version={staleVersion} hat das Rennen gegen die dauerhafte Version={latestVersion} auf '{path}' verloren und wurde in sie eingefügt.", "activity.writeConflict.durableRow": "Der dauerhafte Datensatz: nodeType='{nodeType}' name='{name}' category='{category}' mainNode='{mainNode}' lastModifiedBy='{lastModifiedBy}'.", @@ -1395,6 +1396,7 @@ "activity.gitsync.reconcileAtProvenCommit.title": "{space} mit dem versiegelten Commit {sha} abgleichen", "activity.gitsync.reconcileAtProvenCommit.fetching": "{sha} — der Commit, aus dem die Bundles dieser Instanz gebaut wurden — wird geholt und die Partition damit abgeglichen…", "activity.gitsync.prunedNodes": "{count} Node(s) entfernt, die im Repository nicht mehr vorhanden sind: {paths}", + "activity.gitsync.failedNodes": "⚠ {count} Node(s) konnten NICHT geschrieben werden — das Mesh enthält diesen Inhalt NICHT, alles was sie referenziert wird daher nicht kompilieren: {paths}", "activity.gitsync.import.done": "Importiert: {outcome} ({count} Node(s)).", "activity.gitsync.import.skipped": "Import übersprungen — ein früherer VOLLSTÄNDIGER Import hat genau diesen Inhalt bereits unter dem Fingerabdruck {fingerprint} ({markerPath}) festgehalten; die Partition wurde daher nicht erneut gelesen.", "activity.gitsync.reimport.title": "{space} bei {commitish} neu importieren", diff --git a/src/MeshWeaver.Messaging.Hub/Localization/strings.en.json b/src/MeshWeaver.Messaging.Hub/Localization/strings.en.json index 8543ae113d..089aee28d2 100644 --- a/src/MeshWeaver.Messaging.Hub/Localization/strings.en.json +++ b/src/MeshWeaver.Messaging.Hub/Localization/strings.en.json @@ -1375,6 +1375,7 @@ "activity.import.pruned": "🗑 Pruned {path} (absent from the repo).", "activity.import.pruneRefusedIncompleteListing": "⛔ Pruned nothing in {partition}: the SOURCE listing came back INCOMPLETE (the repository tree was truncated), so a node missing from it is an unread file, not a deleted one. Everything that was read has been imported; nothing was removed.", "activity.import.itemFailed": "⚠ Failed to import {path}: {error}", + "activity.import.itemRefusedBefore": "⚠ {path} was REFUSED at this exact content by an earlier import, so the mesh does NOT hold it — the same bytes break the same rule, and no write was attempted. Fix the source file, or re-import with force to see the refusal again in full.", "activity.prune.heldNodeTypes": "⏸ Held {count} NodeType(s) the source no longer carries, because the mesh still has instances of them — NOT pruned (deleting the type would leave those instances with no per-node hub, reading as Unavailable and rendering empty). Retype or delete the instances and the next sync completes the retirement: {detail}.", "activity.writeConflict.merged": "A write at Version={staleVersion} lost the race against the durable Version={latestVersion} on '{path}' and was merged into it.", "activity.writeConflict.durableRow": "The durable row: nodeType='{nodeType}' name='{name}' category='{category}' mainNode='{mainNode}' lastModifiedBy='{lastModifiedBy}'.", @@ -1395,6 +1396,7 @@ "activity.gitsync.reconcileAtProvenCommit.title": "Reconcile {space} with the sealed commit {sha}", "activity.gitsync.reconcileAtProvenCommit.fetching": "Fetching {sha} — the commit this instance's bundles were baked from — and reconciling the partition against it…", "activity.gitsync.prunedNodes": "Pruned {count} node(s) absent from the repo: {paths}", + "activity.gitsync.failedNodes": "⚠ {count} node(s) did NOT land — the mesh does NOT hold this content, so anything referencing them will not compile: {paths}", "activity.gitsync.import.done": "Imported {outcome} ({count} node(s)).", "activity.gitsync.import.skipped": "Imported Skipped — an earlier FULL import already recorded this exact content at fingerprint {fingerprint} ({markerPath}), so the partition was not re-read.", "activity.gitsync.reimport.title": "Re-import {space} at {commitish}", diff --git a/test/MeshWeaver.Graph.Test/ARefusedNodeDoesNotFreezeThePartitionTest.cs b/test/MeshWeaver.Graph.Test/ARefusedNodeDoesNotFreezeThePartitionTest.cs new file mode 100644 index 0000000000..a80d35f457 --- /dev/null +++ b/test/MeshWeaver.Graph.Test/ARefusedNodeDoesNotFreezeThePartitionTest.cs @@ -0,0 +1,328 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Reactive.Linq; +using System.Threading; +using System.Threading.Tasks; +using MeshWeaver.Data; +using MeshWeaver.Hosting; +using MeshWeaver.Hosting.Monolith.TestBase; +using MeshWeaver.Markdown; +using MeshWeaver.Mesh; +using MeshWeaver.Mesh.Security; +using MeshWeaver.Mesh.Services; +using MeshWeaver.Utils; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace MeshWeaver.Graph.Test; + +/// +/// 🚨 Issues #4459 and #4456 — ONE refused node froze a whole partition out of the mesh, and +/// nothing named the node. +/// +/// Measured on memex.systemorph.com, 2026-09-15. A single file — +/// Hosting/Deployment/Source/SelfUpdateRouting.cs — carried a LITERAL NUL byte (0x00) inside a +/// char literal. PostgreSQL cannot store a NUL in a text column, so the Hosting GitSync import +/// refused that ONE row while forty others landed. The pass therefore earned +/// ImportedWithContentErrors — every failure in it was a verdict about the bytes — and the +/// content-addressed marker recorded that verdict FOR THE PARTITION. Every later import then +/// answered: +/// +/// Skipped — an earlier FULL import already recorded this exact content at fingerprint +/// bb9801101e859a21 (Hosting/_Activity/import-bb9801101e859a21), so the partition was not re-read +/// +/// which is not true: the import lost a node, and that sentence claims the content was +/// recorded. Five Hosting NodeTypes stayed parked on CS0246 SelfUpdateRouting — among them +/// Hosting/InstanceAction, so NO instance action could run on the control instance at all (no +/// Sample, no Logs, no HelmRelease, no Roll) — and the operator's one repair, a re-import, was +/// refused with that same sentence. The failure count was reported without a single path, so nobody +/// could tell WHICH node was missing; the visible symptom was a compile error on a file that is +/// plainly in git. +/// +/// 🚨 The defect is GRANULARITY, not the #3146 rule it came from. #3146's argument is +/// correct and is pinned in : the +/// marker is content-addressed, so re-issuing a write the owner refused re-derives the identical +/// refusal, and doing it anyway cost memex-cloud 19 full passes in 3 h. But a content verdict is +/// earned by a NODE — one file that Postgres, a validator or RLS refuses — and recording it as a +/// verdict about the PARTITION is what turns one unstorable byte into a frozen partition. The +/// memory now lives per node, in the partition's import manifest, so both hold at once: the failing +/// write is not re-issued, AND every other node is still evaluated. +/// +/// The two tests below are the two halves. The first is the freeze (#4459/#4456); the second +/// is the storm the fix must not reintroduce, measured on WRITE REQUESTS rather than on the word +/// "Skipped" — which is the quantity #3146 was ever about. +/// +public class ARefusedNodeDoesNotFreezeThePartitionTest(ITestOutputHelper output) + : MonolithMeshTestBase(output) +{ + // 🚨 A DERIVED FIELD INITIALIZER RUNS BEFORE THE BASE CONSTRUCTOR, and the base constructor is + // what calls ConfigureMesh — so both of these are already set when the validator is registered. + private readonly string _partition = "Rf" + Guid.NewGuid().ToString("N")[..8]; + + /// + /// The one path the test validator refuses. A validator refusal is mapped to + /// NodeUpsertRejectionReason.ValidationFailed, which StaticRepoImporter.IsContentVerdict + /// classifies as a verdict about the bytes — the same classification the live Postgres NUL-byte + /// refusal earned, reproduced without needing a store that refuses a byte. + /// + private string? _refusePath; + + /// + protected override MeshBuilder ConfigureMesh(MeshBuilder builder) + => base.ConfigureMesh(builder) + .ConfigureServices(services => services.AddSingleton( + new RefuseOnePathValidator(() => _refusePath))); + + /// + /// 🚨 THE PIN FOR #4459/#4456. One node is refused; every other node in the partition must + /// still be evaluated by the next import — including one that has since gone MISSING from the + /// mesh, which is precisely the repair an operator reaches for and precisely the repair the + /// whole-partition skip refused. + /// + /// Deleting Good between the passes stands in for the ways a partition drifts from + /// its marker (a cross-partition prune, a manual delete, a botched migration) — the same drift a + /// GREEN marker has had a content sentinel for since the "a user didn't see the core app skills" + /// report. A marker recording a content verdict had no such path at all: it short-circuited + /// before reading anything. + /// + /// Pre-fix the second pass answers Skipped and writes nothing, so Good stays + /// missing for ever at this fingerprint. Post-fix the refusal is remembered against + /// Bad alone and Good is re-created. + /// + [Fact(Timeout = 300_000)] + public async Task AContentRefusalOnOneNode_DoesNotStopTheNextImportRepairingTheRest() + { + var ct = TestContext.Current.CancellationToken; + _refusePath = $"{_partition}/Bad"; + var source = new FixtureSource(_partition) + { + Root = Space(_partition), + Nodes = [Page(_partition, "Good"), Page(_partition, "Bad")], + }; + + var first = await Import(source, ct); + first.Outcome.Should().Be("ImportedWithContentErrors", + "the one failure in this pass is a validator refusing these bytes — a verdict about the " + + "content, which is what the live NUL-byte refusal was too"); + first.WrittenPaths.Should().Contain($"{_partition}/Good", + "the refusal is per file: every other node still lands"); + + // 🚨 THE WAIT BELOW MUST BE ABLE TO FAIL. It waits for `Good` to LEAVE the parent listing, so + // a listing that never contained `Good` in the first place would satisfy it instantly and the + // test would pass having deleted nothing and repaired nothing. Assert the positive case first: + // if the query shape ever stops seeing the node, this goes red here rather than passing + // vacuously three lines later. + (await ChildPaths(_partition, ct)).Should().Contain($"{_partition}/Good", + "the listing this test waits on must actually see the node, or waiting for it to " + + "disappear proves nothing"); + + // The partition now drifts from what the marker claims — the state an operator re-imports to + // repair. Waited on through the QUERY the importer itself reads, not merely posted: a stale + // snapshot would let the incremental skip pass over the node and prove nothing. + await Delete($"{_partition}/Good", ct); + await WaitUntilAbsentFromTheIndex(_partition, "Good", ct); + + var second = await Import(source, ct); + + second.Outcome.Should().NotBe("Skipped", + "the marker recorded a verdict earned by ONE node; answering Skipped for the PARTITION " + + "claims 'an earlier FULL import already recorded this exact content' about content the " + + "import demonstrably lost, and it refuses the operator's only repair. That sentence is " + + "what memex.systemorph.com was given while Hosting/InstanceAction sat parked on a " + + "symbol whose file was in git the whole time (#4459/#4456)"); + second.WrittenPaths.Should().Contain($"{_partition}/Good", + "a node that has gone missing must be re-created by the next import — the refusal belongs " + + "to Bad, and nothing about Bad's bytes says anything about Good"); + (await Body($"{_partition}/Good", ct)).Should().Contain("page", + "and it is really back in the mesh, not merely reported as written"); + } + + /// + /// 🚨 THE CASE THAT COULD FALSIFY THE ONE ABOVE — #3146's storm must stay prevented. + /// + /// The freeze is removed by re-importing where the old code skipped, so the obvious way to + /// get this wrong is to re-issue the refused write on every trigger: 19 full passes in 3 h, ≈425 + /// identical failing upserts plus a NodeType compile each, on a portal already at 8/8 replicas. + /// So the second pass is measured on — the + /// requests actually issued, counted at subscribe — which is the quantity that storm was made of. + /// Asserting an outcome word would stay green through a revert to re-attempting every node. + /// + /// And the pass must still SAY the partition is incomplete. Going quiet would be the other + /// failure: a green import over a partition missing a node is exactly how the Hosting outage went + /// unnoticed until five NodeTypes parked. + /// + [Fact(Timeout = 300_000)] + public async Task TheRefusedWriteIsNotReIssued_AndTheIncompletenessIsStillReported() + { + var ct = TestContext.Current.CancellationToken; + _refusePath = $"{_partition}/Bad"; + var source = new FixtureSource(_partition) + { + Root = Space(_partition), + Nodes = [Page(_partition, "Good"), Page(_partition, "Bad")], + }; + + var first = await Import(source, ct); + first.WriteRequests.Should().BeGreaterThan(0, "the first pass really did try to write"); + + var second = await Import(source, ct); + + second.WriteRequests.Should().Be(0, + "nothing in this pass can do anything: Good is unchanged and present, and Bad was already " + + "refused at exactly this token. Re-issuing either is the #3146 storm — ≈425 identical " + + "failing upserts plus a NodeType compile per pass, 19 passes in 3 h on memex-cloud"); + second.Failed.Should().Be(1, + "and it must not go QUIET about it either: the partition is still missing a node the " + + "source declares, and a green import over that is how one dropped file went unnoticed " + + "until five NodeTypes parked on it"); + second.Outcome.Should().Be("ImportedWithContentErrors", + "the verdict is unchanged — these bytes still break the same rule"); + } + + /// + /// 🚨 The reporting half of #4459 (defect 2) and #4456 (suggestion 1): "names a count (9), + /// never the paths". Every other thing an import can leave behind names its paths — + /// , PrunedPaths, + /// BlockedCreatePaths, HeldNodeTypePaths, RefusedContent. A failure was the + /// one reported as a bare number, and with it the operator had nothing to act on. + /// + [Fact(Timeout = 300_000)] + public async Task AFailedNode_IsNamedWithItsReason() + { + var ct = TestContext.Current.CancellationToken; + _refusePath = $"{_partition}/Bad"; + var source = new FixtureSource(_partition) + { + Root = Space(_partition), + Nodes = [Page(_partition, "Good"), Page(_partition, "Bad")], + }; + + var result = await Import(source, ct); + + result.FailedPaths.Should().HaveCount(1, + "one node did not land, and the result must carry it — the count and the paths have to " + + "agree, because Failed alone is what the sync baseline reads and the paths are what a " + + "person reads"); + var failure = result.FailedPaths[0]; + failure.NodePath.Should().Be($"{_partition}/Bad"); + failure.Reason.Should().NotBeNullOrWhiteSpace( + "'refused' alone is indistinguishable between a byte Postgres cannot store, a validator " + + "rule and an RLS denial — three problems with three different fixes (#3101's argument, " + + "for nodes this time)"); + failure.Deterministic.Should().BeTrue( + "a validator refusal is a verdict about the bytes, and that flag is what the manifest " + + "records so the next pass does not re-issue the write"); + } + + private async Task Import(FixtureSource source, CancellationToken ct) + { + // 🚨 .Await(), never a bare `await` on the observable: Rx's own awaiter resumes the + // continuation INLINE on the signalling thread, inside the trampoline, and every later await + // in the method inherits that scheduler. + var result = await StaticRepoImporter.ImportSource(Mesh, source) + .FirstAsync().Timeout(240.Seconds()).Await(ct); + Output.WriteLine( + $"outcome={result.Outcome} count={result.Count} failed={result.Failed} " + + $"writeRequests={result.WriteRequests} written=[{string.Join(", ", result.WrittenPaths)}]"); + return result; + } + + private async Task Delete(string path, CancellationToken ct) => + await Mesh.ServiceProvider.GetRequiredService() + .DeleteNode(path) + .FirstAsync().Timeout(60.Seconds()).Await(ct); + + /// + /// Waits until the eventually-consistent index the importer's own snapshot reads no longer lists + /// the node — the re-query shape the house rules prescribe for a request/response source, never a + /// Task.Delay. Without it the next import could read a stale "present", skip the node on + /// its token, and the test would pass having measured nothing. + /// + /// 🚨 A scope:children LISTING of the parent, never a path: point query. + /// Existence of a specific path is not a valid query use — a point read of an absent node is a + /// routing NotFound that terminates the stream and opens the storm-breaker on that path — so the + /// existence half of the question is asked the way the CQRS rules say to ask it: list the parent + /// and look for the child. (Copilot review.) + /// + private async Task WaitUntilAbsentFromTheIndex(string partition, string id, CancellationToken ct) + { + var meshService = Mesh.ServiceProvider.GetRequiredService(); + var path = $"{partition}/{id}"; + await Observable.Interval(50.Milliseconds()).StartWith(0L) + .SelectMany(_ => ChildPathsOnce(meshService, partition)) + .Where(paths => !paths.Contains(path, StringComparer.OrdinalIgnoreCase)) + .FirstAsync().Timeout(60.Seconds()).Await(ct); + } + + /// One reading of the parent's child listing — the same listing the wait above polls, so + /// the pre-delete assertion and the wait can never disagree about what they are looking at. + private async Task> ChildPaths(string partition, CancellationToken ct) => + await ChildPathsOnce(Mesh.ServiceProvider.GetRequiredService(), partition) + .FirstAsync().Timeout(60.Seconds()).Await(ct); + + private static IObservable> ChildPathsOnce( + IMeshService meshService, string partition) => + meshService + // .Complete() — this is an ENUMERATION used as an existence gate, so it must never be + // silently served as a page. + .Query(MeshQueryRequest.FromQuery($"path:{partition} scope:children").Complete()) + .Take(1) + .Select(change => (IReadOnlyList)change.Items.Select(n => n.Path).ToArray()); + + private async Task Body(string path, CancellationToken ct) + { + var node = await Mesh.GetWorkspace().GetMeshNodeStream(path) + .Where(n => n is not null) + .FirstAsync().Timeout(60.Seconds()).Await(ct); + return node.ContentAs(Mesh.JsonSerializerOptions)?.Content ?? ""; + } + + private static MeshNode Space(string partition) => new(partition) + { + Name = partition, NodeType = "Space", State = MeshNodeState.Active, + Content = new MarkdownContent { Content = $"# {partition}\n\nfixture." } + }; + + private static MeshNode Page(string partition, string id) => new(id, partition) + { + NodeType = "Markdown", Name = id, State = MeshNodeState.Active, + Content = new MarkdownContent { Content = $"# {id}\n\npage" } + }; + + private sealed class FixtureSource(string partition) : IStaticRepoSource + { + public string Partition => partition; + public bool Versioned => false; + + /// + /// Immutable, per the repository's collections policy — the fixture only ever initializes it, + /// so nothing is lost by refusing a mutable one here. (Copilot review.) + /// + public ImmutableList Nodes { get; init; } = ImmutableList.Empty; + + public MeshNode? Root { get; init; } + + public IReadOnlyList EnumerateSourceNodes() => Nodes; + public MeshNode? PartitionRoot => Root; + public IReadOnlyList EnumerateInlineContentSyncs() => []; + } + + /// + /// Refuses ONE path. The bulk verb runs the same INodeValidator pass the singular create + /// runs, so the refusal reaches the importer as + /// NodeUpsertRejectionReason.ValidationFailed — a content verdict. + /// + private sealed class RefuseOnePathValidator(Func path) : INodeValidator + { + public IReadOnlyCollection SupportedOperations { get; } = [NodeOperation.Create]; + + public IObservable Validate(NodeValidationContext context) + => Observable.Return( + string.Equals(context.Node.Path, path(), StringComparison.Ordinal) + ? NodeValidationResult.Invalid( + $"'{context.Node.Path}' is refused by the test validator") + : NodeValidationResult.Valid()); + } +} diff --git a/test/MeshWeaver.Graph.Test/FailedImportIsNotRetriedAtTheSameFingerprintTest.cs b/test/MeshWeaver.Graph.Test/FailedImportIsNotRetriedAtTheSameFingerprintTest.cs index 450423aa2d..1f9385e106 100644 --- a/test/MeshWeaver.Graph.Test/FailedImportIsNotRetriedAtTheSameFingerprintTest.cs +++ b/test/MeshWeaver.Graph.Test/FailedImportIsNotRetriedAtTheSameFingerprintTest.cs @@ -35,6 +35,18 @@ namespace MeshWeaver.Graph.Test; /// re-importing. So only a pass whose failures were every one a content verdict records a /// final verdict; a single retryable failure among them keeps the ordinary Warning. The two tests /// below are that distinction, and the second is the one that could falsify the first. +/// +/// 🚨 Updated for #4459/#4456 — the rule is unchanged, its GRANULARITY is not. The +/// memory of "these bytes cannot be written" moved from the content-addressed marker (a verdict +/// about the PARTITION) to the per-node import manifest (a verdict about the NODE that earned it), +/// because recording it per partition is what froze Hosting out of memex.systemorph.com over +/// a single NUL byte: the next import answered Skipped without reading anything, and the +/// operator's repair was refused with a sentence claiming the content had been recorded. So these +/// tests now assert on — the write requests +/// actually issued — which is the quantity the memex-cloud storm was made of and the one thing the +/// word Skipped was ever standing in for. The partition-level skip of a clean import +/// () is untouched. +/// is the other side of this pair. /// public class FailedImportIsNotRetriedAtTheSameFingerprintTest(ITestOutputHelper output) : MonolithMeshTestBase(output) @@ -92,9 +104,10 @@ private static MeshNode NestedSpace(string partition, string id) => /// /// 🚨 THE PIN. Import twice with the SAME content. The second pass must not touch the mesh. /// - /// Pre-fix both passes return an importing outcome and re-run every upsert and every - /// compile; post-fix the second is Skipped, which is the same word the green short-circuit - /// uses — because it is the same fact: this fingerprint has a verdict. + /// Pre-#3146 both passes re-ran every upsert and every compile. The pin is that the second + /// pass issues NO write request at all — measured, not inferred from an outcome word, because a + /// word can stay right while the writes come back (#4459's fix re-imports where this used to + /// answer Skipped, and the storm protection now lives in the per-node refusal memory). /// [Fact(Timeout = 300_000)] public async Task AContentVerdict_IsFinalForThatFingerprint() @@ -118,12 +131,17 @@ public async Task AContentVerdict_IsFinalForThatFingerprint() // Same source, same fingerprint — the state memex-cloud was in on every webhook. var second = await StaticRepoImporter.ImportSource(Mesh, source) .FirstAsync().Timeout(240.Seconds()).Await(TestContext.Current.CancellationToken); - Output.WriteLine($"second = {second.Outcome}"); - - second.Outcome.Should().Be("Skipped", - "the marker is content-addressed, so re-running re-reads the same nodes and re-derives " - + "the same refusal; doing it anyway cost memex-cloud 19 full passes in 3 h — ≈425 " - + "failing upserts plus a NodeType compile each — on a portal already at 8/8 replicas"); + Output.WriteLine($"second = {second.Outcome} writeRequests={second.WriteRequests}"); + + second.WriteRequests.Should().Be(0, + "the refusal is remembered against the node that earned it, so re-running re-reads the " + + "same bytes and issues NOTHING; doing it anyway cost memex-cloud 19 full passes in 3 h " + + "— ≈425 failing upserts plus a NodeType compile each — on a portal already at 8/8 " + + "replicas"); + second.Outcome.Should().Be("ImportedWithContentErrors", + "and it still SAYS the partition is missing a node the source declares. Answering " + + "Skipped for the whole partition is what froze Hosting out of memex.systemorph.com " + + "over one NUL byte (#4459) — the refusal belongs to Nested, never to the partition"); } /// @@ -201,9 +219,10 @@ public async Task AThrowingValidator_IsRecordedAsAContentVerdict() var second = await StaticRepoImporter.ImportSource(Mesh, source) .FirstAsync().Timeout(240.Seconds()); - Output.WriteLine($"second = {second.Outcome}"); + Output.WriteLine($"second = {second.Outcome} writeRequests={second.WriteRequests}"); - second.Outcome.Should().Be("Skipped", - "and it follows through: the marker is final, so the next trigger skips"); + second.WriteRequests.Should().Be(0, + "and it follows through: the verdict is remembered against Flaky, so the next trigger " + + "re-issues nothing"); } }