Skip to content

fix(stella-pipeline): unbreak main — two clippy failures merged with #1813 - #1859

Merged
macanderson merged 2 commits into
mainfrom
fix/witness-stage-clone-on-copy
Aug 6, 2026
Merged

fix(stella-pipeline): unbreak main — two clippy failures merged with #1813#1859
macanderson merged 2 commits into
mainfrom
fix/witness-stage-clone-on-copy

Conversation

@macanderson

@macanderson macanderson commented Aug 6, 2026

Copy link
Copy Markdown
Owner

What this is

An unbreak: 608a0aa (#1813) landed two clippy -D warnings failures in crates/stella-pipeline/src/pipeline/witness_stage.rs, so the fmt+clippy+test gate is red on main and on every PR branched from it. The first masks the second because cargo stops at the first error.

Verification

Pure lint fixes, no behavior change, so no witness test — the witness is clippy itself: cargo clippy -p stella-pipeline --all-targets -- -D warnings fails on main at both sites and passes on this branch. cargo test -p stella-pipeline shaping (including #1785's verifier_shaping_overlays_the_worker_engine_config) passes.

After this merges, open PRs (#1843, #1836, #1844) need gh pr update-branch to go green.

Summary by Sourcery

Fix clippy lint violations in stella-pipeline’s witness_stage to restore a clean build.

Bug Fixes:

  • Avoid cloning a Copy GenerationParams value when applying role shaping overrides.
  • Construct test EngineConfig instances with struct update syntax instead of mutating a default value to satisfy clippy lints.

…s witness stage

608a0aa (#1813) landed two clippy -D warnings failures in
pipeline/witness_stage.rs, red on main and on every PR branched from it
(the first masks the second because cargo bails at the first error):

- clone_on_copy: `params.clone()` where GenerationParams became `Copy`
  in a parallel change — copy it out of the borrow instead.
- field_reassign_with_default: the #1785 witness test built its worker
  config by mutating a `default()` — use struct-update syntax.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@vercel

vercel Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
stella-cli-docs Ignored Ignored Preview Aug 6, 2026 6:31pm

@sourcery-ai

sourcery-ai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Fixes two Clippy lint violations in witness_stage.rs by avoiding an unnecessary clone of a Copy type and replacing a mutating-default pattern in a test with struct-update syntax, restoring the lint gate on main.

File-Level Changes

Change Details Files
Avoid unnecessary cloning of Copy generation parameters in role shaping logic.
  • Change apply_role_shaping to pattern-match overrides.params by value instead of by reference.
  • Assign config.params directly from the copied GenerationParams value without calling .clone().
crates/stella-pipeline/src/pipeline/witness_stage.rs
Refactor test worker EngineConfig setup to use struct update syntax instead of mutating a default instance.
  • Replace constructing a mutable EngineConfig::default() and then setting fields with a single EngineConfig { ..EngineConfig::default() } literal.
  • Explicitly set temperature, max_output_tokens, and effort fields in the struct literal while inheriting the remaining defaults.
crates/stella-pipeline/src/pipeline/witness_stage.rs

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@macanderson
macanderson merged commit 2c1bf69 into main Aug 6, 2026
4 checks passed
@macanderson
macanderson deleted the fix/witness-stage-clone-on-copy branch August 6, 2026 18:32
macanderson added a commit that referenced this pull request Aug 6, 2026
…he verdict call (#1867)

## What & why

The witness author has a hard independence gate; the **verdict** call
had none — with one configured provider, the "independent code reviewer"
is the same model that wrote the code, its PASS ends the run, and the
only trace is a once-per-run prose caveat that scrolls away (the
46%-agreement measurement in `verify.rs` was made under exactly this
condition). Two halves, per the issue:

- **Opt-in refusal, before spend.**
`PipelineConfig::require_independent_verifier` mirrors
`require_independent_witness` — same shared probe
(`witness_author_independence`, worker-vs-verifier model-ref), same
placement ahead of triage so a refused run costs $0, its own error
variant (`PipelineError::VerifierNotIndependent`). Default off: a
single-provider BYOK seat keeps working. Reachable via the new settings
key `pipeline_require_independent_verifier` (settings.json and
stella.toml), wired in `apply_pipeline_tuning` so every driver honours
it. The probe's reason wording was made role-neutral ("no model
independent of the worker resolves") since three framings now share it.
- **The structured fact, unconditionally.** Every model verdict's
`LadderSnapshot` now carries `verifier_independent: Option<bool>` —
`Some(false)` = self-graded — stamped at the call seam
(`Pipeline::verifier`) where the actual resolution is in hand, threaded
through `verify::Verdict`, and additive on the wire (absent on
deterministic/waived/abstain rungs, on pre-existing snapshots, and when
the worker itself is unresolvable). `replay::verdict_provenance`
narrates it (`grader=self-graded (worker's own model)`), and
`docs/wire/` is regenerated.

Exemplar: the existing `require_independent_witness` gate (#1147) — same
refusal shape, same before-spend contract — and `diff_coverage`'s
three-valued honesty for why the fact is `Option<bool>`, never a
defaulted bool.

Two god-file limits would have been newly crossed by this diff, so two
test extractions ride along (repo rule: split, not grow):
`replay/late_reconciliation_tests.rs` out of `replay.rs`, and
`verify/tests/witness_strip.rs` out of `verify/tests.rs`. The
regenerated baseline *tightens* `pipeline.rs` (3642 → 3616) and grows
only the two irreducible one-liners (the `mod` declaration in
`pipeline/tests.rs`, the required field in `event.rs`'s test
constructor).

Closes #1795

## The witness

- [x] This PR includes a witness test (fails on `main`, passes here)

Flip-checked by restoring the old behavior in place and re-running —
three behavioral witnesses fail, the two invariant guards pass on both
sides, as designed:

-
`verifier_independence::requiring_an_independent_verifier_refuses_before_spending_anything`
— refusal with an empty provider script, $0, no stage opened
-
`verifier_independence::a_self_graded_verdict_states_the_fact_on_its_snapshot`
— `Some(false)` on the stored verdict
- `verifier_independence::an_independent_verdict_states_that_too` —
`Some(true)`, distinguishable from pre-fact snapshots
-
`verifier_independence::the_verifier_independence_requirement_is_opt_in`
/ `..._lets_a_distinct_verifier_through` — guards (pass on both)

Plus protocol round-trip + additive-parse tests in `ladder.rs`
(invariant 4).

## The gate

- [x] `cargo fmt --check`
- [x] `cargo clippy --workspace --all-targets -- -D warnings` — clean
**except** the two pre-existing `witness_stage.rs` warnings #1859
unbreaks; none in files this PR touches
- [x] `cargo test` — stella-pipeline (549), stella-protocol, stella-cli
all green; rustdoc `-D warnings` clean; `make guards` green except
`gate-parity`, which is red on main and owned by #1845
- [x] Docs updated (field docs, settings-key docs, wire schema
regenerated)
- [x] CLA signed
- [x] `Closes #N` above and as a commit trailer

**CI note:** until #1845 (gate-parity) and #1859 (witness_stage clippy)
merge, the required job will be red on those pre-existing steps; I'll
`gh pr update-branch` after they land.

## Nothing left behind

- [x] Filed: #1865 — partition the calibration false-positive rates by
the new grader-independence fact (the reading side of #1284); #1866 —
the trusted posture auto-sets the witness flag but not the verifier flag
(equivalent today via the shared probe, drifts if the probes diverge;
related #1785)

## Ground-rule check

- [x] No I/O added to `stella-core`; no new deps
- [x] New cross-boundary field round-trips through serde (test included;
additive, alias-free)

## Anything reviewers should know?

- The refusal also fires when the verifier role is *unresolvable* (not
only same-model): a caller requiring an independent verifier is also
refusing "no verifier at all", where the verdict would degrade to the
heuristic — the conservative read, stated in the error.
- The heuristic-fallback verdict leaves the fact absent on purpose: no
model answered, so grader independence is not a fact about it (mirrors
`diff_coverage`'s unmeasured honesty).
- The uncorroborated-PASS relabel (`Unverifiable`) also leaves it absent
— by the field's own contract the fact rides only the `ModelVerdict`
rung; #1865 covers reading it at scale.

## Summary by Sourcery

Enforce optional independence between worker and verifier models for
verdicts and record verifier independence as structured data on verdict
snapshots, while keeping wire formats and configuration in sync and
extracting oversized tests into dedicated modules.

New Features:
- Add a configurable pipeline option to require the VERDICT call to use
a model independent of the worker, refusing runs before any spend when
independence cannot be ensured.
- Record whether a verdict’s grader model is independent of the worker
on LadderSnapshot and propagate this fact through verdicts, events, and
replay provenance narration.

Enhancements:
- Update verifier routing and error handling to use role‑neutral wording
for independence failures and introduce a dedicated error for
non‑independent verifiers.
- Expose the new verifier-independence requirement through CLI settings
(JSON and TOML) and apply it when constructing pipeline configurations.
- Extend replay provenance strings to describe whether a verdict was
self‑graded or independently graded.

Documentation:
- Regenerate and extend wire documentation and TypeScript definitions to
include the new verifier_independent field on LadderSnapshot.

Tests:
- Add end-to-end tests covering the verifier-independence requirement,
verifier independence recording on snapshots, and the opt‑in nature of
the new flag.
- Add serialization and wire-contract tests to ensure LadderSnapshot
with verifier_independent remains backward compatible and round-trips
correctly.
- Extract late reconciliation tests from replay.rs and witness-strip
tests from verify/tests.rs into separate modules to stay within file
size limits.

---------

Co-authored-by: Stella Test <test@stella.local>
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
…nd three parameter objects (#1880)

## What — #1809, Option A as triaged

The candidate plane (`run` → `run_best_of_n` →
`dispatch_isolated_candidates` → `run_candidate` → `verify_candidate` →
`revise_turn` → `run_engine_turn`) threaded the same values positionally
through every layer, and **nine** functions across `pipeline.rs` and
`fanout_stage.rs` carried `#[allow(clippy::too_many_arguments)]` to keep
it lawful (the issue counted seven; the sweep found two more in
`fanout_stage.rs`). Each new stage input — #1701's `mutating_actions`,
#1798's `opaque_actions` — meant widening half a dozen signatures and
re-justifying allows.

Three parameter objects, each grouping values that already always travel
together (transport, not semantics — every field keeps its own
documented meaning):

- **`TaskFrame`** (new `pipeline/task_frame.rs`, per the
`driver/settlement.rs` sibling-module pattern): goal, staged prefix,
plan, assessment. Built once in `run` where its last field settles;
`Copy`, so the fan-out closures each take their own.
- **`Spend`** (in `pipeline/stage_budget.rs`, beside `FanOutBudget`):
the budget guard + running total. Borrowed rather than owned so every
mutation lands at `run`'s locals with no write-back on early returns —
one missed return would be a silently vanished spend. The fan-out builds
a per-candidate `Spend` over its claimed allowance, which is the shape
that code already had.
- **`ChangeSignals`** — no new type: the warrant's input struct replaces
the three loose `u32` counters on `CandidateState`, so `run_engine_turn`
accumulates directly into the type the warrant reads. The deleted
`change_signals()` accessor existed to stop the counts being transposed
en route (the #1701 recurrence); holding the struct makes that
protection structural.

`revise_turn` now takes `&mut CandidateState` — its only caller already
held one and exploded seven fields out of it.

## Result

- `rg too_many_arguments crates/stella-pipeline/src` → **zero allows**
(one doc-comment mention).
- Every function in the plane is at or under clippy's threshold; `cargo
clippy -p stella-pipeline --all-targets` adds no warnings from this
change.
- `pipeline.rs` **shrinks by 88 lines** (3487 against its 3642 ceiling);
the god-file and file-size guards pass for every touched file.

## Verification

Pure refactor — no behavior change, so no witness test (per AGENTS.md).
Evidence: all **566** existing `stella-pipeline` tests pass unchanged
(544 lib + integration suites), `cargo fmt --check` clean, scoped
`RUSTDOCFLAGS="-D warnings" cargo doc` clean.

**Known-red main caveat:** clippy currently fails on main with two
`witness_stage.rs` warnings from #1813's auto-merge, and the file-size
gate on `event.rs`/`deck_render.rs` skew — peer unbreaks #1859 and #1845
cover those; this PR deliberately does not duplicate them, so its CI
stays red on those two axes until they land.

Exemplar for the parameter-object shape: `rustc`'s own `Session`/context
threading and this repo's existing `WitnessAuthoring` bundle.

Closes #1809
Refs #1798, #1808

## Summary by Sourcery

Introduce parameter objects for shared task context, budget, and change
signals to simplify the candidate execution/verification pipeline and
remove all too_many_arguments allowances.

Bug Fixes:
- Eliminate the risk of misordered change-signal counters by storing and
passing them as a typed ChangeSignals struct rather than separate u32
fields.

Enhancements:
- Thread immutable task context through the candidate pipeline via a new
TaskFrame struct instead of multiple positional parameters.
- Bundle budget guard and running cost into a Spend helper struct to
centralize turn-level spending and reporting.
- Replace loose change counters on CandidateState with the existing
ChangeSignals struct to align execution tallies with warrant inputs and
prevent miswiring.

Tests:
- Rely on the existing stella-pipeline test suite to validate that the
refactor preserves behavior without adding new tests.

Co-authored-by: Stella Test <test@stella.local>
macanderson added a commit that referenced this pull request Aug 6, 2026
…the fallback, budget stops degrade, arming symptoms recorded (#1890)

## What this is

Round 3 of the verification-role work (#1798, #1813 were rounds 1–2):
three decision-gated issues from the audit backlog, decided with the
reasoning recorded in code, each with a witness test.

## The decisions

1. **A verifier outage is not a refutation** (`Closes #1788`).
`heuristic_fallback` now passes on an observed fail→pass flip, not only
on green touched tests. The asymmetry it closes: `Unverifiable` abstains
when the *evidence* is absent, but a missing *checker* drove a
flip-verified candidate (diff over budget → ModelVerdict → provider
down) to `VerificationFailed` — the checker's absence treated as the
work's failure. With nothing deterministic positive the fallback still
fails closed. This deliberately inverts the "even a flip doesn't rescue
an unconfirmed suite" pin; the test now states why.

2. **The scaffolding's budget must not discard the work** (`Refs #1789`
— the reclassification + de-panic half). A budget stop during witness
authoring/repair was `rejected`, aborting a candidate whose worker
change was already complete. It degrades now: the budget guard still
gates every later paid call (no overspend is possible), and what
degrading buys is the deterministically-resolvable endings — a warranted
waiver, an abstention — that need no further spend. The stage's two
`expect`s on workspaces became degradable aborts, per its own
produce-vs-trust contract. The issue stays open for the end-to-end
complete-without-further-spend scenario.

3. **A build-failure baseline is recorded, never refused** (`Closes
#1790`). Resolved per the on-issue analysis: refusing
`SymptomClass::BuildFailure` baselines (the issue's original framing)
would reject the most common Rust witness shape — a missing-API test
fails to compile on the old code *by design*. Instead the arming
failure's class is recorded (`witness_baseline=build_failure` in the
verifier's trusted evidence, plus a run warning), so a compile-armed
flip is visible to the verdict and to anyone reading the evidence — the
honest treatment for a shape that is legitimate for missing-API goals
and identical to two-tree environment drift.

4. **The cache-minimum measurement is a stated fact** (`Closes #1786` —
the split landed in #1813). The management-prompt module doc records the
measured prefix sizes: no fixed instruction block clears Anthropic's
1024-token minimum alone (verdict ~520, witness author ~620); the raw
calls cache only with an `agents.<role>.prompt` override padding the
prefix, while the witness author's engine turn crosses the minimum
within its first tool round-trip — which is exactly where #1813's split
pays.

## Witness tests

`heuristic_fallback_passes_only_on_confirmed_green_tests` (inverted pin
+ new no-evidence case),
`a_build_failure_baseline_is_recorded_and_an_assertion_one_is_not`. Both
fail on main.

## CI note

Until #1859 (witness_stage clippy, merged with #1813's premature
auto-merge) and #1845 (file-size baseline skew: `deck_render.rs` +3)
land, the required job is red on those pre-existing steps — none are in
hunks this PR touches. I'll update-branch after they merge.
Coordination: my #1795 implementation was dropped from this round in
favor of the more complete open #1867; my duplicate unbreak #1874 was
closed in favor of #1859/#1845.

## Still tracked

#1787 (structured verdict output, per-candidate degradation records),
#1789 (the e2e half), #1793 (FlipHalt — gated on a loop-bench
measurement), #1794 (semantic witness review — needs its design pass;
recommendation on the issue is extending the mutation audit to
flip-corroborated ModelVerdict passes).

Closes #1788
Closes #1790
Closes #1786
Refs #1789

## Summary by Sourcery

Adjust verification pipeline behavior to treat verifier outages, budget
stops, and build-failure witnesses as recorded, degradable conditions
rather than hard rejections, and document cache minimum measurements for
management prompts.

Bug Fixes:
- Allow heuristic fallback verdicts to pass when a confirmed fail→pass
flip exists even if the verifier is unavailable.
- Prevent witness authoring and repair stages from discarding completed
work on budget stops or missing workspaces by degrading the run instead
of rejecting or panicking.
- Record build-failure baselines for witnesses in pipeline state and
evidence summaries instead of refusing them, ensuring missing-API
compile errors are treated as legitimate but weaker evidence.

Enhancements:
- Expose the authored witness baseline symptom class in candidate state
and evidence output so verifiers can distinguish build failures from
assertion failures.
- Clarify management prompt documentation with measured token-prefix
sizes and their implications for provider cache behavior.

Tests:
- Extend heuristic fallback tests to cover verifier outage behavior and
no-positive-evidence failure cases.
- Add a witness-stage unit test to assert that build-failure baselines
are recorded while assertion-based baselines are not.

Co-authored-by: Stella Test <test@stella.local>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant