Skip to content

test(module): wait for the setup future to be reattached before asserting it is retained - #111

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/terminal-test-wait-for-reattached-future
Oct 23, 2026
Merged

M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:fix/terminal-test-wait-for-reattached-future

Conversation

@M3gA-Mind

Copy link
Copy Markdown
Collaborator

Summary

Main CI went red on module-ci / coverage (run 39949099619, at 8dc8a33): terminal_runtime_tests::actual_linked_rejected_physical_spawn_retains_received_resources_without_retry panicked at lib_terminal_tests.rs:803, which asserts that initial_future is still in its slot. The same test failed the same way on 2026-10-22 (run 39785116193, feature-tests (ubuntu-latest)), so this is not specific to one change.

Cause: TerminalSetupPoll::poll takes the setup future out of its slot, polls it with no lock held, and puts it back, so no mutex is ever held across user code. The test read the slot directly, so a slow runner could sample it in the middle of a poll and find it empty. The future was still owned, by the poll that was running.

Fix (test only): wait, with a 5 s deadline, until the future is back in its slot before taking the resources lock and asserting retention. The physical-completion test above already waits for its anchor the same way (lib_terminal_tests.rs:667-678). If the future really were dropped, the wait times out and the test still fails.

Related issue

None.

API or behavior changes

None. Test only.

Validation

  • Red/green proof on our remote check host, using two probe branches on the fork that will not be merged. Each adds a 300 ms delay between the poll and putting the future back, so the race happens on every run:

    • probe/terminal-race-before: main + probe, expected to FAIL at line 803;
    • probe/terminal-race-after: this fix + probe, expected to PASS.

    The results will be posted here.

  • Full module CI on this head (GitHub, and our remote check host).

Tests

actual_linked_rejected_physical_spawn_retains_received_resources_without_retry now waits for the poll to put the future back instead of reading its slot directly. It still asserts that the factory, the runtime, the initial payload and the initial future are all retained.

Documentation

Not needed.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No sleep used as synchronisation: a deadline around an awaited condition
  • No secrets, tokens, or .env contents in the diff or the description

…ting it is retained

A setup poll takes the initial future out of its slot, polls it with no
lock held and puts it back. The rejected-spawn retention test sampled the
slot directly, so a slow runner could catch it mid-poll and see it empty.
Wait, bounded, for the future to be back in its slot, as the
physical-completion test already does for its anchor.
@coderabbitai

coderabbitai Bot commented Oct 23, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 148cfd25-9add-420f-b57e-9e0634fc4a01
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@tinysweeper

tinysweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: 1c622a3bed0735dbd467fb28b57317b93e0937c4

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

Diagnostics and supporting details

Full summary

Pull request purpose (title): test(module): wait for the setup future to be reattached before asserting it is retained. This test-only change addresses a flaky failure in `terminal_runtime_tests::actual_linked_rejected_physical_spawn_retains_received_resources_without_retry`, which panicked asserting that the setup future was still in its slot. Because `TerminalSetupPoll::poll` removes the future from the slot, polls it without holding the lock, and then reattaches it, the test could sample an empty slot mid-poll. The fix waits (bounded by a 5s deadline) for the future to be reattached before locking resources and asserting retention; the pre-existing retention assertions remain unchanged. 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: Test coverage is assessed from changed tests and lane evidence; execution is not claimed without trusted check data.

What changed

In `crates/tinybus-module/src/lib_terminal_tests.rs`, a bounded `tokio::time::timeout` wait loop (5s) now spins with `yield_now` until the cloned `initial_future` slot is no longer `None`, replacing a direct read that could race against an in-flight poll. After the wait, the existing assertions that `retained_factory`, `runtime`, the initial payload, and the initial future are retained remain in place. No production code, API, or behaviour changes.

Features

  • Modified — Test waits for setup future reattachment before asserting retention: Eliminates the sampling race where the initial-future slot could be observed empty while a poll held the future, de-flaking the test while preserving the retention assertions and failing (via 5s timeout) if the future were truly dropped. (crates/tinybus-module/src/lib_terminal_tests.rs async fn actual_linked_rejected_physical_spawn_retains_received_resources_withou, crates/tinybus-module/src/lib_terminal_tests.rs)

Tests

  • modification — `actual_linked_rejected_physical_spawn_retains_received_resources_without_retry` now clones the initial-future slot Arc, awaits up to 5s for the slot to be repopulated, then re-locks resources and asserts factory, runtime, and future retention as before.: Lanes confirm the existing assertions already pin reattachment and this is a de-flake rather than a behavioural change; no execution or passing status is claimed. (crates/tinybus-module/src/lib_terminal_tests.rs async fn actual_linked_rejected_physical_spawn_retains_received_resources_withou, crates/tinybus-module/src/lib_terminal_tests.rs)

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Critique lane answered with no defects: the loop resolves a real transient `None` sampling race, and releasing the resources lock before the wait avoids a new deadlock.
  • Lane summary: The change adds a bounded wait that re-checks the retained initial-future slot until it is reattached, then keeps the existing assertions. The slot is an `Arc<StdMutex<Option<SetupFuture>>>` and `TerminalSetupPoll::poll` puts the future back after polling, so a transient `None` sampling race is real and the loop resolves it; no new deadlock is introduced because `anchored.resources` is released before the wait and only re-locked afterwards. I found no defect introduced by this diff. _The code index is behind this pull request (indexed at `38ef2fd5e268`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Security lane answered: test-only bounded 5s wait, no production code touched, no security defect.
  • Lane summary: The diff only adds a polling wait in a test so it samples the future slot after the setup poll reattaches it, with a bounded 5s timeout as the repository rules require. It does not touch production code and introduces no security or behavioural defect. _The code index is behind this pull request (indexed at `38ef2fd5e268`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Tests lane answered: diff only adds a wait-for-reattach preamble in the existing test, adds no new assertions, and requires no new test.
  • Lane summary: The diff only adds a wait-for-reattach preamble inside an existing test (`lib_terminal_tests.rs`); it changes no production behaviour and adds no new assertion beyond the existing `assert!(resources.initial_future.lock().unwrap().is_some())` at line 814 that already pins the slot's reattachment. This is a de-flake of a test, not a behavioural change, so no new test is needed and I found no defect. _The code index is behind this pull request (indexed at `38ef2fd5e268`), so retrieved context may be out of date._

commits

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

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Description lane answered: the change matches the PR description (test-only, no API change, deadline around an awaited condition rather than sleep-as-synchronisation).
  • Lane summary: The change adds a bounded wait for the initial-future slot to be reattached before asserting retention in the terminal test, which matches the PR description exactly (test-only, no API change, no sleep used as synchronisation). The code clones the slot Arc before the wait and then re-locks for the assertions, which is consistent with the described race and its fix. _The code index is behind this pull request (indexed at `38ef2fd5e268`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.001520
  • Tokens: 127193 input · 3105 output · 11776 cached · 85 embedding
  • Updated: 2026-10-23T13:19:40Z
Head State Pass summary
1c622a3bed07 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-23T13:19:40Z)

tinysweeper 0.1.2

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0015

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 23, 2026
@M3gA-Mind
M3gA-Mind merged commit 809eef2 into tinyhumansai:main Oct 23, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant