[rbuergi]

Follow-up to #3912 (Refs #3888), acting on the Copilot review it received. The branch was already in
the merge queue when the review landed, and a queued branch cannot be pushed to — so this is the
same change set's second commit, carried on its own PR rather than dequeued out from under the group
behind it.

The finding, which is the right one to catch on this change of all changes

Please add a deterministic test for the NodeReadStatus.Unavailable arm … The six new cases cover
Absent and Compiled/Script, so a regression that maps an unavailable read back to Compiled would
still leave the suite green — the fail-closed guarantee this change is meant to provide.

Correct. #3912 fixed an instrument that could not fail, and shipped one arm of its own fix with no
control on it. A wedged owner is the case that made the outcome type worth having in the first
place (#1592: memex-cloud went 2 h 20 m with per-node hubs not answering, and every diagnostics probe
in that window would have read clean), and it was the one arm nothing could have caught going back.

Why no live-mesh test could reach it

GetMeshNodeOutcome's budget is 15 s and Unavailable depends on an owner that will not answer —
not something a MonolithMeshTestBase can arrange on demand, and the suite's methodTimeout is
30 s. An arm no control can reach is precisely where a regression puts Compiled back unnoticed.

So the mapping is extracted — MeshNodeLanguageService.NothingWasChecked(NodeReadOutcome), the
WHOLE decision rather than a fragment, the same idiom as CanReuseWorkspace three methods below —
and driven directly.

The controls

Test	What it pins
NoReadThatProducedNoNodeMayReadAsCompiled	over Enum.GetValues<NodeReadStatus>(): no status may render as Compiled or IsClean, including one added later
AnOwnerThatDidNotAnswerIsUnavailable_AndTheReasonSurvives	the arm itself, plus that the cause reaches DescribeProblem — a refusal naming neither path nor reason is only a differently-shaped dead end
ADeleteInFlightReadsAbsent_NotUnavailable	the other direction, so the mapping cannot pass by answering Unavailable to everything

Enumerating the enum rather than testing per value is deliberate: a per-value test goes silent on
a new NodeReadStatus, which is the same failure shape as the bug — the next status arrives as a red
instead of as a hole.

Falsified on purpose. With the _ arm returning Compiled(empty), the two new negative controls
go red and 7 of 9 stay green:

Failed  NoReadThatProducedNoNodeMayReadAsCompiled
  Did not expect value to be Compiled because a read that produced no node compiled nothing,
  so status Present must not render as a verdict about the proposed source.

Failed  AnOwnerThatDidNotAnswerIsUnavailable_AndTheReasonSurvives
  Expected value to be Unavailable because a hub that did not answer is not a hub that answered
  'fine' — this is the case that makes the pre-flight a gate rather than a nicety, but found Compiled.

Failed! - Failed: 2, Passed: 7, Total: 9

Restored: Passed! - Failed: 0, Passed: 9, Total: 9.

The review's second half was a correction to the COMMENT, and it was also right

GetMeshNodeOutcome surfaces Unauthorized via OnError and RLS-hidden nodes as Absent, so the
test should target a status that actually reaches this switch (or adjust the comment/contract
accordingly).

#3912's comment said the Unavailable arm covers "a delivery-level denial". It does not. A
delivery-level RLS denial rides a raw GetDataRequest whose NACK surfaces as OnError
(DeliveryFailureException) and is caught at the tool boundary (#2554) — it never reaches this
switch. What does reach it via access control is a read validator hiding the node: a filtered
node is invisible to the reader by contract, which lands on Absent, and that is how #3888's live
case — a NodeType in a partition the caller held no grant on — got there. Both routes now say so
where they are, so the next reader is not sent looking for a denial in the wrong arm.

Local verification

• dotnet build src/MeshWeaver.Compiler.Pipeline -c Release -warnaserror → 0 Error(s) 0 Warning(s)
• dotnet build test/MeshWeaver.Compiler.Pipeline.Test -c Release -warnaserror → 0 Error(s) 0 Warning(s)
• SpeculativeCheckCannotAnswerGreenForAnUncheckedNodeTest → 9 passed (2 red under the sabotage above)
• MeshWeaver.Documentation.Test link/embed/topic/timeout guards → 18 passed

Doc/Architecture/LanguageServices records the general point: an arm no live test can reach is not
an excuse, it is an instruction to extract the decision and assert the invariant over the enum
rather than over the values you happened to think of.

🤖 Generated with Claude Code

https://claude.ai/code/session_01S25PJZbVeCbkWhCfMub9P7

[Copilot]

🟢 Approval recommended

Final comments are minor nits only, with no approval-blocking issues.

Pull request overview

Adds fail-closed handling and deterministic tests for unavailable node reads.

Changes:

• Extracts and tests no-node outcome mapping.
• Adds coverage for unavailable and delete-in-progress reads.
• Updates language-service documentation.

File summaries

File	Summary
test/MeshWeaver.Compiler.Pipeline.Test/SpeculativeCheckCannotAnswerGreenForAnUncheckedNodeTest.cs	Adds deterministic fail-closed mapping tests.
src/MeshWeaver.Documentation/Data/Architecture/LanguageServices.md	Documents the control strategy. Nit (1 vote): reference CheckSpeculativeOutcome rather than CheckSpeculative.
src/MeshWeaver.Compiler.Pipeline/MeshNodeLanguageService.cs	Extracts the outcome mapping. Nit (1 vote): clarify that unauthorized delivery failures remain a separate tool-boundary obligation.

Review details

Suppressed comments (2)

src/MeshWeaver.Compiler.Pipeline/MeshNodeLanguageService.cs:260

• This comment overstates the current behavior: issue #2554 reports that the LSP tool wrappers let an Unauthorized DeliveryFailureException escape as an unhandled MCP error, and the dependent plugin changes are not part of this core PR. GetMeshNodeOutcome does surface the denial via OnError, but this code does not convert it to {ok:false}; please describe that as a separate tool-boundary obligation (or point to the change that actually implements it) so the source does not claim permission denials are already structured refusals.

            // 🚨 A DELIVERY-level RLS denial does NOT arrive here: GetMeshNodeOutcome rides a raw
            // GetDataRequest whose NACK surfaces as OnError (DeliveryFailureException), which the
            // tool boundary converts into the same {ok:false} shape (#2554). Two different routes,
            // both refusing; neither may read as clean.

src/MeshWeaver.Documentation/Data/Architecture/LanguageServices.md:120

• Now that this table labels the first column Method, the row below names CheckSpeculative, but that overload is intentionally still the editor's silent list API. The fail-closed verdict surface added for #3888 is CheckSpeculativeOutcome; leaving this row as-is contradicts the explanation at lines 131–136 and directs readers to the wrong method.

| Method | Fixed in | The symptom |
|---|---|---|
| `GetDiagnostics` | #1592 / #1618 | `lsp_diagnostics_for_node @Edu/DefinitelyNotARealNodeType` → `{"ok":true,"diagnostics":[]}`; the mandated pre-prod sweep reported all-green over stale paths having verified nothing |
| `CheckSpeculative` | #3888 | `lsp_check_node` on a NodeType in a partition the caller cannot read → `{"ok":true,"diagnostics":[]}` — **including for `proposedCode` that is not C# at all**; the `/code` edit loop's pre-flight blessed every proposed edit against every path it could not reach |

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Test Results (shard 1)

496 tests   496 ✅  36s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit c2a277e.

[github-actions]

Test Results (shard 3)

389 tests   389 ✅  56s ⏱️
  2 suites    0 💤
  2 files      0 ❌

Results for commit c2a277e.

[github-actions]

Test Results (shard 0)

254 tests   254 ✅  2m 17s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit c2a277e.

[github-actions]

Test Results (shard 2)

1 229 tests   1 227 ✅  3m 39s ⏱️
    3 suites      2 💤
    3 files        0 ❌

Results for commit c2a277e.

[github-actions]

Test Results (shard 4)

1 888 tests   1 888 ✅  3m 21s ⏱️
    4 suites      0 💤
    4 files        0 ❌

Results for commit c2a277e.

[github-actions]

Test Results (shard 5)

    6 files      6 suites   4m 30s ⏱️
2 599 tests 2 407 ✅ 192 💤 0 ❌
2 603 runs  2 411 ✅ 192 💤 0 ❌

Results for commit c2a277e.

[github-actions]

Test Results

   17 files     17 suites   15m 21s ⏱️
6 855 tests 6 661 ✅ 194 💤 0 ❌
6 859 runs  6 665 ✅ 194 💤 0 ❌

Results for commit c2a277e.