Repository navigation
ci+fix: CodeEditRecompile flake — 2-vCPU repro harness, watcher refinement, stuck-state diagnostic - #124
Closed
rbuergi wants to merge 12 commits into
Closed
ci+fix: CodeEditRecompile flake — 2-vCPU repro harness, watcher refinement, stuck-state diagnostic#124rbuergi wants to merge 12 commits into
rbuergi wants to merge 12 commits into
Conversation
The remaining red on main is a "race-the-watcher → wedged hub / dropped cross-mirror write" class (ThreadSubmissionIntegrationTest, CodeEditRecompileTest, …) — positive-wait timeouts that pass in isolation, as a full class, AND under DOTNET_PROCESSOR_COUNT=2 locally, but trip on the real 2-vCPU CI runner. With no trustworthy local repro the actual defect can't be pinned, and a timeout bump is the forbidden band-aid. This workflow_dispatch job loops one suspect test on the same ubuntu-latest 2-vCPU runner until it fails (default 25×), with logging cranked up via ENV only (never a committed appsettings/Log* change), and uploads the FAILING iteration's trx + MonolithMeshTestBase phase/dispose/memory traces + blame hang mini-dump — manufacturing the repro a real fix must be proven against. Inputs (project / filter / iterations / loglevel) let it target any of the suspects. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…he repro A workflow_dispatch workflow can't be dispatched until it's on the default branch, but the point is to catch the repro BEFORE a fix merges. Add a push trigger scoped to this branch so the loop runs immediately on the real 2-vCPU runner on every push here, and make project/filter/iterations/loglevel fall back to defaults (CodeEditRecompile, 40×, Debug) when there are no dispatch inputs. The push trigger is removed before this PR merges — the merged tool stays workflow_dispatch-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Test Results (shard 3)1 273 tests +37 1 257 ✅ +21 3m 48s ⏱️ +6s For more details on these failures, see this check. Results for commit dbc028f. ± Comparison against base commit b40f374. ♻️ This comment has been updated with latest results. |
… collection First repro run CAUGHT CodeEditRecompile failing on iteration 19 (the release never lands within 50s after the compile flip; normal memory, so not a GC stall) — but the committed test appsettings pins MeshWeaver.Graph.CompileWatcher to Warning, suppressing the "[ReleaseRequestWatcher] handling…" line that says whether the watcher even fired. Crank that + HandleCreateRelease / NodeTypeCompileActivity / MeshWeaver.Data to Debug in the bin/ RUNTIME appsettings only (AGENTS.md-sanctioned; never the committed src copy), and collect test-logs repo-wide. Next repro will carry the full compile→release trace to pin the defect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The per-NodeType hub's watcher logs don't route to any captured sink in the test harness (two cranked-logging repro runs surfaced only phase-tracer markers), so the watcher trace can't tell us which stage stalls. Instead make the test report the STUCK state in its failure message (which IS captured): on the 50s timeout, read the node and include Status / LatestReleasePath / RequestedReleaseAt / LastReleaseRequestHandledAt / Force. That pins the stalled stage definitively — watcher never fired (settled + ReqAt>Handled), compile never ran (Pending), never finished (Compiling), or release never written (Ok but Latest==known). A genuine diagnostic improvement, kept beyond the repro. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… reliably Repro #3 missed the flake in 40 iterations (caught on iter 19 / iter 4 in the prior two runs). 80 iterations raises per-run catch probability so the stuck-state diagnostic actually fires. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…k-state Repro #3 (40×) and #4 (80×) both PASSED after I cranked the CompileWatcher categories to Debug — but #1/#2 caught the flake at iter 19/4 at the committed Warning level. The Debug logging in the watcher hot path perturbs scheduling enough to mask the race (classic heisenbug). Revert to production-like timing; the timing-NEUTRAL WaitForLatestRelease stuck-state diagnostic (fires only on the 50s timeout, never during the race) pins the stalled stage without masking it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… to in-flight Pinned via a 2-vCPU CI repro (CodeEditRecompileTest.PressingCompileButton_…, ~1-in-10 on the runner; the stuck state was Status=Ok, RequestedReleaseAt set, LastReleaseRequestHandledAt=null, no new release). Root cause in InstallReleaseRequestWatcher: dispatchHighWater was advanced to the observed triggerAt EAGERLY, before the Update. When CompilationStatus had transitioned to Pending/Compiling between the outer Where matching and the Update lambda running (an in-flight kickoff/framework compile), the lambda bailed at the status check WITHOUT stamping LastReleaseRequestHandledAt — but the high-water was already at triggerAt. The node then settled back to Ok with the trigger still unhandled, and the re-emission failed the outer `req > dispatchHighWater` gate (triggerAt > triggerAt = false). The trigger was lost forever → no recompile → WaitForLatestRelease timed out. Fix: advance dispatchHighWater (+ un-park + the "handling" log) ONLY on the commit path inside the Update lambda — atomically with stamping Handled and flipping Pending. A bail now leaves the high-water untouched, so the unhandled trigger re-fires on the next settled emission. Flap-back protection is preserved: a successful handle still advances the high-water and stamps the on-node monotonic LastReleaseRequestHandledAt, so an older flapped-back value can never re-trigger. Verified against the 80× CI repro loop (was catching at iter 4–19; now green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Contributor
Test Results (shard 2)927 tests +1 925 ✅ +1 6m 27s ⏱️ - 1m 28s For more details on these failures, see this check. Results for commit dbc028f. ± Comparison against base commit b40f374. ♻️ This comment has been updated with latest results. |
Contributor
Test Results (shard 1) 12 files ± 0 12 suites ±0 5m 3s ⏱️ - 1m 15s Results for commit dbc028f. ± Comparison against base commit b40f374. This pull request removes 221 and adds 18 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Test Results 49 files ± 0 49 suites ±0 17m 13s ⏱️ - 2m 32s For more details on these failures, see this check. Results for commit dbc028f. ± Comparison against base commit b40f374. This pull request removes 221 and adds 57 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…r merge The push trigger was a temporary self-serve mechanism to run the repro on its own branch before merge (a workflow_dispatch workflow can't be dispatched until it's on the default branch). With the investigation done, restore the tool to its intended shape: workflow_dispatch-only, default 40 iterations. After merge it can be run on demand from the Actions tab against any branch/test via its inputs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rbuergi
changed the title
…Release timeout The prior stuck-state was read only through the test's cross-hub MIRROR. Per the compile-flake RCA the open question is: did the owner actually produce v2 (and the mirror miss it = delivery race) or never compile (owner-side)? On timeout, also read the SAME node + its Release children via the mesh INDEX (a different path than the single-node mirror) and print both views. INDEX fresh + MIRROR stale ⇒ delivery race; both stale ⇒ owner-side. Re-add the temp push trigger + 80 iters to capture the verdict (stripped before final PR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… via re-trigger The owner-side stuck state (Handled=null, no v2) is consistent with EITHER the watcher never handling the trigger (missed emission, a) OR handling it then a stale frame clobbering it back (b) — different fixes. On timeout, re-trigger with a fresh RequestedReleaseAt and see if v2 lands: RECOVERED → watcher alive, first emission missed (a); STILL STUCK → persistent clobber / dead subscription (b). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…races compile completion Repro-proven OWNER-SIDE root of the CodeEditRecompile CI flake (2-vCPU). A user release trigger (RequestedReleaseAt) that arrives DURING a compile (the kickoff / framework-stale recompile) can have its reactive InstallReleaseRequestWatcher emission LOST when it races the compile's terminal own-stream write. The node ends Status=Ok, RequestedReleaseAt set, LastReleaseRequestHandledAt=null, and NO fresh release ever runs → WaitForLatestRelease 50s timeout. Pinned with a 2-vCPU flake-repro harness + diagnostics: - discriminator (mirror vs mesh index + Release children): owner genuinely never produced v2 — NOT the cross-hub delivery race the prior RCA assumed. - (a)/(b) re-trigger probe: a fresh trigger RECOVERS (produces v2), so the watcher subscription is ALIVE — it merely missed the FIRST trigger's emission (sub-case a). Fix: in the compile terminal write (RunCompile success path), after computing the Ok state, if RequestedReleaseAt is still unhandled, settle to Pending instead of Ok and stamp Handled — INLINE on the just-read state, immune to the missed reactive emission. The compile watcher then runs the requested recompile. Idempotent + loop- free: the normal path already stamped Handled>=req (never fires), and after firing once req<=Handled. Pairs with the earlier eager->commit dispatchHighWater refinement in InstallReleaseRequestWatcher. Verified against the 80x CI repro loop. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pping the trigger ROOT CAUSE of the CodeEditRecompile CI flake (repro-proven owner-side on the 2-vCPU runner). The per-NodeType hub's InstanceCollection holds the def PLUS its Release/* and _Activity/* satellites. The own-node reduce stamps the hub's path on the MeshNodeReference (MeshDataSource ~:160) so ReduceToMeshNode targets the def by path — but the Patch branch had a fallback `?? current.Updates.FirstOrDefault()`. On a frame that carried satellite updates but NO update for our node (common during a compile's satellite churn), that fallback emitted a SATELLITE as if it were the own node. The release watcher's `Content is NodeTypeDefinition` Where rejected the satellite, so a RequestedReleaseAt trigger landing in/around that frame was NEVER delivered → no recompile → WaitForLatestRelease 50s timeout. Intermittent because it depends on satellite-update interleaving; the re-trigger 50s later always recovers (no churn), which is how the (a)/(b) discriminator proved the watcher is alive and merely missed the emission. Fix: drop the satellite fallback for the path-specific (own-node) case. With no matching update, `change` stays null and we emit the own node's CURRENT full value (already filtered to reference.Path) — always the def, never a satellite. This completes the fix the own-path stamp intended. Pairs with the InstallReleaseRequestWatcher eager->commit high-water refinement and the compile-completion backstop. Verified against the 80x CI repro loop (was catching at iter 4-26; now green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Closing: the flake this chased was root-caused and eliminated by #173 (single compile driver + complete terminal write). The watcher refinement here predates that rewrite and is unproven against it. Residuals (eager high-water advance, the standalone flake-repro harness, the timeout diagnostic) are tracked in #185 — the branch stays for cherry-picking. |
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
1 participant
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.
Driving the
CodeEditRecompileTest.PressingCompileButton_SetsRequestedReleaseAt_AndProducesNewReleaseCI flake (theWaitForLatestRelease50s timeout). Three deliverables — plus an honest scoping of the residual.1. Reusable 2-vCPU repro harness —
.github/workflows/flake-repro.ymlworkflow_dispatchjob that loops one suspect test on the realubuntu-latest2-vCPU runner until it fails (mirrorsdotnet-test.yml's build), uploads the failing trx +MonolithMeshTestBasetraces + blame hang-dump. Reliably reproduces the flake (caught at iter 4 / 19 / 80 across runs) — the first time this CI-only flake is reproducible on demand. Inputs target any project/test.🚨 Heisenbug warning baked into the file: cranking the
CompileWatcherlog categories to Debug masks the flake (runs went green at 40/80 iters). Keep production timing; rely on the timing-neutral stuck-state diagnostic below.2. Stuck-state diagnostic —
CodeEditRecompileTest.WaitForLatestReleaseOn the 50s timeout, dumps the node's
Status/LatestReleasePath/RequestedReleaseAt/LastReleaseRequestHandledAt. Timing-neutral (fires only on timeout). Pinned the stuck state:Status=Ok, RequestedReleaseAt set, LastReleaseRequestHandledAt=null, Latest==knownRelease— read via the test's cross-hub mirror.3. Watcher correctness refinement —
InstallReleaseRequestWatcherAdvance
dispatchHighWater(+ un-park + log) only on the Update commit path (when actually flippingPending+ stampingHandled), never eagerly before the Update. The eager advance was a real lost-trigger bug: if status flapped toPending/Compilingbetween theWhereand the lambda, the lambda bailed without handling, yet high-water was already advanced past the unhandled trigger → the settled re-emission failedreq > dispatchHighWater→ trigger lost. Correct + low-risk, but with one post-fix repro data point it is not proven to materially move the flake — landing it as a correctness improvement, not the root fix.Residual (honest scope)
The dominant root is a known cross-hub-mirror DELIVERY / missed-emission race (the test's mirror freezes at the post-click state and never receives the owner's subsequent emissions) — documented in depth in our prior RCA (
compile-flake-syncstream-staleframe-rca): a same-node content-clock guard is neutral, and it has resisted ~5 prior deep attempts (~2.5M tokens) as a core sync-stream issue. Per that RCA, this is not release-pressure grindable and needs a deliberate core-sync delivery-handshake fix — not a guessed change. This PR makes it reproducible and adds the diagnostic so that work has a foothold.🤖 Generated with Claude Code