[M3gA-Mind]

Summary

• Fixes the flaky thread_delete_tests::purging_threads_cancels_driver_children_and_discards_completions. It failed in roughly 1 of 4 runs on main, with left: Some(Completed), right: Some(Cancelled).
• Test-only change. The product's purge/cancel ordering is correct (see Problem).

Problem

• The test parks a spawned child in a gated model call, then started a task that released the gate 100 ms later, before calling threads_purge.
• threads_purge first closes thread admission (spawn_blocking), lists the threads and fences each one. Only then does running_subagents::cancel_all cancel the live driver job.
• Whenever that preamble took longer than 100 ms, which happens under coverage instrumentation and load, the released child completed on its own first. The job was already terminal when purge listed it, so it settled Completed, and the Cancelled assertion failed.
• Not a product race. In SubAgentJobRegistry, once cancel_owned sets cancellation_requested under the controls lock, a later Ok from the child still settles as Cancelled (mark_driver_result/settle). A child that genuinely finishes before purge cancels it is correctly Completed, and its completion is still discarded.

Solution

• Keep the child gated until purge returns, and release it afterwards. The harness races the in-flight model call against the job's cancellation token for both unary and streaming calls (tinyagents-harness agent_loop/model_call.rs). Purge's cancel therefore unwinds the child deterministically without a release.
• This also tightens the test: if cancellation ever stopped reaching an in-flight model call, purge would hit the test's 6 s timeout instead of passing because of timing.

Submission Checklist

• Tests added or updated: this PR fixes the flaky test itself.
• Diff coverage ≥ 80% — N/A: only a _tests.rs file changes.
• Coverage matrix updated — N/A: behaviour-only test fix.
• All affected feature IDs listed — N/A: no feature change.
• No new external network dependencies introduced.
• Manual smoke checklist — N/A: no release-cut surface.
• Linked issue — N/A: there is no filed issue for this flake.

Impact

• None at runtime; test-only.

Related

• Closes: N/A
• Follow-up PR(s)/TODOs: none.

Summary by CodeRabbit

• Tests
  • Updated a purge test to verify that the child task is canceled and settled before it is released.

[tinysweeper]

Tiny Sweeper review

Reviewed head: 0e9166e8e6e9325026b9c96b098d2ccf742e0983

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 1 of 1 files.
Pull request purpose (title): test(threads): fix f…

Diagnostics and supporting details

Full summary

Pull request purpose (title): test(threads): fix flaky purge cancellation test by releasing the gated child after purge. The prior 100 ms timer could fire before purge's admission/fencing preamble completed, letting the child settle `Completed` instead of the asserted `Cancelled`. No production behavior changes; the latest-push delta is unavailable.

Review snapshot

Change surface	Files	Review signal	Count
Production	0	Active findings	0
Tests	1	Noted findings	0
Documentation	0	Resolved findings	0
Configuration	0	Pending checks/questions	0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

A comment explains that the harness races the in-flight model call against the job's cancellation token, so purge's cancel unwinds the child deterministically. The diffstat is 1 file, +6/-11.

Features

None identified with supported citations.

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: Change aligns with the purge path ordering (cancel_all before completion wipe, job registry reaching Cancelled as asserted), removing the race between child self-completion and purge.
• Lane summary: The change removes the timer-driven release of the gated child model and instead relies on purge's cancellation unwinding the child, then notifies once at the end so the provider future settles. This matches the purge path (cancel_all runs before the completion wipe, and the job registry reaches Cancelled as asserted), so the test no longer races the child's self-completion against purge. No defects found in this file. _The code index is behind this pull request (indexed at `60110f0e2af9`), so retrieved context may be out of date._

security

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: Test-only change removing a flake; raises no security concern.
• Lane summary: Test-only change to a single Rust E2E test: the timer-based release of the gated child model call is removed and the release now happens after purge settles. The change removes a flake rather than introducing a defect, and it raises no security concern. _The code index is behind this pull request (indexed at `60110f0e2af9`), so retrieved context may be out of date._

tests

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: Removing the sleep-based release satisfies the repository rule against time-based flakes and makes the test strictly more deterministic; the now-redundant `notify_one()` is a harmless no-op.
• Lane summary: The diff only edits an existing E2E test: it drops the timer-driven release task and keeps the child gated with the `release` Notify until after purge, then notifies once and awaits nothing. That removes a `tokio::time::sleep`-based race, which is exactly what the repository rule against time-based flakes asks for, and makes the test strictly more deterministic. The `release.notify_one()` call is now redundant (a `Notify` wakes one waiter or stores one permit), but that is a harmless no-op, not a defect. No production behaviour changed, so no new test is owed. _The code index is behind this pull request (indexed at `60110f0e2af9`), so retrieved context may be out of date._

commits

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: No sensitive content in committed files.
• Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `60110f0e2af9`), so retrieved context may be out of date._

description

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: Description accurately reflects the diff: timer release removed, child gated until after `threads_purge`, gate released at test end.
• Lane summary: The description matches the diff: the 100 ms timer-based release task is removed, the child stays gated until after `threads_purge` returns, and the gate is released at the end of the test. Test-only change, no findings on the description. _The code index is behind this pull request (indexed at `60110f0e2af9`), so retrieved context may be out of date._

Evidence and run details

• Models: ladder/vectors-oai3, deepseek-v4.1-flash
• Spend: $0.000170
• Tokens: 106655 input · 2505 output · 17408 cached · 110 embedding
• Updated: 2026-10-26T14:05:04Z

Head	State	Pass summary
0e9166e8e6e9	ready for maintainer review	0 active finding(s), 0 resolved finding(s) (at 2026-10-26T14:05:04Z)

[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-10-26T14:06:56.561022Z	0e9166e	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]

Caution

Review failed

The pull request is closed.

Walkthrough

The purge test no longer releases the child on a timer while purge runs. It waits for purge and its assertions to complete before notifying the child’s release signal.

Changes: Purge cancellation test

Layer / File(s)	Summary
Control child release during purge crates/openhuman-core/src/agent/orchestration/tools/tools_e2e_thread_delete_tests.rs	The test removes the timed release and release-task join. It keeps the child gated through purge and its assertions, then notifies the release signal.

Priority: Not assessed

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

Change: Other

Possibly related PRs

• tinyhumansai/openhuman#8368: Changes thread-purge behavior and tests for asynchronous deletion and child cleanup.
• tinyhumansai/openhuman#8399: Changes purge admission handling on the path exercised by this test.

Merge Risk: Minimal · up to 0e916

The change is limited to test synchronization, with no confirmed merge-blocking risk.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit waits beside the gate
While purge completes its careful run
The child stays held until checks are done
Then one small signal says, “You’re free”
And off it hops beneath the moon}

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

[tinysweeper]

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0002