fix(release): the re-cut's failure carries its reason — a bound that faults, not one that substitutes by rbuergi · Pull Request #5152 · Systemorph/MeshWeaver

rbuergi · 2026-09-21T19:58:48Z

Addresses issue 5057 — and deliberately does NOT close it. No closing keyword is used on purpose: this lands the diagnosability the issue asked for first, and the root it immediately revealed is named here but not fixed. Production verification and what is still owed are at the bottom, and on the issue itself: #5057 (comment)

What the defect actually is

ReleasePostCondition exists because a consumed release request can leave latestReleasePath naming an earlier build, and its answer is to re-cut the release from the bytes the compile just produced. That remedy can fail, and when it did it reported the failure in the one shape nobody can act on. The production line, in full:

[ReleasePostCondition] Hosting/InstanceRequest: a release request (requestedReleaseAt=…) was consumed and this compile succeeded, yet latestReleasePath still names '…' — cut for an EARLIER build (lastCompiledVersion 4650 → 4658) — AND the release could not be re-cut. The node advertises a build no release names; instances will keep binding '…' until a release is created for it.

It names the consequence, the stale path, the build and the cost. It cannot name the cause — and the cause is the only thing that decides what to do. The reason was discarded one frame below: NodeTypeBuildState.TryCreateReleaseNode collapsed four unrelated failures into a bare IObservable<string?> emitting null:

channel	what it logged
no `IMeshService` on the hub	nothing
the create was refused (attribution, validation, a partition the requester may not write)	`Warning` + stack
the create threw while being composed	`Warning` + stack
the 10 s bound expired	nothing at all

The last row is the channel the incident names, and it was unobservable by construction: Timeout(bound, Observable.Return<string?>(null)) substitutes the fallback sequence rather than faulting, so the expiry never reached the Catch that logs. Its only trace in production is a gap between two adjacent lines — Hosting/InstanceRequest logged "Re-cutting…" at 22:16:14Z and "…could not be re-cut" at 22:16:24Z, exactly the bound. And the three channels that did log logged at Warning, which the log watcher does not ingest — so the loudest line in the log is the one that cannot say why.

A remedy that reports its own failure without being able to report the failure is the defect it exists to catch, one level up: the output looks like a diagnosis and carries none.

Still live, measured today

get @Hosting/InstanceRequest on the control instance, 2026-09-21 ~19:50Z: compilationStatus: Ok, lastCompiledVersion: 4898 (compiled 09-21 09:57Z), requestedReleaseAt == lastReleaseRequestHandledAt == 2026-09-20T23:23:19Z (consumed), and latestReleasePath naming …/Release/20260920223612-BEscMFNh — cut the previous day. The node still advertises a build no release names, and reads perfectly healthy from every field.

What changed

ReleaseCreateOutcome carries the path or the reason, with Attempted separating a failure from a create nobody asked for. Three states, because a string? holds two.
Timeout(CreateBound) FAULTS, routing the expiry into the same Catch as every other failure, where it becomes a reason naming the bound it waited out and saying the create's fate is UNKNOWN rather than known not to have happened — the difference between re-issuing it and going to look.
Every sentence the post-condition emits — the Error line and the compile _Activity diagnosis — names the cause of both attempts: the settle's own create (until now a Warning reaching no operator-facing surface at all) and the re-cut.
Because never returns the empty string: an attempted failure arriving with no reason SAYS the reason is missing, so this defect cannot reappear wearing terseness.
The batch bake runs no post-condition at all and its stamp keeps the previous release path, so it now logs the reason itself. It is unwatched by design, which is exactly why the reason has to be in its log rather than inferred from a release that never appeared.
Sentence composition extracted as pure functions (FirstAttemptClause, RestoredDiagnosis, FailedDiagnosis, Describe) so the property is asserted rather than hoped for — the same shape Violation already had.

🚨 The bound was NOT widened and must not be. A create that cannot land in ten seconds is not a slow create, and raising the number is precisely the band-aid that would have kept this unobservable for longer. CreateBound is named so the refusal can quote it and so no test writes the number again.

Controls — one on each side, both measured

The failure is reported, and the assertion CAN fail. With the pre-fix wording restored on top of the fix, run against the same tests:

MeshWeaver.Graph.Test.ReleaseRecutReportsWhyItFailedTest.TheFailedDiagnosis_NamesBothCauses [FAIL]
      Assert.Contains() Failure: Sub-string not found
MeshWeaver.Graph.Test.ReleaseRecutReportsWhyItFailedTest.ATimeout_NamesTheBoundItWaitedOut [FAIL]
      Assert.Contains() Failure: Sub-string not found
   MeshWeaver.Compiler.Pipeline.Test  Total: 11, Errors: 0, Failed: 2, Skipped: 0, Not Run: 0

A healthy pass still reads healthy. After restoring the fix, the three release classes together:

   MeshWeaver.Compiler.Pipeline.Test  Total: 29, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0

TheRestoredDiagnosis_ReadsAsARepair is the load-bearing half of that side: a re-cut that WORKED must read as a repair, naming where the release landed, with no "could not" or "VIOLATED" in it — because a fix that made every outcome sound like a failure would satisfy every other assertion here and be worse than the defect. ALandedCreate_Succeeds_AndCarriesNoReason and ALandedFirstCreate_IsWordedAsALanding keep the success wording total.

Builds, one project per invocation, -c Release -warnaserror, each 0 Warning(s) / 0 Error(s): MeshWeaver.Compiler.Pipeline, MeshWeaver.Hosting, MeshWeaver.Graph, MeshWeaver.Compiler.Pipeline.Test, MeshWeaver.Documentation.Test. Doc gates: DocumentationLinkIntegrityTest + ArchitectureTopicMapTest + NoConflictMarkersGuard — 6 tests, 0 failed. scripts/check-record-signatures.py: ✓ 5 changed C# file(s): no binary-breaking primary-constructor changes.

Surface, and why no pair declaration

TryCreateReleaseNode, ReleaseCreateOutcome, CreateBound, Describe and ReleasePostCondition.* are all internal (InternalsVisibleTo: MeshWeaver.Graph, MeshWeaver.Hosting, and the test projects). No public top-level type or public member is removed, and no interface member is added — so Pairs-with: and Implementers: do not apply. Two i18n catalog keys ARE added, for review finding 2 below, so this PR carries the Mirror-sync: declaration the mirror gate wants.

🚨 But internal is not the same as unreachable, and no gate checks it. MeshWeaver.Hosting.Monolith.Test — which lives in MeshWeaver.Plugins, not here — is on the InternalsVisibleTo list, and it is where ReleasePostConditionAtSettleTest pins this exact settle end-to-end. A changed internal signature is not a public-surface removal, so Cross-repo pair (public surface) cannot fire on it: this is one of the break shapes the gates do not see. Checked by hand against Systemorph/MeshWeaver.Plugins origin/main:

ReleasePostConditionAtSettleTest references neither Restore nor TryCreateReleaseNode — it drives the settle through the mesh, not through these symbols.
The only two files in that repo mentioning TryCreateReleaseNode at all are src/Memex.Database.Migration/Migrations/V19_DeleteLegacyReleaseNodes.cs:17 and src/MeshWeaver.PluginCatalog.Test/RetiredNodePruneTest.cs:189, and both are comments — naming MeshDataSource.TryCreateReleaseNode, a spelling that no longer exists either way.

So there is no compile-time caller outside this repo and no pairing is owed. Stated because the absence had to be measured, not assumed.

Review findings, both accepted and both answered on their threads

1 — the controls did not cover the load-bearing change. Correct, and it is the change's own standard turned back on it: reverting Timeout(CreateBound) to a substituting Timeout(CreateBound, other) left every new test green, because Describe was only reached through a TimeoutException I built by hand. The bounded wait is now a seam — Bounded(IObservable<Unit>, string, IScheduler?, ILogger?), pure over its source and its clock, the same shape FleetWatchHeartbeat.Track uses — driven by a HistoricalScheduler: no mesh, no clock, no sleep. Four cases through the production .Timeout(…).Catch(…).

On the scheduler argument, pre-empting the obvious question: Bounded passes scheduler ?? DefaultScheduler.Instance where the code previously called the no-scheduler Timeout(dueTime) overload. That is behaviour-identical in production, not a change: Rx's Timeout(source, dueTime) delegates to Timeout(source, dueTime, SchedulerDefaults.TimeBasedOperations), and SchedulerDefaults.TimeBasedOperations is DefaultScheduler.Instance. Production passes null and therefore gets exactly the scheduler it got before; only the tests pass anything else. Making it a parameter is what lets the expiry be driven rather than waited out — there is no timeout binary on this host and a sleep-based test of a ten-second bound would be both slow and a different measurement.

Measured, with the substituting bound restored on top of the fix:

MeshWeaver.Graph.Test.ReleaseRecutReportsWhyItFailedTest.AnElapsedCreate_ReportsTheBound_ThroughTheRealChain [FAIL]
      Assert.NotNull() Failure: Value is null
   MeshWeaver.Compiler.Pipeline.Test  Total: 15, Errors: 0, Failed: 1

2 — the _Activity transcript entry was unkeyed, so it rendered its English interpolation to every viewer forever. Verified rather than taken: LogMessage says it in its own words (a transcript is a RENDERED surface, Message alone cannot be localized, #3236), and the sibling entry two lines above already keys itself as activity.compile.releaseCreated. Pre-existing — it came in with #781 — but this diff makes that string longer and richer, which is exactly when the rule applies. Restore now returns the keyed entry carrying its own level; activity.compile.releasePostCondition.{restored,violated} added to both catalogs with the English text kept as the FALLBACK, so an old persisted row and a key that later leaves the catalog still render as they do today. The arguments stay English on purpose: a node path, a release path, a store version and an exception's own message are not translatable.

Mirror-sync: MeshWeaver.Plugins runs the i18n sync (npm run sync:i18n) against this merge sha after it lands — two keys added, activity.compile.releasePostCondition.restored and .violated, with no existing value re-worded. The mirror pins a core commit rather than tracking main, so adding a key reds nothing there and this handover is the only thing that keeps it from going stale silently.

🚨 What the reportable expiry immediately revealed — the ROOT, measured

Once the expiry had a name, one read settled what it means, and it is not what "timed out" reads as. Timeout disposes this process's subscription to the CreateNode response; the request is already on the bus, so the owning hub writes the node whether or not anyone is still listening. A release id is {yyyyMMddHHmmss}-{8 chars of SHA256(Collection/ContentPath)} — so the id records when the attempt STARTED and createdDate records when it LANDED, and the gap is measurable from the store with no instrumentation at all.

Control instance, Hosting/InstanceRequest/Release/*, 200 nodes (a floor — the listing truncated at the limit):


median id-mint → landed	0.7 s
beyond the 10 s bound	8 of 200 (4%)
slowest	17.8 s — `20260920222136-3V8XoerZ`, landed 2026-09-20T22:21:53Z, minutes after the incident burst

So a refusal reading "the release could not be re-cut" was, 4% of the time, emitted over a release node that exists, with the pointer never advanced to it.

🚨 And the retry compounds it, because the id encodes the SECOND:

20260917173651-dU1GWMZG   landed 2026-09-17T17:37:06.550Z
20260917173701-dU1GWMZG   landed 2026-09-17T17:37:15.169Z

Identical content hash — by the id's own construction, identical bytes — with ids exactly 10 seconds apart: the bound. The first wait expired, the re-cut minted a new id for the same build, and both landed. AdoptOnOwnCollision cannot rescue this: it adopts only a create REFUSED for NodeAlreadyExists, which needs the same id, and #3407's reasoning rests explicitly on the collision happening in the same second. A retry one bound later collides with nothing.

The remedy this points at, deliberately not taken here: the re-cut should reuse the abandoned attempt's release path rather than mint a fresh one — the same id turns the late landing into a NodeAlreadyExists the adoption mechanism already resolves correctly, and the duplicate stops being minted. That is a behavioural change to release-id minting with a genuine in-flight race to design against, needing a control that drives expiry → retry → adopt. Widening the bound is not the alternative: a tail reaching 17.8 s would only move the same failure further out while making it rarer and therefore harder to catch.

Deploy: addresses to recycle

The change is in compiled src/, so it ships with the platform image — no NodeType recompile is needed for the fix itself. But every per-node hub for a NodeType binds its configuration once, so after the roll the compile-settle path only runs the new code for activations created after it. Nothing needs recycling to get the fix; the types whose next compile will exercise it are the seven in the incident: Store/Catalog, Store/Purchase, Store/Provision, Store/Tier, Store/Publishing, Store/Subscription, Hosting/InstanceRequest.

What this does NOT establish, and what production verification needs

🚨 This does not establish WHY the re-cut's create fails to land. That is the point: it was unobservable, the filed issue's own first task was "log why the re-cut fails", and inventing a cause from a ten-second gap would be a guess dressed as a finding. The candidates the evidence admits — a boot-time storm (the Store/* requests were consumed a day and a half after they were made), a cross-hub write refused mid-flight (Ops/Logs from 09-12/13 show cross-hub write PARTIALLY refused on this lane), an owner that never answered — are distinguishable only by the next occurrence.

#5057 must not be closed on this merge. What would verify it:

The image carrying this commit is rolled onto memex (control instance) and the running tag confirmed on the Fleet Console.
Request a release on one of the seven node types — or wait for the next occurrence — and read the Error line. It must now end with a reason: either did not land within 00:00:10 — the owning hub did not answer, so whether the node exists is UNKNOWN from here, not no, or a named exception, or the settle's own create's refusal.
That reason names the actual root, which is then a new, separately fixable defect — and the first one where a fix can be aimed at something.
The negative half: a release request that succeeds must produce release restored at … / no post-condition line at all — no new noise on the healthy path.

A second finding, measured and deliberately NOT fixed here

🚨 The post-condition's scope predicate never expires, so once a type has ever had a release request consumed it is held to "every build must have a release" forever. Violation gates on LastReleaseRequestHandledAt >= RequestedReleaseAt, and RequestedReleaseAt is never cleared — a repo-wide grep finds six assignment sites and not one that nulls it. So every later compile of such a type re-enters the check, finds the build has moved past the standing release, and attempts a re-cut nobody asked for.

Confirmed on live data, not inferred: Hosting/InstanceRequest carries requestedReleaseAt == lastReleaseRequestHandledAt == 2026-09-20T23:23:19Z and lastCompileSucceededAt: 2026-09-21T09:57:42Z — the 09-21 compile fired the post-condition off a request made the previous day. That is also why the incident shows the same requestedReleaseAt repeating.

The consequence a reviewer should weigh: the release list for these types is sparse and irregular (Hosting/InstanceRequest: 08-20, 08-21, 08-23, 09-14, 09-17, … 09-20) precisely because the re-cut keeps failing. Once the underlying create is fixed, the same predicate will cut a release per compile on every type that was ever released. Whether that is the intended reading of #781's invariant ("latestReleasePath must never name a build older than lastCompiledVersion" — under which it follows) or a latent release-node explosion is a real question. This change does not touch the predicate, the scope or when the re-cut is attempted — narrowing #781's guard is not something to do inside a diagnosability fix.

A follow-up worth filing from the same evidence, deliberately NOT fixed here: the Store/* release requests sat unconsumed from 2026-09-19 06:33 to 2026-09-20 22:19. Whether the release-request path stalls or loses requests before the post-condition is even evaluated is a separate question this change makes no claim about.

🤖 Generated with Claude Code

…faults, not one that substitutes Issue #5057. ReleasePostCondition re-cuts the release a consumed request owes, and logs an ERROR when the re-cut does not land. In production that error read, in full: "...AND the release could not be re-cut. The node advertises a build no release names; instances will keep binding 'X' until a release is created for it." — 8 occurrences over 3 minutes across two memex pods and seven node types, naming the consequence, the stale path, the build and the cost, and not once the cause. The cause was discarded one frame below. TryCreateReleaseNode collapsed four unrelated failures into a bare IObservable<string?> emitting null: · no IMeshService on the hub → logged nothing · the create was refused → Warning + stack · the create threw while composing → Warning + stack · the 10 s bound expired → logged NOTHING AT ALL The last is the channel the incident names, and it was unobservable by construction: Timeout(bound, Observable.Return<string?>(null)) SUBSTITUTES the fallback rather than faulting, so the expiry never reached the Catch that logs. Its only trace in production is a gap between two adjacent lines — Hosting/InstanceRequest logged "Re-cutting..." at 22:16:14Z and "...could not be re-cut" at 22:16:24Z, exactly the bound. And the three channels that did log logged at Warning, which the log watcher does not ingest, so the loudest line in the log is the one that cannot say why. A remedy that reports its own failure without being able to report the failure is the defect it exists to catch, one level up: the output looks like a diagnosis and carries none. · ReleaseCreateOutcome carries the path OR the reason, with Attempted separating a failure from a create nobody asked for. Three states, because a string? holds two. · Timeout(CreateBound) FAULTS, routing the expiry into the same Catch as every other failure, where it becomes a reason naming the bound AND saying the create's fate is UNKNOWN rather than known not to have happened. · Every sentence the post-condition emits — the Error line and the compile _Activity diagnosis — now names the cause of BOTH attempts: the settle's own create (until now a Warning reaching no operator-facing surface) and the re-cut. · Because never returns the empty string: an attempted failure arriving with no reason SAYS so, so this defect cannot reappear wearing terseness. · The batch bake runs no post-condition at all and its stamp keeps the previous release path, so it logs the reason itself. It is unwatched by design, which is exactly why. The bound was NOT widened and must not be: a create that cannot land in ten seconds is not a slow create, and raising it is the band-aid that would have kept this unobservable for longer. Control on each side, measured: with the pre-fix wording restored on top of the fix, TheFailedDiagnosis_NamesBothCauses and ATimeout_NamesTheBoundItWaitedOut both go red — the assertions can fail. Green after restore: 29 tests, 0 failed, .trx written. The other side is TheRestoredDiagnosis_ReadsAsARepair — a re-cut that WORKED must read as a repair with no failure wording, because a fix that made every outcome sound like a failure would satisfy every other assertion here and be worse than the defect. What this does NOT establish: why the create does not land. That was the point — it was unobservable, and inventing a cause from a ten-second gap would be a guess dressed as a finding. The candidates the evidence admits (a boot-time storm — the Store/* requests were consumed a day and a half after they were made; a cross-hub write refused mid-flight; an owner that never answered) are distinguishable only by the next occurrence, which is now diagnosable. Doc: Architecture/ReleaseFailureClasses — same lesson as that page's own, one layer down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved moderate issues affect timeout correctness, operator visibility, localization, and production-path test coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Add reactive test covering real create timeout failure · New
Localize persisted compilation diagnoses with keyed log messages · New

What changed in this PR

This PR improves release re-cut failure diagnostics by preserving failure reasons and surfacing timeout outcomes.

Changes:

Adds reason-carrying ReleaseCreateOutcome states.
Propagates timeout and creation failures through diagnostics and batch baking.
Adds tests and architecture documentation.

File	Summary	Review notes
`test/MeshWeaver.Compiler.Pipeline.Test/ReleaseRecutReportsWhyItFailedTest.cs`	Tests outcome and diagnosis behavior.	—
`src/MeshWeaver.Hosting/NodeTypeBatchBake.cs`	Logs failed batch release creation.	Moderate: use operator-visible severity and avoid asserting an unknown timeout did not create a release. Nit: update stale comment.
`src/MeshWeaver.Documentation/Data/Architecture/ReleaseFailureClasses.md`	Documents the failure model.	—
`src/MeshWeaver.Compiler.Pipeline/ReleasePostCondition.cs`	Composes recovery and failure diagnoses.	Moderate: accurately describe unknown re-cut outcomes and add localization keys.
`src/MeshWeaver.Compiler.Pipeline/NodeTypeCompilationHelpers.cs`	Integrates outcomes into compile settlement.	Nit: update stale outcome documentation.
`src/MeshWeaver.Compiler.Pipeline/NodeTypeBuildState.cs`	Carries release outcomes and timeout reasons.	Moderate: add an end-to-end timeout test covering the reactive chain. Nit: correct contradictory XML documentation.

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

rbuergi · 2026-09-21T20:04:44Z

+                .Timeout(CreateBound)
+                .Catch<ReleaseCreateOutcome, Exception>(ex =>


Accepted — and this is the finding that matters most, because it is the exact standard the change is about: a verification step that cannot fail is not a verification step, and my controls only proved the wording can fail.

I did measure a negative control, but on the wrong subject. Restoring the pre-fix sentence on top of the fix reddens TheFailedDiagnosis_NamesBothCauses and ATimeout_NamesTheBoundItWaitedOut — so the sentences are pinned. The Timeout(bound, Observable.Return(null)) → Timeout(bound) change, which is the load-bearing half, is pinned by nothing: you are right that reverting it leaves every new test green, because Describe is reached through a TimeoutException I construct by hand rather than one the operator produced.

Fixing it the way the house does this: extracting the bounded-create composition as Bounded(IObservable<Unit> create, string releasePath, IScheduler, ILogger) — pure over its source and its clock, the same seam FleetWatchHeartbeat.Track uses — and driving it with a HistoricalScheduler, no mesh and no sleep. Three cases through the real .Timeout(...).Catch(...) chain:

a create that never emits, clock advanced past CreateBound → the outcome carries a Failure naming the bound. This is the discriminator you asked for: a substituting fallback yields an outcome with no reason at all, so Because reports "the attempt reported no reason" and the assertion on the bound fails. The revert is caught.

a create that emits inside the bound → Landed, no failure wording.

a create that errors with NodeAlreadyExists → adopted at the same path, which also puts A release re-cut collides with its own first attempt, and the type is left advertising a build no release names #3407 under the production chain rather than only under AdoptOnOwnCollision in isolation.

Pushing that now.

rbuergi · 2026-09-21T20:04:58Z

+    internal static string RestoredDiagnosis(string violation, string firstAttemptClause, string restored) =>
+        $"Release post-condition (#781): {violation}.{firstAttemptClause} Restored at {restored} "
+        + "from the bytes this compile produced — no recompile.";


Accepted. I checked the premise rather than taking it, and it holds on both halves:

src/MeshWeaver.Data.Contract/LogMessage.cs says it in its own words — an activity transcript is a RENDERED surface, Message alone cannot be localized (ActivityLog messages are persisted English strings — a rendered surface that cannot be localized (53 sites, 0 localizable) #3236), a localizable entry carries MessageKey + MessageArgs and is resolved at render time off the viewer locale, with Message as the English fallback.

Two lines above the site you flagged, the sibling entry already does it: new LogMessage($"Release created: {newReleasePath}", …).WithKey("activity.compile.releaseCreated", ("path", newReleasePath)). So the keyed pattern is established on this very transcript and the post-condition line is the one that skipped it.

It is pre-existing — the unkeyed new LogMessage(diagnosis, …) came in with #781, not with this diff — but that is not a defence: my diff makes that string longer and richer, which is precisely when the rule applies. Fixing it here.

What I am adding: activity.compile.releasePostCondition.restored and activity.compile.releasePostCondition.violated in both strings.en.json and strings.de.json, with the composed parts passed as named arguments, and the existing English sentence kept as the fallback so old persisted rows and any future catalog drift still render as they do today.

One thing I want to be explicit about rather than overclaim: the frame becomes localizable, the detail stays English. The arguments are a node path, a release path, a store version, and an exception type plus its message — none of which is translatable, and inventing German for an exception message would be worse than leaving it. A German viewer gets a German sentence around English identifiers, which is what the keyed pattern gives everywhere else on this transcript.

This adds catalog keys, so the PR body now carries the Mirror-sync: statement the i18n mirror gate wants — the mirror in MeshWeaver.Plugins/clients/react/src/i18n/ pins a core commit and reds nothing when a key is added here, so handing the sync over explicitly is the only thing that keeps it from going stale silently.

github-actions · 2026-09-21T20:04:52Z