test(runner): add start-loop test observer

[seven332]

Summary

• replace the single-purpose runner start-loop test probe with a typed test-only observer that stores event history
• record budget-exhausted and idle-cleanup lifecycle events through the observer
• migrate cleanup_tick_evicts_expired_entries away from fixed sleep gating to observer-based synchronization

Tests

• cargo test --manifest-path crates/Cargo.toml -p runner start::tests::budget::cleanup_tick_evicts_expired_entries
• cargo test --manifest-path crates/Cargo.toml -p runner start::tests::budget
• cargo test --manifest-path crates/Cargo.toml -p runner start::tests::main_loop
• cargo test --manifest-path crates/Cargo.toml -p runner
• cd crates && cargo fmt --all --check
• cargo clippy --manifest-path crates/Cargo.toml -p runner --all-targets --all-features -- -D warnings
• lefthook pre-commit: cargo-fmt, cargo-clippy, cargo-doc

Closes #12206

[codecov]

Codecov Report

❌ Patch coverage is 96.22642% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
crates/runner/src/cmd/start/mod.rs	96.22%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[seven332]

Code Review: PR #12250

Summary

Reviewed the single test-only runner commit against docs/testing.md and docs/bad-smell.md. The change is well-scoped: production behavior remains unchanged, the observer is behind #[cfg(test)], and the migrated cleanup test still asserts externally meaningful budget/idle-pool behavior.

Key Findings

Critical Issues (P0)

• None.

High Priority (P1)

• None.

Low Priority (P3)

• crates/runner/src/cmd/start/mod.rs:553 — StartLoopTestObserver::wait_for scans full event history and returns the first matching event. This is fine for the current one-shot wait, but repeated same-kind waits can match stale historical events unless future callers use unique payload predicates or a cursor/baseline API. Not a blocker for this PR.

Testing Review

Coverage

• test-only change: runner start-loop observer and cleanup tick synchronization -> covered by cleanup_tick_evicts_expired_entries, start::tests::budget, start::tests::main_loop, and cargo test -p runner per PR validation.
• no production feature/fix behavior changed, so no additional regression test requirement.

Convention Compliance

• No new mocks.
• No direct fetch/db/fs mocking.
• No JS fake timers.
• The removed active sleep is replaced by observer synchronization; tokio::time::advance remains appropriate for the existing start_paused interval test.
• Behavior assertions remain on budget release and idle-pool state, not just hook calls.

Testing Verdict: Adequate

Bad Smell Analysis

• Mock violations: 0
• Test coverage issues: 0
• Defensive programming: 0
• Dynamic imports: 0
• Type safety issues: 0
• Lint suppressions: 0
• Artificial active sleep synchronization: 0
• Test-only hook boundary violations: 0

Recommendations

• No blocking changes required.
• If the observer grows to support repeated lifecycle waits, add a cursor/baseline wait API before relying on repeated same-kind event predicates.

Verdict

LGTM

---

Full review details: codereviews/20260509/
Testing standards: docs/testing.md