Skip to content

fix(modules): the reload agent reads the commit the feed announced, never a trailing mirror - #6226

Merged
rbuergi merged 4 commits into
mainfrom
fix/module-reload-faulted-beside-activation
Oct 7, 2026
Merged

rbuergi merged 4 commits into
mainfrom
fix/module-reload-faulted-beside-activation

Conversation

@rbuergi

@rbuergi rbuergi commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Root cause

ModuleReloadFaultedBesideActivationTest.OneModulesCrashedAdopt_BesideAnotherModulesLiveActivation_IsFaulted times out on core main (push run 37569688010, shard 3) and on #6210 (run 37570022272, shard 4). #6210's diff does not touch this code; the defect was already on main. The test is not in .github/known-flakes.json.

The per-process ModuleReloadAgent hears a request on the invalidation feed, which fires post-commit. This process's mirror of the node gets the owner's echo by a separate path, and under load that echo arrives after the feed event. Handle read the mirror with a bare Take(1), so for the event that announced Activating it saw the previous state, Landing. It swapped nothing and reported nothing. The executor writes nothing more until a replica reports, so no later event came to wake the agent. The request stayed in Activating, logged nothing, and the test's AwaitRequest ran out its time (108 s in CI, with no log line after the crashed download).

Fix

Evidence

  • New ModuleReloadAgentReadsTheAnnouncedCommitTest makes the "mirror behind the announcement" moment deterministic: both handlers subscribe before the commit is written.
    • The handler with the floor waits for the commit and reports.
    • Its negative control, the same handler without a floor (the old read), answers from the earlier state and reports nothing.
  • Local Release run: 12/12 passed across the new test, ModuleReloadFaultedBesideActivationTest, ModuleReloadLiveTest, ModuleReloadByRestartTest and InstanceRebootTest. Built -c Release -warnaserror with 0 errors.

Not established: CI logs only record Warning level, so the exact interleaving in the CI failure was not captured in a log. The mechanism is inferred from where it stopped (after the crashed download, before any report) and pinned deterministically by the test above.

Doc: Doc/Architecture/ModuleReload step 5.

Pairs-with: none — no public surface removed (internal Handle gains an optional parameter)
Mirror-sync: none — no catalog change
Recycle: none — process-level agents, picked up by the roll

🤖 Generated with Claude Code

…ever a mirror that trails it

The invalidation feed fires post-commit while this process's mirror receives the
owner's echo on its own path - under load AFTER the feed. The agent's bare Take(1)
then read Landing for the event that announced Activating, swapped nothing,
reported nothing, and nothing later woke it: the request sat in Activating until
ModuleReloadFaultedBesideActivationTest timed out (main run 37569688010, #6210's
run 37570022272). The read now waits for a state at or past the event's Version
(the RebaseSource floor, #1174); the boot listing reads unfloored. The instance
reboot agent had the same read and gets the same floor.

Pinned by ModuleReloadAgentReadsTheAnnouncedCommitTest, whose unfloored control
reports nothing.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@meshweaver-cloud
meshweaver-cloud Bot enabled auto-merge October 7, 2026 06:52
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   11 files     11 suites   28m 1s ⏱️
6 891 tests 6 891 ✅ 0 💤 0 ❌
6 903 runs  6 903 ✅ 0 💤 0 ❌

Results for commit 9d0db69.

♻️ This comment has been updated with latest results.

@systemorph-com
systemorph-com Bot disabled auto-merge October 7, 2026 06:56
@rbuergi
rbuergi requested a balanced review from Copilot October 7, 2026 07:07

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.

Copilot review overview

🟡 Changes recommended

Both agents can still complete silently before reaching the announced version, bypassing their warning paths.

Review effort: Balanced
Findings: 3 Medium severity

Open (3)
What changed in this PR

Prevents reload and reboot agents from acting on stale mirrored state after a commit notification.

Changes:

  • Uses announced versions as minimums for both agents’ reads.
  • Adds a delayed-mirror regression test for module reloads.
  • Documents version floors and includes announced versions in warnings.
File Description
test/​Memex.Portal.Shared.Test/​ModuleReloadTest.cs Adds delayed-mirror regression and unfloored control.
src/​MeshWeaver.PluginCatalog/​ModuleReloadExecutor.cs Applies announced-version floors to reload reads.
src/​MeshWeaver.PluginCatalog/​InstanceRebootExecutor.cs Applies equivalent floors to reboot reads.
src/​MeshWeaver.Documentation/​Data/​Architecture/​ModuleReload.md Explains mirror lag and version-floor behavior.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/MeshWeaver.PluginCatalog/InstanceRebootExecutor.cs
Comment thread src/MeshWeaver.PluginCatalog/InstanceRebootExecutor.cs Outdated
Comment thread src/MeshWeaver.PluginCatalog/ModuleReloadExecutor.cs Outdated
@systemorph-com systemorph-com Bot added the thread:owner-systemorph-meshweaver-6226-544096 Agent thread Hosting/Triage/pull-request/systemorph-meshweaver-6226/_Thread/owner-systemorph-meshwe… label Oct 7, 2026
@systemorph-com

systemorph-com Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

🔗 thread:owner-systemorph-meshweaver-6226-544096 — assigned to the agent thread https://memex.systemorph.com/Hosting/Triage/pull-request/systemorph-meshweaver-6226/_Thread/owner-systemorph-meshweaver-6226 (origin).

1 similar comment
@systemorph-com

systemorph-com Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

🔗 thread:owner-systemorph-meshweaver-6226-544096 — assigned to the agent thread https://memex.systemorph.com/Hosting/Triage/pull-request/systemorph-meshweaver-6226/_Thread/owner-systemorph-meshweaver-6226 (origin).

…mit is an error, not a silent empty

Both agents' floored read used Take(1): a node stream that completes before reaching
the announced version (a disposed synchronization stream forwards OnCompleted) left
it empty, Timeout passed the completion through, and Arm's warning never ran — no
report, no diagnostic. FirstAsync turns that completion into an error on the existing
warning path.

Adds InstanceRebootAgentReadsTheAnnouncedCommitTest, the reboot twin of the module
reload regression: a handler subscribed below a nonzero floor waits for the
AwaitingRestart commit and reports, while the unfloored control reads the earlier
state and reports nothing.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 7, 2026 07:20
@meshweaver-cloud
meshweaver-cloud Bot enabled auto-merge October 7, 2026 07:20

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.

Copilot review overview

🟢 Approval recommended

The narrowly scoped version-floor changes have regression coverage, and no blocking issues were identified.

Review effort: Balanced
Findings: None

Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Low severity Use ImmutableHashSet for the read-only fixture

test/​Memex.Portal.Shared.Test/​ModuleReloadTest.cs:1023

Use an ImmutableHashSet for this read-only fixture, as required by the collection policy in AGENTS.md:413. It satisfies LoadedNames's IReadOnlySet<string> return type and preserves the case-insensitive comparer without introducing a mutable collection.

@systemorph-com
systemorph-com Bot disabled auto-merge October 7, 2026 07:35
Copilot AI balanced review requested due to automatic review settings October 7, 2026 07:56
@meshweaver-cloud
meshweaver-cloud Bot enabled auto-merge October 7, 2026 07:57

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.

Copilot review overview

🟡 Changes recommended

Versionless feed events can still bypass the freshness check and leave requests pending after a stale read.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)

Comment thread src/MeshWeaver.PluginCatalog/ModuleReloadExecutor.cs
…instead of reading an unfloored mirror in silence

A path-only invalidation (StorageChangeFeedRelay.PathOnlyFallback) keeps the node type
but carries version 0, which the agents could not tell from the boot listing's no-floor
read. Each agent now warns, by request path, that the announced commit is unknown and
its read may trail it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 7, 2026 08:40
@meshweaver-cloud
meshweaver-cloud Bot enabled auto-merge October 7, 2026 08:41

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.

Copilot review overview

🟢 Approval recommended

The focused fix addresses the stale-read race with regression coverage, and only minor nonblocking refinements remain.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (2)

In code that hasn't changed since last review

Low severity Document best-effort handling of versionless feed events

src/​MeshWeaver.Documentation/​Data/​Architecture/​ModuleReload.md:110

Document the intentional best-effort exception for versionless feed events. Both agents log a warning for Version <= 0 but still read unfloored, so they can miss the announced phase when the mirror trails it. As written, this paragraph implies that only boot-listing reads lack a version floor.

Low severity Use ImmutableHashSet for LoadedNames

test/​Memex.Portal.Shared.Test/​ModuleReloadTest.cs:1023

This fixture does not mutate the set, but the new HashSet conflicts with the immutable-collection requirement in AGENTS.md:413. Use ImmutableHashSet to preserve the case-insensitive lookup and satisfy LoadedNames's IReadOnlySet<string> contract.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

  1 files    1 suites   3m 15s ⏱️
354 tests 354 ✅ 0 💤 0 ❌
358 runs  358 ✅ 0 💤 0 ❌

Results for commit 9d0db69.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

519 tests   519 ✅  2m 42s ⏱️
  1 suites    0 💤
  1 files      0 ❌

Results for commit 9d0db69.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 104 tests   1 104 ✅  4m 49s ⏱️
    1 suites      0 💤
    1 files        0 ❌

Results for commit 9d0db69.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

    1 files      1 suites   4m 57s ⏱️
1 301 tests 1 301 ✅ 0 💤 0 ❌
1 305 runs  1 305 ✅ 0 💤 0 ❌

Results for commit 9d0db69.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

    2 files      2 suites   6m 19s ⏱️
1 211 tests 1 211 ✅ 0 💤 0 ❌
1 212 runs  1 212 ✅ 0 💤 0 ❌

Results for commit 9d0db69.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

    5 files      5 suites   5m 57s ⏱️
2 402 tests 2 402 ✅ 0 💤 0 ❌
2 405 runs  2 405 ✅ 0 💤 0 ❌

Results for commit 9d0db69.

@rbuergi
rbuergi merged commit 04d0f4e into main Oct 7, 2026
@rbuergi
rbuergi deleted the fix/module-reload-faulted-beside-activation branch October 10, 2026 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests-before-review thread:owner-systemorph-meshweaver-6226-544096 Agent thread Hosting/Triage/pull-request/systemorph-meshweaver-6226/_Thread/owner-systemorph-meshwe…

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants