Take Task out of the mesh core: delete a dead startup TCS, end GitCli's sync-over-async, ratchet the IMeshService shim

[rbuergi]

Part of the "tasks are evil" sweep, framework half. Companion to #2535 (portal-API half).

Two Task sites removed, and a third not removed — with a measured reason that is the more useful half of this PR.

1. MessageService.startupCompletionSource was DEAD

Declared, cancelled at dispose, never completed and never read — no TrySetResult anywhere, no reader in src/, test/, memex/ or samples/. Deleted with its dispose block. No behaviour to preserve.

2. GitCli.Exec stopped doing sync-over-async

It started two ReadToEndAsync tasks and unwrapped them with GetAwaiter().GetResult() — inside the method whose entire job is to be the synchronous body IIoPool.InvokeBlocking takes. It now pumps stderr through BeginErrorReadLine (into a ConcurrentQueue, since the runtime raises that handler on its own thread) while reading stdout to EOF on the pool thread. Both pipes still drain concurrently; no Task remains in the assembly.

stdout deliberately stays ReadToEnd() rather than moving to the line pump as well — git show <rev>:<path> returns FILE CONTENT (GitWorkingTreeService.ShowFile, GitPackageSource.ShowFile), and a line pump would silently rewrite its CRLFs.

🚨 Reading synchronously dropped a property ReadToEndAsync(ct) gave for free: a cancelled run raised OperationCanceledException. Without it the kill leaves git with a signal exit code, and returning that makes every Expect(...) in the assembly raise GitWorkingTreeException — an unsubscribe would masquerade as "git failed". ThrowIfCancellationRequested now guards both entry and exit. Caught in review; it was a real regression, and there is now a test for it.

3. NOT done — and this is the finding

MeshServiceExtensions (CreateNodeAsync/UpdateNodeAsync/DeleteNodeAsync over a TaskCompletionSource) stays on MeshWeaver.Mesh.Contract.

The move to MeshWeaver.Fixture was written, built green, and withdrawn. Its doc comment claims "~180 existing callers"; measured, that is zero in src/, samples/, memex/ and content/ — every caller left in this repo is a test, and all four consuming test projects already reference Fixture. So on this repo's evidence the move was free.

Measured twice by different means — a tree grep here, and an org-wide GitHub code search, which is what reaches the satellite repos and their in-mesh source. That second measurement is what found the blocker, and the first one alone would have shipped the break.

What blocks it is a repo no compiler here can look at: MeshWeaver.Reinsurance has 58 call sites across 22 in-mesh Source/*.cs files (TreatySubmission, UwPortfolio, CatScenario, UWDeepfieldHome, UwLossWatch). In-mesh source compiles at RUNTIME in the portal, so deleting the shim turns 22 NodeTypes CompileError — and a CompileError NodeType refuses portal readiness. CI would have been green the whole way.

Tracked as MeshWeaver.Reinsurance#102. Exit is two steps, in this order:

1. Port those 58 sites to CreateNode(...).Subscribe(...) — a real fix, not paperwork: each is an await on hub-reachable layout-area code, i.e. already the deadlock shape.
2. Then move the shim beside the test-only QueryAsync bridge in MeshWeaver.Fixture.

Until then MeshServiceHasNoTaskShimGuard holds the line: exactly one shipped assembly, exactly three methods. A fourth verb or a second assembly fails.

Tests

• GitCliProcessDrainTest (4 new, green): stdout byte-exact through a CRLF blob; ~1 MB of stdout past any pipe buffer arriving whole (a regression there hangs rather than fails, hence its own timeout); a non-zero exit still carrying its stderr; a cancelled run raising cancellation instead of reporting a result.
• MeshServiceHasNoTaskShimGuard (2 new, green): IMeshService declares no Task member, and the bridge stays one assembly / three methods. It loads MeshWeaver.*.dll from the test output rather than walking the live AppDomain — a lazily-unloaded assembly would make a domain walk pass having looked at nothing — and asserts it found a populated set and the known shim before trusting any zero.
• Local -c Release -warnaserror: Fixture, GitSync, PluginCatalog, Documentation, Documentation.Test, GitSync.Test — all 0 Warning(s) 0 Error(s). DocumentationLinkIntegrityTest green (it caught two .md sibling links on the first pass).

Docs: AsynchronousCalls.md and SatelliteNodePatterns.md now carry the measured caller count and the two-step exit, instead of describing the shim as ordinary back-compat.

Also reported, not done here

IMessageHub.Started is a public Task readiness gate — the twin of the disposal defect in #2488, where I have left the full consumer list and an assessment: #2488 comment. It is the right fix, it lands in the same files #2301/#2488 are already changing, and it wants its own PR.

No What's New entry: pure-internal refactor, no user-visible effect.

Refs #2489

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR continues the “tasks are evil” sweep by removing several Task/TaskCompletionSource surfaces from mesh-reachable framework code, and by hardening the remaining test-only bridges and process-leaf behavior so they don’t reintroduce async hazards.

Changes:

• Removed a dead TaskCompletionSource startup gate from MessageService.
• Moved the IMeshService await-based CRUD extension shim out of MeshWeaver.Mesh.Contract into the test-only MeshWeaver.Fixture, and added a guard test to prevent reintroduction in shipped assemblies.
• Rewrote GitCli.Exec to avoid sync-over-async while still draining stdout/stderr concurrently, with new tests pinning the drain contract.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/MeshWeaver.GitSync.Test/GitCliProcessDrainTest.cs	Adds regression tests covering stdout byte-exactness, large output drain, and stderr on failure.
test/MeshWeaver.Documentation.Test/MeshServiceHasNoTaskShimGuard.cs	Adds governance tests ensuring IMeshService exposes no Task-shaped surface in shipped assemblies.
src/MeshWeaver.Messaging.Hub/MessageService.cs	Deletes a dead startup TaskCompletionSource field and its disposal-time cancellation block.
src/MeshWeaver.Mesh.Contract/Services/MeshServiceExtensions.cs	Removes the Task-returning CRUD extension shim from the core contract assembly.
src/MeshWeaver.Mesh.Contract/Services/IMeshService.cs	Updates interface comments to reflect the removal/move of the await-shaped shim.
src/MeshWeaver.GitSync/GitCli.cs	Replaces sync-over-async process draining with stderr event pumping + stdout ReadToEnd.
src/MeshWeaver.Fixture/MeshServiceTestExtensions.cs	Reintroduces the await-shaped CRUD bridge in a test-only assembly for use at test boundaries.
src/MeshWeaver.Documentation/Data/Architecture/SatelliteNodePatterns.md	Updates docs to reflect that the Task shim no longer ships in production assemblies.
src/MeshWeaver.Documentation/Data/Architecture/AsynchronousCalls.md	Updates the “no async shims” rule to point at the test-only location.
src/MeshWeaver.Documentation/Data/AI/ExecuteScript.md	Updates scripting guidance to reflect the shim’s test-only location.

---

💡 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 0)

1 312 tests   1 312 ✅  6m 10s ⏱️
    6 suites      0 💤
    6 files        0 ❌

Results for commit 0271708.

[github-actions]

Test Results (shard 2)

1 087 tests   895 ✅  5m 28s ⏱️
    6 suites  192 💤
    6 files      0 ❌

Results for commit 0271708.

[github-actions]

Test Results (shard 5)

1 601 tests   1 600 ✅  9m 44s ⏱️
    9 suites      1 💤
    9 files        0 ❌

Results for commit 0271708.

[github-actions]

Test Results (shard 4)

    8 files      8 suites   11m 31s ⏱️
4 408 tests 4 405 ✅ 3 💤 0 ❌
5 010 runs  5 007 ✅ 3 💤 0 ❌

Results for commit 0271708.

[github-actions]

Test Results (shard 3)

1 095 tests  +4   1 095 ✅ +4   18m 9s ⏱️ + 3m 55s
    7 suites ±0       0 💤 ±0 
    7 files   ±0       0 ❌ ±0 

Results for commit 0271708. ± Comparison against base commit 5574979.

[github-actions]

Test Results (shard 1)

739 tests   735 ✅  7m 21s ⏱️
  8 suites    4 💤
  8 files      0 ❌

Results for commit 0271708.

[github-actions]

Test Results

    44 files      44 suites   58m 26s ⏱️
10 240 tests 10 040 ✅ 200 💤 0 ❌
10 844 runs  10 644 ✅ 200 💤 0 ❌

Results for commit 0271708.

[rbuergi]

The Reinsurance side is ported — the shim can move now

Systemorph/MeshWeaver.Reinsurance#124 landed the last of it, closing MeshWeaver.Reinsurance#102.
This PR's §3 finding — that the org-wide code search, not the tree grep, is what found the blocker —
holds up: the blocker was real, and it was 58 in-mesh call sites in a satellite repo's runtime-compiled
Source/*.cs, invisible to every build in this repo.

Measured on MeshWeaver.Reinsurance@main after the merge: grep -rn "CreateNodeAsync\|UpdateNodeAsync\|DeleteNodeAsync" over the WHOLE tree — legacy/ and the node JSON included — returns zero code
references. Two prose mentions remain, both describing the old shape historically.

The 58 went in three passes: #106 (2 sites, the worked template), #112 (10 leaf handlers), and #124
(the remaining 42, across 19 files — the TaskAssignmentService / UwUnderwritingTeam cascade that
is byte-identical across three and two type folders, the 13-call-site decision Upsert, and 14
read-modify-writes).

So MeshServiceExtensions can move to MeshWeaver.Fixture and MeshServiceHasNoTaskShimGuard's
expectation can become "none". Two things worth re-measuring before you do, since neither is
visible from here:

1. Re-run the org-wide code search, not just the tree grep. That is what caught this the first
   time. The other satellites (MeshWeaver.Plugins, .Education, .SocialMedia, .Manufacturing,
   Crm) each carry in-mesh Source/*.cs too, and a NodeType's configuration lambda is C# inside
   a JSON string, so --include='*.cs' alone under-counts.
2. Sweep the live mesh (search_chunks) as well — a node tree can hold a caller the repo has
   already dropped.

Two findings from the port that are about the framework, not about UWDeepfield, in case they are
useful elsewhere:

• The shim was never the whole defect at a site. The dominant shape was
  try { existing = await QueryAsync($"path:{p}").FirstOrDefaultAsync(); } catch { } followed by a
  branch to UpdateNodeAsync or CreateNodeAsync. A stale query negative AND the swallowed probe
  failure both route to the create branch on a node that already exists — Write lane degraded on memex-cloud: create chain dies on a NotFound stream, a finished Store/Provision storms compile-state, plus a permanent-silent import order bug and a 2/s identity-cache loop #2229's duplicate-data
  hazard, reached two ways. IMeshService.CreateOrUpdateNode replaces the whole shape, and its
  null-keeps-state merge (UpdateAccordingToSourceNode) turned out to be exactly what the old
  existing with { … } was hand-rolling.
• Fourteen sites were read-modify-writes that wrote the WHOLE record back from a snapshot, four
  of them with a guard ("already sent", "already signed off", "already superseded", "already
  awaiting FOTs") the snapshot rendered useless — two reviewers signing off in the same window each
  overwrote the other's recorded verdict. GetMeshNodeStream(path).Update(…) fixes both halves,
  because the owning hub hands the lambda the live node. The semantic that replaces an early
  return in an async body — hand the node straight back — is the one a compile cannot check, so
  each guard is now a pure Apply* transform pinned by a mutation-proven test.

[rbuergi]

🚨 Caveat on the sweep this thread asks for before the shim moves: the search_chunks half cannot be executed today — see #2741.

Content indexing is inactive on both MCP-reachable portals, and search_chunks answers {"count":0,"results":[],"message":"Content indexing is not active on this deployment…"}. Verified on both this morning. So a live-mesh sweep run now returns count: 0 — indistinguishable from "no callers found", which is exactly the false pass that would license removing a public surface that in-mesh code still binds.

Until #2741 is resolved, the honest basis for moving MeshServiceExtensions is: repo trees across the fleet plus node JSON (the configuration lambdas are C# inside JSON strings and are invisible to grep --include='*.cs'), plus node-level search — which does work and covers node content, unlike the chunk index. State that basis explicitly rather than citing a live-mesh sweep that did not run.

(Reinsurance's side is clear: #102 merged as ec1ee9b, and git grep over all of that repo's main — legacy/ and node JSON included — now finds zero code references to the three shim methods.)