cast: structured failure detection + advance over Skipped phases

[BunsDev]

Summary

Re-applies the two improvements the Copilot SWE bot wrote on PR #98's branch (commit `438690b`) but that never reached main. The original PR was overtaken by the squash-merges of #99 and #100, and the bot's follow-up commit was orphaned.

This commit brings the bot's fix forward on its own, adapted to the current `quest.rs` surface (phases 7–10 evolved it further). All three open review threads on PR #98 are genuinely resolved by this change.

What changes

1. Structured failure detection. Today's `handoff_reason` keys off string patterns (`failed`, `error`, `exit 1`, `interrupted`) which misclassifies non-zero exit codes other than 1 (e.g. 2, 137 from SIGKILL, 130 from SIGINT) as success framing. New `phase_failed(summary)` helper checks `exit_code != 0` first, then falls back to the status-string match for runs that produced no exit code.
2. Advance over Skipped phases. `advance` (and `skip_phase` when nudging the cursor) now use a new `next_pending_index` helper so the cursor never lands on a Skipped or Complete row. The shell loop's defensive guard stays as a belt-and-braces safety net for hypothetical async paths that might mutate `cursor` directly, but the common path is now correct at the data layer.

Tests

Two new and one updated:

• ✨ `advance_skips_over_skipped_phases_and_preserves_skip_reason` — explicit assertion that the cursor jumps directly to the next pending phase and that the Skipped row's reason is preserved.
• ✨ `non_zero_exit_codes_use_failure_handoff_reason` — pins the structured exit-code check (status string says "completed" but exit code is 2 → failure framing).
• 🔧 `skip_phase_advances_cursor_and_marks_status` — updated to assert quest exhaustion (`q.is_complete()`) instead of "cursor lands on the Skipped row" since the new cursor advances past Skipped.

Gate results

• `cargo fmt --all -- --check` — clean
• `cargo clippy -p coven-cli --tests --no-deps -- -D warnings` — clean
• `cargo test -p coven-cli` — 346 unit + 4 smoke, 0 failures (2 net new)

Attribution

Authored by `@Copilot` SWE bot on the PR #98 branch; co-authored on this commit so the bot keeps proper credit. Closes the loop on the three `copilot-pull-request-reviewer` threads from #98 that flagged the underlying issues.

Test plan

• Run a quest where a phase produces `exit 2`. Confirm the next phase's handoff card reads "incorporate the failure context" rather than the success framing.
• Run a quest, skip the middle phase via `/skip `, then approve the first phase. Confirm the cursor jumps to the third phase (not the skipped one).

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR updates the Cast quest state machine (cast::quest) to (1) classify phase failures based on structured exit codes rather than brittle status-string matching, and (2) ensure quest cursor progression never lands on already-Skipped or Complete phases—keeping the data-layer cursor consistent with the shell loop’s expectations.

Changes:

• Add phase_failed(summary) to treat any non-zero exit_code as failure (with a status-string fallback when exit_code is missing).
• Add next_pending_index(quest, start) and use it from advance and skip_phase so the cursor always moves to the next Pending phase (or to phases.len() when exhausted).
• Update and add unit tests to lock in the new cursor and failure-framing behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.