CodeNodeType's last-execution stamp swallows its failure, and nothing can tell that it did

Split out of Systemorph/MeshWeaver.Plugins#1256 (ask 3), which invited exactly this: "Core seam (split out if preferred)". Asks 1 and 2 are fixed in Plugins (#1266, #1288); this is the remainder, and it belongs here.

The seam

CodeNodeType, src/MeshWeaver.Graph/Configuration/CodeNodeType.cs ~line 380: after dispatching a run, the handler stamps Last{ExecutedAt,ExecutedBy,ActivityPath,ExecutedCodeHash} onto the Code node. The write is fire-and-forget, and both of its failure paths only log:

.Subscribe(_ => { },
    ex => stampLogger?.LogWarning(ex, "CodeNodeType: stamp UpdateMeshNode failed for {Hub}", hub.Address));
…
catch (Exception stampEx)
{
    // The workspace might not be ready (cold-start race) — the missing stamp is a UI nicety,
    // not a correctness invariant, and the Activity log IS written.
    logger?.LogWarning(stampEx, "CodeNodeType: could not stamp last-execution fields on {Hub} …");
}

Then ExecuteScriptResponse { Success = true } is posted regardless — and it is posted before the stamp can have failed, so it could not report the failure even if it wanted to.

Why the "UI nicety" reasoning no longer holds — and why it is now smaller, not larger

When that comment was written the stamp was the pane's only pointer to the run: LastActivityPath is what the cell's output pane keys on. A swallowed stamp therefore meant a Succeeded run that was invisible until a reload — the symptom Plugins#1256 reported (two runs, both Succeeded, nothing on screen, output present four minutes later).

Plugins#1288 removed that dependency: the pane now prefers the activity path carried on ExecuteScriptResponse.ActivityLog, falling back to the stamp. So the stamp is genuinely a nicety again — for the pane. What remains is narrower and still real:

"Last executed: <when> by <who>" silently goes stale or missing;
LastExecutedCodeHash is what drives the stale-cell indicator (IsOutputStale), so a swallowed stamp can leave a cell reading "up to date" against code it did not run — a wrong claim rather than a missing one;
nothing anywhere can distinguish "this cell has never been run" from "this cell ran and we failed to record it".

🚨 The design tension, stated rather than assumed

The issue's wording is "if it fails, the response should say so". That cannot be taken literally: the response is posted before the stamp resolves, and delaying it would undo Plugins#1266, whose whole point is that a Run click is acknowledged immediately and is never silent. The two asks pull opposite ways, which is why I have not implemented a shape here — picking one silently would be the wrong kind of decisiveness.

Three candidate shapes, for whoever owns this:

Keep the response immediate; carry the failure on the NODE. A LastStampFailedAt (or an existing diagnostics channel) that the view can surface as a quiet chip. Costs a field on a persisted record; makes the state observable by the thing that already observes the node.
Keep the response immediate; raise the log to Error and ticket it. Cheapest, and honest about it being an operator-facing fault rather than a viewer-facing one — but a viewer still cannot tell.
Make the stale-cell indicator fail CLOSED. Independent of 1 and 2 and arguably the highest value: if LastExecutedCodeHash was not recorded, IsOutputStale should not answer "not stale". A cell that cannot prove it is current should say so.

(3) is separable and could land alone.

Not in scope here

The pane behaviour (Plugins#1288) and the dispatch acknowledgement (Plugins#1266) are done. This issue is only about the stamp's own failure being unobservable.

Option (3) is implemented in #3270. The issue stays open for the part it deliberately
does not touch.

What landed

The stale-cell verdict is no longer a boolean, and no longer lives in whichever view renders a cell.
CodeConfiguration.OutputCurrency() (MeshWeaver.Mesh.Contract, beside CodeFingerprint and the
field it interprets) answers four states, and only Current may be rendered as up to date:

Node state	Verdict
no run recorded at all	`NeverRun` — silent
run recorded with its fingerprint	`Current` / `Stale`, as before
run recorded without its fingerprint	`Unverified` — fail closed

Unverified is deliberately neither neighbour. Not Current, because nothing proves it. Not Stale
either — that would light up every node last executed before LastExecutedCodeHash existed, and a
warning nobody believes is the same failure as no warning. "A run is recorded" accepts any of the
three run markers (LastExecutedAt, LastActivityPath, LastExecutedBy), so a stamp that landed
only in part cannot fall back into NeverRun and go silent on a cell that ran.

Localized chip strings (code.unverifiedCell, code.unverifiedCellHint) ship in the core catalog in
en + de. The toolbar that renders them is CodeViews.IsOutputStale in MeshWeaver.Plugins, which
adopts the rule in a paired PR there — core is the source of truth and merges first.

The rule and its reasoning are written into Doc/Architecture/ScriptExecution → "What a cell may
claim about its own output — the currency rule", so this does not live only in a PR body.

What remains — why this is `Refs`, not `Closes`

Options 1 and 2 are untouched, on purpose. The design tension you stated is real and it is a
maintainer call, not something to settle silently in a PR: the response is posted before the stamp
resolves, and delaying it undoes Plugins#1266.

And (3) does not reach the whole of the third bullet. It makes every state the node can express
honest. It cannot help with the state the node cannot express at all: a cell whose very first run
stamps nothing records nothing, so it stays indistinguishable from a cell nobody has run. That is
exactly what a LastStampFailedAt (option 1) or an operator-facing Error (option 2) would fix, and
neither is in scope here.

One thing found while reading the seam, not fixed here

CodeNodeType.cs line ~390, inside the stamp's Update:

curr.Content is CodeConfiguration cfg ? curr with { … } : curr

That is the is-cast trap-door AGENTS.md rules out. It is a fourth failure path the issue does
not name, and the only one with no signal at all: when the content arrives as untyped JSON (the
polymorphic converter degrading an unresolvable $type, the as-written JsonObject DOM, or a
same-named type from another collectible assembly) the pattern simply does not match, Update
returns curr unchanged, and the stamp is dropped with no exception and no log — the two paths the
issue quotes at least log.

I did not change it, because it is not option (3) and it is not risk-free: ContentAs<T> on a
node whose content genuinely is not a CodeConfiguration would re-type it and write it back, which
is worse than a missing stamp. It wants its own change with its own reasoning about that case.

fix(#3249): decide currency from the fingerprint first — the mirrored…

Point the residual at #3301 rather than at the issue this closes

Fixed on main by #3300 (merged today). Verified against origin/main rather than inferred from the PR:

modified +132/-43  src/MeshWeaver.Graph/Configuration/CodeNodeType.cs
added    +248/-0   test/MeshWeaver.Graph.Test/CodeNodeStampFailureTest.cs
modified +23/-1    src/MeshWeaver.Documentation/Data/Architecture/ScriptExecution.md
added    +42/-0    …/WhatsNew/2026-09-04-a-cell-that-ran-no-longer-forgets-it-silently.md

The decision, and why the issue's option (1) was not taken

The issue offered three shapes and deliberately declined to pick. Option (1) — record the failure as
LastStampFailedAt on the node — is unsound, and that is worth stating so it is not proposed again:
it records the failure of a node write by performing another node write to the same node through the
same GetMeshNodeStream().Update path. In every failure mode the issue names — workspace not ready,
cold-start race, the partition write erroring — the second write fails for the same reason. That is
recovery inside the failure domain that just failed.

Option (3) had already landed in #3270 (CodeConfiguration.OutputCurrency(), four states, fail-closed
Unverified).

What #3300 found that the issue did not

The stamp had a worse defect than "the failure is only logged": the write was guarded by
curr.Content is CodeConfiguration cfg — the object-payload type test AGENTS.md rules out. When the
content arrives as untyped JSON (the polymorphic converter degrading an unresolvable $type, the
as-written JsonObject before materialisation, or a same-named type from another collectible assembly
— all three routine in a running mesh) that test is simply false, the update short-circuits to a no-op,
and the stamp is dropped with no exception and no log at all. So the "swallowed failure" this issue
reports was in places not even reaching the swallowing catch.

CodeNodeStampFailureTest drives the stamp path directly and pins that a failure cannot pass silently.

Residual, tracked elsewhere

#3301 — a cell cannot find its own runs without the last-execution stamp. That is the viewer-facing
half and it is deliberately a separate issue, not an unfinished part of this one; #3300's commit
5da35c79c retargeted the residual there explicitly.

Navigation Menu

Repository navigation

CodeNodeType's last-execution stamp swallows its failure, and nothing can tell that it did #3249

The seam

Why the "UI nicety" reasoning no longer holds — and why it is now smaller, not larger

🚨 The design tension, stated rather than assumed

Not in scope here

What landed

What remains — why this is `Refs`, not `Closes`

One thing found while reading the seam, not fixed here

The decision, and why the issue's option (1) was not taken

What #3300 found that the issue did not

Residual, tracked elsewhere

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

CodeNodeType's last-execution stamp swallows its failure, and nothing can tell that it did #3249

Description

The seam

Why the "UI nicety" reasoning no longer holds — and why it is now smaller, not larger

🚨 The design tension, stated rather than assumed

Not in scope here

Activity

rbuergi commented on Sep 4, 2026

What landed

What remains — why this is Refs, not Closes

One thing found while reading the seam, not fixed here

rbuergi commented on Sep 4, 2026

The decision, and why the issue's option (1) was not taken

What #3300 found that the issue did not

Residual, tracked elsewhere

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

What remains — why this is `Refs`, not `Closes`