[senamakel]

Main CI on the v0.3.6 merge failed once in coverage: a_module_whose_setup_faults_after_load_fails_the_load_with_its_reason got a 'bus name already owned' attach error. The test faulted the module from inside init, racing admission. It now waits until the host lists the module before faulting.

[tinysweeper]

Tiny Sweeper review

Reviewed head: 699c7ba08c003fdd1059dede5025d86cf2aad7ea

Findings

No active actionable findings.

Advisory notes

• medium: Replace the 5 ms sleep poll with an awaitable admission signal ((pull request description))
  Problem: The repository rule says:
  Suggested fix:
  No concrete fix was supplied.

Verdict: Ready for maintainer review · Priority: medium
Coverage: Reviewed 1 of 1 files.

Diagnostics and supporting details

Full summary

Pull request purpose (title): test(module): wait for attachment before faulting in setup-fault test. Tiny Sweeper completed its review; deterministic results follow.

Review snapshot

Change surface	Files	Review signal	Count
Production	0	Active findings	0
Tests	1	Noted findings	1
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

The test now waits for the module to appear in the host's listing (matched by `bus_name` against the `ReleaseRecord`) before calling `fault()` on the spawned setup task, so the injected fault occurs during setup after admission rather than racing admission. The wait is bounded at 2000 iterations of 5 ms and panics with "module never finished attaching" if the module never appears. Behavioural assertions on the resulting error reason and `Failed` state are unchanged.

Features

• Added — wait_until_attached bounded polling helper: Polls the shared module host's listing every 5 ms for up to 2000 iterations, returning once a module with matching `bus_name` is registered and panicking otherwise, so callers cannot silently proceed without the module attached. (crates/tinybus/src/module/resolve_tests.rs mod resolver {, crates/tinybus/src/module/resolve_tests.rs)
• Modified — Setup-fault test ordering: await attachment before faulting: `a_module_whose_setup_faults_after_load_fails_the_load_with_its_reason` now waits for host registration after `FAULTING_ENTERED` is notified and before `fault()`, eliminating the admission race that produced 'bus name already owned' attach errors in CI. (crates/tinybus/src/module/resolve_tests.rs mod resolver {, crates/tinybus/src/module/resolve_tests.rs)

Tests

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

Before merge

None.

Agent review details

critique

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: Critique lane (Answered) found no defect, noting the helper is a bounded poll only used on a path where the spawned load task keeps running until the fault arrives.
• Lane summary: The diff adds a `wait_until_attached` helper that polls the shared module host until the slow-setup module appears in the registry, closing a real race where `fault()` could land before admission. The helper is a plain poll loop bounded at 10 seconds, consistent with the repository's rules (the `sleep` here is bounded polling, and `Timeout`/`Tokio` are already imported/qualified in this file), and it is only used on a path where the spawned load task keeps running until the fault arrives. I found no defect in this change. _The code index is behind this pull request (indexed at `48ca032491de`), so retrieved context may be out of date._

security

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: Security lane (Answered) recorded no findings, characterizing the change as a test-harness ordering change with no production-path impact.
• Lane summary: The plumbing of the question I intended to settle (whether the helper's deadline and the faulting test's flow are sound) is unresolved without seeing the surrounding test bodies; I already have the plainest evidence in hand, so I record no findings: the added helper only reads the host's listing in a bounded polling loop, and the fault injection happens after attachment is observed, which is a test-harness ordering change rather than a defect on a production path. _The code index is behind this pull request (indexed at `48ca032491de`), so retrieved context may be out of date._

tests

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Lane summary: The change adds a bounded polling helper `wait_until_attached` to a test in `resolve_tests.rs`, used by `a_module_whose_setup_faults_after_load_fails_the_load_with_its_reason` to avoid faulting before the host registers the module. The test still asserts the fault reason is carried through the load error and that state becomes `Failed`, so the existing behavioural assertions are unchanged; the new helper only narrows a race. I examined the helper's callers and the `ReleaseRecord.bus_name` field it matches on: it fails loudly via `panic!` after the bounded poll, so it cannot silently pass. Two things worth noting, neither a defect introduced here: the helper uses `tokio::time::sleep` in a polling loop, which the repository's rule "No `sleep` as synchronisation; await the thing, using `tokio::time::timeout` only as a deadline" would ordinarily forbid — but the helper is added by this diff and the rule sits on the fence of the "no test file changed" classification, and the sleep is a bounded poll with an explicit failure rather than an un-anchored synchronisation primitive. The lookup into `ReleaseRecord` confirms `bus_name` is `&'static str` and directly comparable, so the match in the helper is sound. I found no concrete input that misbehaves and no invariant stated in the diff without a test pinning it, so I have no findings. _The code index is behind this pull request (indexed at `48ca032491de`), so retrieved context may be out of date._

commits

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

description

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: Description lane (Answered) confirmed the description matches the diff: the test awaits the module appearing in the host list before signalling the fault.
• Lane summary: The description matches the diff: the setup-fault test now awaits the module appearing in the host list before signalling the fault, exactly as the body says. One problem remains in the new helper: the wait is implemented as a fixed 5 ms sleep-poll loop, which the repository's own rules forbid. _The code index is behind this pull request (indexed at `48ca032491de`), so retrieved context may be out of date._

Evidence and run details

• Models: ladder/vectors-oai3, deepseek-v4.1-flash
• Spend: $0.001866
• Tokens: 130476 input · 7734 output · 18176 cached · 79 embedding
• Updated: 2026-10-26T06:02:45Z

Head	State	Pass summary
699c7ba08c00	ready for maintainer review	0 active finding(s), 0 resolved finding(s) (at 2026-10-26T06:02:45Z)

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 1 billable file and costs up to $0.25.}

Or wait 9 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

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: 96cc707c-0ff7-4dbb-9f03-fa8fef0e72ae

Commits

Reviewing files that changed from the base of the PR and between 6ce06bc and 699c7ba.

Files selected for processing: 1

• crates/tinybus/src/module/resolve_tests.rs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{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-10-26T06:02:57.547200Z	699c7ba	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.

[tinysweeper]

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0019

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 699c7ba08c

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Replace sleep-based attachment polling with an explicit notification

This helper synchronizes the setup-fault test by repeatedly sleeping until attachment becomes visible, contrary to the explicit AGENTS.md requirement that tests never use sleep as synchronization. It also introduces a scheduler-dependent retry budget into the test intended to eliminate a CI race. Await an explicit attachment notification instead, with tokio::time::timeout providing the failure deadline.

AGENTS.md reference: AGENTS.md:L64-L66

Useful? React with 👍 / 👎.