fix(gate): unbreak main — clippy clone_on_copy, file-size baseline, gate-parity step count - #1873
Closed
macanderson wants to merge 2 commits into
Closed
fix(gate): unbreak main — clippy clone_on_copy, file-size baseline, gate-parity step count#1873macanderson wants to merge 2 commits into
macanderson wants to merge 2 commits into
Conversation
…s instead of cloning it `clippy::clone_on_copy` fails the workspace at -D warnings on main: `apply_role_shaping` in witness_stage.rs clones `GenerationParams`, which is `Copy`. One-word fix, exactly clippy's suggestion. Landed red because the introducing PR merged without a green gate (enforce_admins is off); every open PR inherits the failure until this lands.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
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
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideFixes a Clippy lint in stella-pipeline by replacing an unnecessary clone of a Copy type in witness_stage role shaping logic, with no behavior change. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This was referenced Aug 6, 2026
… count Two more pre-existing gate failures on main, found by running guards-fast at its tip: - file-size: crates/stella-protocol/src/event.rs (+2) and crates/stella-tui/src/deck_render.rs (+3) grew past their recorded ceilings via already-merged PRs. Regenerated with `make file-size-update`, which also RETIGHTENS command_deck.rs (4740->4691), fleet_cmd.rs (1507->1504) and pipeline.rs (3642->3580) to their shrunken sizes. - gate-parity: GATE_STEPS runs 25 steps, but AGENTS.md/CONTRIBUTING.md still spelled "twenty-four" (the step lists themselves were current).
macanderson
changed the title
This was referenced Aug 6, 2026
Merged
Owner
Author
|
Pushed 6a-style completion: CI's |
Owner
Author
|
Superseded: #1894 landed on main with all three fixes (both clippy errors in witness_stage.rs — the clone_on_copy AND the field_reassign_with_default it masked — the file-size baseline, and gate-parity, which it resolved by deleting the spelled count from the check entirely). I rebased this branch onto main and every commit dropped as already-upstream, so this PR now carries zero diff. Recommend closing without merge — I don't close PRs from an automated session. The follow-up context lives in #1878, whose description I've updated to point at #1894 instead of this PR. |
macanderson
added a commit
that referenced
this pull request
Aug 6, 2026
…-home too (#1877) ## What & why Refs #1755 #1810 factored the **legacy** self-driving roots into `stella-home` so the writer and the read-only reader could not drift, and moved `stella-observatory` onto `stella_home::self_driving_root` for the current one — but left `LoopState::open` building its own: ```rust let dir = home.join("self-driving").join(&slug); ``` Half the job. The **current** root — the one every live loop actually uses — was still two literals in two crates: ``` $ git grep -n 'self_driving_root()' -- crates/ crates/stella-home/src/lib.rs:181:pub fn self_driving_root() ... crates/stella-observatory/src/self_driving.rs:73: roots.extend(stella_home::self_driving_root()); ^ only the reader used it ``` That is the exact shape #1755 was filed for. Renaming the directory in one of them would point the dashboard at nothing, and **the failure is silent** — an empty Self-Driving tab reads as "no runs on this machine", never as "the reader is looking in the wrong place". I introduced this in #1810, so it is mine to close. ## The witness - [x] This PR includes a witness test. `the_current_root_comes_from_the_shared_resolver` pins that the shared resolver still yields the directory this crate writes and the observatory reads — the twin of the existing `the_writer_and_the_reader_agree_on_the_legacy_roots`. The migration test helper keeps its own hard-coded literal **deliberately**: a test that re-derives the expected path from the code under test cannot catch a change to it. The tests state the path; the code resolves it. ## The gate - [x] `cargo test -p stella-cli --bin stella self_driving_cmd::state` — 7 / 7 - [x] `cargo fmt --check` — clean - [x] `cargo clippy -p stella-cli --all-targets -- -D warnings` — clean **for this crate** > Workspace clippy currently fails on `stella-pipeline` (`clone_on_copy` in `witness_stage.rs:55`), which is pre-existing on `main` and already covered by #1873 and #1859. Not touched here. ## Nothing left behind Nothing new. `main` is currently red on three fronts, all with fixes in flight: the clippy error above (#1873/#1859), the `file-size` baseline skew and the stale gate-parity count (both in #1845; #1863 carries the same count fix). ## Summary by Sourcery Unify resolution of the current self-driving state root in stella-cli with the shared stella-home resolver to prevent drift between writer and reader paths. Bug Fixes: - Use the shared stella-home self-driving root resolver in LoopState::open instead of constructing the path manually, ensuring the CLI and observatory agree on the current state directory. Tests: - Add a regression test that pins the current self-driving root produced by the shared resolver to the directory used by stella-cli and read by stella-observatory. Co-authored-by: Stella Test <test@stella.local>
macanderson
added a commit
that referenced
this pull request
Aug 6, 2026
…sed writers too (#1826) (#1878) ## Problem PR #1828 (#1653) taught the supervised registry writers (`daemon::outcome_status` and its callers) to record `SessionStatus::Stopped` for a policy stop — but `SessionPresence::finish(ok: bool, …)` and its three call sites still collapsed the outcome to a bool on the way in. On a **supervised** run this was masked, because `record_outcome_if_supervised` writes after `finish` and corrects it. On an **unsupervised** headless run (a pipe, CI, `--foreground`) there is no later write: a deliberate stop (`AbortKind::DeliberateStop` — stuck-loop escalation, step cap, enforced budget, ended scope review; exit 3) aged into the SESSIONS overlay as `Error`, indistinguishable from a crash. ## Approach Mirrors #1828's shape: widen the writer past the bool, and keep **one decider**. - `crates/stella-cli/src/agent/outcome.rs` — `pipeline_session_status`, the fifth projection the module doc now names: `Result<PipelineOutcome, PipelineRunError>` → terminal `SessionStatus`, routed **through `crate::daemon::outcome_status`** rather than re-matched, so every registry writer — supervised or not — reads a deliberate stop (`Stopped`), a user interrupt (`Cancelled`), and a crash (`Error`) off the same function. - `crates/stella-cli/src/agent/presence.rs` — `SessionPresence::finish` now takes the projected `SessionStatus`; `one_shot_notification` words the inbox entry by how the run actually ended (a policy stop notifies "run stopped by policy", an interrupt "run cancelled" — "FAILED" for a deliberate stop was the same dishonesty the status carried; the issue delegated this wording decision). - Call sites: `agent.rs` (pipeline one-shot — projects the `PipelineStatus` it holds; the interactive exit passes `Complete` explicitly), `agent/goal.rs` raw one-shot (feeds its `Result<(), CliFailure>` straight to `outcome_status`, the same idiom as `agent/resume.rs`). - `agent/goal.rs` goal loop: audited — its `Result<(), String>` has **no room for the abort kind** (the #1637 collapse one level deeper), so a policy-stopped goal round still records `Error`; it now at least projects through `outcome_status` so an interrupt records `Cancelled`. Filed as **#1862**, cited at the call site. - Deck `session_exit` (`command_deck.rs:2123`): audited, unchanged — `run_lead_turn` returns the same stringly `Result<(), String>`, so the deck cannot see a deliberate stop today; covered by **#1862** (and `command_deck.rs` is a god file closed to growth). - `daemon.rs`: the `record_outcome_if_supervised` doc said the presence "sees only a bool" — now stale, updated (stale comments are bugs). God files: `agent.rs` sits at its exact 2269-line ceiling and this diff keeps it at **exactly 2269** (all edits line-neutral); new logic lands in `agent/outcome.rs` and `agent/presence.rs`, the siblings the issue names. ## Witness `outcome::tests::an_unsupervised_deliberate_stop_projects_stopped_not_error` — a deliberate stop projects `Stopped`, a crash `Error` (plus `the_remaining_terminal_arms_project_unchanged` pinning `Complete` and the hard-error arm). **The witness is structural, mirroring #1828's own** (`daemon::tests::a_deliberate_stop_records_a_status_distinct_from_a_crash`, whose doc says "on the old signature this test does not even compile"): on main, `pipeline_session_status` does not exist and the widened `finish` call sites do not compile, so the fail-on-old is a compile failure rather than an assertion failure. On the old code the collapse is visible in source: `finish` wrote `if ok { Complete } else { Error }` — a deliberate stop could not reach any other status. ## Verification - `cargo test -p stella-cli --bin stella -j 2` — **1429 passed, 0 failed** (includes the new witnesses, `daemon::tests`, and the goal tests). - `cargo clippy -p stella-cli -j 2 --all-targets -- -D warnings` — clean (with main's pre-existing `stella-pipeline` lint patched locally, see below). - `cargo fmt -p stella-cli -- --check` — clean; `check-file-size` — `agent.rs` unchanged at its exact ceiling. **Main-side gate note**: main was red on three pre-existing gate steps when this branch was cut (clippy `clone_on_copy` in `stella-pipeline` — which masked a second `field_reassign_with_default` error behind it — file-size ceilings for `event.rs`/`deck_render.rs`, and gate-parity). All three were fixed on main by #1894 (my parallel unbreak #1873 was superseded and is closed with zero diff). This branch has since had main merged in and its CI runs against the green tip; it contains none of those failures and none of their fixes. Closes #1826 Refs #1653 Refs #1862 ## Summary by Sourcery Ensure unsupervised runs record distinct session statuses for deliberate policy stops, user interrupts, and crashes, aligning all registry writers on a single outcome projection. Bug Fixes: - Fix unsupervised headless runs incorrectly recording deliberate policy stops as generic errors in the SESSIONS registry. Enhancements: - Add a shared projection from pipeline outcomes to terminal session status so supervised and unsupervised writers use the same outcome classification. - Improve headless inbox notification wording to reflect whether a run completed, was stopped by policy, cancelled, or genuinely failed. Tests: - Add tests confirming the new pipeline-to-session status projection distinguishes deliberate stops from crashes and preserves existing terminal states. Co-authored-by: Stella Test <test@stella.local>
macanderson
added a commit
that referenced
this pull request
Aug 6, 2026
…pipe (#1838) (#1881) ## What & why The nine `scripts/check-*.sh` guards that #1815's sweep (PR #1844) left out already decide their verdict before printing, but their final green OK line was still an unguarded pipe write. With stdout piped into a reader that has already exited (`| true` is the deterministic repro), that write dies of SIGPIPE (exit 141) or fails with EPIPE and `set -e` turns it into exit 1 — either way a green verdict reports as a failure. Only the exit code was forged, which is exactly the half a caller reads. This PR copies the epilogue PR #1844 established (exemplar: `scripts/check-left-behind.sh`): the verdict is decided first, then the final write runs under `trap '' PIPE` with its failure discarded (`|| true`). Output text is byte-identical. Scripts hardened: - `scripts/check-action-pins.sh` — the non-fatal missing-tag-comment report is part of the same decided-green tail, so it rides under the same trap; the unreachable skip branches are left alone, matching #1844's treatment of `check-cargo-install-pins.sh` - `scripts/check-brand-case.sh` - `scripts/check-design-refs.sh` - `scripts/check-empty-diff.sh` - `scripts/check-license-allowlist-parity.sh` - `scripts/check-no-scratch.sh` - `scripts/check-no-secrets.sh` - `scripts/check-stat-portability.sh` - `scripts/check-wire-schema.sh` `scripts/test-guard-sigpipe.sh` (`make guard-sigpipe-test`) grows a case pair (`| true`, `| head -1`) per hardened guard: the seven scanning guards join the main loop, `check-empty-diff.sh` runs against the real `HEAD~1 HEAD` pair (it takes a `<base> <head>` pair rather than scanning the tree), and `check-wire-schema.sh` — which compiles the two schema exporters — is gated on a cargo toolchain being on `PATH`, skipping loudly otherwise, per the issue's allowance. Based on `fix/1815-guards-survive-sigpipe` (PR #1844) because that PR carries the harness and the epilogue pattern and has not merged yet; retarget to `main` after it lands. Closes #1838 Refs #1815 ## The witness - [x] This PR includes a witness test (fails on the old scripts, passes here) `scripts/test-guard-sigpipe.sh`, extended with 16 new cases, run both ways on this tree (cargo masked off `PATH`, so the wire-schema case skips): - **Old scripts + extended harness: 32 passed, 11 failed.** 8 of the failures are the new `| true` cases dying on the final OK write with **rc=141** (action-pins, brand-case, design-refs, license-allowlist-parity, no-scratch, no-secrets, stat-portability, empty-diff) — the forged exit this PR removes. The new `| head -1` cases pass on the old scripts because these guards emit a single line that fits the pipe buffer. - **Hardened scripts: 40 passed, 3 failed.** All 16 new cases pass. The 3 residual failures are identical in both runs and are **not** pipe-related: they are pre-existing main breaks reporting genuine verdicts at rc=1 (gate-parity's stale "twenty-four" step count, and file-size ceilings on `event.rs`/`deck_render.rs`), already covered by open unbreak PRs #1845 / #1863 / #1873. `check-wire-schema.sh` could not be exercised here (its harness case needs a cargo toolchain and pays a workspace build; this change was verified shell-only) — its epilogue is byte-for-byte the same shape as the eight witnessed ones, and its harness case will run wherever cargo is present. ## The gate - [x] `shellcheck` clean on all 10 touched scripts (`make shellcheck` set) - [x] No Rust touched — fmt/clippy/test unaffected by this diff - [x] Docs: no behavior/flag changes; harness header already documents the posture - [x] CLA signed - [x] `Closes #1838` appears both above and as a commit trailer ## Nothing left behind - [x] There is nothing new: the pre-existing main breaks the harness surfaced (gate-parity count, file-size ceilings) already have open unbreak PRs #1845 / #1863 / #1873 ## Ground-rule check - [x] No I/O added to `stella-core`; no new deps - [x] No new outbound network calls ## Anything reviewers should know? `trap '' PIPE` is process-wide from the point it is set, but in every script it is set only after the last verdict-bearing computation, so it can only affect the best-effort report writes — the same placement PR #1844 reviewed nine times. ## Summary by Sourcery Harden gate guard shell scripts against closed-pipe failures, align their reporting patterns, and add a dedicated test harness and Make target to ensure guards remain robust when their output is piped to early-exiting readers. New Features: - Add a dedicated guard-sigpipe-test Make target and scripts/test-guard-sigpipe.sh harness to verify gate guard behavior when their output pipes are closed early. Bug Fixes: - Prevent gate guard scripts from failing with SIGPIPE/EPIPE when their stdout is piped to readers that exit early by ensuring final report writes are best-effort and do not affect exit codes. Enhancements: - Refine multiple guard scripts to buffer verdict output before emission and use consistent reporting patterns that avoid subshell state loss and pipe races, including a more robust membership check in check-god-files.sh and simplified failure propagation in check-role-names.sh. Build: - Extend the Makefile with a guard-sigpipe-test target to exercise guard robustness against closed pipes. Tests: - Introduce a comprehensive test harness in scripts/test-guard-sigpipe.sh that runs key guard scripts under early-terminating pipe readers (e.g., `| true`, `| head -1`) to ensure exit codes remain correct and stable. --------- Co-authored-by: Stella Test <test@stella.local> Co-authored-by: Mac Anderson <macanderson.mail@gmail.com>
macanderson
added a commit
that referenced
this pull request
Aug 6, 2026
… turn to the terminal writers (#1893) ## What & why The goal loop (`run_goal_cmd` / `run_goal_turn` / `run_goal_pipeline_turn`) and the deck's lead turn (`run_lead_turn` / `run_lead_pipeline_turn`) answered with `Result<(), String>`, which has no room for the abort's typed `AbortKind` — so on their paths a deliberate stop (stuck-loop escalation, step cap, enforced budget) was indistinguishable from a crash by the time the terminal SESSIONS-registry status was written. A policy-stopped goal run recorded `SessionStatus::Error`, never `Stopped`, and exited `1` instead of `3`. This chases #1637's shape one level deeper, exactly along the seam #1862 specs: - **`stella-core`**: `GoalOutcome::Unmet` now carries `kind: Option<AbortKind>` — the typed kind of the working turn's abort, `None` for the backstops that are not turn aborts (round cap, unreachable verifier). `GoalOutcome` has no consumer outside `stella-cli`. - **`stella-cli` goal loop**: the three drivers answer with `crate::failure::CliFailure`. The folds that stringified `PipelineStatus::Aborted` / `GoalOutcome::Unmet` are now shared projections in `agent/outcome.rs` (`goal_round_break`, `goal_unmet_failure` — siblings of `pipeline_status_result` / `turn_outcome_result`, same messages as before), and `run_goal_cmd`'s terminal `presence.finish` projects the *real* failure through `daemon::outcome_status` instead of a reconstructed `CliFailure::error`. The inbox notification for a deliberate stop now says "stopped by policy" rather than "FAILED". - **`stella-cli` deck**: `run_lead_turn` / `run_lead_pipeline_turn` answer with `CliFailure` through the existing `turn_outcome_result` / `pipeline_status_result` projections, and the `session_exit` write reads `daemon::outcome_status` — one decider for every terminal writer (#1653/#1826/#1862). `command_deck.rs` shrank by 7 lines; `agent.rs` stayed at its exact ceiling (one line-neutral visibility edit: `pub(crate) mod outcome;`). Design exemplar: the same total-`match` projection module pattern `agent/outcome.rs` already established (and `std`'s "constructors on the error type" shape for `CliFailure::from_abort`) — no new patterns invented. Closes #1862 Refs #1826, #1653, #1637 ## The witness - [x] This PR includes a witness test (fails on `main`, passes here) Same family and justification as #1826's `an_unsupervised_deliberate_stop_projects_stopped_not_error`: - `agent::outcome::tests::a_policy_stopped_goal_round_projects_stopped_not_error` — the fold an aborted working round takes to the terminal registry write keeps `AbortKind::DeliberateStop`, and `outcome_status` projects `Stopped`; the `Failure` kind still projects `Error`. Fails on the old code the way #1826's witness does: the projection did not exist, and the old fold stringified the status so the terminal write could only reconstruct `CliFailure::error` → `Error`. - `agent::outcome::tests::a_policy_stopped_raw_goal_loop_projects_stopped_not_error` — the raw (`--no-pipeline`) half; the kind-less backstops (round cap) stay `Error`. - `stella-core`: `goal::tests::session_budget_caps_total_spend_across_rounds` now asserts the enforced-budget stop reaches `Unmet` as `Some(AbortKind::DeliberateStop)`, and `aborted_working_turn_ends_the_goal_loop` asserts a provider failure reaches it as `Some(AbortKind::Failure)` — neither pattern compiles against the old kind-less enum. ## The gate - [x] `cargo fmt --check` (touched crates) - [x] clippy `-D warnings` — `stella-cli` + `stella-core` clean. **Pre-existing break, not this PR's**: `stella-pipeline/src/pipeline/witness_stage.rs:55` fails `clone_on_copy` on the base branch and on `main`; open unbreak PRs #1873 / #1859 own it. I verified my crates lint clean with that one line patched locally (patch not included — a peer PR owns the fix). - [x] `cargo test -p stella-core` goal suite (18/18) and `cargo test -p stella-cli --bin stella` (1431/1431) — scoped per the 16GB-machine constraint; CI runs the full workspace. - [x] `RUSTDOCFLAGS="-D warnings" cargo doc -p stella-core -p stella-cli --no-deps` clean - [x] Docs: doc comments updated where the folds moved; no flags changed - [x] CLA signed - [x] `Closes #1862` above and as a commit trailer `scripts/check-file-size.sh` flags `stella-protocol/src/event.rs` (+2) and `stella-tui/src/deck_render.rs` (+3): both overages exist verbatim on the base branch and are named by unbreak PR #1873 — untouched here. ## Nothing left behind - **Base branch**: PR #1878 (`fix/1826-unsupervised-stop-status`) is not merged yet, and this change builds directly on its `finish(status, …)` projections — so this PR targets that branch as base. **Retarget to `main` after #1878 merges.** - Behavioral note, deliberate: a deliberately stopped goal run now exits `3` (was `1`), consistent with the exit-code taxonomy in `failure.rs` and with what #1637 already did for the resume driver; `bench/harbor_adapter` reads exactly this code. Also the deck's soft-stop (`SOFT_STOP_REASON`) session exit now records through `outcome_status` like every other writer. - The pre-existing `main` breaks encountered during verification (pipeline clippy, two file-size overages) are already tracked by open PRs #1873 / #1859 — nothing new to file. ## Summary by Sourcery Preserve typed abort information through goal and lead execution paths so terminal session status and exit codes distinguish deliberate policy stops from crashes. New Features: - Propagate AbortKind via GoalOutcome::Unmet and new CLI projections so policy-stopped goal runs now surface as "Stopped" with dedicated messaging instead of generic failures. Bug Fixes: - Ensure goal runs and lead turns that stop due to policy or enforced budgets are recorded as SessionStatus::Stopped and exit with the correct non-error code instead of being treated as crashes. - Fix deck soft-stop handling so the session exit status and user-facing error events are derived from the shared outcome projection rather than raw strings. Enhancements: - Standardize CLI failure handling for goal and lead flows by returning CliFailure instead of String and reusing shared outcome projection helpers. - Expose agent outcome helpers for reuse across goal and deck paths, consolidating terminal status decisions into a single outcome_status-based mechanism. - Extend goal-related tests in stella-core and stella-cli to assert that enforced budgets and provider failures carry the correct AbortKind through to session status projection. Tests: - Add witness tests under agent::outcome validating that policy-stopped goal rounds and raw goal loops project SessionStatus::Stopped instead of Error. - Update stella-core goal tests to assert that budget caps and provider failures are reflected as GoalOutcome::Unmet with the appropriate AbortKind values. --------- Co-authored-by: Stella Test <test@stella.local>
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
1 participant
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.
Problem
main's tip (5650c88) fails three gate steps locally under the pinned 1.97.0 toolchain (CI for the last several main commits is still queued, so the server-side verdict has not surfaced yet):
-D warnings:apply_role_shapingincrates/stella-pipeline/src/pipeline/witness_stage.rs:55clonesGenerationParams, which isCopy(clippy::clone_on_copy).crates/stella-protocol/src/event.rsgrew to 2964 (ceiling 2962) andcrates/stella-tui/src/deck_render.rsto 1531 (ceiling 1528) via already-merged PRs.GATE_STEPSruns 25 steps; AGENTS.md and CONTRIBUTING.md still spelled the count "twenty-four" (their step lists were already current).Every open PR inherits all three until this lands.
Approach
Some(params.clone())→Some(*params). No behavior change — the clone and the copy are byte-identical.make file-size-update, committed as the reviewable baseline diff the guard asks for. It raises the two ceilings the merged growth already spent (+2/+3) and retightens three files that shrank on main:command_deck.rs4740→4691,fleet_cmd.rs1507→1504,pipeline.rs3642→3580.Verification
cargo clippy -p stella-pipeline -j 2 -- -D warnings— clean after, fails on main's tip before.make guards-fast— exits 0 on this branch, fails on main's tip (file-size, then gate-parity).Found while shipping the fix for #1826 (that PR notes this dependency). Per the parallel-unbreak rule: if a peer unbreak lands first, re-verify main still fails before merging this.