[Aschenbath]

What a user gets

Compaction receipt tests now exercise their modeled transitions without a scheduler pause consuming the fixture's 20 ms idle budget. Previously, the enforcer used a stepped clock while the real SessionTransport used wall-clock deadlines and timers. An idle first submission could therefore become not_sent before a cancellation assertion expected submitted_unconfirmed.

The fixture now shares its clock with Date.now and transport sleeps. Receipt helpers restore only their send spy, keeping that clock active through the rest of each test; afterEach restores all mocks. The test guide documents the clock boundary. Production code, the 20 ms budget and behavioral assertions are unchanged.

Found while investigating the first CI attempt on #2153. That PR's unchanged compaction test passed in isolation and on an upstream rerun. This change addresses the reproducible fixture sensitivity; it does not claim to establish what delayed the original CI observation.

How you verified it

Source: f3d6b3addc21356c72b26645210e69d62783d10d, based on a5e42d3d6b5d954e3c06304de6fd6d7278fc5f03.

• Injected a bounded 35 ms performance.now() stall into the readiness stub, without advancing the modeled clock. Both existing cancellation cases failed on the original fixture at the submitted-receipt assertion, receiving not_sent. The same injection passes both cases on the final fixture. The injection was removed after each run and source restoration was verified; no busy-wait is included in the PR.
• Seven related daemon files: 463 passed, including all 45 command-submission tests, compaction preparation/restart/drain/draft controls, and session-transport.test.ts's modeled and real deadline cases.
• npm ci, npm run build, npm run lint and npm run test:repo passed; repository tests were 327 passed / 1 skipped, with first-run references, docs, skill mirrors and generated context packs also checked. After narrowing the three send-spy restorations, the seven-file suite and lint were repeated successfully. Build and repository-check sources were unchanged by that final cleanup.
• git diff --check passed. All 10 CI jobs passed on this head, including every package suite, repository checks, installed scenario and portability. The PR has been approved and merged.

Anything you were unsure about

The fault injection is a synthetic event-loop stall, not evidence of a particular GC or scheduler event in CI. The fixture uses the real enforcer and transport over a recording tmux adapter; no tmux server or native Claude process is needed. The transport suite retains separate real-timer coverage for late or unresolved observations.

• One concern per PR; no version bump; no CHANGELOG.md edit
• Tests added or updated where the change is testable
• I listed the checks I ran, their results, and any checks I could not run

Summary by CodeRabbit

• Tests
  • Improved consistency of timing in automated checks, reducing sensitivity to real-time scheduling delays.
• Documentation
  • Added guidance on how test fixtures handle mixed clock sources and restore clock behavior after each test. These changes affect testing and documentation only; they do not change production timeout behavior.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: fd297de0-4ecd-4021-9cc5-207db8622bf5

Commits

Reviewing files that changed from the base of the PR and between 80616b7 and f3d6b3a.

Files selected for processing: 2

• docs/as-built/test-layers.md
• packages/daemon/test/compaction-command-submission.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

Walkthrough

The command-submission test fixture now uses one clock for Date.now and transport sleeps. Tests and a helper restore their send spies directly. The test-layer documentation describes the mixed-clock gotcha and its effect on the fixture’s idle deadline.

Changes: Compaction test timing

Layer / File(s)	Summary
Shared clock and spy restoration packages/daemon/test/compaction-command-submission.test.ts, docs/as-built/test-layers.md	The fixture uses its clock for Date.now and advances it during transport sleeps. Tests and a helper restore their send spies directly. The documentation describes the mixed-clock gotcha and notes that transport tests cover real deadline behavior.

Priority: Low

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

Change: Other

Possibly related PRs

• mvschwarz/openrig#2149: Uses a controllable fixture clock for Date.now to prevent simulated waits from consuming a real deadline.
• mvschwarz/openrig#1585: Uses one controllable clock for injected sleeps and transport and enforcer timing in a related compaction fixture.
• mvschwarz/openrig#1884: Changes the same command-submission test and its transport result.

Suggested reviewers: mvschwarz, openrig-review

Merge Risk: Minimal · up to f3d6b

The test-only clock alignment and mock cleanup introduce no concrete merge-blocking risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: sharing the compaction submission fixture clock across the relevant daemon tests.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 2 functions across 1 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

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

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

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

[mvschwarz]

Thanks again, @Aschenbath. As its description says, this makes the compaction submission tests share one clock between the fixture and the transport's sleeps and deadlines, so a scheduler pause can't use up the 20 ms idle budget and turn an expected submitted_unconfirmed into not_sent, with production code and the assertions unchanged. It's with the review desk, and we'll follow up here.

[openrig-review]

Approved at f3d6b3a. The compaction submission fixture now runs the enforcer, the injected clock, Date.now and the transport's sleeps on one clock, so a scheduler pause can no longer spend the idle budget while the modeled seat stays idle; the cancellation and receipt assertions are unchanged, mocks are still restored after each test, and real-timer deadline cases remain in the transport suite. Reviewed by the merge desk's Claude reader. Required CI passes at this head.

[mvschwarz]

Merged into main as be52fa0, and it isn't in a release yet. Thanks again, @Aschenbath. The compaction receipt tests now run on one clock, so a scheduler pause no longer fails them on CI, as it did on #2153's first CI run. It's a test-only change, and the test guide is updated to match.