test: isolate blocking console render workers

[thomhurst]

Problem and change

Linux CI on #5160 failed while waiting for a dependent module in RunSchedulerAsync_QueuesDependentWhenDependencyCompletes, matching the starvation symptoms tracked in #5106. Console output tests had eight Task.Run workers that deliberately blocked on synchronous render gates, occupying workers needed by their own cleanup continuations and concurrent scheduler tests.

Run those blocking workers on dedicated threads using TaskCreationOptions.LongRunning; unwrap asynchronous flush tasks so failures remain observable. Move acquisition waits inside existing try/finally cleanup blocks so a failed wait still releases its worker. Scheduler implementation, assertions, and test timeouts remain unchanged.

Validation

• Before: Flush_CancellationInterruptsRenderGateWait alone stalled with the portable thread pool limited to one worker. The 45-second agent guard terminated it with zero tests completed.
• After: 12 affected console/scheduler tests passed under the same one-worker limit and 45-second guard in 1.285 seconds.
• 81 related console, scheduler, state-tracker, and exit-condition tests passed with normal thread-pool settings.
• Core test project built successfully; scoped formatting passed through the pinned SDK guard.

Related to #5106 and #5160. The controlled reproduction establishes a starvation source; subsequent full Linux CI must confirm that the observed scheduler timeout is resolved before closing #5106.

Summary by CodeRabbit

• Tests
  • Improved concurrency test reliability by running blocking test workers on dedicated threads.
  • Reorganized asynchronous test setup and cancellation handling to reduce timing-related failures.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-16T03:28:55.319990Z	6edcf12	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 75ebb9f0-a4c6-4b03-8a68-fbc65c3294b1

📥 Commits

Reviewing files that changed from the base of the PR and between 67555c6 and 6edcf12.

📒 Files selected for processing (1)

• test/ModularPipelines.UnitTests/Console/ModuleOutputBufferTests.cs

Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

Concurrency-sensitive console tests now run blocking workers on dedicated long-running threads. Tests create workers before awaiting synchronization signals, and selected cleanup scopes now include those awaits.

Changes

Console concurrency test scheduling

Layer / File(s)	Summary
Dedicated blocking worker helper test/ModularPipelines.UnitTests/Console/ModuleOutputBufferTests.cs	Adds RunBlockingWorker overloads for Action and Func<Task> workers. Both use long-running tasks on TaskScheduler.Default.
Concurrency test orchestration test/ModularPipelines.UnitTests/Console/ModuleOutputBufferTests.cs	Updates incremental flush, cancellation, timeout, filtering, and concurrent logging tests to use the helper. Worker creation now occurs before lock-signal waits. Selected try blocks now include the waits and worker operations.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 6edcf

The test-only scheduling changes have no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: isolating blocking console render workers in tests.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-5106-scheduler-ci-repair

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit starts a worker thread
On a path where pool threads rest
Locks signal, tests proceed
Cleanup wraps each waiting deed
Console flows now hop with speed}

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Code review

Reviewed the diff in test/ModularPipelines.UnitTests/Console/ModuleOutputBufferTests.cs (all 8 blocking Task.Run worker call sites plus the two new RunBlockingWorker helper overloads).

Summary: This is a targeted, well-reasoned test-reliability fix with two independent improvements, both correctly applied consistently across all affected tests:

1. Dedicated threads for blocking workers. Replacing Task.Run(...) with Task.Factory.StartNew(..., TaskCreationOptions.LongRunning, TaskScheduler.Default) (via the new RunBlockingWorker overloads) moves synchronously-blocking render-gate holders off the shared thread pool. This directly addresses the starvation scenario described in the PR (a worker occupying a pool thread while other continuations/tests compete for the same limited pool). The Func<Task> overload correctly .Unwrap()s so exceptions from the wrapped async delegate still propagate to awaiters.
2. Moving WaitAsync(...) calls inside the try blocks. This is the more important correctness fix: previously, if the initial wait (e.g. lockAcquired.Task.WaitAsync(...)) timed out, the exception would propagate before the try/finally was entered, so releaseLock.TrySetResult()/releaseRendering.TrySetResult() would never run. That left the background worker permanently blocked on the lock/gate — leaking a blocked thread indefinitely and likely contributing to the very starvation this PR is fixing. Moving the wait inside try ensures cleanup always runs, even on timeout.

Checked for consistency: all 8 Task.Run occurrences were converted (matches the "eight Task.Run workers" in the PR description), and grepping the PR head confirms no leftover blocking Task.Run calls in this file. The added comment on RunBlockingWorker explains the non-obvious rationale (why dedicated threads) rather than restating the code, consistent with repo conventions.

This is test-only code (no production/library changes), scoped correctly to the core UnitTests project, and doesn't touch generated code or CLAUDE.md-governed areas (options generator, CLI packages, PublicAPI baselines). No CLAUDE.md violations found.

No outstanding findings from CodeRabbit/Codex bots at review time (both were still running with no posted issues), so nothing to cross-check as addressed.

No issues found. Nice fix — the try/finally reordering in particular closes a real resource-leak window that could itself have been feeding the flakiness under investigation.

[thomhurst]

CodeRabbit reports no actionable findings. Its docstring-coverage warning counts existing regression methods and the two private worker helpers. The helpers include the non-obvious threading rationale, and named tests follow the existing test-project convention without repetitive XML summaries. No public API documentation changes are needed.