fix(stella-cli): unbreak main — broken intra-doc link in #1698's boot guard - #1929
fix(stella-cli): unbreak main — broken intra-doc link in #1698's boot guard#1929macanderson wants to merge 1 commit into
Conversation
…ot guard
`main` fails the gate's `doc-warnings` step:
error: could not document `stella-cli`
--> crates/stella-cli/src/daemon/boot.rs:130
|
| /// ([`supervised::APPROVAL_REQUEST`]).
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no item named `supervised` in scope
|
= note: `-D rustdoc::broken-intra-doc-links` implied by `-D warnings`
`boot.rs` imports `stella_store::{SessionRecord, SessionRegistry, SessionStatus}`
but not the `supervised` module, so the shortened path does not resolve. Fully
qualified, it does.
This is mine, from PR #1920 (#1698). I ran `cargo test`, clippy, `fmt --check`
and four guard scripts on that branch and reported them — but not
`RUSTDOCFLAGS="-D warnings" cargo doc`, which is a separate gate step, and the
one this needed.
It reached `main` because the required checks never ran: every `ci.yml` run
since 19:03 is still `queued` (#1899), so the branch merged on stale/absent
contexts. That is #1645's failure mode arriving by a different road — not a red
check that was overridden, but no check at all.
Verified: `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps` exits 0.
`cargo test -p stella-cli --bin stella boot` — 24 passed.
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
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideFixes a broken Rustdoc intra-doc link in stella-cli’s daemon boot guard by updating the reference to use a fully-qualified path so documentation builds cleanly under -D rustdoc::broken-intra-doc-links. 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 Worth recording why it got in, since the mechanism will repeat: my PR #1920 reported tests, clippy, fmt and four guard scripts as clean — all true — and I did not run the rustdoc step. Normally the required check catches that. Here the required checks never reported at all, so the branch merged on absent contexts rather than green ones. That is a different failure from #1645's (a red check overridden); it is no check, and branch protection cannot distinguish "passed" from "never ran" when the run is stuck in Verification for this PR specifically: |
|
Closing as a duplicate of #1927, which fixes the same dangling intra-doc link with the same one-line qualification and was opened first. Two sessions found this independently within minutes, which is the #1722 shape exactly — and worth noting that the duplicate-claim guard I shipped for it (#1922, now on main) would not have caught this pair: both PRs say Not obviously worth fixing — widening the guard to My gate-audit comment is moved to #1927. No code lost — the fix is identical. |
…1930) fix(stella-cli): make the resume frame impossible to forget The pipeline frame that lets a resume say what it is not restoring was declared at exactly one of four `Pipeline::new` call sites. The Command Deck, the goal loop and fleet workers declared nothing, so a checkpoint any of them left read as a plain engine turn and any resume from it degraded in silence — the exact failure #1615 closed for `stella run` alone. ## A mechanism, not a convention The obvious fix is one `resume_frame::declare` line beside each construction. That is what I wrote first, and the file-size gate rejected it: two of the four sites are in grandfathered god files closed to growth, and +1 line each put `command_deck.rs` and `fleet_cmd.rs` over their ceilings. The issue anticipated this and suggested routing through a helper. That turns out to be the better design regardless: `resume_frame::pipeline` declares and constructs in one call, so a surface **cannot come to hold a pipeline whose checkpoints do not say what they are**. Remembering is no longer part of the contract. One call replaces one call, so all three god files land exactly at their ceilings with no baseline bump: command_deck.rs 4621 / 4621 fleet_cmd.rs 1504 / 1504 agent.rs 2266 / 2266 `agent.rs` needed one line back to get there; extending its existing `use crate::{…}` group rather than adding an import line is where it came from. Callers still chain what is genuinely theirs — the deck and fleet workers add `with_turn_gate`, because the gate is per-surface while the frame is not. ## Witness `every_pipeline_construction_declares_its_resume_frame` walks `stella-cli/src` and fails on any `Pipeline::new(` outside the one helper, naming file and line. Verified to catch the regression it exists for by deleting the goal loop's call: these sites call `Pipeline::new` directly and so declare no resume frame … ["…/crates/stella-cli/src/agent/goal.rs:775"] It greps the source rather than driving a turn, deliberately, and the issue asks for exactly this: what went wrong was **wiring**, not logic. Every unit test here already passed while three surfaces never called the function. A behavioural test would need one scripted run per surface and would still only cover the surfaces somebody remembered to write a test for — the same gap one level up. The repo uses source-grep guards for this shape already (the `stella fullauto` wrapper guards from #1619). The guard skips its own file: the needle appears in its string literal, so it would otherwise report itself forever. Found by running it. ## One rustdoc repair Dropping the now-unused `Pipeline` imports orphaned an intra-doc link in `goal.rs` that resolved through one of them; it is fully qualified now. Caught by `RUSTDOCFLAGS="-D warnings" cargo doc`, which is a gate step — and which is also how I found that `main` was already red for an unrelated broken link of mine (PR #1929, which this branch is stacked on). `cargo test -p stella-cli` — 1446 passed, 0 failed. Clippy `-D warnings`, `fmt --check`, workspace rustdoc `-D warnings`, `check-file-size`, `check-god-files` and `check-left-behind` all clean. Closes #1672 Refs #1615, #1671 ## Summary by Sourcery Ensure all stella-cli pipelines declare their resume frame via a single construction helper to prevent silent degradation when resuming from checkpoints. Bug Fixes: - Prevent checkpoints created by certain stella-cli surfaces (command deck, goal loop, fleet workers) from resuming as plain engine turns without indicating missing stages by centralizing resume frame declaration. - Fix an intra-doc link in the goal agent module after removing direct Pipeline imports. Enhancements: - Introduce a `resume_frame::pipeline` helper that couples resume frame declaration with pipeline construction so surfaces cannot forget to declare the frame. - Add a source-grep test that enforces all Pipeline constructions go through the new helper and flags any direct uses as errors. Tests: - Add a test that scans stella-cli sources to ensure every `Pipeline::new` call goes through the resume-frame-aware helper and fails if any direct constructions remain.
fix(stella-cli): unbreak main — a broken intra-doc link in #1698's boot guard
mainfails the gate'sdoc-warningsstep:boot.rsimportsstella_store::{SessionRecord, SessionRegistry, SessionStatus}but not the
supervisedmodule, so the shortened path does not resolve. Fullyqualified, it does.
This is mine, from PR #1920 (#1698). I ran
cargo test, clippy,fmt --checkand four guard scripts on that branch and reported them — but not
RUSTDOCFLAGS="-D warnings" cargo doc, which is a separate gate step, and theone this needed.
It reached
mainbecause the required checks never ran: everyci.ymlrunsince 19:03 is still
queued(#1899), so the branch merged on stale/absentcontexts. That is #1645's failure mode arriving by a different road — not a red
check that was overridden, but no check at all.
Verified:
RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-depsexits 0.cargo test -p stella-cli --bin stella boot— 24 passed.Summary by Sourcery
Bug Fixes:
-D warnings.