fix(stella-core): read every instant, wait and timeout through the Sleeper port - #6486
Merged
Conversation
…eeper port The engine read the monotonic clock itself in 19 places, hid nine more behind `.elapsed()`, and bounded four calls with `tokio::time::timeout`, which is the runtime's timer rather than the port. `Sleeper` gains `now() -> Instant`; every deadline is a reading from it, every elapsed time is the difference of two, and `retry::bounded` — the port's sleep racing a call — is the timeout. stella-core's tokio features are `sync` alone, and `make core-no-io`'s clock-read baseline is empty. The shared test doubles sleep on tokio's clock under `start_paused`, so a pending provider or tool is bounded the way it is in production and a backoff costs nothing while the runtime is idle. The lib suite went from 46 s to 2.5 s.
Contributor
There was a problem hiding this comment.
Sorry @macanderson, your pull request is larger than the review limit of 150,000 diff characters
…te timeout race rustdoc's private-intra-doc-links lint failed the doc step on the link from `Sleeper` to `bounded`; the header sentence claiming one clock read remained was stale as well.
macanderson
added a commit
that referenced
this pull request
Sep 10, 2026
… sleeper doubles #6486 routed every instant, wait and timeout in stella-core through retry::Sleeper and left the real sources copied: a wall clock in stella-cli, stella-runtime and stella-serve, a Tokio sleeper in two of them, a sleeper trait of stella-fleet's own, and the same two test doubles in about thirty files. stella-time now holds TokioSleeper, WallClock and MonotonicClock (which replaces the CLI's per-construction SystemClock), and ships PausedSleeper and NoopSleeper behind a test-util feature. The copies are deleted; stella-fleet's monitor takes the engine's Sleeper. stella-core's own unit tests keep one copy in src/tests.rs, because a lib's unit tests are a second build of the lib and a dev-dependency that links the lib implements the trait for the first. tests/one_home.rs reads the tree and fails on any other copy. ADR 0042 records the design #6486 merged and where the sources live. Closes #6484
macanderson
added a commit
that referenced
this pull request
Sep 10, 2026
… sleeper doubles #6486 routed every instant, wait and timeout in stella-core through retry::Sleeper and left the real sources copied: a wall clock in stella-cli, stella-runtime and stella-serve, a Tokio sleeper in two of them, a sleeper trait of stella-fleet's own, and the same two test doubles in about thirty files. stella-time now holds TokioSleeper, WallClock and MonotonicClock (which replaces the CLI's per-construction SystemClock), and ships PausedSleeper and NoopSleeper behind a test-util feature. The copies are deleted; stella-fleet's monitor takes the engine's Sleeper. stella-core's own unit tests keep one copy in src/tests.rs, because a lib's unit tests are a second build of the lib and a dev-dependency that links the lib implements the trait for the first. tests/one_home.rs reads the tree and fails on any other copy. ADR 0042 records the design #6486 merged and where the sources live. Closes #6484
macanderson
added a commit
that referenced
this pull request
Sep 10, 2026
…Sleeper and Clock ports #6486 routed every instant, wait and timeout in stella-core through retry::Sleeper and left the real sources copied: a wall clock in stella-cli, stella-runtime and stella-serve, a Tokio sleeper in two of them, and a sleeper trait of stella-fleet's own with a third. stella-time now holds TokioSleeper, WallClock and MonotonicClock (which replaces the CLI's per-construction SystemClock), and ships PausedSleeper and NoopSleeper behind a test-util feature for the test sweep that follows. The copies are deleted; stella-fleet's monitor takes the engine's Sleeper. tests/one_home.rs reads the shipping tree and fails on any other copy. ADR 0042 records the design #6486 merged and where the sources live. Refs #6484
macanderson
added a commit
that referenced
this pull request
Sep 10, 2026
…Sleeper and Clock ports #6486 routed every instant, wait and timeout in stella-core through retry::Sleeper and left the real sources copied: a wall clock in stella-cli, stella-runtime and stella-serve, a Tokio sleeper in two of them, and a sleeper trait of stella-fleet's own with a third. stella-time now holds TokioSleeper, WallClock and MonotonicClock (which replaces the CLI's per-construction SystemClock), and ships PausedSleeper and NoopSleeper behind a test-util feature for the test sweep that follows. The copies are deleted; stella-fleet's monitor takes the engine's Sleeper. tests/one_home.rs reads the shipping tree and fails on any other copy. ADR 0042 records the design #6486 merged and where the sources live. Refs #6484
12 tasks
macanderson
added a commit
that referenced
this pull request
Sep 11, 2026
…Sleeper and Clock ports (#6488) ## What & why #6486 routed every instant, wait and timeout in `stella-core` through `retry::Sleeper`. It left the other half of the same defect in place: the real time sources are copied. A Unix-epoch wall clock lived three times (`stella-cli` `WallClock`, `stella-runtime` `HostClock`, `stella-serve` `WallClock`); a Tokio sleeper twice (`stella-cli`, `stella-serve`); and `stella-fleet` kept a `Sleeper` trait of its own with a third `TokioSleeper`. `stella-serve` may not link `stella-cli` or `stella-runtime`, and `stella-core` may not link the Tokio timer, so the copies had no home. This PR gives them one, and ships the two test doubles the follow-up sweep will move every test onto. - **`stella-time`** holds `TokioSleeper` (the engine's real sleeper and `now`), `WallClock` (Unix epoch, for a stamp another process reads) and `MonotonicClock` (one origin per process, for a span compared as a number; replaces `stella-cli`'s per-construction `SystemClock`). The copies are deleted; `stella-fleet`'s trait is gone and its monitor takes the engine's, re-exported under the old name. Justified under AGENTS.md § "When a new crate is justified" on two counts: it holds the effects the ports keep out of `stella-core`, and it sits below `stella-serve`. Exemplar: tokio / tokio-test. - **`stella_time::test_util`** ships `PausedSleeper` and `NoopSleeper` behind a `test-util` feature. Nothing takes them yet: the sweep that retires the ~30 per-file copies in `stella-core` and `stella-engine` is the stacked follow-up PR, kept separate so each diff stays under Sourcery's review limit and reads as one change. - **ADR 0042** records the design #6486 merged (why `now` sits on `Sleeper`, why the reading is an `Instant`) and where the sources and doubles live. #6486 closed nothing and wrote no ADR; SCR-002 asks for one. Refs #6484 — the issue closes with the follow-up sweep, which lands its last two checklist items. ## The witness - [x] This PR includes a witness test (fails on `main`, passes here) `crates/stella-time/tests/one_home.rs` reads every shipping `.rs` under `crates/*/src` (test directories and `tests.rs` files skipped) and fails on any `impl … Sleeper for` outside `stella-time`, and on any `struct WallClock | HostClock | SystemClock | MonotonicClock` outside it. The `impl`s that stay are named with reasons: two special-shape doubles (the retry tests' recording sleeper, the monitor's advancing sleeper) and five inline test doubles the follow-up sweep retires; a second test fails if a named one disappears, so the list cannot go stale. On `main` the sleeper test fails on `stella-cli/src/runtime.rs` and `stella-serve/src/remote.rs`, and the clock test on `stella-cli/src/runtime.rs` (`SystemClock`, `WallClock`) and `stella-runtime/src/wrapper/stamp.rs` (`HostClock`); `stella-fleet/src/monitor.rs` is on the kept list for its advancing double, so its production `TokioSleeper` was reachable only through the trait it also deleted. Locally the time crate's 6 + 3 tests pass, and `stella-core`, `stella-engine`, `stella-serve`, `stella-runtime`, `stella-fleet` and `stella-cli` type-check with their tests. ## The gate - [x] `cargo fmt --check` - [ ] clippy over the workspace at `-D warnings` — CI; ran locally, scoped and clean, on the six touched crates - [ ] the workspace test suite — CI - [x] Docs updated where behavior/flags changed (README, `--help`, doc comments) - [x] CLA signed - [x] No `Closes` here by design: this PR advances #6484 and the follow-up closes it (`closes-nothing`) ## Fix over file - [x] Extra fixes in this PR, each its own commit: - **The two high-severity Dependabot alerts on `main`** (`sharp` <0.35.4, GHSA-rgj7-g3m4-5g8c; `js-yaml` <4.3.2, GHSA-2883-xcg3-v3hh) are both transitive under `website/`, so `website/pnpm-workspace.yaml` raises the `sharp` floor and adds a `js-yaml` one, the way that file already handles `postcss` and `nanoid`; `js-yaml` takes a caret because a bare floor resolves to 5.x, which fumadocs does not call. - **`dependency-review`** re-surfaced sharp's fourteen LGPL-3.0 libvips tuples on that bump (not a required check). They are named in `allow-dependencies-licenses`, which is the immediate remedy issue #2532 records, with the reasoning in the workflow comment: the docs site is private, imports no `next/image`, and ships nothing into either license track. - `AGENTS.md` carried two crate counts ("Twenty-nine crates", "The other twenty-four crates") that a new crate makes wrong; both are now phrased without a number. - [x] Nothing was deferred beyond the stacked follow-up. ## Ground-rule check - [x] No I/O added to `stella-core`; it is untouched except its README - [x] No new outbound network calls - [x] No new cross-boundary serde types ## Deleted tests Three tests in `stella-cli/src/runtime.rs` tested the clocks that module no longer defines, and each has a counterpart in `stella-time/src/lib.rs`: - `system_clock_starts_near_zero_and_advances_monotonically` → `the_monotonic_clock_never_goes_backwards` (plus `every_monotonic_clock_shares_one_origin`, the property the old per-construction clock lacked) - `default_constructs_a_fresh_clock` → gone with the constructor; `MonotonicClock` is a unit struct - `wall_clock_reads_epoch_milliseconds_not_a_process_origin` → `the_wall_clock_counts_from_the_unix_epoch` ## Anything reviewers should know? - An earlier head of this branch redesigned the engine's time as `u64` readings of `Clock`. That design lost to #6486 on the merge order and on the merits the ADR states (a double has to answer `now` and `sleep` from one timeline), and it was dropped rather than rebased over the merged one. - Sourcery declined the earlier, combined head (208k characters of diff against a 150k limit). That is why the test-double sweep is its own PR. - `stella-cli`'s `runtime` module is now three re-exports and `one_shot_budget_guard`. The `SystemClock` rename to `MonotonicClock` reaches `fleet_cmd`, `agent/engine.rs` and two test files, and nothing else. ## Summary by Sourcery Centralize real time sources and shared sleeper doubles in `stella-time` while preserving the existing time ports across all hosts. New Features: - Add the `stella-time` crate as the shared home for Tokio-backed sleeping, wall-clock timestamps, process-monotonic timing, and reusable test sleeper doubles. Bug Fixes: - Remove duplicated clock and sleeper implementations across host crates and standardize fleet monitoring on the engine's `Sleeper` port. - Raise transitive website dependency floors to address the sharp and js-yaml security advisories. Enhancements: - Add a source-tree witness test that prevents production time-source implementations from being duplicated outside `stella-time`. - Replace per-construction monotonic clocks with a process-wide shared origin and re-export the shared implementations through existing host modules. Build: - Register `stella-time` in the workspace and add it as a dependency for the affected host crates. CI: - Allow the newly surfaced sharp LGPL libvips dependency tuples in dependency review with documented rationale. Documentation: - Document the shared time-source boundary in the workspace guidance, add ADR 0042, and update the ADR index and crate documentation. Tests: - Add coverage for wall-clock, monotonic-clock, Tokio sleeper, and jitter behavior, plus the `stella-time` source-layout witness tests. Chores: - Update workspace guidance to avoid stale crate counts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is
The follow-up #6482 said it was leaving behind. That PR fixed the two ambient reads that were also I/O-adjacent (OS entropy, wall clock) and recorded 19
Instant::now()reads as ratchet debt. This one removes every ambient read of time fromstella-core's shipping code — the 19 explicit reads, the 9 hidden ones (.elapsed()is anInstant::now()in disguise), and the 4tokio::time::timeouts, which are the runtime's timer rather than the port — and drops tokio'stimefeature from the crate. The ratchet baseline is empty.The port
retry::Sleeperis now the engine's whole time port:now() -> Instantjoinssleepandjitter. One port rather than a secondClockbecause a double cannot answer them apart — a sleeper that suspends virtually has moved its ownnow, and a timeout is a sleep racing a call. That is tokio's own shape: a paused runtime puts one virtual clock behindsleepandInstant::now.ports::Clock(now_ms) stays what it was — the millisecond clock hosts hand to things that stamp records (fleet ledger, runtime stamps, the hook bus) — and is untouched.retry::bounded(sleeper, limit, call)istokio::time::timeoutwritten against the port:futures_util::future::selectover the call andsleeper.sleep(limit), call polled first so a ready call never loses to a ready sleep,Optionrather thanResultbecause the limit passing is an answer, not an error.What moved, in shipping code
Instant::now()→self.sleeper.now()/sleeper.now(): driver (deadline notice, step timing, settlement, speculation pool), dispatch, completion, rate-limit park allowance, sub-agent deadline and tick, accounted call, retry attempt timing..elapsed()→sleeper.now().duration_since(started)at each of the nine sites;CancelUsageGuardcarries&dyn Sleeperbecause aDrophas no caller to hand itnow.tokio::time::timeout→retry::bounded: the tool dispatch ceiling, the idle generation bound, the task-deadline bound, the accounted call's idle bound.stella-core's tokio features are["sync"].TurnState::new/TurnState::from_checkpoint/BorrowedTurn::adopttakenow: Instant;bounded_generation/deadline_bounded_generationtake the sleeper.from_checkpointis public, sostella-cli's resume path andstella-serve's checkpoint test passInstant::now()— hosts are where that read belongs.Clockit already holds (wall milliseconds); the quarantine rule needs three consecutive overruns, so one wall-clock step cannot quarantine anything. Its test advances a hand-stepped clock instead ofthread::sleep(100ms).The guard
check-core-no-io.py's floor gains.elapsed()and anytokio::timeuse; the tokio feature allowlist losestime; the baseline is empty and--updaterefuses to add to it, so anyInstant::now()now fails. Harness: 27 cases (+3:.elapsed(), a tokio timer, thetimefeature). AGENTS.md rule 2, the crate README and theci.ymlcomment say the count is zero.Test doubles — the part worth reading
An instant-returning
Sleeperdouble was tolerable while timeouts bypassed the port and is a lie once they go through it:EngineConfig::default()arms a 816 s model timeout and a 15 min tool timeout, and an instant sleep makes both fire the moment a provider future waits on another task. Twelve tests failed that way on the first run. The faithful double sleeps on tokio's clock and readsnowfromtokio::time::Instant::now().into_std(), and the files that use it run#[tokio::test(start_paused = true)]: virtual time is free while the runtime is idle and correct when it is not.driver/tests.rs's andsubagent/tests.rs's shared doubles (now namedTokioSleeper) and 17 + 7 test files changed that way;hard_drop_write_backand the streaming accounted-call test got the same double. Files with their own instantNoSleepthat passed were left alone.A side effect worth stating: the
stella-corelib suite went from 46 s to 2.5 s, because several tests had been paying real backoff sleeps throughtokio::time::timeoutthat the paused clock now skips.Witness
retry::tests::bounded_races_the_ports_own_sleep: with a sleeper whose sleep returns at once,bounded(1h, pending)isNoneimmediately andbounded(1h, ready(5))isSome(5), and the port recorded exactly one 3 600 000 ms sleep. Ontokio::time::timeoutthis waits the real hour.check-core-no-io.pyonmainfails on 19Instant::now()reads, 9.elapsed()calls, 4tokio::timeuses and thetimefeature; here it passes with an empty baseline.No test was deleted.
Verified
cargo check -p stella-core -p stella-engine -p stella-tools -p stella-serve -p stella-cli --all-targets: clean.cargo test -p stella-core: 893 lib tests + every integration test pass (2.5 s lib).cargo test -p stella-engine: pass.make guards-fast: green;./scripts/test-core-no-io.sh: 27 passed; shellcheck clean.Closes nothing by design (
closes-nothing): this is the remaining half of the audit the maintainer asked for directly.