fix(stella-pipeline): unbreak main — four breaks two merges left behind, incl. a silently deleted #1793 witness - #1971
Merged
Conversation
`cargo clippy -p stella-pipeline --all-targets -- -D warnings` fails on b5ab7f8. A compile error in the lib masks the test target entirely, so this reads as one failure and is really four, from two different merges. From #1953 (the #1778 research stage): 1. `management_prompt/tests.rs` — `ModelCallRole::Research` is a new variant and `management_system_block`'s match is exhaustive on purpose (E0004). Research rides the sub-agent primitive, so its system prompt travels on the `SubAgentSpec`, never through `metered_raw_call`: it joins the never-dispatched arm, and `ALL_ROLES` grows to 15. 2. `pipeline.rs` — the new `research` parameter pushed `plan_stage` to 8 arguments, one over clippy's cap. Bundled `budget`/`total` into the `Spend` struct every stage downstream of the fan-out already takes, rather than `#[allow]`-ing the lint. From #1951, which rewrote `tests/verification_hardening.rs` wholesale and dropped three items #1945 had added to it hours earlier — a same-seam clobber, in a file #1951's own subject (per-candidate verifier degradation) never needed to touch: 3. `PassingShell` and `shell_call_result` went with it, leaving the child module `flip_halt_arming.rs` referencing two helpers that exist nowhere in the tree (E0425 ×2). Restored to their original home, which the child reaches through `use super::*`. 4. `a_revision_halts_at_the_step_where_the_tracked_test_flips` went too — the configured-command **witness for #1793**. Deleting it did not fail any gate, because the crate stopped compiling for reason 3 first: #1793 has been shipping with half its witness silently gone. Restored verbatim. Both #1793 witnesses now run and pass. Neither is vacuous: each asserts a scripted-prompt count, so a `PassingShell` that omitted the `[exit code: 0]` marker `flip_halt::exit_status` parses would leave the halt unarmed, the revision would consume the steps scripted beyond the flip, and the count would be wrong. `cargo test -p stella-pipeline`: 605 passed, 0 failed. `cargo clippy -p stella-pipeline --all-targets -- -D warnings`: clean.
![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
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Reviewer's GuideFixes stella-pipeline main by addressing clippy/compiler failures introduced by two merges, wiring the new Research role into management prompts, refactoring plan_stage budget handling to use Spend, and restoring deleted verification hardening helpers and a key #1793 witness test so tests and clippy pass cleanly. Sequence diagram for Spend usage in plan_stage budget handlingsequenceDiagram
participant ScopeStage
participant Pipeline
ScopeStage->>Pipeline: scope_stage(budget, total)
ScopeStage->>Pipeline: Spend{budget, total}
loop per_revision
ScopeStage->>Pipeline: plan_stage(goal, scope, research, repo_structure, revision, spend)
Note right of Pipeline: spend bundles budget and total to satisfy clippy arg cap
Pipeline->>Pipeline: metered_raw_call(..., spend.budget, spend.total)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
4 tasks
Owner
Author
|
Branch updated from Your
So the two PRs deadlock: this one can't be green without #1970's doc fix, and #1970 can't be green without this one's pipeline fix. Neither is a duplicate of the other; together they are the whole repair. Suggested order: land this one once the ratchet clears (its only remaining red will be that known, separately-fixed doc link), then #1970 goes fully green on a branch update and auto-merges — it is already armed. For the record, the root cause of all of it: #1953 merged with four checks already failing (ratchet, clippy, doc, test), which |
Owner
Author
|
Heads-up from a parallel session — I verified this branch fixes every break I independently found on main ( Restoring the #1793 witness (+123 lines) pushed The witness restoration is the right call and must not be dropped to satisfy the guard — the fix is to move it (or an existing sibling group) into a child module under Verified on my side by merging this branch with mine locally: with these two guards set aside, Related: my #1954 fix is #1978, which touches disjoint files ( |
Restoring `a_revision_halts_at_the_step_where_the_tracked_test_flips` in the parent commit took `tests/verification_hardening.rs` to 1557 lines, which `file-size` rejects outright — the baseline takes no new entries. Split rather than exempted, and the split is the one the content was asking for: both #1793 witnesses and the two doubles they share (`PassingShell`, `shell_call_result`) now live in `verification_hardening/flip_halt_arming.rs`, the module already named for the concern. The parent drops to 1434. That the two witnesses were ever in separate files is what let #1951's clobber happen quietly: it rewrote the parent wholesale, taking the configured-command witness and both doubles with it, and nothing failed that named the missing test — the crate had already stopped compiling for the missing doubles. With the cluster in one file the same rewrite is a merge conflict instead of a silent deletion, so the module doc says so. `cargo test -p stella-pipeline`: 605 passed, 0 failed — both witnesses among them, at their new path.
macanderson
force-pushed
the
unbreak-main-pipeline-tests
branch
from
August 7, 2026 02:19
2b33d45 to
2aa2586
Compare
Owner
Author
|
Filed #1997 for the systemic gap this PR exposed: a test can be deleted by an unrelated PR and no gate objects. This PR fixes the instance (restores the #1793 witness, co-locates the cluster so the next wholesale rewrite is a merge conflict); #1997 carries the question of whether that should be mechanically enforced, with |
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…ead's tasks rows (#1708) (#1967) ## What A sub-session worker's closeout wrote **its own** private task board into the shared `tasks` table under the **lead's** `session_id` (`crates/stella-cli/src/subsession.rs`, `run_worker`). At the table's `UNIQUE(session_id, task_id)` key that write was pure corruption, twice over: 1. **Ordinal collision** — a worker board numbers from "1" in its own namespace, so its task "1" upserted over the lead's unrelated task "1". 2. **`/clear` seal bypass (#1692)** — the seal that quarantines pre-clear workers lives on the driver (`SubSessions::seal_task_board` → `session_clear::settle_worker_task`); this write ran on the worker's own thread where no seal can reach, so a pre-clear worker repopulated the mirror the user destroyed. ## The decision (of the three shapes in #1708) **Drop the write** — the shape #1708's investigation identified as the only one that fixes both defects alone: - The write's stated purpose ("so `tasks` queries see sub-agent boards too") is unachievable at this key — neither reader (`Store::list_session_tasks`, the Observatory's sessions query) can distinguish a worker row from a lead row. - The delegation outcome the session cares about is the **lead's** board row, which the driver already mirrors at both of its own write sites (the lead's turn end in `command_deck.rs`, worker settlement in `session_clear::settle_worker_task`). - Namespacing fixes only defect 1; routing through the driver fixes only defect 2. Dropping fixes both, with no migration and no cross-thread plumbing — and it makes the `/clear` seal airtight rather than advisory, because the driver becomes the session mirror's *sole* writer. ## The witness seam (the part #1708's investigation could not find) The worker's store closeout moves to a new sibling module, `crates/stella-cli/src/subsession/closeout.rs` — required anyway, since `subsession.rs` sits 24 lines under the file-size gate's 1500-line ceiling and could not absorb tests. `close_worker_execution` is the exact production path `run_worker` calls, and its signature takes **no session id**, so the closeout structurally cannot address session-keyed rows. Two witness tests drive that seam against a real in-memory store: - `a_workers_private_board_never_lands_in_the_sessions_tasks_rows` — the lead's mirror survives a worker closeout byte-identical, the table row **count** is pinned too (so a NULL-session append can't hide), and the execution row still closes (so "fixed by deleting the closeout" can't pass). - `a_pre_clear_workers_closeout_cannot_repopulate_a_cleared_mirror` — after `clear_session_tasks` (the persisted half of `/clear`), a worker closeout leaves the mirror exactly as empty as the user made it. **Fail→pass flip, demonstrated:** the seam is new, so `git stash` alone can't show the flip. Instead the pre-#1708 board mirror was temporarily grafted back into `close_worker_execution` (same write, keyed to the lead's session exactly as `run_worker` was) — both witnesses **fail** against the graft with the exact corruption #1708 describes ("the lead's mirror survives a worker closeout untouched" assertion trips), and **pass** with it removed. The graft was reverted before shipping. ## Verification Measured on this branch **with current `main` merged in** (head `73f99926`): - `cargo test -p stella-cli` — **1456 pass**, 0 fail, including both new witnesses - `cargo clippy -p stella-cli --all-targets -- -D warnings` — clean - `cargo fmt --check` — clean - `make guards-fast` (all 25 guards) — clean; `file-size` reports **"none grew"** against main's regenerated baseline ## CI is red for reasons outside this diff `fmt + clippy + test` fails on **five breaks inherited from `main`**, none in a file this PR touches (the diff is 3 files, all `stella-cli`): | Break | Crate | Already covered by | |---|---|---| | `unresolved link to SkipReason::NoResumePoint` (`daemon/boot.rs:59`, added by `a2806246` / #1939) | stella-cli | **#1970** | | `too many arguments (8/7)` (clippy) | stella-pipeline | **#1971** | | `PassingShell` not found | stella-pipeline | **#1971** | | `shell_call_result` not found | stella-pipeline | **#1971** | | non-exhaustive match: `ModelCallRole::Research` not covered | stella-pipeline | **#1971** | The last one is the root cause of three of these: `main` added a `Research` variant to `stella_protocol::ModelCallRole` without updating downstream exhaustive matches. This branch point has no `Research` variant at all, which is why the break is provably inherited rather than caused. No competing unbreak is included here deliberately — duplicate PRs racing the same seam is how `main` gets re-broken. This PR should go green once #1970 and #1971 land; it is already merged up to current `main` and conflict-free. No migration: the corrupted rows self-heal — the driver's next lead-board mirror upserts the colliding ordinals back to the lead's state, and `/clear` deletes the rest; stray rows beyond the lead's ordinal range in existing databases are dead rows in a table whose only production readers are per-session queries the driver now exclusively feeds. Exemplar for the shape: the pipeline's witness stage gives authored witnesses exactly this lifetime — scaffolding for one run, discarded with it — which is the model applied to a worker's private board here. ## Filed, not fixed - **#1968** — this PR deliberately closes the door on persisting sub-agent boards. If they are wanted, it takes a `lane` column + migration, routing through the driver, *and* a lane-aware reader, all three together; the issue scopes each and says why any one alone reintroduces a defect. - **#1969** — existing databases already hold corrupt rows from this bug, and they are **indistinguishable by construction** (that is #1708's whole complaint), so no migration can repair them. They render in the Observatory's Sessions tab. Needs a maintainer's call between accept / caveat / document. Refs #1692, #1631. Closes #1708
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…ellation (#1954) (#1978) ## What A caller that drops a sub-agent future mid-flight — a latency ceiling, a hard cancel — left `Started` open forever. Every ceiling-bearing caller therefore had to forge its own `Finished`, and could only guess `steps: 0`, because the committed-call count lived inside the dropped turn. The pre-plan research stage (#1778, merged as 1bdf2da) was the first pipeline caller to hit this and forged exactly such a bracket. `CancelBracket` moves the obligation into the primitive (`crates/stella-core/src/subagent.rs`) — design 2 of the two the issue proposed, so the next caller with a ceiling inherits the fix instead of repeating the bug. This is the same argument that moved the goal verifier onto `Engine::run_sub_agent`. **Drop order does the sequencing**, which is what makes the numbers honest: the in-flight turn future is declared *after* the guard, so it drops first — the engine's own cancel guard has already emitted the abandoned call's `UsageIncomplete { Cancelled }` envelope and `SettleChildOnDrop` has already folded the money back by the time the bracket closes. The bracket therefore reports only what was **committed**; the in-flight call's usage rides its own envelope and is never guessed at here. The two committed numbers travel as one `CommittedTally` type rather than two loose `Arc`s — a bracket reporting a step count without the cost that produced it would be half an answer, and bundling them also keeps `run_child_turn` under the argument cap without an `#[allow]`. `research_stage.rs` drops its forged bracket accordingly. ## Witness Both fail on the old code and pass with the change — verified by checking out `origin/main`'s copies of the two production files and re-running: - `subagent::tests::a_cancelled_child_closes_its_bracket_with_committed_steps_and_cost` — on old code the event stream contains `Started` with **no `Finished` at all**. (The dump also shows `UsageIncomplete { Cancelled }` already present on old code, which is the evidence that only the bracket needed owning.) - `pipeline::tests::research::a_child_past_the_ceiling_closes_its_bracket_with_committed_steps` — the issue's verbatim scenario: balanced bracket, `UsageIncomplete` with reason `cancelled`, and `Finished.steps` equal to the committed `StepUsage` count. Old code fails on `"Finished.steps is the committed StepUsage count, not a forged zero"`. No protocol change was needed — `UsageIncompleteReason::Cancelled` already exists, so there is no wire-schema regeneration. ## Verification `main` is currently red (see #1971), so these were run on a local merge of this branch with `unbreak-main-pipeline-tests`: - `cargo test -p stella-core -p stella-pipeline` — 0 failures - `cargo clippy -p stella-core -p stella-pipeline --all-targets -- -D warnings` — clean - `cargo fmt --all -- --check` — clean - `scripts/check-file-size.sh` on this branch alone — OK, none grew **This PR's CI will stay red until #1971 lands**, for reasons that have nothing to do with this diff. Closes #1954 ## Summary by Sourcery Ensure sub-agent cancellations close their Started/Finished bracket with accurate committed steps and cost, and rely on the core primitive rather than callers to emit synthetic finishes. Bug Fixes: - Preserve a balanced Started/Finished sub-agent bracket on mid-flight cancellation, reporting the true committed step count and cost instead of leaving it open or forging zeros. Enhancements: - Track committed sub-agent step count and cost via a shared tally that survives cancellation, and introduce a cancel guard to emit the final Finished event when the sub-agent future is dropped. Tests: - Add core and pipeline tests that deterministically cancel hanging sub-agents to verify brackets close correctly, usage incomplete events are emitted, and budgets settle the committed spend.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
… board in isolated runs (#1719) (#1995) Closes #1719 > **Stacks on #1971.** `main` is red, and this branch carries #1971's unbreak commits so it can compile and be tested. Once #1971 merges, this PR's diff reduces to the single commit described below — 7 files, +431/−25. ## The defect The PLAN rail shows every step as a hollow grey `○` for the whole turn. The panel's own dot moves (`PLAN ● approved 0/7`) but no individual step ever reaches started / complete — the rail reports a plan and never reports progress through it. A step's ring moves only on `AgentEvent::TaskUpdate`, whose only emitter is the deck's session `TaskTap`. `Pipeline::run_isolated_candidate` builds its engine on `ws.tools()` — the candidate workspace's own stack — and `TaskTap` is not in that chain. Two independent failures follow, either of which alone freezes the rail: 1. **The board is private and unseeded.** The forwarder seeds the *session* registry's board from the approved proposal, but a candidate `ToolRegistry` builds its own empty one. `task_start "3"` answers `UnknownTask` for a step the gate already numbered 3. A worker routing around it with `task_create` then builds rows whose subjects *overwrite* the approved step titles. 2. **Nothing announces it.** Even a correct mutation reaches no surface. Execution 66 of a real session: 1168 events, **zero** `task_update`, while the model called `task_create` ×3 and `task_start` ×1 exactly as the system prompt asks. This is the same shape as the already-fixed "isolated runs blind the Files tab" bug — see the `attach_read_events` comment in `candidate_ws.rs`. The task board was missed in that sweep. ## The fix **Seeding** — `CandidateWorkspace` grows a default-no-op `seed_task_board(steps, announce)`. The fan-out stage drives it right after `create`, before any dispatch, so the worker's very first `task_start` already resolves the gate's ordinals. It passes the plan's step descriptions verbatim, which is what `scope::build_proposal` renders the approval card from — so a candidate board and the session board seeded from `ScopeReview` cannot disagree on a step's id or title. **Announcing** — the candidate tool stack gains a `TaskTap`-shaped decorator (`candidate_ws/task_events.rs`), outermost over registry + customs + candidate MCP + policy, so it observes every route a `task_*` call can take. **Why the `announce` latch, and why boards stay private.** The issue notes `CandidateWorkspacePort::create` carries no fan-out width, so the "how many candidates may report" decision cannot read `n`. It is taken in `create_candidate_workspaces`, which is the one seam that knows both `n` and the plan. `TaskUpdate` carries a full board snapshot with no candidate tag, so several boards reporting onto one channel would splice into a checklist that is nobody's — the same reasoning that already mutes `TextDelta`/`Reasoning` on a shared event lane. Only a lone candidate announces. Boards are **never** shared, at any width: `TaskBoard::set_status` rejects a transition out of a terminal state, so the second sibling to finish a step would receive a tool error for work it really did. ## Definition of done, item by item | From the issue | Where | |---|---| | An isolated candidate's `task_*` moves the gate-numbered step and emits `TaskUpdate` | `a_seeded_lone_candidate_moves_the_approved_step_and_announces_it` | | Best-of-N does not regress — candidates must not share one board | `fanout_candidates_keep_separate_and_silent_boards`, `best_of_two_adopts_only_the_winner_and_removes_every_workspace` | | A host rendering no plan (fleet worker, headless) is unchanged | Default no-op on the port; `events: None` skips the tap entirely; the witness author's pristine snapshot is created via `port.create()` directly (`witness_stage.rs:615`), never seeded, so its latch stays `false` | | Witness proving the fail→pass flip | below | ## Witness The new API means the tests cannot compile on `main` (the feature is genuinely absent). That is the weaker half of the proof, so both halves of the fix were **independently neutered** on this branch to show each is load-bearing: **Seeding neutered** — reproduces the issue's exact symptom: ``` a_seeded_lone_candidate_moves_the_approved_step_and_announces_it ... FAILED the gate-numbered step must resolve on the candidate board: Error { message: "no task with id 1 — call task_list to see the board" } fanout_candidates_keep_separate_and_silent_boards ... FAILED ``` **Announcing neutered:** ``` a_seeded_lone_candidate_moves_the_approved_step_and_announces_it ... FAILED a lone candidate's task_start must announce a TaskUpdate ``` (The fan-out test correctly still passes with the tap dead — it asserts *silence*, which a dead tap trivially satisfies. Its real job, the separate-boards guarantee, fails in the seeding run above.) Restored, both pass. The two CLI-side witnesses run against **real git worktrees** through the production `GitCandidateWorkspaces` port, not a fake. ## Constraints honoured - `candidate_ws.rs` was near the 1500-line ceiling with no baseline entry, so it could not cross it at all: it takes **+14 lines** (1480 → 1494) and all new logic lands in the new sibling `candidate_ws/task_events.rs`. - `pipeline.rs` is a god file — **+0 net lines** (unchanged at its 3462 ceiling). - No baseline entry added or raised. ## Verification - `cargo test -p stella-pipeline -p stella-cli` — **2121 passed, 0 failed** (20 binaries) - `cargo clippy -p stella-pipeline -p stella-cli --all-targets -- -D warnings` — clean - `make guards-fast` — green, `file-size` and `god-files` included ## Summary by Sourcery Ensure isolated candidate workspaces share the approved task plan and surface task progress while tightening flip-halt and management accounting coverage. New Features: - Seed each candidate workspace with the approved plan steps and optionally allow a lone candidate to announce task-board updates on the shared event stream. Enhancements: - Extend the candidate workspace port to accept plan step seeding and an announce latch, and wire this through the fan-out stage and Git-backed candidate workspaces. - Clarify management accounting for the Research role and refactor planning budget handling to use a shared Spend wrapper parameter. - Add a passing-shell tool executor and supporting helpers to exercise flip-halt behavior when observing real test commands. Tests: - Add pipeline and CLI tests to verify seeded candidate task boards resolve gate-numbered steps, keep per-candidate boards, and only announce from lone candidates. - Add a configured-command flip-halt witness test to ensure revisions halt when the tracked test flips from fail to pass. - Update management accounting tests to work with the new Spend wrapper and expanded role set. - Extend best-of-N isolation tests to assert that seeded task boards remain private and silent in multi-candidate runs.
This was referenced Aug 7, 2026
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…hree things and the merge kept both #1985 and #1971/#1995 independently repaired the breaks #1953 left, converged on the same designs, and landed within minutes of each other. Git merged the two additively rather than conflicting, so `main` at e0fbbe0 carries each fix twice and fails `cargo clippy -p stella-pipeline --all-targets -- -D warnings` three ways: 1. `management_prompt/tests.rs` — `ModelCallRole::Research` appears twice in the same or-pattern (`unreachable_patterns`). Kept one. 2. `pipeline/scope_stage.rs` — both PRs bundled `plan_stage`'s budget+total into `Spend`, but the call site kept #1985's per-iteration reborrow AND the other's hoisted `let mut spend`, now unused (`unused_variables` + `unused_mut`). Kept #1985's: the loop replans after a rejected scope card, and only a reborrow per attempt survives that. 3. `tests/verification_hardening.rs` — both restored `PassingShell` and `shell_call_result` after #1951 deleted them, one into this file and one into its `flip_halt_arming` child, leaving the parent's pair dead (`dead_code` ×3, counting `SHELL_TOOL`). For (3) the two copies were not equivalent, so this is not an arbitrary pick: #1985's are better documented — they name `SHELL_TOOL` as a const distinct from `WRITING_TOOL` and say why the `[exit code: 0]` marker is load-bearing (without it the halt never latches and the arming test passes for no reason). Those are the ones kept. They move to the child, which is where both #1793 witnesses now live, because co-location is what makes the next wholesale rewrite of the parent a merge conflict instead of the silent deletion that started this (#1997). The parent's now-stale `mod` doc is corrected in place rather than left describing a layout that no longer holds. `cargo clippy -p stella-pipeline --all-targets -- -D warnings`: clean.
5 tasks
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…d match arm, and the shell doubles #1971 left orphaned Three more `main` failures, all of which the earlier lib-level errors were hiding: `cargo clippy --all-targets` stops at the first crate that fails, so `stella-pipeline`'s *lib test* target was never checked until the `spend` fix let the lib compile. Each is in a file this branch had not otherwise touched. `cargo fmt --check` — `event/tests.rs` lost its trailing newline in #1994's split: Diff in crates/stella-protocol/src/event/tests.rs:1487: mod tag_table; + Fixed by `cargo fmt --all`; that one byte is the entire formatting diff. `unreachable pattern` — `management_prompt/tests.rs` names `ModelCallRole::Research` twice in the same or-pattern: error: unreachable pattern --> crates/stella-pipeline/src/management_prompt/tests.rs:98:11 90 | | ModelCallRole::Research <- matches all the relevant values 98 | | ModelCallRole::Research <- no value can reach this Dropped the trailing one. The surviving arm is the one the comment directly above the match names ("`Research` (#1778) rides the sub-agent primitive"), and it sits in the position that comment describes; the copy after `Summarization` is a merge appending a role that was already there. `dead_code` ×3 — `SHELL_TOOL`, `shell_call_result` and `PassingShell` in `verification_hardening.rs` are unreachable, because `flip_halt_arming.rs` defines its own `PassingShell` and `shell_call_result` and an explicit item shadows a `use super::*` glob. Nothing errored when #1971 restored the deleted witness with its own doubles; the parent's pair just went quietly dead. Deleted the parent's three rather than the child's two, because the child's module doc is the normative statement and it argues for exactly this layout: They were split apart once already, and the parent's next wholesale rewrite deleted the configured-command witness and both doubles without failing a gate [...] Keeping the cluster in one file is what makes that clobber a merge conflict instead of a silent deletion. The child is already self-contained — its doubles spell the tool name `"bash"` inline and never read the parent's `SHELL_TOOL` — so removing the parent's copy takes nothing away and restores the one-cluster-one-file invariant that #1971 and #1997 exist to protect. Retargeted the parent's `mod` doc, which still claimed the child reaches the shell fakes through this file.
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
… stacked behind one clippy error (#2000) ## What & why `main` was red at `e0fbbe02` on **five distinct breaks stacked behind one another**. CI's log showed only the first, because both compile-tier gates report one unit at a time: clippy stops at the first *hard* error in a crate, and `cargo doc --workspace` stops at the first crate that fails to document. That is why this is the sixth consecutive unbreak PR — each one can only reveal the next layer. Root cause tracked in #1986 (`ci.yml` does not run on a push to `main`); evidence from this session added there. ### 1. `pipeline/scope_stage.rs:34` — dead `spend` local ``` error: variable does not need to be mutable error: unused variable: `spend` ``` `plan_with_review` binds `let mut spend = Spend { budget, total };` and never reads it — the re-planning loop builds a fresh `Spend` by reborrowing on each iteration, which is the only construction the code uses. Dead since #1971, unmasked when #1985 cleared the `plan_stage` arg-count error above it. ### 2. `management_prompt/tests.rs` — `ModelCallRole::Research` listed twice `unreachable_patterns`. Kept the documented placement beside `Unknown`, whose comment explains why `Research` never reaches the chokepoint; dropped the copy appended after `Summarization`. ### 3. `verification_hardening.rs` — three items nothing constructs `dead_code` ×3 on `SHELL_TOOL`, `shell_call_result`, `PassingShell`. The child `flip_halt_arming` module defines its own, which shadow the parent's through `use super::*` — a glob import loses to a local definition silently, so this was never a name clash, just quietly unreachable code. The child's are the live pair *and* the newer one: a per-command `call_id: format!("call-shell-{command}")` that `FlipHalt` correlates on, versus the parent's fixed `"call-shell"` which cannot distinguish two shell calls. So the parent's stale copies go. Its `mod` doc claimed the child existed in order to reach the parent's fakes — the pre-split rationale, now false — and is rewritten to point at the child's own doc, where the anti-clobber reason for colocating them lives (#1997). **#2 and #3 are the same shape**: a merge landed the same addition twice. Neither side conflicts textually, so review saw nothing. ### 4. `stella-protocol/src/event.rs` — unresolved intra-doc link `AgentEvent::Compaction::rewrites` documents itself with `[`CompactionRewrite`]`, but `event.rs` never imports the type (the field spells it `crate::CompactionRewrite` inline), so `broken_intra_doc_links` failed `doc-warnings`. A *different gate step* from #1–#3, invisible while clippy was red. Fourth recurrence of the shape #1986 tracks. ### 5. `file-size` — two ceilings exceeded on `main` `driver.rs` at 2572/2571 and `pipeline/tests.rs` at 2537/2536. **Neither file is touched by this branch**; both were grown on `main` by merges that did not regenerate the baseline. That mechanism is #2004. Also regenerated `docs/wire/*` — the protocol types' doc comments *are* that contract, so break #4's fix mechanically changed the emitted `description`. ## About the two raised ceilings A raised ceiling is normally a defect, so this is stated plainly rather than buried: `make file-size-update` moved `driver.rs` and `pipeline/tests.rs` up by one line each, for growth **this branch did not author**, because the growth has already landed on `main` and reverting another PR's line is outside this task. The alternative was leaving the gate red. The same regeneration also **tightens** `pipeline.rs` from 3451 to 3181 — a 270-line shrink the baseline had not captured. This branch adds no lines to any god file. A maintainer who would rather see those two lines pushed into submodules should say so; that is their call, not mine. ## The witness - [x] No witness test. Four of the five are dead code, a duplicate match arm, and a doc link — no runtime behavior exists to witness, and the compiler is the oracle. The fifth is a generated baseline. Per CONTRIBUTING's carve-out for changes with no behavior delta, here is how it was verified instead: - `cargo clippy --workspace --all-targets -- -D warnings` — fails on `main` at break #1, exits 0 here. - `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps` — fails on `main` at break #4, exits 0 here. - `cargo fmt --all --check` — exits 0. - `cargo test -p stella-pipeline` — 618 pass, 0 fail. That suite **did not compile at all** on `main` (break #3), so these tests were not running. - `scripts/check-file-size.sh` and `make wire-schema` — both exit 0. Exit codes were read from cargo directly, not through a pipe: `cargo … | tail` reports *tail's* status, which is always 0, and cargo colorizes `error` so a plain `^error` grep matches nothing. Both produced a false green in this session before being corrected. ## The gate - [x] `file-size` and `god-files` pass; baseline regenerated, never hand-edited. - [x] `wire-schema` passes; the diff is comment-only — no field added, removed, renamed or re-tagged, and no optional field made required, so the additive-only contract holds. - [x] No behavior change, no new flags, no new dependencies. ## Nothing left behind - **#2013** (filed) — sharing `CARGO_TARGET_DIR` between worktrees produces compile errors naming symbols that do not exist. Hit during this work: a phantom `E0004` for `AgentEvent::TurnParked`/`TurnWoken`, variants present in neither checkout, because a parallel job's build was linked in. Nearly caused a wrong "fix". - **#1986** — commented with the full five-layer breakdown as evidence for fixing the trigger rather than the instances. - **#2004** — owns the file-size baseline skew behind break #5. Refs #1972, #1986, #1997, #2004, #2013
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…e same three things, plus the file-size ratchet blocking every PR (#2008) >⚠️ **Overlaps #2000 — merge exactly one of these, never both.** We built the same unbreak in parallel and reached the *identical* resolution on all three collisions. This PR additionally fixes a fourth break (the file-size ratchet) that is currently failing #2000's checks and every other open PR. If #2000 picks up that one commit, close this; otherwise close #2000. Merging both is how the collision being fixed here happened. ## Why main is red `main` at e0fbbe0 fails `cargo clippy -p stella-pipeline --all-targets -- -D warnings` **and** `file size ratchet`. Two unbreak PRs (#1985, and #1971 via #1995) independently repaired the breaks #1953 left, **converged on the same designs**, and landed minutes apart. Git merged them additively rather than conflicting, so main now carries each fix twice: | # | Break | Where | |---|---|---| | 1 | `ModelCallRole::Research` twice in one or-pattern (`unreachable_patterns`) | `management_prompt/tests.rs` | | 2 | Both a hoisted `let mut spend` **and** a per-iteration reborrow (`unused_variables` + `unused_mut`) | `pipeline/scope_stage.rs` | | 3 | `PassingShell`/`shell_call_result` restored into *both* the parent and its child (`dead_code` ×3) | `tests/verification_hardening.rs` | | 4 | Two grandfathered files one line over their ceiling | `scripts/file-size-baseline.txt` | ## The judgment calls **(2) — kept #1985's per-iteration reborrow, not the hoisted binding.** Not arbitrary: `plan_with_review` loops, replanning after a rejected scope card, and only a `Spend` reborrowed per attempt survives that. The hoisted version would have been moved on the first iteration. **(3) — kept #1985's doubles, in the child.** The two copies were *not* equivalent. #1985's are better documented: they name `SHELL_TOOL` as a const distinct from `WRITING_TOOL`, and say why the trailing `[exit code: 0]` marker is load-bearing — without it `FlipHalt::observe` never latches and the arming test passes for no reason. Those are the ones kept. They live in `flip_halt_arming` with both #1793 witnesses, because co-location is what turns the next wholesale rewrite of the parent into a merge conflict instead of the silent deletion that started this (#1997). The parent's `mod` doc is corrected in place rather than left describing a layout that no longer holds. **(4) — recording growth that already merged, and saying so.** Two ceilings go **up** by one line each: ``` crates/stella-core/src/driver.rs 2571 → 2572 crates/stella-pipeline/src/pipeline/tests.rs 2536 → 2537 ``` Per CLAUDE.md, a raised ceiling to turn a gate green is normally a defect against the PR that raises it, so this is flagged rather than slipped through. The difference: **this branch touches neither file.** Both grew on main via #1979 and #1962, which did not regenerate the baseline in the same commit. The choice is therefore not "grow or don't" but "record what already merged, or leave main red for everyone". The two lines are somebody's to reclaim; neither is mine to judge irreducible. The same regeneration **tightens** `pipeline.rs` from 3451 to 3181 — 270 lines of stale headroom now closed off, which is the ratchet working as intended and more than offsets the two. Regenerated via `make file-size-update`, never hand-edited. ## Verification - `cargo clippy -p stella-pipeline --all-targets -- -D warnings` — clean - `cargo test -p stella-pipeline --lib` — **596 passed, 0 failed**, both #1793 witnesses among them - `make guards-fast` — green, `file-size` and `god-files` included ## Related - #1997 — why a deleted test failed no gate in the first place - #1985, #1995, #2000 — the colliding unbreaks ## Summary by Sourcery Unbreaks main by reconciling overlapping clippy and test fixes in stella-pipeline, consolidating flip-halt arming test doubles, and updating the file-size baseline so guards and ratchet checks pass again. Bug Fixes: - Resolve unreachable pattern warning in management_prompt tests by removing the duplicate ModelCallRole::Research arm - Fix clippy unused variable warnings in scope_stage by relying on per-iteration Spend reborrows - Restore and colocate shell tooling doubles for flip halt arming tests so dead-code warnings are cleared while preserving #1793 coverage Enhancements: - Clarify documentation and structure of flip halt arming tests by moving shared shell doubles into the child module and updating the parent module description Build: - Regenerate file-size baseline to reflect recent growth in driver.rs and pipeline tests while tightening the pipeline.rs ceiling so file-size ratchet gates pass again
macanderson
added a commit
that referenced
this pull request
Aug 7, 2026
…es left behind (#2014) ## What & why `main` is red at `6c345532` on **three separate gates** — `cargo fmt --check`, `cargo clippy -D warnings`, and the file-size ratchet — plus workspace rustdoc. Every open PR inherits all of it. The cause is not one bad change. Four sessions fixed the *same* red base concurrently (#1964, #1970, #1971, and an earlier push to #1964's branch). The merge that closed #1964 resolved every overlap by **keeping both sides**, which is the dangerous resolution here: it produces code that still compiles, so nothing conflicted and nobody had to look at it, and the damage only shows up under `-D warnings`. ### clippy (`-D warnings`) — four merge artefacts | Site | Lint | |---|---| | `management_prompt/tests.rs` | `ModelCallRole::Research` appears **twice** in one `\|` chain → `unreachable_patterns` | | `pipeline/scope_stage.rs` | a hoisted `let mut spend` **and** a per-iteration inline `Spend` → `unused_variables` + `unused_mut` | | `tests/verification_hardening.rs` | `SHELL_TOOL`, `shell_call_result`, `PassingShell` duplicated into the `flip_halt_arming` child → three `dead_code` | In each case the duplicate is deleted and the *used* copy kept. For `scope_stage` that is the inline `Spend`, because the loop replans after a rejected scope card and a moved bundle could not be handed to the next attempt — #1971's comment beside it already says so. ### fmt `crates/stella-protocol/src/event/tests.rs` is missing the trailing newline `rustfmt` wants after `mod tag_table;`. Unrelated to the merges and failing on its own. ### rustdoc `StepUsage` links a bare `` [`CompactionRewrite`] ``, re-exported at the crate root but never in `event`'s scope — the next line already spells the field `crate::CompactionRewrite`, so the link now matches. This one was **invisible** until the `stella-cli` link above it was fixed: `cargo doc` stops at the first failing crate, so a broken link one dependency layer down masks every link beneath it. Third occurrence of that pattern here. ### file-size ratchet `driver.rs` and `pipeline/tests.rs` each sit one line over a stale ceiling. Regenerated with `make file-size-update` rather than hand-edited — which is why the diff mostly **tightens**: `pipeline.rs` drops 3451 → 3181 and `bus.rs` 2126 → 1891. Both were already true and neither was recorded. ## The witness No witness test: this is a build/lint/format repair with no behaviour change. The gate *is* the witness, and each failure was reproduced locally before and after. ## The gate Run on this tree, not inferred: - [x] `make guards-fast` — all 25 guards plus `cargo fmt --check` - [x] `cargo clippy --workspace --all-targets -- -D warnings` - [x] `RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps` (24 crates) - [x] `cargo test --workspace` - [x] `./scripts/check-file-size.sh` and `check-god-files` ## Nothing left behind Already filed and linked, not duplicated: - **#1986** — `ci.yml` does not run on a push to `main`, which is *why* all eight of these landed unnoticed. This PR is the fourth cleanup in a row caused by that gap; it is the fix worth prioritising. - **#1972** — the original red-main report this chain started from. - **#1645** — red PRs keep landing (`enforce_admins` off). - **#1977** — `ALL_ROLES` is a hand-maintained array that can silently under-test the role family; it is exactly what let the `Research` arm drift in the first place. Refs #1986 Refs #1972 Refs #1977 ## Summary by Sourcery Repair main by resolving merge artefacts and bringing formatting, linting, documentation, and file-size checks back to green. Bug Fixes: - Remove duplicated management prompt role arm to fix unreachable pattern lint. - Drop unused scope-stage budget variable to clear unused variable lints. - Delete duplicate shell tooling fakes from verification hardening tests to remove dead code lints. - Correct rustdoc link for compaction rewrite events so documentation builds cleanly. - Add missing trailing newline in event tests module to satisfy rustfmt. Enhancements: - Clarify documentation around daemon boot resume-point handling and location of shell doubles for flip-halt tests. Build: - Regenerate file-size baseline to reflect current driver and pipeline module sizes, re-aligning with the file-size ratchet checks. Tests: - Tighten test layout by consolidating flip-halt shell doubles into a single module referenced by the arming tests. Chores: - Minor comment and whitespace cleanups across daemon boot, pipeline scope stage, and event tests.
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.
What
mainat b5ab7f8 did not build:cargo clippy -p stella-pipeline --all-targets -- -D warningsfailed. A compile error in the lib masks the test target, so it read as one failure and was really four, from two different merges.From #1953 (the #1778 research stage)
management_prompt/tests.rs(E0004) —ModelCallRole::Researchis a new variant andmanagement_system_block's match is exhaustive by design.Researchrides the sub-agent primitive, so its system prompt travels on theSubAgentSpec, never throughmetered_raw_call— it joins the never-dispatched arm, andALL_ROLESgrows to 15.pipeline.rs(too_many_arguments) — the newresearchparameter pushedplan_stageto 8 args, one over the cap. Bundledbudget/totalinto theSpendstruct every stage downstream of the fan-out already takes, rather than#[allow]-ing the lint.pipeline.rsis a god file, so the doc comment is written to land the file back at exactly its 3462 ceiling — no growth.From #1951 — a same-seam clobber
#1951 rewrote
tests/verification_hardening.rswholesale and dropped three items #1945 had added to it hours earlier, in a file its own subject (per-candidate verifier degradation) never needed to touch:PassingShell+shell_call_result(E0425 ×2) — the child moduleflip_halt_arming.rswas left referencing two doubles that exist nowhere in the tree.a_revision_halts_at_the_step_where_the_tracked_test_flips— the configured-command witness for FlipHalt never arms on the authored-witness path, and revise turns pass None even for configured commands #1793. Deleting it failed no gate, because the crate had already stopped compiling for reason 3. FlipHalt never arms on the authored-witness path, and revise turns pass None even for configured commands #1793 has been shipping with half its witness silently gone. Restored verbatim from eddf970.The structural fix, not just the restore
Restoring the witness took the parent file to 1557 lines, which
file-sizerejects outright — and the baseline takes no new entries. So this splits rather than exempts, and the split is the one the content was asking for: both #1793 witnesses and the two doubles they share now live inverification_hardening/flip_halt_arming.rs, the module already named for the concern. Parent drops to 1434.That the two witnesses were ever in separate files is what let the clobber happen quietly. With the cluster in one file, the same wholesale rewrite is a merge conflict instead of a silent deletion — the module doc records why.
Witness
Not a pure refactor: items 3 and 4 restore two witness tests, and neither passes vacuously. Each asserts a scripted-prompt count, so a
PassingShellthat omitted the[exit code: 0]markerflip_halt::exit_statusparses would leave the halt unarmed, the revision would consume the steps scripted beyond the flip, and the count would be wrong.cargo test -p stella-pipeline— 605 passed, 0 failedcargo clippy -p stella-pipeline --all-targets -- -D warnings— cleanmake guards-fast— green,file-sizeandgod-filesincludedOverlap with #1965
This branch originally also retired the stale
crates/stella-protocol/src/event.rsbaseline entry (1454 lines against a recorded 2965 — a fifth red gate on main). #1965 landed the same fix while this was in flight, so that work was dropped here in favour of theirs on rebase; the remaining commit is only the split. No duplicate baseline edit.