Skip to content

The plugins gate blamed a compile break for a failure that compiled fine - #1103

Merged
rbuergi merged 7 commits into
mainfrom
fix/1077-plugins-gate-message
Aug 10, 2026
Merged

rbuergi merged 7 commits into
mainfrom
fix/1077-plugins-gate-message

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #1077

1. The message named a cause the report contradicted

The gate judges three checks per NodeType — compile, render, tests — and its own log says which failed:

RED Store/Catalog: compile=Ok render=ok tests=FAILED
    **Area not found**
    No renderer is registered for area `Tests` on hub `Store`.

CI annotated that as "MeshWeaver.Plugins does not compile against this PR … A public API change here breaks plugin node source that is Roslyn-compiled at runtime." One message for all three checks, asserting a cause the report contradicts — and the most expensive wrong lead available, because it is plausible: a public-API break here genuinely does break that repo (plugin node source is Roslyn-compiled from mesh content, so a repo-local rg can never prove a deletion safe — that is how AddSkillSource broke 8 NodeTypes). So the claim reads as authoritative and the investigator goes to diff public signatures. It did exactly that on #1043, whose whole diff was MessageService.cs and two test files.

Now the tester emits one verdict line naming the failing check and the node(s):

GATE FAILED — tests: Store/Catalog — 1 new failure(s), 0 stale allow entr(ies).

CI lifts that line verbatim — it does not re-derive or re-word a cause — and the public-API sentence is kept, conditional on the phase actually being compile. Two failures previously reported as "0 new failure(s)" now name themselves: a fatal error says fatal: …, a stale allow entry says so. A run that fails with nothing recorded says no failing check was recorded rather than picking a phase.

GateSummaryTest pins each of these two-sided: the line must name the check that failed and must not name one that did not — a message that can drift back is the thing being fixed. CI's grep is anchored on ^GATE FAILED (the GateReport.FailedPrefix constant, ASCII), so re-punctuating the line cannot silently unhook the annotation.

2. The Store/Catalog failure is a framework defect, not plugin rot — and not the probe path

The hypothesis in the issue (the probe landed on a type path, where "Area not found" is by design) does not hold. Store is the Store package's root, retyped to Store/Catalog by the installer — a correctly-typed instance. The evidence is the available-areas list the failure printed: item for item it is ConfigureDefaultNodeHub's set — AddDefaultLayoutAreas (incl. AddEducationLayoutAreas' CourseNav/Learn/StartExercise/GoToMyCopy and AddDomainLayoutAreas' Catalog/Details/DataModel) plus Invite. Not one area from Store/Catalog's compiled configuration, and not the Space placeholder's either. Hub Store was serving the mesh default configuration: the known partition-root defect (#1055, and 2026-07-29 before it).

It recurred on a commit that already carried #1055's fix (run 31361446933 at 06:18Z; #1055 merged 04:39Z). NOT cached is not NOT pinned: a hub is pinned by address in Mesh.GetHostedHub and never re-reads its node's NodeType, so fixing the resolution does not un-pin the hub a bad resolution already activated. The only thing that does is PackageInstaller's DisposeRequest — fire-and-forget, and conditional on the placeholder dance having run. That residual is not fixed here (it needs a repro and a design decision, not a guess); it is written into both comment blocks that carry this ledger, which now ask three questions instead of two.

The gate now prints which node hosted the Tests run and how it was picked (an instance already in the mesh vs. a throwaway probe it created) — the two are not equivalent, and that difference is what separates the same commit's green and red runs. That line is what makes the third question answerable from CI alone. The host resolution is also bounded now: it queries the index and may create a node, neither with a budget of its own, so a wedge there spent the whole job timeout and reported nothing.

Not done

No continue-on-error, no retry around the probe, no widened timeout, no allow-list entry for Store/Catalog. The gate still fails exactly when it failed before — it just says what failed.

Verification

  • MeshWeaver.PluginTester.Test: 28/28 pass (12 new).
  • mw-plugin-test run end-to-end against an empty root: emits GATE FAILED — fatal: …, exit 1 — the line CI greps, produced by the real binary.
  • -c Release -warnaserror clean: MeshWeaver.PluginTester, MeshWeaver.PluginTester.Test, MeshWeaver.PluginCatalog, MeshWeaver.Hosting.

🤖 Generated with Claude Code

The gate judges three checks per NodeType — compile, render, tests — and its own
log says which one failed:

    RED Store/Catalog: compile=Ok render=ok tests=FAILED
        **Area not found**
        No renderer is registered for area `Tests` on hub `Store`.

CI then annotated that as:

    MeshWeaver.Plugins does not compile against this PR … A public API change
    here breaks plugin node source that is Roslyn-compiled at runtime.

One message for all three checks, asserting a cause the report contradicts.
And it is the most expensive wrong lead available, because it is PLAUSIBLE: a
public-API break here genuinely does break that repo — plugin node source is
Roslyn-compiled from mesh content, so a repo-local `rg` can never prove a
deletion safe (that is how `AddSkillSource` broke 8 NodeTypes). So the claim
reads as authoritative and the investigator goes to diff public signatures that
are fine. It did exactly that on #1043, whose entire diff was MessageService.cs
and two test files.

Now the tester emits ONE verdict line naming the failing check and the node(s):

    GATE FAILED — tests: Store/Catalog — 1 new failure(s), 0 stale allow entr(ies)

CI lifts that line verbatim — it does not re-derive or re-word a cause — and the
public-API sentence is kept, CONDITIONAL on the phase actually being `compile`.
Two failures that were previously reported as "0 new failure(s)" now name
themselves: a fatal error says `fatal: …`, a stale allow entry says so. A run
that fails with nothing recorded says "no failing check was recorded" rather
than picking a phase — an unexplained failure is cheaper than a confidently
wrong one. GateSummaryTest pins each of these two-sided: the line must name the
check that failed AND must not name one that did not.

Second half: the report now names WHICH node ran the Tests area, and how it was
picked. A type's `Tests` area is served by INSTANCE hubs, so the gate probes
either an instance already in the mesh or a throwaway probe it creates — and the
two are not equivalent. `Store` is the Store package's ROOT, retyped to
`Store/Catalog` by the installer; its hub can have been activated while it was
still the Space placeholder, and an instance hub never re-binds its
configuration, so it serves only the framework's default areas. That is the
difference between the same commit's green and red gate runs, and without the
host in the report the RED reads as plugin rot (it was in fact read as "the
probe landed on a type path" — it had not). The host resolution is also bounded
now: it queries the index and may create a node, neither with a budget of its
own, so a wedge there spent the whole job timeout and reported nothing.

Fixes #1077
…will look

The gate's `Store/Catalog` RED is not plugin rot and not the type-hub quirk it
was read as. The available-areas list the failure printed is, item for item,
`ConfigureDefaultNodeHub`'s set — `AddDefaultLayoutAreas` (Overview, Settings,
Search, Files, Threads, Chat, NodeTypes, AccessControl, Groups, Edit, Copy,
Move, Recycle, Versions, Delete, Pin/Unpin, StopSync, $Data/$Schema/$Model,
plus AddEducationLayoutAreas' CourseNav/Learn/StartExercise/GoToMyCopy and
AddDomainLayoutAreas' Catalog/Details/DataModel) plus Invite. Not one area from
`Store/Catalog`'s compiled configuration, and not the Space placeholder's
either. So hub `Store` — the Store package's ROOT, retyped to the package's own
NodeType by the installer — was serving the mesh DEFAULT configuration. That is
the known partition-root defect, not a broken plugin.

It recurred on a commit that already carried #1055's no-cache fix (run
31361446933, 06:18Z; #1055 merged 04:39Z). NOT cached is not NOT pinned: a hub
is pinned by address in Mesh.GetHostedHub and never re-reads its node's
NodeType, so fixing the RESOLUTION does not un-pin the hub a bad resolution
already activated. The only thing that does is PackageInstaller's DisposeRequest
— fire-and-forget, and conditional on the placeholder dance having run.

Both comment blocks that carry this ledger now say so, and both now ask three
questions instead of two, the third being "is the live hub the one activated
inside the provisioning window?" — which the gate's new Tests-host line is what
makes answerable from CI alone.

Also drops a double period from the no-allowlist verdict line (verified against
a real run: `GATE FAILED — fatal: …`), and anchors CI's grep on `^GATE FAILED`
with no punctuation, so re-wording the line cannot silently unhook the
annotation and drop CI back to a guessed cause.
Copilot AI lite review requested due to automatic review settings August 10, 2026 11:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR makes the cross-repo plugins gate report what actually failed (compile/render/tests) and which node hosted a failing Tests run, then wires CI to lift that verdict line verbatim instead of emitting a generic “does not compile / public API change” annotation.

Changes:

  • Add a stable GATE FAILED — … headline that names the failing check(s) + scope(s), including fatal/stale-allow cases, and print it as the single CI-consumed verdict line.
  • Record and print the resolved Tests host (shipped instance vs. created probe) and bound the host-resolution path with the same timeout budget as the render/test probe.
  • Update the GitHub Actions workflow to tee the tester output, extract the ^GATE FAILED line into job outputs, and only mention “public API change” when the verdict actually includes compile:.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tools/MeshWeaver.PluginTester/PluginGateRunner.cs Bounds and instruments Tests host resolution; reports which node hosted Tests and how it was chosen.
tools/MeshWeaver.PluginTester/GateReport.cs Adds FailedPrefix and prints a phase-accurate verdict headline as the single CI-consumed failure line; prints TestsHost detail.
tools/MeshWeaver.PluginTester/GateAllowlist.cs Introduces GateVerdict.Headline(...) to generate a stable, phase-accurate one-line failure summary from report/verdict data.
test/MeshWeaver.PluginTester.Test/GateSummaryTest.cs Pins the failure headline against drift and asserts “two-sided” behavior (names failing phase, does not name non-failing phase).
src/MeshWeaver.PluginCatalog/PackageInstaller.cs Updates the comment ledger to reflect the 2026-08-10 recurrence and why host reporting matters diagnostically.
src/MeshWeaver.Hosting/PathResolutionService.cs Extends the comment ledger explaining the “not cached vs not pinned” residual and ties it to the new host reporting.
.github/workflows/dotnet-test.yml Lifts the tester’s ^GATE FAILED line verbatim into job output + annotations; keeps the API-break sentence conditional on compile:.

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

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

701 tests   701 ✅  13m 4s ⏱️
 10 suites    0 💤
 10 files      0 ❌

Results for commit a5e44db.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

1 193 tests   1 193 ✅  5m 55s ⏱️
   11 suites      0 💤
   11 files        0 ❌

Results for commit a5e44db.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 351 tests   1 346 ✅  9m 58s ⏱️
   11 suites      5 💤
   11 files        0 ❌

Results for commit a5e44db.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

2 018 tests   2 016 ✅  6m 50s ⏱️
   11 suites      2 💤
   11 files        0 ❌

Results for commit a5e44db.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 783 tests   1 591 ✅  5m 44s ⏱️
   11 suites    192 💤
   11 files        0 ❌

Results for commit a5e44db.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

   11 files     11 suites   9m 39s ⏱️
1 984 tests 1 876 ✅ 108 💤 0 ❌
2 161 runs  2 053 ✅ 108 💤 0 ❌

Results for commit a5e44db.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   65 files     65 suites   51m 13s ⏱️
9 030 tests 8 723 ✅ 307 💤 0 ❌
9 207 runs  8 900 ✅ 307 💤 0 ❌

Results for commit a5e44db.

♻️ This comment has been updated with latest results.

@rbuergi

rbuergi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

This PR's own CI red is the proof

The Plugins compile against this PR job failed on this run — on the pre-existing Store/Catalog defect, which this diff cannot reach (all 6 test shards pass; the diff is the tester's reporting, the workflow's annotation, and three comment blocks). That is exactly the phenomenon #1077 describes: "it currently reds PRs whose diffs cannot reach it."

Before, that would have been annotated as:

MeshWeaver.Plugins does not compile against this PR … A public API change here breaks plugin node source that is Roslyn-compiled at runtime.

What this run actually printed (job):

##[error]GATE FAILED — tests: Store/Catalog — 1 new failure(s), 0 stale allow entr(ies).

And the new host line settles the second half of the issue outright:

RED Store/Catalog: compile=Ok render=ok tests=FAILED
    Tests host: Store — an instance of Store/Catalog already in the mesh (1 candidate(s)); its hub may predate this install
    **Area not found**
    No renderer is registered for area `Tests` on hub `Store`.

Every other type in the same run reports its host too, and the pattern is unambiguous:

Type Host Result
Store/Catalog Store — instance already in the mesh FAILED
Store/Enrollment Store/Enrollment/GateProbe — probe the gate created 13/13 passed
Store/Provision Store/Provision/GateProbe — probe the gate created 9/9 passed
Store/Plugin GateEnroll120df… — instance already in the mesh 290/290 passed

So it is not "the probe landed on a type path" (the hypothesis in the issue), and not "a pre-existing instance is bad" either — Store/Plugin used one and passed. It is specifically the partition root Store, whose hub was activated during the install/provisioning window and never re-binds, serving ConfigureDefaultNodeHub's area set instead of Store/Catalog's. Same defect as #1055, still live after it, and the residual is written up in PathResolutionService and PackageInstaller.

Not fixed here, deliberately — un-pinning an already-activated hub needs a repro and a design decision, not a guess. This PR makes the failure legible enough that the next occurrence starts from the right place.

Three conflicts, all in files #1143/#1102 also touched.

PathResolutionService + PackageInstaller (the two comment ledgers): merged, not
picked. Main's account is the correct spine — #1143 resolved the residual my
version posed as open, so "check THREE things … is the live hub the one
activated inside the provisioning window?" is deleted in favour of
NodeTypeRebindWatcher being the un-pin. What mine contributed and main did not
is the FINGERPRINT: the available-areas list being exactly
ConfigureDefaultNodeHub's set (AddDefaultLayoutAreas + Invite) and not one area
of the node's real type, plus the gate run (31361446933) it was observed on.
That is what lets someone recognise this defect instead of re-deriving it, so it
is folded into main's paragraphs.

PluginGateRunner: main's design wins outright. #1102 deleted the
shipped-instance preference — the probe host is ALWAYS a freshly created
GateProbe, which binds the just-verified release by construction. My
ResolveTestsHost and its two-branch "shipped vs probe" description are obsolete
and are gone with it; the branch they described no longer exists.

Kept on top of it, because neither is about branch selection:
  • the host still goes into the report (TestsHost). With one branch it no
    longer discriminates a cause, but "No renderer … on hub X" is still
    unreadable without knowing which node X was — that ambiguity is what made
    the original RED get read as "the probe landed on a type path".
  • the probe creation is still BOUNDED. CreateNode is a round-trip with no
    budget of its own; unbounded, a wedge there spends the whole 12-min job
    timeout and reports nothing.

Also drops a stale duplicate <summary> on CreateTestsProbe left by #1102 — the
old "Prefers an instance the package ships" text survived above the new one,
contradicting it, with two <summary> elements on one member.

Verified after the merge: 33/33 tester tests, including all ten GateSummaryTest
cases — the two-sided ones (TestsFailure_NamesTests_AndNeverCompile, and the
same with the allowlist) confirmed present in the trx, so the misattribution
cannot have crept back. mw-plugin-test run end-to-end still emits the CI-facing
line: GATE FAILED — fatal: …. Release -warnaserror clean for
MeshWeaver.PluginTester (+ Hosting and PluginCatalog, built transitively).
…ger occur

Post-merge drift in my own file. TestsHost_IsPrinted_SoAreaNotFoundIsDiagnosable
still fed the summary "Store — an instance of Store/Catalog already in the mesh"
— the shipped-instance description from before #1102 deleted that branch. The
test passed (it constructs the string itself), which is exactly the problem: a
green test documenting a host the runner can no longer produce.

Now it uses the shape RenderGate actually emits for the probe CreateTestsProbe
returns, with a note to keep the two in step. The assertion is unchanged in
kind — the host line must reach the summary.

Also retires the comment's "either an instance already in the mesh or a probe"
framing for the reason that outlives the branch: the hub named in an
"Area not found" is NEVER the type under test, because a Tests area is served by
instance hubs — which is what made the original failure get read as both "the
probe landed on a type path" and "the plugin is broken", neither true.

33/33 tester tests; both two-sided cases (TestsFailure_NamesTests_AndNeverCompile
and the allowlist variant) verified Passed by name in the trx.
@rbuergi

rbuergi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on main — the gate that blocked this PR is now green

Plugins compile against this PR: **pass** (59s). #1143 fixed #1104 (a per-node hub binds HubConfiguration from its NodeType once at activation and is then pinned by address, since RoutingServiceBase.RouteInMesh short-circuits on Mesh.GetHostedHub; a rebind watcher off IMeshChangeFeed now un-pins it). That is the residual this PR documented as open — the two comment ledgers now point at the resolution instead of posing the question.

Merge resolution (3 conflicts)

PathResolutionService + PackageInstaller — the comment ledgers, merged rather than picked. Main's account is the correct spine; what this branch contributed and main did not is the fingerprint: the available-areas list being exactly ConfigureDefaultNodeHub's set (AddDefaultLayoutAreas + Invite) and not one area of the node's real type. That is what lets someone recognise the defect rather than re-derive it, so it is folded in. The "check THREE things … is the live hub the one activated inside the provisioning window?" framing is deleted — #1143 answered it.

PluginGateRunner — main's design wins outright. #1102 deleted the shipped-instance preference: the probe host is now ALWAYS a freshly created GateProbe, which binds the just-verified release by construction. ResolveTestsHost and its two-branch "shipped vs probe" description are obsolete and gone with it.

Kept on top, because neither is about branch selection:

  • the host still goes into the report — with one branch it no longer discriminates a cause, but No renderer … on hub X is still unreadable without knowing which node X was, and that ambiguity is precisely what made the original RED get read as "the probe landed on a type path";
  • probe creation is still bounded — CreateNode is a round-trip with no budget of its own, and unbounded it spends the whole 12-min job timeout reporting nothing.

Also drops a stale duplicate <summary> on CreateTestsProbe left by #1102 (the old "Prefers an instance the package ships" text survived above the new one, contradicting it), and corrects this branch's own test fixture, which still fed the summary a shipped-instance description the runner can no longer produce.

Verification

33/33 tester tests. Both two-sided cases — TestsFailure_NamesTests_AndNeverCompile and TestsFailure_WithAllowlist_NamesTests_AndNeverCompile — confirmed outcome="Passed" by name in the trx, with both assertion directions present in source: the line must name the check that failed AND must not name one that didn't. -c Release -warnaserror clean.

Remaining red is main's, not this branch's

Run tests (shard 4) fails on StaleStampRootBindingTest, added by #1146. It fails on main's own runs at 09cd35739 and 269dcbc76, and reproduces locally (Failed: 1, Passed: 2). This diff cannot reach it — its only PackageInstaller.cs change is a comment block. Filed as #1151 with the fault site (Workspace.SubscribeToClient throws inside the SubscribeRequest handler; the handler's error arm posts nothing, so the requester burns its full 60s budget).

Not merging — the gate is absolute and Consolidate test results is red on main's regression.

…esolution reverted

The earlier 'Merge origin/main — keep both ledger accounts, adopt main's probe
design' resolved the PackageInstaller.cs conflict in the BRANCH's favour, which
silently undid #1153: SettleRetypedRoot was absent entirely and the write-time
fire-and-forget hub.Post(new DisposeRequest(), ...) was back. #1153 is what
greened main — merging that resolution would have red main again on both halves
of StaleStampRootBindingTest, and the diff read as -158 lines in a file this PR
only wants to add a comment to.

Resolution: main's side for PackageInstaller.cs, so the ordered recycle-and-wait
stands. The branch's genuine contribution to that file — the gate-run 31361446933
fingerprint (the available-areas list IS ConfigureDefaultNodeHub's set, which is
what distinguishes this from a compile failure) — is preserved on top, as is the
same enrichment in PathResolutionService.cs and all of the gate ratchet work.
@rbuergi

rbuergi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

⚠️ The previous conflict resolution silently reverted #1153 — I've restored it in 5b0686a09.

The commit "Merge origin/main — keep both ledger accounts, adopt main's probe design" got PluginGateRunner.cs right (main's CreateTestsProbe is the correct side — a shipped instance's hub activates mid-import, before the type's compile produced its release, and never rebinds). But it resolved PackageInstaller.cs in the branch's favour, and this branch predates #1153:

#1153 is what greened main earlier today. Merging that resolution would have re-red main on both halves of StaleStampRootBindingTest — the exact test main had just been rescued from.

The tell, for next time: git diff origin/main HEAD --stat read PackageInstaller.cs | 158 +++------ on a PR whose only intent in that file was to enrich a comment. A negative delta that large in a file you meant to add to is the signature of a resolution that swallowed someone else's fix. git merge-base --is-ancestor origin/main HEAD also returned false after a commit titled "Merge origin/main", meaning an older main was merged.

Nothing of this branch's own work was dropped. The gate ratchet (GateAllowlist/GateVerdict), Headline, the GateSummaryTest suite, the workflow wiring, and the gate-run 31361446933 fingerprint comments in both PackageInstaller.cs and PathResolutionService.cs are all preserved — layered on top of main's mechanism rather than instead of it.

MeshWeaver.PluginCatalog and MeshWeaver.PluginTester both build clean under -c Release -warnaserror.

@rbuergi
rbuergi merged commit 7c4e076 into main Aug 10, 2026
18 checks passed
@rbuergi
rbuergi deleted the fix/1077-plugins-gate-message branch August 19, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The plugins gate reports 'does not compile — a public API changed' when nothing failed to compile

2 participants