test: avoid a forced thread-pool hop in cache concurrency checks

[thomhurst]

Problem and change

The Linux pipeline on #5170 timed out two repetitions of Cache_Lookups_For_Ready_Modules_Run_Concurrently. Its trace shows both lookups started and the release signal was sent within 0.5 seconds, but the first lookup never resumed before the five-second deadline. The same post-release stall is recorded in #5101.

Allow the test's release gate to resume its held lookup inline, removing the unnecessary forced thread-pool hop. Assert that execution remains incomplete after both lookup starts have been observed. Keep all 50 repetitions, the five-second timeout, and cancellation cleanup. This preserves the requirement that independent ready-module cache lookups overlap; it makes no production scheduling change.

Validation

• Baseline and changed distributed suite: 254 tests passed with coverage in each run.
• Release core solution: zero warnings/errors.
• Scoped formatting at severity info passed.
• Linux CI and reviews must pass before merge; fix(generator): avoid masking non-secret CLI metadata #5170 also validates this commit with the generator changes.

Fixes #5101.

Failure evidence: https://github.com/thomhurst/ModularPipelines/actions/runs/35070619377/job/104712855730

Summary by CodeRabbit

• Tests
  • Updated cache concurrency coverage to verify that execution remains incomplete while the initial cache lookup is held.
  • Simplified test synchronization setup for more reliable validation.

[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-16T08:18:47.121920Z	45e5185	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: 5c847df1-7dd6-42ad-90fa-9eb737067e12

📥 Commits

Reviewing files that changed from the base of the PR and between 7be4a6e and 45e5185.

📒 Files selected for processing (1)

• test/ModularPipelines.Distributed.UnitTests/Master/DistributedModuleExecutorTests.cs

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

---

📝 Walkthrough

Walkthrough

The cache concurrency test now uses inline continuation for the held lookup. It also verifies that execution remains incomplete before the first lookup is released.

Changes

Distributed cache concurrency test

Layer / File(s)	Summary
Cache lookup synchronization test/ModularPipelines.Distributed.UnitTests/Master/DistributedModuleExecutorTests.cs	The test uses the default TaskCompletionSource configuration and checks that execution has not completed while the first cache lookup remains held.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 45e51

The test synchronization change preserves the concurrency and cleanup checks, so the PR is mergeable.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check	❓ Inconclusive	The change addresses #5101 by removing the forced thread-pool hop from Cache_Lookups_For_Ready_Modules_Run_Concurrently. It adds an assertion that execution remains incomplete while the first lookup…	Provide evidence from the focused distributed test run and the Linux CI pipeline that includes Cache_Lookups_For_Ready_Modules_Run_Concurrently and confirms successful completion.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The reported change is limited to the distributed cache concurrency test. It removes the synchronization operation identified as the timeout cause and strengthens the existing overlap test. No product…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: removing a forced thread-pool hop from cache concurrency checks.

Full details: Linked Issues check

Explanation

The change addresses #5101 by removing the forced thread-pool hop from Cache_Lookups_For_Ready_Modules_Run_Concurrently. It adds an assertion that execution remains incomplete while the first lookup is held. The summary states that cancellation cleanup, 50 repetitions, and the five-second timeout remain unchanged. The summary also reports 254 distributed tests passed. However, the supplied evidence does not confirm a successful Linux CI pipeline, and the repository inspection was unavailable. Therefore, full compliance with the issue acceptance criteria cannot be established.

• 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-5101-cache-continuation

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.

---

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 holds the cache gate tight
Inline hops keep tasks in sight
The first lookup waits in place
The second runs its parallel race
Before release, completion stays away
Then both return to finish the day}

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

[thomhurst]

Linux validation for the cache-release fix now passes on #5170 at 716cec4: https://github.com/thomhurst/ModularPipelines/actions/runs/35072807013/job/104719889014. All 254 distributed tests passed with coverage in 19.481 seconds, including all 50 cache concurrency repetitions; the entire Linux pipeline passed. This contrasts with two post-release timeouts on the prior head under the same workflow. #5171's own Linux/macOS checks are still running.

CodeRabbit review disposition: the named test and adjacent two-line synchronization comment document this existing test change; no library API or documentation warning was introduced. Its repository-clone limitation remains external to this diff (local checkout, build, and tests succeeded). Codex completed review of 45e5185 with no findings. Claude's workflow completed without publishing a review; all inline/review surfaces have no findings.

[thomhurst]

Merged as 6c1341e after all 17 checks passed on 45e5185. This branch's Linux pipeline passed all 254 distributed tests with coverage in 26.743 seconds: https://github.com/thomhurst/ModularPipelines/actions/runs/35072756973/job/104718960749. The same test fix also passed all 254 tests and the complete Linux pipeline in #5170. No unresolved review threads or findings remain. All branch workflow runs are complete.