fix: a content verdict is final for the NODE that earned it, never for the partition (#4459, #4456) by rbuergi · Pull Request #4467 · Systemorph/MeshWeaver

rbuergi · 2026-09-16T05:13:08Z

Pairs-with: none — this diff removes no public type and no public member from src/. It ADDS one public record (FailedImport) and one init property (StaticRepoImportResult.FailedPaths); the only signature change is to the private StaticRepoImporter.WriteManifest.

Mirror-sync: npm run sync:i18n -- --ref <this PR's merged core sha> — two keys added (activity.import.itemRefusedBefore, activity.gitsync.failedNodes), no existing value re-worded.

Implementers: none — no member is added to any public interface or abstract class.

The root cause

On 2026-09-15 one 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.

A pass whose failures were all deterministic earns ImportedWithContentErrors. That word means "every failure was a verdict about the bytes", not "everything failed" — but it was recorded on the content-addressed marker, whose whole job is to say something about the partition. Every later import read it and answered:

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.

A claim about forty nodes, made on the evidence of one, and false: the import lost a node. Five Hosting NodeTypes stayed parked on CS0246 SelfUpdateRouting — among them Hosting/InstanceAction, so no instance action would run on the control instance at all (no Sample, Logs, HelmRelease or Roll) — and the operator's one repair, a re-import, was refused with that same sentence.

So #3146's rule is right and its GRANULARITY was the defect. Re-issuing a write the owner refused cannot help — that is real, and it cost memex-cloud 19 full passes in 3 h. But a content verdict is earned by a node, and recording a per-node fact as a per-partition verdict is what turned one unstorable byte into a frozen partition.

#4456 reads the same evidence as "applies modifications but not additions". It is the same single dropped file: nothing named it, so the shape of the loss had to be inferred. Its "a force update imports 0 nodes" is the same file being refused again — correctly, on bytes that genuinely cannot be stored.

The fix

1. The memory moves to the per-node import manifest.

WriteManifest may only claim what the run wrote. A refused node used to be recorded under its plain source token — the ledger asserting the partition holds content that is provably not in it, which is the marker's false claim one layer down, and it had to become honest before the marker's skip could be retired.

what happened to the node	what the manifest now records
written	its source token
refused, deterministically	`!` + its source token
failed, retryably	nothing at all — which is what makes the next pass look again

The ! prefix is deliberately a prefix and not a schema change: 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, no write attempted — and ImportedWithContentErrors stops being a fingerprint-recording outcome. The marker keeps it as a record; it is no longer a licence to answer Skipped without reading the partition. Force and Reconcile bypass the refusal memory exactly as they bypass the marker.

Both properties then hold at once: the failing write is not re-issued (no storm), and every other node is still evaluated (no freeze) — so a partition that has drifted from its marker can be repaired, which is the repair the whole-partition skip refused.

Cost: a partition holding a refused node re-evaluates on every trigger instead of short-circuiting — no node writes and no recompiles, just one subtree read, the prune evaluation and two bookkeeping writes. That is the same cost the marker already accepts for an absent or non-converged verdict.

3. An import that loses a node NAMES it. Every other thing a pass leaves behind already named its paths (written, pruned, blocked creates, held NodeTypes, refused content); failures were the one category reported as a bare number. StaticRepoImportResult.FailedPaths now carries each failed node with the write path's own reason and whether it was deterministic — #3101's argument, for nodes this time. The terminal summary names them, and the GitSync activity logs them as their own line.

4. A partial import no longer renders green. Every outcome line in GitHubActivityExtensions was written at Information whatever the import did, and RunActivity derives the activity's maxSeverity and terminal status from those levels — which is how the import that lost SelfUpdateRouting reported maxSeverity: Information, status: Succeeded. The level now follows the outcome: Failed → error, any ImportedWith… → warning.

The control

Both new behavioural tests use only pre-existing API, so the fix could be reverted and the suite still compiles.

Fix reverted (src/MeshWeaver.Graph/StaticRepoImporter.cs + the GitSync half back at main), test project rebuilt, run — 3 failed / 2 succeeded, with the defect printed verbatim:

outcome=ImportedWithContentErrors count=1 failed=1 writeRequests=3 written=[Rf9a6279eb/Good]
[StaticRepoImport] Rf9a6279eb already FAILED at b7b778c951b8aabb on content rules — skipping.
outcome=Skipped count=0 failed=0 writeRequests=0 written=[]
=== TEST FAILED: Did not expect value to be "Skipped" …

Fix restored, rebuilt, re-run — 6 succeeded / 0 failed.

suite	result
`ARefusedNodeDoesNotFreezeThePartitionTest` + `FailedImportIsNotRetriedAtTheSameFingerprintTest`	6/6
`MeshWeaver.Graph.Test` (full)	1717/1717
`MeshWeaver.Hosting.Test` (full)	728/728
`MeshWeaver.Documentation.Test` (full)	476/476
`LocalizationTest`	60/60

dotnet build -c Release -warnaserror clean (0 Warning(s), 0 Error(s)) on MeshWeaver.Graph, MeshWeaver.GitSync, MeshWeaver.Documentation and each test project. check-record-signatures.py and check-type-forwards.py pass locally against origin/main.

🚨 #3146's own pins changed deliberately. They asserted the word Skipped; they now assert WriteRequests == 0 — the quantity that storm was actually made of (≈425 failing upserts plus a compile, per pass). A word can stay right while the writes come back; a request count cannot.

🚨 What this deliberately does NOT fix

Both issues list a third item, and it is the same one: a NodeType parked on a symbol whose source the import could not write still fails on CS0246 for a file that is plainly in git, with nothing in that error connecting it to the import. Split out as #4469 rather than left implied — closing two issues while one of their listed defects stays live, without saying so, is the failure mode where an answer reads like a pass.

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 #4469 needs are already recorded here (the manifest's !<token> entries say which declared nodes a partition is missing), so it is a wiring problem, not a measurement one — with the granularity trap noted in it: scoping a compile refusal to a whole partition would re-create exactly the over-broad reading this PR removes.

Also unchanged on purpose: a remembered refusal is still reported as a failure, so MayAdvanceBaseline still holds the git baseline (the mesh genuinely is not at that commit), and VerdictIsFinal is untouched, so #3945's cheap-delivery path is unaffected.

Work products

Doc/Architecture/AContentVerdictIsPerNode — the incident, the granularity argument, the manifest encoding, the cost, the residue (A NodeType parked on a source node the import could not write should name the import, not just CS0246 #4469) and what is deliberately unchanged. Linked from Architecture.md and from ImportMarkerRecordsConvergence.md.
A What's New entry (Category: Fix).
A NodeType parked on a source node the import could not write should name the import, not just CS0246 #4469 — the compiler-side half, filed rather than implied.

🤖 Generated with Claude Code

…r the partition (#4459, #4456) One literal NUL byte in `Hosting/Deployment/Source/SelfUpdateRouting.cs` took every instance action on memex.systemorph.com offline for an evening. Postgres cannot store a NUL in a text column, so the import refused that ONE row while forty others landed. The pass therefore earned `ImportedWithContentErrors` — which means "every failure was deterministic", not "everything failed" — and the content-addressed marker recorded that verdict FOR THE PARTITION. Every later import answered: Skipped — an earlier FULL import already recorded this exact content at fingerprint bb9801101e859a21 … so the partition was not re-read a claim about forty nodes made on the evidence of one, and false: the import lost a node. Five Hosting NodeTypes stayed parked on CS0246 SelfUpdateRouting — among them Hosting/InstanceAction, so no Sample, Logs, HelmRelease or Roll would run at all — and the operator's one repair, a re-import, was refused with that same sentence. #3146's rule is right; its GRANULARITY was the defect. A content verdict is earned by a NODE, so the memory of "these bytes cannot be written" moves to the per-node import manifest: - WriteManifest may only claim what the run WROTE. A refused node used to be recorded under its plain source token — the ledger asserting the partition holds content that is provably not in it, the marker's false claim one layer down. A deterministic refusal is now recorded as `!<token>`; a retryable failure leaves no entry at all, which is what makes the next pass look again. The prefix is forward/backward compatible: a reader that does not know it sees a token mismatch and re-evaluates — the safe direction. - The import loop skips a node whose manifest entry is a refusal OF THIS EXACT TOKEN, reported with no write attempted, so #3146's storm stays prevented per node. - `ImportedWithContentErrors` stops being a fingerprint-recording outcome. The marker keeps it as a record; it is no longer a licence to answer Skipped without reading the partition, so a later import can create, repair and prune again. Force and Reconcile bypass the refusal memory exactly as they bypass the marker. And an import that loses a node now NAMES it. Every other thing a pass leaves behind named its paths — written, pruned, blocked creates, held NodeTypes, refused content; failures were the one category reported as a bare number. `FailedPaths` carries each failed node with the write path's own reason and whether it was deterministic, the terminal summary names them, and the GitSync activity logs them on their own line. That activity's outcome line was also written at Information whatever the import did, and RunActivity derives the activity's severity and terminal status from those levels — which is how an import that dropped a source node reported maxSeverity Information and status Succeeded. The level now follows the outcome. Control: with the fix reverted, `AContentRefusalOnOneNode_DoesNotStopTheNextImportRepairingTheRest` and `TheRefusedWriteIsNotReIssued_AndTheIncompletenessIsStillReported` fail on `outcome=Skipped count=0 failed=0 writeRequests=0`, the log line naming the defect verbatim ("already FAILED at … on content rules — skipping"); restored, 6/6 pass. MeshWeaver.Graph.Test 1717/1717, MeshWeaver.Hosting.Test 728/728, MeshWeaver.Documentation.Test 476/476, LocalizationTest 60/60. #3146's own pins move from asserting the word "Skipped" to asserting WriteRequests == 0 — the quantity that storm was actually made of, and one a rename cannot make vacuous. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…y what is true of both A content verdict can refuse an UPDATE as easily as a CREATE: a validator rule, an invalid path or an RLS denial on a node that already exists leaves that node PRESENT and STALE, not missing. Three of the new sentences said "is NOT in the mesh", which is right for the incident that prompted them and wrong for the other half of the cases — and an operator told a node is absent goes looking for the wrong thing. They now say the mesh does not hold this CONTENT, which is exactly what is true in both: the import's own summary, the per-node ⚠ line, the GitSync activity line, and both catalogs. MeshWeaver.Graph.Test import suites 43/43, LocalizationTest 60/60, `-c Release -warnaserror` clean on MeshWeaver.Graph and MeshWeaver.GitSync. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…ing actually returns The argument for the '!' sigil rests on the token's alphabet, so the comment stating that alphabet has to be checkable. ComputeNodeToken ends in Convert.ToHexString, which returns 0-9A-F; the comment said lowercase. The argument is unaffected (no hex of either case contains '!'), but a reader verifying it would have found the stated fact wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…s at nothing (#4469) Both issues list a third item: a NodeType parked on a symbol whose source the import could not write fails on CS0246 for a file that is plainly in git, with nothing connecting the error to the import that produced the state. This change fixes the other direction — the sync activity now names the missing file and is no longer green — and does not reach that one. Closing two issues while one of their listed defects stays live, without saying so, is the failure mode where an answer reads like a pass. Split out as #4469, with the note that the facts are already recorded (the manifest's !<token> entries say which declared nodes a partition is missing) and the warning that a fix must be scoped to the types that actually reference the missing node — refusing to compile a whole partition would re-create the over-broad granularity this change removes. Also states what is deliberately unchanged about the baseline: a remembered refusal is still a failure, so MayAdvanceBaseline still holds (the mesh is not at that commit), and VerdictIsFinal is untouched so the delivery stays cheap. MeshWeaver.Documentation.Test 476/476 (assembly re-embedded — verified newer than the edited page, never --no-build over a stale one). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

An unresolved critical GitSync finality issue and additional moderate findings remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR moves deterministic import refusals from partition-wide markers to per-node manifests, preventing one failed node from freezing a partition.

Changes:

Records refusals per node and reports failed paths.
Updates GitSync severity and localization.
Adds regression tests and documentation.

File summaries

File	Review summary
`test/MeshWeaver.Graph.Test/FailedImportIsNotRetriedAtTheSameFingerprintTest.cs`	No final findings.
`test/MeshWeaver.Graph.Test/ARefusedNodeDoesNotFreezeThePartitionTest.cs`	Moderate (2): use a parent-child listing rather than an exact-path existence check. Nit (2): use an immutable collection.
`src/MeshWeaver.Messaging.Hub/Localization/strings.en.json`	No final findings.
`src/MeshWeaver.Messaging.Hub/Localization/strings.de.json`	No final findings.
`src/MeshWeaver.Graph/StaticRepoImporter.cs`	Critical (3): content-error attempts can still persist partition-level finality and cause webhook skips. Moderate findings cover legacy manifest entries, failed-update reporting, stale-node messaging, and duplicate-path handling. Nit (2): localize the failed-path summary.
`src/MeshWeaver.GitSync/GitHubActivityExtensions.cs`	Moderate (2): failed updates should not be reported as absent nodes.
`src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-16-one-file-the-import-cannot-write-no-longer-freezes-the-whole-space.md`	No final findings.
`src/MeshWeaver.Documentation/Data/Architecture/ImportMarkerRecordsConvergence.md`	No final findings.
`src/MeshWeaver.Documentation/Data/Architecture/AContentVerdictIsPerNode.md`	Nit (1): align the documentation with the existing `StaticRepoImport.md` contract.
`src/MeshWeaver.Documentation/Data/Architecture.md`	No final findings.

Review details

Suppressed comments (4)

src/MeshWeaver.Documentation/Data/Architecture/AContentVerdictIsPerNode.md:101

This new rule contradicts the existing StaticRepoImport.md contract, whose outcome table still says ImportedWithContentErrors leaves a Failed marker that skips the next same-fingerprint import. Since this page says the marker no longer licenses that skip, the canonical import documentation also needs to be updated; otherwise operators and implementers get two different behaviors for the same outcome.

**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.

src/MeshWeaver.Graph/StaticRepoImporter.cs:3215

Existing manifests written before this change store failed nodes as plain tokens. This loop only recognizes the new !token; the later ordinary prevToken == token branch then treats an old failed update to an existing node as successfully imported, so the first post-upgrade pass never retries it. Add a one-time manifest migration/versioned success encoding (or otherwise invalidate old plain entries) before allowing the ordinary skip.

                    g => PartitionSourceFingerprint.ComputeNodeToken(g.First(), opts),
                    StringComparer.OrdinalIgnoreCase);
            var builder = map.ToBuilder();
            foreach (var failure in failures)
            {

src/MeshWeaver.Graph/StaticRepoImporter.cs:2127

This terminal summary has the same factual problem as the GitSync line: a failed WriteOne may be an update, in which case an older node is still present. Saying the path is "NOT in the mesh" and that every reference cannot compile misleads operators about what repair is needed. Use neutral failed-write wording or distinguish failed creates from failed updates.

                            // 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

src/MeshWeaver.Graph/StaticRepoImporter.cs:3205

count.Failures can include a failed earlier attempt for a path that a later duplicate source entry successfully wrote; ImportWriteOrder deliberately preserves those duplicates, but both this map and the failure-token map use g.First(). The manifest then retains !<first-token> even though the final node landed from the later entry, so every subsequent import re-reports the stale refusal and re-writes the successful duplicate. Derive the manifest token from the final attempted/written duplicate (or resolve duplicate paths before applying refusal memory), rather than unconditionally taking the first.

        // 🚨 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)
        {

Files reviewed: 10/10 changed files
Comments generated: 6
Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    /// <para>🚨 <b>It is NOT a fingerprint-recording outcome (issue #4459).</b> The marker keeps it
+    /// as a RECORD of what the pass found; it is no longer a licence to answer <c>Skipped</c> 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 <c>StaticRepoImporter.RefusedTokenPrefix</c>),
+    /// which keeps #3146's storm protection — the failing write is not re-issued — without the


+        return new LogMessage(
+                $"⚠ {result.FailedPaths.Count} node(s) did NOT land and are NOT in the mesh — "
+                + $"anything referencing them will not compile: {paths}",


+                                    Failure: new FailedImport(path, RememberedRefusalReason, true),
+                                    Log: new LogMessage(
+                                        $"⚠ {path} was REFUSED at this exact content by an earlier import "
+                                        + "and is NOT in the mesh — the same bytes break the same rule, so "


+        var meshService = Mesh.ServiceProvider.GetRequiredService<IMeshService>();
+        await Observable.Interval(50.Milliseconds()).StartWith(0L)
+            .SelectMany(_ => meshService
+                .Query<MeshNode>(MeshQueryRequest.FromQuery($"path:{path}"))
+                .Take(1))


+                            var failedNote = count.Failures.Count > 0
+                                ? $" ⚠ {count.Failures.Count} node(s) that did NOT land — they are NOT in "
+                                  + "the mesh, so anything referencing them will not compile: "
+                                  + string.Join("; ", count.Failures.Take(BlockedPathsNamed)
+                                      .Select(f => $"{f.NodePath} ({f.Reason})"))


+    {
+        public string Partition => partition;
+        public bool Versioned => false;
+        public List<MeshNode> Nodes { get; init; } = [];


github-actions · 2026-09-16T05:28:33Z