test(logging): start blocking concurrency workers independently

[thomhurst]

Two logging concurrency tests failed in Linux CI while waiting for Task.Run workers to start. Start their blocking workers with TaskCreationOptions.LongRunning and TaskScheduler.Default, matching the dedicated second writer already used by the serialization test. The nested child still inherits ExecutionContext, so the test continues to exercise inherited secret-emission scopes.

The serialization, nested-emission, masking, and timeout assertions remain intact. This removes ThreadPool scheduling from worker startup; a fresh full Linux run is still required to confirm the original CI blocker is resolved.

Validation:

• Release build of ModularPipelines.Tests.slnf passed (0 errors; 185 existing warnings).
• All 87 SecretMaskingPatternTests passed.
• Scoped formatting and git diff --check passed.
• Full Windows core coverage run exceeded the existing 2 GB process-tree guard (2076 MB measured). Before that limit, a separate command-log ordering assertion failed; recorded in Command logging ordering test misses streamed output during coverage run #5130. The full run was not retried and limits were not raised. Full validation is deferred to CI.

Closes #5129

Refs #5100

[coderabbitai]

Warning

Review limit reached

Next included review available in 7 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 65be27f8-42e6-4b1c-951a-eb381214a7a5

📥 Commits

Reviewing files that changed from the base of the PR and between de11378 and 0f0f7af.

📒 Files selected for processing (1)

• test/ModularPipelines.UnitTests/Logging/SecretMaskingPatternTests.cs

---

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

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

[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-14T21:40:09.934085Z	0f0f7af	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.

[greptile-apps]

Greptile Summary

This PR makes two logging concurrency tests independent of ThreadPool worker availability by starting their blocking workers as long-running tasks on the default scheduler.

• Preserves serialized custom-obfuscator coverage across module buffers.
• Preserves inherited stable-secret scope and nested-emission coverage.
• Retains existing synchronization, masking, completion, and timeout assertions.

Confidence Score: 5/5

The PR appears safe to merge; the scheduling changes preserve the tests’ execution-context, synchronization, exception-observation, and cleanup semantics.

The changed delegates are synchronous, the nested worker continues to inherit the outer ambient secret-emission scope, and both tests release their blocking gates before awaiting worker completion. No actionable regression was identified.

Important Files Changed

Filename	Overview
test/ModularPipelines.UnitTests/Logging/SecretMaskingPatternTests.cs	Replaces ThreadPool-dependent test workers with dedicated long-running tasks while preserving execution-context flow and cleanup.

_{Reviews (1): Last reviewed commit: "test(logging): isolate blocking workers ..." | Re-trigger Greptile}

[github-actions]

Review

Change: In SecretMaskingPatternTests.cs, two blocking worker tasks (firstWrite in DifferentModuleBuffers_SerializeCustomObfuscatorCalls, and outerEmission/childEmission in StableSecretEmission_DoesNotWaitForInheritedEmissionBeforeNestedEmission) are switched from Task.Run(...) to Task.Factory.StartNew(..., CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default).

Correctness: This is a sound fix for the described flakiness. These workers block synchronously on ManualResetEventSlim gates immediately after starting, so if the ThreadPool is momentarily saturated (common on constrained Linux CI runners), Task.Run scheduling can delay worker startup past the tests' 5-second Wait timeouts. TaskCreationOptions.LongRunning with TaskScheduler.Default requests a dedicated thread instead of a pool thread, removing that dependency on ThreadPool queue depth. LongRunning doesn't affect ExecutionContext flow, so the nested-emission test's assertions about inherited secret scopes are unaffected — the change is scoped correctly and the diff itself is minimal and low-risk (test-only, no production code touched).

The pattern also matches the file's own precedent: secondWrite in the same DifferentModuleBuffers_SerializeCustomObfuscatorCalls test already used this exact Task.Factory.StartNew(..., LongRunning, TaskScheduler.Default) form, so the PR is bringing firstWrite in line with a convention that already existed one call below it.

Non-blocking suggestion: The same "block on a gate right after starting" shape appears in many other Task.Run(...) call sites throughout this file (e.g. around lines 1608, 1640, 1718, 1993, 2011, 2050/2054, 2112, 2128, 582/587). Since the root cause is ThreadPool-starvation-driven startup delay rather than something specific to these two tests, those call sites are exposed to the same class of CI flakiness and may surface as future one-off "fix this test too" PRs. Consider extracting a small test helper (e.g. StartBlockingWorker(Action) wrapping the Task.Factory.StartNew call) so blocking workers get the safe scheduling behavior by default, rather than requiring each test author to remember the pattern. This is a suggestion for follow-up, not a blocker for this PR, since it's correctly scoped to the two tests that were observed failing in CI per issue #5129.

No other issues found. The description's validation notes (Release build, all SecretMaskingPatternTests passing, deferred full-suite run due to the 2 GB guard) are consistent with the project's stated policy of deferring expensive validation to CI.

[thomhurst]

@coderabbitai review