fix(stella-cli): unbreak main — resolve the boot.rs doc link #1920 left dangling - #1927
Conversation
…ft dangling The parked field's intra-doc link named supervised::APPROVAL_REQUEST with no supervised in scope in boot.rs, so RUSTDOCFLAGS="-D warnings" cargo doc fails and the doc-warnings gate is red for every open PR. ci.yml does not run on a push to main, which is how #1920's merge never saw it. Refs #1920
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Reviewer's guide (collapsed on small PRs)Reviewer's GuideFixes a failing intra-doc link in stella-cli’s daemon boot documentation by fully qualifying the referenced constant so File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Full local gate audit of Since no
So merging this returns One correction to the description, since it changes where to look: For the record, #1920 is mine, and I reported tests, clippy, fmt and four guard scripts clean on it — all true, and none of them is the rustdoc step. Thanks for catching it. |
|
CI is running again, and it confirms this PR is the single thing blocking every open branch. Hard evidence rather than my local gate run.
That branch touches one string literal in A push to Merging this returns |
…n cannot stall the sweep (#1921) (#1928) ## What & why `resume_all` streamed each resumed run to completion with no upper bound, so any turn that never ends — a wedged tool, a provider that never returns, a model call retrying forever — stalled every id behind it, exactly as an unanswered approval used to, and just as silently. #1920 fixed the one such state visible on disk before spawning (a parked approval); this is option 2 of the pair #1698 said compose: a **per-run wall-clock ceiling** on the boot sweep — `stella daemon resume-all --ceiling MINUTES`, default 30. The design calls the issue asked to be stated: - **The stop path is the existing graceful one, not a second one.** Expiry never kills the child outright — a `SIGKILL` at the ceiling would land mid-edit, worse than the stall it fixes. The ceiling goes through `Supervised::interrupt_and_drain`, the same discipline as Ctrl-C and `stella daemon stop`: `SIGTERM`, the measured 8-second `STOP_GRACE` for the engine to abort at a safe boundary (invariant 6) and write its own terminal status, escalation only then. `STOP_GRACE` is untouched. - **A timed-out run IS charged its boot attempt.** The attempt is recorded before the spawn (as every attempt already was) and deliberately not refunded. The two endings compose with the existing brakes rather than needing new ones: a child that honours the stop ends deliberately (checkpoint discarded, `Cancelled` recorded — same as any operator stop), so the next sweep skips it as ended and returns its attempts; a child so wedged it had to be killed wrote nothing, keeps its resume point, and is swept again next boot — where the charge counts against `MAX_BOOT_ATTEMPTS`. So the charge only ever persists for the genuinely wedged run, which is exactly the recurring failure the three-attempt bound exists to stop. A run that was merely slower than the ceiling is `stella daemon resume <id>`'s to continue by hand — that verb has no ceiling. - **Nothing is quiet** (#1627's "a run silently resumed at boot is as bad as one silently lost"): the watch names the ceiling the moment it fires, and the sweep prints one operator-actionable line per stopped run (`ceiling_report`, pure and witnessed like `SkipReason::explain`). Mechanically: `watch` gains an `Option<Duration>` ceiling and answers `Watched::{Finished, CeilingReached}`; the bounded race is `tokio::time::timeout` over the existing `follow()` future (all streaming state lives on `Supervised`, so the abandoned future drops losslessly). `resume_supervised` threads it; hand resumes pass `None`. Also in this PR: `Tail` moved verbatim from `daemon.rs` into `daemon/console.rs`, where its readers already live. `daemon.rs` stood at 1493 lines with the ceiling in place — file-size policy says a file approaching the 1500 gate gets split, not grown. It now sits at ~1400, and no baseline entry changed. Closes #1921 ## The witness - [x] This PR includes a witness test (fails on `main`, passes here) `daemon::tests::a_run_that_outlives_the_ceiling_is_stopped_gracefully_and_the_watch_returns` — a real supervised child that traps `TERM` outlives a 250ms ceiling; the test asserts the watch *returns* (the property the sequential sweep rests on), that the child saw `SIGTERM` before anything harder (the trap's marker file), and that the stop is recorded `Cancelled` rather than aging into a crash. `a_run_that_finishes_under_the_ceiling_is_left_to_finish` pins the no-misfire half, and `boot::tests::a_ceiling_stop_names_the_ceiling_the_safe_stop_and_the_next_step` pins the console contract. On `main` these fail as the feature is genuinely absent (`watch` has no ceiling parameter and `Watched`/`ceiling_report` do not exist). ## The gate - [x] `cargo fmt --check` - [x] `cargo clippy --workspace --all-targets -- -D warnings` - [x] `cargo test` (`make gate CARGO_SCOPE="-p stella-cli"`) - [x] Docs updated: `website/content/docs/commands/daemon.mdx` § `resume-all` (new "What bounds each resume" block + synopsis), `--help` text on `ResumeAll`, module docs in `daemon/boot.rs` (new "What stops a stalled sweep" section stating the attempt-charging decision) - [x] CLA signed - [x] `Closes #1921` appears both above and as a commit trailer ## Nothing left behind While gating this, main turned out doc-red: #1920 merged a dangling intra-doc link in `boot.rs` (`ci.yml` does not run on pushes to main). The second commit here fixes it — and the same one-line fix is also open as a dedicated unbreak, #1927, so every other PR heals without waiting on this one; the two apply cleanly in either order. That commit also swaps the bounded race to the canonical `tokio::time::timeout` and teaches `describe_ceiling` to speak milliseconds rather than round a sub-second test ceiling to "0-second". Refs #1585, #1627, #1698, #1920, #1927.
…a dangling boot.rs doc link (#1985) ## What & why `main`'s required `fmt + clippy + test` job has been **red since #1951** (five consecutive merges), so every open PR inherits a red gate. Four distinct errors are involved. This PR fixes the two that **no open PR covers**. ### 1. `cargo clippy -D warnings` — `plan_stage` has 8 arguments (limit 7) `crates/stella-pipeline/src/pipeline.rs`. #1953 added a `research` parameter, pushing `plan_stage` over `clippy::too_many_arguments`. `Spend<'_>` (`crates/stella-pipeline/src/pipeline/stage_budget.rs`) already groups the `budget`/`total` pair, and **seven** sibling stage methods take it; `plan_stage` was the last one carrying the two loose. Adopting it takes the count to 7. This is the right fix rather than `#[allow(clippy::too_many_arguments)]` because there is no argument that the lint is wrong *here* — the grouping type the lint is asking for already exists — and because `pipeline.rs` is a grandfathered god file closed to growth, which this change **shrinks**. ### 2. `cargo doc -D warnings` — unresolved link in `boot.rs` `crates/stella-cli/src/daemon/boot.rs`. #1939 left ``[`SkipReason::NoResumePoint`]`` in the module doc unresolvable (`no item named `SkipReason` in scope`), which fails `rustdoc::broken_intra_doc_links`. Qualified to the full path — the same remedy #1927 applied to *this same file* for *this same reason* after #1920. That recurrence is filed as #1986. ## What this PR deliberately does NOT fix The other two errors belong to the open PR **#1964**: - `flip_halt_arming.rs` references `PassingShell` / `shell_call_result`, test helpers #1945 landed the test file without. - `crates/stella-pipeline/src/management_prompt/tests.rs`'s exhaustive match omits `ModelCallRole::Research`, which #1953 added. Duplicating them here is how two parallel unbreak PRs collide, so they are left to #1964. **`main` needs both PRs.** This one alone leaves `cargo test` red; #1964 alone leaves clippy and rustdoc red (verified: #1964's own CI still fails both, and its clippy failure is the identical `plan_stage` error). Neither is sufficient on its own, and the merge order does not matter. Because of that, **this PR's own `cargo clippy` and `cargo test` steps will stay red until #1964 merges** — clippy `--all-targets` reaches the missing test helpers once the lib error is gone. That is expected, not a regression. ## The witness - [x] No witness test needed — a lint fix and a doc-link fix, neither a behavior change. Verified the artisanal way: - `RUSTDOCFLAGS="-D warnings" cargo doc -p stella-cli --no-deps` fails on `main` and **exits 0** with this change. - `cargo check -p stella-pipeline --all-targets` reports **only** the three #1964-owned errors; the `plan_stage` arg-count error is gone and both call sites (`pipeline/scope_stage.rs`, `pipeline/tests/management_accounting.rs`) compile. - `cargo fmt --check -p stella-pipeline -p stella-cli` exits 0. ## The gate - [x] `check-file-size.sh` and `check-god-files.sh` pass; `pipeline.rs` shrinks, so no baseline change is needed. - [x] No behavior change, no new flags, no docs pages affected. - [x] No new dependencies. ## Nothing left behind - **#1986** — `boot.rs`'s module doc has now broken `main`'s rustdoc twice in two days by the same mechanism (#1920 → #1927, then #1939 → this PR), because `ci.yml` does not run on a push to `main`. Filed as a handoff. - **#1974** — `CandidateState` is hand-built at two sites, which is how #1951's field addition silently broke PR #1962 at merge time. Noticed in the same investigation. Refs #1953, #1939, #1964
What & why
RUSTDOCFLAGS="-D warnings" cargo doc -p stella-clifails on main: theparkedfield's intra-doc link incrates/stella-cli/src/daemon/boot.rsnamessupervised::APPROVAL_REQUESTwith nosupervisedin scope. The doc-warnings gate is therefore red for every open PR. #1920 merged with it becauseci.ymldoes not run on a push to main.One line: qualify the link as
stella_store::supervised::APPROVAL_REQUEST, the path the code itself uses. Refs #1920.The witness
RUSTDOCFLAGS="-D warnings" cargo doc -p stella-cli --no-deps, which fails on main and exits 0 with this change.The gate
Nothing left behind
The same fix rides on the #1921 feature branch (identical line, no conflict either way).
Summary by Sourcery
Bug Fixes: