Skip to content

Deflake CI: shared-timeout sizing, plus a runner-handshake diagnosis - #2019

Merged
Aaronontheweb merged 3 commits into
devfrom
fix/ci-deflake-omnibus
Aug 19, 2026
Merged

Deflake CI: shared-timeout sizing, plus a runner-handshake diagnosis#2019
Aaronontheweb merged 3 commits into
devfrom
fix/ci-deflake-omnibus

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Consolidates the remaining CI deflake items into one PR, in the shape of #2001.

Doctrine

Every raise here is environment sizing, not a correctness change. A short
budget on a multi-hop actor operation measures scheduler load on a starved
CI runner. It does not measure the property under test. Production code is
untouched, and a green test pays no added time — the raise only slows
failure reporting.

Ask literals stay explicit. Ask falls back to akka.actor.ask-timeout,
which is infinite by default. Deletion of an Ask timeout would trade a
flaky wait for a hang.

1. SubAgent Ask literal inconsistency

SubAgentActorTests.Approve_once_does_not_leak_between_subagent_tool_calls
used a 5-second literal on its outer Ask. Every sibling approval test in
the file uses the class constant ApprovalAskTimeout, which is 30 seconds.

This test drives two sequential approval round-trips. It makes more hops
than any sibling, so it tolerates a 5-second budget least. It flaked on
Windows CI. The literal now uses the class constant.

2. ErrorCorrelation leftovers

ErrorCorrelationTests held four 5-second literals on
ExpectMsgAsync<ErrorOutput>. Each waits for a full provider-failure turn:
actor spawn, Akka.Persistence recovery, a failed stream, and the error
classification.

This commit adds the same Config-property raise that #2014 applies to this
class, then drops the four literals so they inherit it.
LlmSessionTestBase seals ConfigureAkka, so the Config property is the
only seam. The comment text matches #2014 word for word.

Merge note for #2014: the Config property is byte-identical on both
branches, so it merges clean. Two hunks in
Each_error_turn_gets_a_distinct_CorrelationId will conflict, because this
PR edits the ErrorOutput line and #2014 edits the TurnCompleted line
directly under it. No line separates them, so git cannot merge the pair.
The resolution is the union: keep both deletions. This conflict is
unavoidable in either order of merge.

3. Shared-infrastructure raises

Two shared helpers held 5-second budgets. A short budget on a shared helper
makes a whole group of tests flake at once.

  • ShellApprovalHarness stops the approval actor twice — once to prove a
    persistent grant survives a restart, once on dispose. Both stops wait for
    a persistence flush and an actor teardown. Every shell-approval test runs
    through this harness. Both budgets move to 15 seconds.
  • ApprovalRehydrationTests resolves the session child at three sites, one
    of them the shared ColdRespawnAsync helper. The resolve waits for the
    actor spawn and the Akka.Persistence recovery. About twenty cold-respawn
    tests depend on it. All three budgets move to 15 seconds.

Each site carries a comment that names what the budget bounds.

4. Demo.AppHost xUnit v3 handshake corruption — diagnosed, not fixed

Incident:

Netclaw.Demo.AppHost.IntegrationTests: Catastrophic failure:
System.InvalidOperationException: Test process did not return valid JSON (non-object)

The mechanism is now proven from the runner code, and it is not ours:

  1. The VSTest adapter probes the test executable with a single argument,
    -assemblyInfo
    (TestProcessLauncherAdapter.GetAssemblyInfo, xunit.runner.visualstudio
    3.1.5). It then reads all of stdout and parses it as one JSON object.
  2. ConsoleRunner.PrintAssemblyInfo writes that one JSON object and returns.
  3. ConsoleRunner.Run (xunit.v3.runner.inproc.console 3.2.2) queues a
    watchdog. One second after the entry point returns, if the process still
    lives, it writes to stdout:
    Console.WriteLine("Waiting 10 seconds for foreground threads to exit...").
    The plain-text branch runs because -assemblyInfo is not automated mode;
    the automated branch would emit a JSON diagnostic instead.
  4. The adapter reads the JSON object plus that line, and the parse fails
    with non-object.

No Netclaw code writes to stdout on this path. The only Console.WriteLine
in the suite sits inside the opt-in test body, which is skipped without
NETCLAW_RUN_DEMO_SMOKE=1. The assembly has one module initializer, and it
only sets an environment variable. A local -assemblyInfo probe on Linux
returns clean JSON and exits in 72 ms. The trigger is process-exit latency
above one second on the Windows runner.

No fix is applied, because the minimal fix is not in our code. Two options
for a follow-up, both maintainer calls:

  • Exclude samples/Netclaw.Demo.AppHost.IntegrationTests from the
    pr_validation sweep. Every test in it is opt-in and skips without Docker
    and NETCLAW_RUN_DEMO_SMOKE=1, so the sweep buys no coverage and pays a
    fragile process probe. demo_smoke.yml already runs the suite on its own.
  • Report the watchdog race upstream to xunit. The -assemblyInfo probe
    should not write plain text to a stdout channel it defines as JSON.

Related but separate: skunkworks/json-stdout-hygiene covers the Netclaw
CLI --json envelope, not the xUnit runner handshake. It does not apply
here.

Gates

  • dotnet build --nologo -v q — 0 warnings, 0 errors
  • SubAgentActorTests — 66 passed
  • ErrorCorrelationTests — 3 passed
  • ApprovalRehydrationTests — 20 passed
  • ShellPolicyEvidenceFixtureTests + ShellApprovalDispositionMatrixTests — 332 passed
  • dotnet slopwatch analyze — 0 issues
  • ./scripts/Add-FileHeaders.ps1 -Verify — all files have headers

SubAgentActorTests.Approve_once_does_not_leak_between_subagent_tool_calls
used a 5-second literal on its outer Ask. Every sibling approval test in
the file uses the class constant ApprovalAskTimeout, which is 30 seconds.

This test drives two sequential approval round-trips. It makes more hops
than any sibling, so it is the least able to tolerate a 5-second budget.
A starved Windows CI runner made it flake.

The literal now uses the class constant. The Ask keeps an explicit
timeout, because Ask falls back to akka.actor.ask-timeout, which is
infinite by default. Deletion would trade a flaky wait for a hang.
ErrorCorrelationTests kept four 5-second literals on
ExpectMsgAsync<ErrorOutput>. Each one waits for a full provider-failure
turn: actor spawn, Akka.Persistence recovery, a failed stream, and the
error classification. A starved CI runner can exceed 5 seconds on that
path. The literal measures scheduler load, not correctness.

This commit adds the same Config-property raise that PR #2014 applies to
this class, and drops the four literals so they inherit it.
LlmSessionTestBase seals ConfigureAkka, so the Config property is the
only seam. The comment text matches #2014 word for word, so the two
changes agree where they overlap.

The Ask literals in this file stay explicit. Ask falls back to
akka.actor.ask-timeout, which is infinite by default. Deletion would
trade a flaky wait for a hang.

Production code is untouched. A green test pays no added time. The raise
only slows failure reporting.
Two shared test helpers held 5-second budgets. Each one bounds a
multi-hop actor operation, not a correctness property. A short budget on
a shared helper makes a whole group of tests flake at once.

ShellApprovalHarness stops the approval actor twice: once to prove a
persistent grant survives a restart, and once on dispose. Both stops wait
for a persistence flush and an actor teardown. Every shell-approval test
runs through this harness.

ApprovalRehydrationTests resolves the session child at three sites, one
of them the shared ColdRespawnAsync helper. The resolve waits for the
actor spawn and the Akka.Persistence recovery. About twenty cold-respawn
tests depend on it.

Each budget moves to 15 seconds and carries a comment that states what it
bounds. Production code is untouched. A green test pays no added time.
The raise only slows failure reporting.
@Aaronontheweb Aaronontheweb changed the title Deflake CI: shared-timeout sizing and runner-handshake hygiene Deflake CI: shared-timeout sizing, plus a runner-handshake diagnosis Aug 19, 2026
@Aaronontheweb Aaronontheweb added tests All issues related to testing, quality assurance, and smoke testing. reliability Retries, resilience, graceful degradation labels Aug 19, 2026
@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) August 19, 2026 20:10
@Aaronontheweb
Aaronontheweb disabled auto-merge August 19, 2026 20:54
@Aaronontheweb
Aaronontheweb merged commit 5a86764 into dev Aug 19, 2026
23 checks passed
@Aaronontheweb
Aaronontheweb deleted the fix/ci-deflake-omnibus branch August 19, 2026 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reliability Retries, resilience, graceful degradation tests All issues related to testing, quality assurance, and smoke testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant