cast(phase 5): sequential goal flow with deterministic sub-prompting

[BunsDev]

Summary

• Adds cast::quest: a pure module that decomposes a high-level user goal into an ordered Quest (default rhythm: design → implement → verify) where each phase carries a concrete sub_prompt ready to hand to a harness.
• advance(quest, summary) marks the current phase complete, attaches a structured QuestHandoff to the next phase, and recomposes that phase's sub_prompt deterministically — preserving any user-authored override via set_phase_sub_prompt.
• render_quest_handoff produces a visible card showing source phase, prior status, carried context, target harness, and the verbatim sub-prompt the harness will see. Sub-prompts longer than 8 lines clip with a … N more lines indicator; user-authored sub-prompts render a · user-edited tag.

Deliberately keeps Cast deterministic and local-first: no LLM planner runs inside Cast. Sub-prompts are assembled from structured templates plus the recorded prior outcome, so every handoff is reproducible, inspectable, and overridable before the harness sees it.

The shell wiring for a /quest <goal> intent is intentionally deferred to the next phase; the Phase 5 seam is documented in docs/design/cast-quest-flow.md (§6).

Done-when (from the phase goal)

• Each phase can produce a concrete sub-prompt (quest_from_goal composes them up front).
• The next phase can be updated from the previous result (advance + QuestHandoff + recompose; failure status produces a failure-framed reason).
• The user can see what was delegated and why (render_quest_handoff card).

Test plan

• cargo test -p coven-cli --bin coven cast::quest — 11/11 green
• cargo test -p coven-cli --bin coven cast::render::tests::quest_handoff — 4/4 green
• cargo test -p coven-cli --bin coven cast:: — 105/105 green (no regressions in the rest of the Cast suite)
• cargo build -p coven-cli — clean (only the 4 pre-existing theme.rs BORDER_* dead-code warnings; nothing from this change)

Files

• crates/coven-cli/src/tui/cast/quest.rs (new, 534 lines incl. 11 unit tests) — Quest, QuestPhase, QuestPhaseStatus, QuestPhaseSummary, QuestHandoff; quest_from_goal, compose_sub_prompt, advance, set_phase_sub_prompt, skip_phase.
• crates/coven-cli/src/tui/cast/render.rs — adds render_quest_handoff + 4 plain-mode render tests.
• crates/coven-cli/src/tui/cast/mod.rs — re-exports the new public surface so Phase 6 shell wiring has a stable seam.
• docs/design/cast-quest-flow.md (new) — Phase 5 design contract: data model, composer, advance step, visible handoff card layout, and the Phase 6 shell-wiring seam.

[Copilot]

Pull request overview

Introduces a new deterministic “quest” flow to Cast, where a single high-level goal is decomposed into sequential phases (design → implement → verify), each with a concrete sub-prompt and a visible handoff card between phases. This adds a pure, testable core (cast::quest) plus a renderer to surface exactly what will be delegated to the selected harness.

Changes:

• Add cast::quest module implementing Quest/QuestPhase state, deterministic sub-prompt composition, and phase advancement with handoffs.
• Add render_quest_handoff to display a handoff card (including clipping long sub-prompts and marking user-edited prompts).
• Document the Phase 5 contract and Phase 6 shell-wiring seam in a new design doc.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
docs/design/cast-quest-flow.md	Defines the Phase 5 quest contract, handoff composition rules, and the Phase 6 integration seam.
crates/coven-cli/src/tui/cast/quest.rs	Adds the pure quest state machine (composer/advance/edit/skip) with unit tests.
crates/coven-cli/src/tui/cast/render.rs	Adds the quest handoff card renderer plus render-focused tests (plain mode).
crates/coven-cli/src/tui/cast/mod.rs	Re-exports the quest and renderer surface for later shell integration.

Comments suppressed due to low confidence (1)

crates/coven-cli/src/tui/cast/quest.rs:270

• skip_phase only bumps quest.cursor when cursor == index, so pre-skipping a later phase doesn’t prevent the cursor from landing on it later (and currently advance doesn’t skip past it). It’d be more robust to centralize “seek next pending phase” cursor movement and use it here too, so skipped phases are always transparently bypassed.

/// Skip a pending phase with a recorded reason. Useful when the prior
/// phase already satisfied this phase's goal (e.g. tests passed during
/// implement, so verify becomes a no-op).
pub(crate) fn skip_phase(quest: &mut Quest, index: usize, reason: String) -> Result<()> {
    let phase = quest
        .phases
        .get_mut(index)
        .ok_or_else(|| anyhow!("quest phase index {index} out of range"))?;
    if !matches!(phase.status, QuestPhaseStatus::Pending) {
        return Err(anyhow!(
            "phase `{}` is not pending; only pending phases can be skipped",
            phase.name
        ));
    }
    phase.status = QuestPhaseStatus::Skipped { reason };
    if quest.cursor == index {
        quest.cursor = index + 1;
    }
    Ok(())

---

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

[BunsDev]

Comprehensive review — PR #98 (cast phase 5: sequential goal flow)

Verdict: comment / recommend extracting the bot follow-up to a fresh PR, then close this one as superseded.

Status

The Phase 5 commit `abdf3a3` already shipped on main via PR #99 (`48b60f3 cast(phase 6): land Phase 1 launcher + verification`). The Phase 5 module (`cast::quest`) was further evolved by PRs #100–#103 (Phases 7–10): shell wiring, event ledger, re-attach reconstruction, edit/skip UX, and the `QuestPhaseStatus::Running` transition all landed.

However — this PR carries a second commit, `438690b Fix quest advance cursor and handoff failure detection` from the Copilot SWE bot, that never made it to main and contains two genuine improvements:

1. Proper exit-code failure detection (`phase_failed` helper). Today's `handoff_reason` on main keys off string patterns (`failed`, `error`, `exit 1`, `interrupted`). Exit codes like 2, 137 (SIGKILL), or 130 (SIGINT) would slip through as "success framing" because the literal string `"exit 1"` isn't in the status label. The bot's fix checks `summary.exit_code != 0` directly and keeps the status-string fallback for cases where exit_code is None.
2. `next_pending_index` helper that advances past Skipped phases. Without it, `advance` always increments cursor by 1 even if the next phase is already `Skipped`, leaving the shell loop to handle the skip-past defensively. Phase 8's shell loop has exactly that defensive guard (`if matches!(quest.phases[idx].status, QuestPhaseStatus::Skipped { .. })`) — but the cleaner home for that logic is `quest.rs`'s `advance` / `skip_phase`, which is where the bot put it.

Review threads

Three open threads from `copilot-pull-request-reviewer`, all addressed by the bot's follow-up commit on this same PR but not yet by anything on main:

1. `quest.rs:234` — "`advance` always moves `cursor` to `current + 1`… can leave `cursor` pointing at a skipped phase." → addressed by `next_pending_index`.
2. `quest.rs` (file-level) — "Failure detection in `handoff_reason` keys off string patterns like `exit 1`, which will misclassify non-zero exit codes other than 1." → addressed by `phase_failed`.
3. `cast-quest-flow.md:40` — "The spec says `cursor` is 'the index of the next non-complete phase'… current implementation can leave `cursor` pointing at a `Skipped` phase." → addressed by `next_pending_index` + the spec update in the bot's commit.

So the reviewer raised three concerns, the bot wrote a fix, the fix landed on the PR branch — but the PR's wrong base (`cast/phase-4-plan-outcome-cards`) and overtaken-by-main status meant the fix never reached main.

Code quality (Copilot fix specifically)

• `phase_failed` is a clean structured-data helper; the exit_code-first check matches the design doc's intent.
• `next_pending_index` is a small pure helper; the centralized cursor-advance logic is more maintainable than the per-callsite defensive guard now in `tui/shell.rs`.
• The minor formatting churn (`new_phase("design", …)` split across lines) is rustfmt-driven, not stylistic noise.

The patch cherry-picks with conflicts onto current main because phases 7–10 evolved `quest.rs` (new event-kind constants, `mark_phase_running`, `parse_phase_action`). A clean follow-up PR re-applying both improvements on top of current main is straightforward.

Recommendation

1. Open a fresh follow-up PR re-applying the Copilot bot's `phase_failed` + `next_pending_index` on current main, preserving the bot as the commit's co-author.
2. Once that lands, close this PR as superseded — the three reviewer concerns will be genuinely resolved on main at that point.

[BunsDev]

Closing as superseded.

The Phase 5 work itself landed on main via #99 (48b60f3 cast(phase 6): land Phase 1 launcher + verification), and the Copilot bot's follow-up commit on this branch (438690b Fix quest advance cursor and handoff failure detection) has now been re-applied to current main via #104 (bd650f6 cast: structured failure detection + advance over Skipped phases).

The three open review threads from copilot-pull-request-reviewer are genuinely addressed on main now and have been resolved.