Repository navigation
docs: guidance no longer teaches the ToTask bridge, and a ratchet keeps it that way - #2755
Merged
Merged
Conversation
…ps it that way
Maintainer ruling, 2026-08-30: "totask is forbidden", "strictly", "no totask
ever". The older carve-out — "tests are the only sanctioned place" — is
RETRACTED. Source is being swept elsewhere; this is the half no code scanner can
see, and it is the half that MATTERS MORE: a page that presents the bridge as the
right thing to write generates new violations faster than a sweep removes them.
The two highest-value categories:
* .github/copilot-instructions.md drives the automatic Copilot review on every
PR in this repo, and it listed `await …FirstAsync().ToTask()` as "the accepted
bridge" in test/**. Review was actively DEFENDING the banned shape. It now
flags `.ToTask(` repo-wide, flags `await source.FirstAsync()` as the same
defect (Rx's own awaiter is an AsyncSubject<T> that resumes inline —
measured INLINE=True for both spellings, INLINE=False only for
ObserveCompletion), and states the three replacements in priority order.
* The .csx examples in ScriptExecution.md, ActivityControlPlane.md,
ScriptExecutionDemo.md and UnifiedPath.md are pasted into mesh nodes, which
compile at RUNTIME in the portal — the blind spot AGENTS.md names. Converted
to .FirstAsync().ObserveCompletion(report, Ct), keeping the "always pass Ct"
point that was correct.
Also converted: the test sketches (SyncedQueryDataSource, SyncedMeshNodeQueries,
AggregatingProviders, NegativeControls, AccessControl/AccessContextPropagation)
to the reactive assertion surface that owns the wait; the framework-edge examples
(HubDisposalModel, ControlledIoPooling, GUI/Observables, /sigsegv) to
ObserveCompletion; and the five remaining prose sanctions of the retracted
exemption (SendingEmail, PermissionApi, ActorModel, LanguageServices,
NodeTypes/Testing, Mesh.Operations/README).
ReactiveTestAssertions.md and WritingTests.md were also factually WRONG, not
merely stale: they said the assertion "bridges with .ToTask()". It waits through
the package's own ReactiveWait.First, a TaskCompletionSource created with
RunContinuationsAsynchronously. Both passages corrected, along with the diagram
caption that still described the deleted SubscribeOn(TaskPoolScheduler).
Deliberately NOT touched: every place that WARNS. The ❌ anti-pattern blocks, the
war stories, the "delete on sight" checklists (AsynchronousCalls, BlazorAsync,
BlazorDataBinding, ObservableHubPipeline, ActorModel's deleted-shape block,
InitializationGates, ThreadOperations, ActivityOperations, OrleansTaskScheduler,
/async, /storm, /gui). A rule stated without the shape it forbids is
unrecognisable in review.
The ratchet: GuidanceBridgeRatchetGuard counts `.ToTask(` ONLY inside a fenced
code block whose language is csharp/cs/c# — prose is a warning, a copyable C#
example is a teaching defect — over Doc/Data, .claude/skills, .github and the
repo-root *.md. Seeded (GuidanceBridgeSites.allow, 6 files / 12 sites, all ❌
blocks) so it can only shrink; a new file, a raised count or a raised TOTAL
fails, a shrunken count is reported as stale. Non-vacuity is pinned in both
directions through the real scanner — synthetically (csharp and cs fences count;
prose, ```text and unlabelled fences do not) and against the real tree
(.github/copilot-instructions.md names the shape and must NOT be counted).
Verified: Release -warnaserror build 0 Warning(s) 0 Error(s); the whole
MeshWeaver.Documentation.Test project 103/103 passed. Live negative control: a
`.ToTask(` injected into a ```csharp fence in ScriptExecution.md failed the guard
RED naming that file ("NEW SITE src/.../ScriptExecution.md (1)"); the identical
text in PROSE in the same file left it green; reverted, green again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates MeshWeaver’s written guidance to reflect the 2026-08-30 ruling that observable→Task bridges via .ToTask( are forbidden everywhere (including test/**), and adds a documentation-focused ratchet to prevent reintroducing the pattern in copyable C# examples.
Changes:
- Rewrites doc/skill/review-instruction examples to avoid
.ToTask(and prefer reactive assertions orReactiveCompletion.ObserveCompletion. - Adds
GuidanceBridgeRatchetGuard+ a seeded allow-list to prevent new.ToTask(teaching sites inside fenced C# markdown examples. - Adds a What’s New entry documenting the guidance change and the new ratchet.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Documentation.Test/GuidanceBridgeRatchetGuard.cs | Adds a ratchet test that scans guidance markdown for .ToTask( inside fenced C# blocks. |
| test/GuidanceBridgeSites.allow | Seeded allow-list for remaining .ToTask( occurrences that are intentionally anti-pattern examples. |
| src/MeshWeaver.Mesh.Operations/README.md | Updates operations guidance to forbid .ToTask() and point to ObserveCompletion at true Task-shaped boundaries. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-30-guidance-no-longer-teaches-the-forbidden-bridge.md | Adds What’s New entry explaining the guidance change and ratchet. |
| src/MeshWeaver.Documentation/Data/GUI/Observables.md | Updates UI observable bridging example to use ObserveCompletion instead of .ToTask. |
| src/MeshWeaver.Documentation/Data/DataMesh/UnifiedPath.md | Updates script example to use FirstAsync().ObserveCompletion(...). |
| src/MeshWeaver.Documentation/Data/DataMesh/SyncedQueryDataSource.md | Converts test sketch from .ToTask bridging to reactive assertions (Should().Within(...).Match(...)). |
| src/MeshWeaver.Documentation/Data/DataMesh/NodeTypes/Testing.md | Removes “sanctioned ToTask in tests” wording; points to assertion-based waiting or helper methods. |
| src/MeshWeaver.Documentation/Data/Architecture/WritingTests.md | Updates test-writing rules to state .ToTask() is forbidden and to use assertion-owned waits. |
| src/MeshWeaver.Documentation/Data/Architecture/SyncedMeshNodeQueries.md | Converts pre-warm/wait examples to assertion-owned waits; notes script alternative via ObserveCompletion. |
| src/MeshWeaver.Documentation/Data/Architecture/SendingEmail.md | Removes test exemption; shows assertion-based wait for email observable. |
| src/MeshWeaver.Documentation/Data/Architecture/ScriptExecutionDemo.md | Converts .Take(1).ToTask(Ct) waits to FirstAsync().ObserveCompletion(...). |
| src/MeshWeaver.Documentation/Data/Architecture/ScriptExecution.md | Updates script examples and guidance to use ObserveCompletion and explain why .ToTask/bare await are banned. |
| src/MeshWeaver.Documentation/Data/Architecture/SatelliteEntityPatterns.md | Converts waits to assertion-owned shape and adds rationale. |
| src/MeshWeaver.Documentation/Data/Architecture/ReactiveTestAssertions.md | Updates docs to reflect that assertions do not use Rx .ToTask() and explains queued continuation behavior. |
| src/MeshWeaver.Documentation/Data/Architecture/PermissionApi.md | Removes .FirstAsync().ToTask() test guidance; replaces with assertion-based example. |
| src/MeshWeaver.Documentation/Data/Architecture/NegativeControls.md | Converts renderer waits from .ToTask() to .Should().Emit(). |
| src/MeshWeaver.Documentation/Data/Architecture/LanguageServices.md | Updates “foreign Task signature” guidance to use ObserveCompletion instead of .ToTask. |
| src/MeshWeaver.Documentation/Data/Architecture/HubDisposalModel.md | Replaces teardown/deactivation bridge example with ObserveCompletion and explains why. |
| src/MeshWeaver.Documentation/Data/Architecture/ControlledIoPooling.md | Updates disposal waiting example to use ObserveCompletion and adds explanation. |
| src/MeshWeaver.Documentation/Data/Architecture/AggregatingProviders.md | Converts test guidance/examples to assertion-owned waits and forbids .ToTask. |
| src/MeshWeaver.Documentation/Data/Architecture/ActorModel.md | Updates actor-model guidance to state tests are not exempt and points to assertions/ObserveCompletion. |
| src/MeshWeaver.Documentation/Data/Architecture/ActivityControlPlane.md | Converts script examples from .ToTask(Ct) to ObserveCompletion and explains why. |
| src/MeshWeaver.Documentation/Data/Architecture/AccessControl.md | Converts exception assertion sketch from .ToTask() bridge to Materialize() + assertion. |
| src/MeshWeaver.Documentation/Data/Architecture/AccessContextPropagation.md | Same as above: replaces .ToTask() exception-assert sketch with Materialize() pattern. |
| .github/copilot-instructions.md | Updates automated review instructions to flag .ToTask( everywhere (including tests) and treat await FirstAsync() equivalently. |
| .claude/skills/sigsegv/SKILL.md | Updates SIGSEGV triage guidance to forbid .ToTask() and recommend ObserveCompletion at lifecycle boundaries. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+97
to
+101
| var total = allowed.Values.Sum(); | ||
| if (total > TotalBudget) | ||
| failures.Add( | ||
| $" TOTAL {total} allowances > {TotalBudget} budgeted — the inventory GREW. " | ||
| + "Adding a line to " + AllowFileName + " is not a fix."); |
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
Contributor
# Conflicts: # .github/copilot-instructions.md # src/MeshWeaver.Documentation/Data/Architecture/LanguageServices.md # src/MeshWeaver.Documentation/Data/Architecture/ReactiveTestAssertions.md # src/MeshWeaver.Documentation/Data/Architecture/SendingEmail.md
rbuergi
added a commit
that referenced
this pull request
Aug 30, 2026
SelfUpdateAvailabilityGateTest waited on IsHeld(CandidateTag) with a 30 s bound. That single wait conflates two outcomes needing opposite fixes: the poller never ran a check (wedged, or its trigger never fired — nothing about the gate is implicated), and a check ran and decided not to hold (the gate is wrong — a real regression). Both arrived as "it did not hold within 30 s". Observed once on main at ec90061, the merge of #2755 — DOCUMENTATION ONLY (doc pages, .claude/skills/*, AGENTS.md, copilot-instructions.md). There is no path from that diff to the self-update availability gate, and no way to attribute the red to either shape. Widening the 30 s would only make a real regression slower to surface, which is why the issue asked for determinism instead. WaitForHeld is now two stages. Stage 1 waits for LastCheckedAt, which EVERY check stamps beside its verdict (#2553's fix) — the positive signal that the poller is alive and has evaluated once. Stage 2 waits for the hold and, on timeout, reports the check that DID run: when, its trigger, and its verdict verbatim. Mutation-checked by pointing stage 2 at a tag that never holds: System.TimeoutException : the poller DID run a check (LastCheckedAt=2026-08-30T17:00:17Z, trigger=Startup, verdict="HOLDING 9999.0.0-ci.1 — no release-availability gate is registered on this host, ...") but never held 9999.0.0-ci.1. This is the gate deciding NOT to hold — a real regression — not a poll that had not fired yet (#2777). Applied to all three hold-waiting tests in the file, not only the one that failed. dotnet build test/MeshWeaver.Hosting.Monolith.Test -c Release -warnaserror 0 Error(s) dotnet test --filter SelfUpdateAvailabilityGateTest 6 passed, 0 failed, 1 s Fixes #2777 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014FhP7Jg65q8cAATnFfeaTK
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Maintainer ruling, 2026-08-30: "totask is forbidden", "strictly", "no totask ever". The older carve-out — "tests are the only sanctioned place" — is RETRACTED.
Source is being swept in #2743 / #2748. This is the half no code scanner can see, and it is the half that matters more: a doc or instruction file that prescribes the banned shape generates new violations faster than a sweep removes them.
The two highest-value fixes
.github/copilot-instructions.mddrives the automatic Copilot review on every PR in this repo, and it listedawait …FirstAsync().ToTask()as "the accepted bridge" intest/**. Review was actively defending the banned shape. It now flags.ToTask(repo-wide, flagsawait source.FirstAsync()as the same defect rather than a simplification (Rx's own awaiter is anAsyncSubject<T>that resumes inline — measuredINLINE=Truefor both spellings,INLINE=Falseonly forObserveCompletion), and states the three replacements in priority order.The
.csxexamples inScriptExecution.md,ActivityControlPlane.md,ScriptExecutionDemo.mdandUnifiedPath.mdare pasted into mesh nodes, which compile at runtime in the portal — the blind spot AGENTS.md names. Converted to.FirstAsync().ObserveCompletion(report, Ct); the "always passCt" point survives, only the shape changed.Also converted
SyncedQueryDataSource,SyncedMeshNodeQueries,AggregatingProviders,NegativeControls,AccessControl/AccessContextPropagation(the last two via.Materialize(), which keeps the original exception type assertable).ObserveCompletion:HubDisposalModel,ControlledIoPooling,GUI/Observables,/sigsegv.SendingEmail,PermissionApi,ActorModel,LanguageServices,DataMesh/NodeTypes/Testing,MeshWeaver.Mesh.Operations/README.Two passages were factually WRONG, not merely stale
ReactiveTestAssertions.mdandWritingTests.mdsaid the assertion "bridges with.ToTask()" and called it "the sanctioned test-edge Rx→Task bridge". It waits through the package's ownReactiveWait.First— aTaskCompletionSourcecreated withRunContinuationsAsynchronously. Corrected, along with the diagram caption that still described the long-deletedSubscribeOn(TaskPoolScheduler).Deliberately NOT touched
Every place that warns. The ❌ anti-pattern blocks, the war stories, the "delete on sight" checklists:
AsynchronousCalls,BlazorAsync,BlazorDataBinding,ObservableHubPipeline,ActorModel's deleted-shape block,InitializationGates,ThreadOperations,ActivityOperations,OrleansTaskScheduler,/async,/storm,/gui. A rule stated without the shape it forbids is unrecognisable in review.Files owned by #2743 (
AGENTS.md,/async,/testing,AsynchronousCalls.md) and #2748 (RemovingObservableToTaskBridges.md,ObservableToTaskBridgeGuard.cs, the other.allowfiles) are untouched.The ratchet
GuidanceBridgeRatchetGuardcounts.ToTask(only inside a fenced code block whose language iscsharp/cs/c#— a mention in prose is a warning, a mention in a copyable C# example is a teaching defect. Unlabelled and```text/```output/```bashfences are not counted; this repo keeps captured stack traces and red-flag lists in those. It scanssrc/MeshWeaver.Documentation/Data/,.claude/skills/,.github/and the repo-root*.md.Seeded rather than zero-tolerance —
GuidanceBridgeSites.allow, 6 files / 12 sites, every one an ❌ anti-pattern block that must survive:.claude/skills/async/SKILL.md.claude/skills/sigsegv/SKILL.mdDoc/Architecture/ActorModel.mdDoc/Architecture/AsynchronousCalls.mdDoc/Architecture/BlazorDataBinding.mdDoc/Architecture/InitializationGates.mdHouse pattern: a new file, a raised count or a raised TOTAL fails; a shrunken count is reported as stale, never failed.
Non-vacuity is pinned in both directions, through the real scanner — synthetically (a
```csharpand a```csfence count, including one indented inside a list item; prose, a```textfence and an unlabelled fence do not), and against the real tree (.github/copilot-instructions.mdcontains the marker and must NOT be counted — it names the shape precisely in order to tell the reviewer to flag it).Verification
dotnet build -c Release -warnaserror test/MeshWeaver.Documentation.Test/…→0 Warning(s),0 Error(s).Failed: 0, Passed: 103..ToTask(injected inside a```csharpfence inScriptExecution.mdfailed the guard RED, naming it —NEW SITE src/MeshWeaver.Documentation/Data/Architecture/ScriptExecution.md (1). The identical text in prose in the same file left the guard green. Reverted → green again (3/3).🤖 Generated with Claude Code