Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
251 changes: 196 additions & 55 deletions src/MeshWeaver.Compiler.Pipeline/NodeTypeBuildState.cs

Large diffs are not rendered by default.

31 changes: 20 additions & 11 deletions src/MeshWeaver.Compiler.Pipeline/NodeTypeCompilationHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4438,17 +4438,23 @@ curr.Content is NodeTypeDefinition def
var releasePathObservable = ok
? NodeTypeBuildState.TryCreateReleaseNode(
hub, hubPath, outcome.Result!, outcome.PendingNode, resolvedActivityPath, logger)
: Observable.Return<string?>(null);
// 🚨 NOT-ATTEMPTED, not failed (#5057). A compile that produced no assembly
// was never asked to release anything, and the post-condition below must not
// be told a create failed when none was made.
: Observable.Return(NodeTypeBuildState.ReleaseCreateOutcome.NotAttempted);

releasePathObservable
.Take(1)
// 🚨 Same totality guard as the compile pipeline above: the terminal
// Status write below runs in THIS OnNext — a release-create observable
// that completed empty would silently skip it and wedge the NodeType at
// Compiling. TryCreateReleaseNode is bounded (null on timeout/fault) by
// contract; DefaultIfEmpty makes the write unconditional even if that
// contract is ever violated.
.DefaultIfEmpty()
// Compiling. TryCreateReleaseNode is bounded (a reasoned failure on
// timeout/fault) by contract; DefaultIfEmpty makes the write unconditional
// even if that contract is ever violated — and says so rather than producing
// a null indistinguishable from a refusal (#5057).
.DefaultIfEmpty(NodeTypeBuildState.ReleaseCreateOutcome.Failed(
"the release create completed without emitting — the pipeline's "
+ "exactly-once contract was violated"))
// 🚨 THE POST-CONDITION (#781), checked where the compile SETTLES — the one
// moment at which "does a release name the build this node is about to
// advertise?" is answerable from facts all in hand. A consumed request whose
Expand All @@ -4458,12 +4464,12 @@ curr.Content is NodeTypeDefinition def
// remedy re-cuts from the bytes this compile just produced (no recompile,
// under System) and is loud either way. Also totality-safe: exactly one
// emission, never a fault.
.SelectMany(newReleasePath => ok
.SelectMany(firstAttempt => ok
? ReleasePostCondition.Restore(
hub, hubPath, outcome.Result!, outcome.PendingNode,
resolvedActivityPath, newReleasePath, logger)
: Observable.Return<(string? ReleasePath, string? Diagnosis)>(
(newReleasePath, null)))
resolvedActivityPath, firstAttempt, logger)
: Observable.Return<(string? ReleasePath, LogMessage? Diagnosis)>(
(firstAttempt.ReleasePath, null)))
// 🚨 #4469 — THE JOIN POINT. A failure on an unresolved NAME asks the
// partition's import bookkeeping whether it lost the file that would have
// defined it, so the operator who lands on the compile error is told about
Expand Down Expand Up @@ -4592,9 +4598,12 @@ curr.Content is NodeTypeDefinition def
.WithKey("activity.compile.releaseCreated", ("path", newReleasePath)));
// The post-condition's verdict belongs on the OFFICIAL diagnosis surface, not
// only in a log sink — a stale release is invisible everywhere else (#781).
// 🚨 KEYED, so a German viewer reads it in German (#3236 / review on #5057). The
// entry arrives carrying its own catalog key, args and level — it used to be
// wrapped in a bare `new LogMessage(string, …)` here, which renders its English
// interpolation to every viewer forever.
if (settle.Diagnosis is { } diagnosis)
activityMessages.Add(new LogMessage(diagnosis,
newReleasePath is null ? LogLevel.Error : LogLevel.Warning));
activityMessages.Add(diagnosis);
NodeTypeCompilationActivity.Complete(hub, resolvedActivityPath,
ok ? ActivityStatus.Succeeded : ActivityStatus.Failed,
activityMessages.ToImmutable(), logger!);
Expand Down
160 changes: 138 additions & 22 deletions src/MeshWeaver.Compiler.Pipeline/ReleasePostCondition.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ namespace MeshWeaver.Graph;
/// against a build the compile is about to supersede — or the release create can simply not land:
/// <c>TryCreateReleaseNode</c> is best-effort by design (compile correctness must not depend on a
/// MeshNode create), so a bound that expires, a fault, or a refusal — it runs under the REQUESTER's
/// identity for attribution, and a partition the requester may not create in refuses it — all emit
/// <c>null</c>, logged at Warning and swallowed. <c>ApplyCompileSuccess</c> then stamps
/// identity for attribution, and a partition the requester may not create in refuses it — all yield
/// a <see cref="NodeTypeBuildState.ReleaseCreateOutcome"/> carrying no path (and, since #5057, the
/// REASON; they used to yield a bare <c>null</c>). <c>ApplyCompileSuccess</c> then stamps
/// <c>releasePath ?? def.LatestReleasePath</c>, i.e. the PREVIOUS build's release, and the trigger
/// is already spent (<c>LastReleaseRequestHandledAt == RequestedReleaseAt</c>, stamped on the
/// dispatch commit). Nothing revisits it.</para>
Expand All @@ -47,6 +48,18 @@ namespace MeshWeaver.Graph;
/// <para>🚨 And it stays LOUD either way: the violation is logged as an ERROR naming the type, the
/// stale path and the build, and the remedy's outcome is recorded on the compile <c>_Activity</c>
/// (the official diagnosis surface). Silence is what made the incident invisible for a day.</para>
///
/// <para>🚨 <b>LOUD IS NOT THE SAME AS DIAGNOSABLE — issue #5057.</b> The remedy can fail, and when
/// it did, this reported it in the one shape nobody can act on: an ERROR reading <i>"AND the release
/// could not be re-cut"</i> with no reason, because the reason was discarded one frame below where
/// <c>TryCreateReleaseNode</c> collapsed four distinct failures into a bare <c>null</c> — and the
/// channel the incident actually names, a <see cref="NodeTypeBuildState.CreateBound"/> that expired,
/// wrote no log line at all. Eight occurrences over three minutes across seven node types, and the
/// filed issue's first task was <i>"log why the re-cut fails"</i>. A remedy that reports its own
/// failure without being able to report the failure is the same defect class as the state it exists
/// to catch, one level up: the output looks like a diagnosis and carries none. Every sentence this
/// class emits now names the cause of BOTH attempts — the settle's own create (previously a
/// <c>Warning</c> that reached no operator-facing surface) and the re-cut.</para>
/// </summary>
internal static class ReleasePostCondition
{
Expand Down Expand Up @@ -132,23 +145,39 @@ private static bool SameSnapshot(
/// observable's OnNext, so a sequence that completed empty or errored would wedge the NodeType
/// at <c>Compiling</c>.</para>
/// </summary>
internal static IObservable<(string? ReleasePath, string? Diagnosis)> Restore(
/// <param name="hub">The hub the compile settled on.</param>
/// <param name="nodeTypePath">The NodeType being settled.</param>
/// <param name="result">The successful compile's result.</param>
/// <param name="pendingNode">The definition as observed at dispatch.</param>
/// <param name="activityPath">The compile <c>_Activity</c>, or null when its create did not land.</param>
/// <param name="firstAttempt">
/// What the settle's OWN release create amounted to — including, when it failed, the reason,
/// which until #5057 existed only as a <c>Warning</c> in the log and reached no operator-facing
/// surface at all. It is the first half of the story this method tells.
/// </param>
/// <param name="logger">Where the violation and the remedy's outcome are published.</param>
internal static IObservable<(string? ReleasePath, LogMessage? Diagnosis)> Restore(
IMessageHub hub,
string nodeTypePath,
NodeCompilationResult result,
MeshNode pendingNode,
string? activityPath,
string? newReleasePath,
NodeTypeBuildState.ReleaseCreateOutcome firstAttempt,
ILogger? logger)
{
var before = pendingNode.ContentAs<NodeTypeDefinition>(hub.JsonSerializerOptions);
if (Violation(before, result, newReleasePath) is not { } violation)
return Observable.Return<(string?, string?)>((newReleasePath, null));
if (Violation(before, result, firstAttempt.ReleasePath) is not { } violation)
return Observable.Return<(string?, LogMessage?)>((firstAttempt.ReleasePath, null));

// 🚨 WHY THE FIRST CREATE FAILED, on the line an operator reads (#5057). This was the missing
// half of #781's own diagnosis: the settle's create is best-effort and logged its refusal at
// Warning, so the ERROR line that announced the consequence never named the cause, and the
// incident's Loki selector — cut at Error — could not have shown it.
var firstFailure = FirstAttemptClause(firstAttempt);
logger?.LogError(
"[ReleasePostCondition] {HubPath}: {Violation}. Re-cutting the release from the bytes "
+ "this compile produced (no recompile) — see issue #781.",
nodeTypePath, violation);
"[ReleasePostCondition] {HubPath}: {Violation}.{FirstFailure} Re-cutting the release from "
+ "the bytes this compile produced (no recompile) — see issue #781.",
nodeTypePath, violation, firstFailure);

// Under SYSTEM, and with the requester cleared so TryCreateReleaseNode does not re-attempt
// the very attribution that was refused. RunAsSystem (not Observable.Using over
Expand All @@ -163,32 +192,119 @@ private static bool SameSnapshot(
.Take(1)
.Catch((Exception ex) =>
{
// The stack goes here; the reason travels, so the Error line below can say it.
logger?.LogError(ex,
"[ReleasePostCondition] {HubPath}: the re-cut faulted", nodeTypePath);
return Observable.Return<string?>(null);
return Observable.Return(NodeTypeBuildState.ReleaseCreateOutcome.Failed(
$"the re-cut faulted: {ex.GetType().Name}: {ex.Message}"));
})
.DefaultIfEmpty()
.Select<string?, (string? ReleasePath, string? Diagnosis)>(restored =>
// 🚨 An EMPTY completion is its OWN reason, not a shared null (#5057). It cannot happen
// today — TryCreateReleaseNode emits exactly once on every path — and naming it is what
// keeps that fact from becoming an assumption the next operator pays for.
.DefaultIfEmpty(NodeTypeBuildState.ReleaseCreateOutcome.Failed(
"the re-cut produced no answer at all — an inner observable completed without emitting"))
.Select<NodeTypeBuildState.ReleaseCreateOutcome, (string? ReleasePath, LogMessage? Diagnosis)>(recut =>
{
if (restored is not null)
if (recut.ReleasePath is { } restored)
{
logger?.LogInformation(
"[ReleasePostCondition] {HubPath}: release restored at {ReleasePath} — the "
+ "node no longer advertises a build no release names",
nodeTypePath, restored);
return ((string?)restored,
(string?)$"Release post-condition (#781): {violation}. Restored at {restored} "
+ "from the bytes this compile produced — no recompile.");
return ((string?)restored, (LogMessage?)RestoredEntry(violation, firstFailure, restored));
}

// 🚨 THE LINE THE INCIDENT WAS FILED FROM, and it now SAYS WHY (#5057). It used to
// end at "could not be re-cut" — an Error naming the consequence, the stale path and
// the build, with the one fact needed to fix it discarded one frame below.
logger?.LogError(
"[ReleasePostCondition] {HubPath}: {Violation} — AND the release could not be "
+ "re-cut. The node advertises a build no release names; instances will keep "
+ "binding '{Stale}' until a release is created for it.",
nodeTypePath, violation, before!.LatestReleasePath);
return ((string?)null,
(string?)$"Release post-condition (#781) VIOLATED: {violation}. The re-cut did "
+ "not land either — this build has no release.");
+ "re-cut{Because}. The node advertises a build no release names; instances will "
+ "keep binding '{Stale}' until a release is created for it.",
nodeTypePath, violation, recut.Because, before!.LatestReleasePath);
return ((string?)null, (LogMessage?)ViolatedEntry(violation, firstFailure, recut));
});
}

/// <summary>
/// What the settle's OWN release create amounted to, as a clause for the sentences below — so
/// "the create was refused because x", "the create timed out" and "no create was made" are three
/// readable statements rather than one absent release path. Pure.
/// </summary>
/// <param name="firstAttempt">The settle's own create.</param>
internal static string FirstAttemptClause(NodeTypeBuildState.ReleaseCreateOutcome firstAttempt) =>
!firstAttempt.Attempted
? " No create was attempted on this settle."
: firstAttempt.Succeeded
? $" The settle's own create landed at {firstAttempt.ReleasePath}."
: $" The settle's own create{firstAttempt.Because}.";

/// <summary>
/// The compile <c>_Activity</c> line for a violation the re-cut REPAIRED. Pure.
/// </summary>
/// <param name="violation">The violation, as <see cref="Violation"/> worded it.</param>
/// <param name="firstAttemptClause"><see cref="FirstAttemptClause"/>.</param>
/// <param name="restored">Where the re-cut landed.</param>
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.";
Comment on lines +248 to +250

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/// <summary>Catalog key for <see cref="RestoredDiagnosis"/>.</summary>
internal const string RestoredKey = "activity.compile.releasePostCondition.restored";

/// <summary>Catalog key for <see cref="FailedDiagnosis"/>.</summary>
internal const string ViolatedKey = "activity.compile.releasePostCondition.violated";

/// <summary>
/// The repaired verdict as an activity entry a GERMAN viewer can read.
///
/// <para>🚨 <b>A transcript entry is keyed or it is English forever</b> (<c>LogMessage</c>, #3236;
/// review on #5057). The entry is written server-side with NO viewer in scope and read later by
/// viewers whose languages differ, so the sentence must be resolved at RENDER time off
/// <c>AccessContext.Locale</c> — the English text stays as the fallback, which is what keeps an
/// old persisted row and a key that later leaves the catalog rendering exactly as they do now.
/// The ARGUMENTS stay English on purpose: a node path, a release path, a store version and an
/// exception's own message are not translatable, and inventing German for an exception message
/// would be worse than leaving it.</para>
/// </summary>
/// <param name="violation">The violation, as <see cref="Violation"/> worded it.</param>
/// <param name="firstAttemptClause"><see cref="FirstAttemptClause"/>.</param>
/// <param name="restored">Where the re-cut landed.</param>
internal static LogMessage RestoredEntry(
string violation, string firstAttemptClause, string restored) =>
new LogMessage(RestoredDiagnosis(violation, firstAttemptClause, restored), LogLevel.Warning)
.WithKey(RestoredKey,
("violation", violation), ("firstAttempt", firstAttemptClause), ("path", restored));

/// <summary>
/// The unrepaired verdict as an activity entry a German viewer can read — same rule as
/// <see cref="RestoredEntry"/>, and <c>Error</c> because this build has no release.
/// </summary>
/// <param name="violation">The violation, as <see cref="Violation"/> worded it.</param>
/// <param name="firstAttemptClause"><see cref="FirstAttemptClause"/>.</param>
/// <param name="recut">What the re-cut amounted to.</param>
internal static LogMessage ViolatedEntry(
string violation, string firstAttemptClause,
NodeTypeBuildState.ReleaseCreateOutcome recut) =>
new LogMessage(FailedDiagnosis(violation, firstAttemptClause, recut), LogLevel.Error)
.WithKey(ViolatedKey,
("violation", violation), ("firstAttempt", firstAttemptClause),
("reason", recut.Because));

/// <summary>
/// The compile <c>_Activity</c> line for a violation the re-cut could NOT repair — the sentence
/// issue #5057 is about.
///
/// <para>🚨 It must NAME THE CAUSE of both attempts. The old wording ended at <i>"The re-cut did
/// not land either"</i>, which reads as a complete report and is not one: it tells the operator
/// the state (this build has no release) and withholds the only fact that decides what to do
/// about it. Pure, so that property is asserted rather than hoped for.</para>
/// </summary>
/// <param name="violation">The violation, as <see cref="Violation"/> worded it.</param>
/// <param name="firstAttemptClause"><see cref="FirstAttemptClause"/>.</param>
/// <param name="recut">What the re-cut amounted to.</param>
internal static string FailedDiagnosis(
string violation, string firstAttemptClause, NodeTypeBuildState.ReleaseCreateOutcome recut) =>
$"Release post-condition (#781) VIOLATED: {violation}.{firstAttemptClause} The re-cut did not "
+ $"land either{recut.Because}. This build has no release.";
}
Loading
Loading