feat(work-graph): cut over work surface inspector

[Hmbown]

Summary

• cut the Ocean Work surface over to the active-session Work Graph and delete the parallel live projection
• replace Active / Tasks / Runs / Workers rows with truthful running / ready / blocked graph counts
• open a full graph inspector from each row and move lifecycle Stop into its two-step-confirmed pager action
• expose explicit empty, error, stale, disconnected, and completed-with-evidence-pending states
• stage Plan-mode edits as validated ProposePlanDiff values with explicit node/edge/scope deltas, render the delta for user review, and apply it only through AcceptPlanDiff
• preserve complete legacy Plan/To-do projections and pre-WG3 proposal deserialization

Product contract

• no new sidebar or modal framework
• no [open] / [stop] row controls
• hover retains the full untruncated row
• inspector includes objective, prerequisites, downstream impact, owner binding/lifecycle, evidence vs acceptance, blockers/approvals, why-next, provenance/reconcile time, and bounded stale output
• completed work with unmet acceptance evidence is attention/blocked, never rendered verified
• Plan revision is fail-closed: a preview failure cannot accept an unseen proposal

Verification

• cargo test -p codewhale-tui --bin codewhale-tui --locked — 7463 passed, 3 ignored, 0 failed
• cargo clippy -p codewhale-tui --all-targets --all-features --locked -- -D warnings
• cargo fmt --all -- --check
• localization suite — 17 passed, including raw key parity for every complete pack
• focused Work surface suite — 13 passed
• focused Plan proposal, delta review, pager confirmation, and modal lifecycle tests passed
• pre-WG3 proposal serialization compatibility fixture passed
• git diff --check
• added-line credential/private-path scan — no matches

Boundaries

No provider credentials, routes, publication, deployment, billing, or customer-data behavior is changed by this PR.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[claude]

Claude finished @Hmbown's task in 1m 56s —— View job

---

Reviewing PR #4567

• Gather context (PR body, changed files, custom instructions)
• Read diff against origin/main
• Review correctness / panics / error handling (work_graph runtime/reducer/events)
• Review work_surface model.rs (view-model / state projection)
• Review work_surface mod.rs (surface controller, inspector, Stop confirm)
• Review work_surface input/interaction/render + live_projection deletion
• Review Plan diff staging (propose/preview/accept fail-closed contract)
• Review app/ui/pager/mouse/sidebar + localization key removals
• Review provider/model/route safety invariant across all areas
• Post final review summary

Dispatched parallel review passes across the diff areas. Working...

[claude]

Claude finished @Hmbown's task in 1m 30s —— View job

---

Reviewing PR #4567

• Gather context (PR body, changed files, custom instructions)
• Read diff against origin/main
• Review work_graph runtime/reducer/events (correctness, panics)
• Review work_surface model.rs (view-model / state projection)
• Review work_surface mod.rs (surface controller, inspector, Stop confirm)
• Review work_surface input/interaction/render + live_projection deletion
• Review Plan diff staging (propose/preview/accept fail-closed contract)
• Review app/ui/pager/mouse/sidebar + localization key removals
• Review provider/model/route safety invariant across all areas
• Post final review summary

Dispatched 6 parallel review passes across the diff areas. Working...

[claude]

Claude finished @Hmbown's task in 10m 40s —— View job

---

Reviewing PR #4567

• Gather context (PR body, changed files, custom instructions)
• Read diff against origin/main
• Review work_graph runtime/reducer/events (correctness, panics)
• Review work_surface model.rs (view-model / state projection)
• Review work_surface mod.rs (surface controller, inspector, Stop confirm)
• Review work_surface input/interaction/render + live_projection deletion
• Review Plan diff staging (propose/preview/accept fail-closed contract)
• Review app/ui/pager/mouse/sidebar + localization key removals
• Review provider/model/route safety invariant across all areas
• Post final review summary

Summary

Solid cutover overall — the live_projection.rs deletion is clean (no dangling refs/imports/dead re-exports anywhere in the tree), the two-step Stop confirmation is genuinely gated (no mouse/click bypass), the [open]/[stop] inline row controls are fully removed with hover still showing the full untruncated row, and completed-work-with-unmet-evidence is correctly rendered as attention (never verified) — this is test-covered in mod.rs. No provider/model/route-safety issues found anywhere in this diff; the only "prefix" parsing (stop_action() in model.rs, on task:/shell:/worker:/workflow: binding strings) is internal lifecycle-cancel dispatch, unrelated to EPIC #2608.

Left 4 inline comments on what I think are real gaps:

1. work_surface/model.rs:234 — the new "N running" header count uses NodeState::is_live() (Active | Waiting), but a Waiting node is rendered per-row as an attention item and is excluded from the blocked bucket entirely. A node waiting on owner/approval is silently counted as "running" and invisible from both ready and blocked — undercuts the PR's own "truthful running/ready/blocked counts" goal. No test exercises NodeState::Waiting in graph_rows.
2. tui/mouse_ui.rs:778 — this diff deletes the localized Stop-control keys (WorkSurfaceStopConfirmControl, WorkSurfaceStoppingControl, etc.) from all 7 locale files with no replacement; the new destructive-action confirmation hardcodes English ("stop", and pager.rs's "confirm {label} · Esc cancels"). Non-English locales lose translation for the Stop confirmation.
3. work_graph/runtime.rs:210 — accept_plan has no proposal_id parameter; it accepts whichever proposal is last in the queue. The fail-closed "can't accept an unseen proposal" contract holds only by the invariant that propose_plan_update always withdraws priors before staging (so exactly one proposal is ever queued). There's no defense-in-depth check that the accepted proposal matches what was actually previewed — worth threading the previewed id through explicitly rather than relying solely on call-site discipline.
4. work_surface/model.rs:192 — Error state renders unconditionally, unlike Empty/Disconnected which are both gated on active_session. Minor; may be intentional (surface errors regardless of session) but flagging the asymmetry.

Other things worth a look, not inline-commented (lower confidence / lower severity):

• ui.rs (~3179-3191): if plan_diff_summary/preview rendering fails after propose_plan_update already staged a validated proposal, the proposal isn't explicitly withdrawn — it sits queued until the next propose call. Currently harmless since nothing reaches accept_plan without plan_prompt_pending being set by a successful render, but it's a latent leaked-proposal state.
• work_graph/tests.rs has no reducer-level test for WithdrawPlanDiff, no test hitting apply_proposal's dangling-edge rejection for removed_nodes directly, and no test proving the new eager validate_proposal at ProposePlanDiff time (vs. only at accept) actually rejects invalid proposals early.
• runtime.rs's propose_plan_update calls preview_plan_diff once itself, then apply_change(ProposePlanDiff) re-validates the same proposal internally — duplicated validation work per proposal, not a bug.
• Pre-WG3 deserialization compat test (pre_wg3_proposal_shape_deserializes_with_empty_delta_extensions) only drops removed_nodes/replacement_compat; doesn't exercise omission of added_nodes/added_edges/updated_nodes (all #[serde(default)], likely fine but untested).
• input.rs:14-17 doc comment still describes a "local stop arm" that this diff removed — stale, no functional impact.

No security issues, no dead references to removed live_projection/InspectText/old localization keys found anywhere.

[claude]

Claude finished @Hmbown's task in 2m 25s —— View job

---

Reviewing new commits on PR #4567

• Gather context (PR body, prior review comments, custom instructions)
• Diff new commits against previously-reviewed state
• Check whether prior inline findings (Waiting-count, i18n Stop label, accept_plan proposal-id, Error-state gating) were addressed
• Review new test commits for correctness/coverage
• Post final review summary

Summary

This push adds three test-only commits on top of the previously-reviewed feat commit; no production code changed since the last full review, so the four earlier inline findings (work_surface/model.rs:234 Waiting-count truthfulness, mouse_ui.rs:778/pager.rs hardcoded English Stop-confirm strings, runtime.rs:210 accept_plan missing a proposal_id check, model.rs:192 unconditional Error render) are all still outstanding — none of model.rs, mouse_ui.rs, pager.rs, or runtime.rs were touched.

2e056f8 (tests/qa_pty.rs) — trims work_surface_real_rows_own_click_wheel_resize_and_stop_confirm down to ..._and_resize, and updates the remaining assertions from "To-do" to "Work". The part that's worth flagging: this deletes the only real-PTY, end-to-end coverage of the two-step Stop confirmation against a live process — spawn a real sleep-backed shell run, click the rendered Stop control, confirm "confirm" appears, reject with Esc and prove the live command survives (the dialog-vs-active-work boundary), re-arm, then accept and prove the process actually stops. The commit message is explicit about why ("Keep shell stop acceptance out of the work-surface cutover test until the lifecycle owner binding lands, so the test cannot resurrect the removed live projection") — reasonable to not leave a test asserting on removed [stop] row controls, but it means the PR's own claim that lifecycle Stop is now real, two-step confirmed, and safe against Esc-leakage is currently backed only by the mocked unit test in pager.rs (destructive_action_requires_two_steps_and_escape_only_disarms), not by anything that exercises a real running process through the new inspector/pager path. Worth tracking as a follow-up before/soon after merge — either restoring PTY coverage once the lifecycle owner binding lands, or confirming in a comment that this is intentionally deferred and tracked elsewhere. No leftover dead imports/vars from the trim (Duration, keys::mouse::click, etc. all still used elsewhere in the file).

9028629 (tests/release_runtime_qa.rs) — drops the "workers 6" aggregate-label assertion in favor of directly counting live "Agent " / "delegate scout [running]" rows. This correctly reflects the cutover (the retired aggregate label is no longer authoritative) and is a good example of adapting a release gate to the new truthful-counts model rather than asserting on stale presentation text.

f992661 (core/engine/tests.rs) — bumps the Windows model-turn timeout from 30s to 60s with a comment explaining CI-runner contention. Reasonable flake mitigation, no concerns.

I wasn't able to run cargo test/cargo clippy in this session (the sandbox required approval for the cargo invocation that wasn't available here) — the PR body's reported local run (7463 passed) should be treated as the source of truth for green CI.

[claude]

Claude finished @Hmbown's task in 1m 39s —— View job

---

Reviewing new commit on PR #4567

• Gather context (PR body, prior review comments, custom instructions)
• Diff new commit 6786816 ("fix(work-graph): close review truth gaps") against previously-reviewed state
• Check whether the 4 prior inline findings were actually fixed
• Check the proposal-id plumbing through ui.rs / plan_todo_bridge.rs / app.rs
• Post final review summary

Summary

All four findings from the earlier review passes are resolved by 6786816, and each fix is real (not just cosmetic):

1. Waiting-count truthfulness (work_surface/model.rs:234) — running now counts only NodeState::Active; Waiting gets its own bucket and is surfaced in the header as "N running · M waiting · ..." only when waiting > 0. Backed by a new test, waiting_operation_is_not_counted_as_running, asserting the exact header string. Good fix — a waiting-on-approval node is no longer invisible.

2. Hardcoded English Stop confirmation (mouse_ui.rs:778, pager.rs) — with_destructive_action now takes both a label and a confirm_label, both sourced via app.tr(MessageId::SidebarStopControl) / app.tr(MessageId::WorkSurfaceStopConfirmHint) instead of literals. All 7 shipped-complete locales (en, ja, ko, vi, zh-Hans, es-419, pt-BR) got real translations, not copies of the English string (spot-checked each) — and the partial zh-Hant pack got the same two keys. A new regression test, work_stop_confirmation_is_explicitly_localized, asserts every non-English shipped-complete locale differs from the English literal, which will catch future regressions of this kind.

3. accept_plan proposal-id defense-in-depth (work_graph/runtime.rs:187) — accept_plan now takes expected_proposal_id: Option<&ProposalId> and explicitly rejects: no pending proposal but one was expected, no expected_proposal_id when one is required, and the queued proposal not matching what was reviewed (with the queued-proposal-must-be-singular check kept). plan_for_review is now backed by a new plan_review_for_confirmation that returns (ProposalId, PlanSnapshot, summary) together, so the UI can't drift from the previewed identity. App.pending_plan_proposal_id is threaded through ui.rs → apply_plan_choice → plan_todo_bridge::project_accepted_plan → accept_plan, and cleared on mode exit, session switch, and after acceptance. New tests cover both rejection paths and confirm a mismatched acceptance leaves the reviewed proposal untouched. This is a solid closure of the earlier "relies entirely on call-site discipline" gap.

4. Unconditional Error render (work_surface/model.rs:194) — now gated with active_session.then_some(...), matching Empty/Disconnected. Covered by the new graph_error_without_an_active_session_stays_suppressed test.

Checked for regressions: plan_for_review and plan_diff_summary remain independently used (tools/plan.rs:785, tests, plan_prompt.rs), so nothing was left dead by introducing plan_review_for_confirmation. ProposalId::derive used in the new test matches the existing crates/tui/src/work_graph/ids.rs definition. No provider/model/route-safety concerns in this commit (no prefix-based provider inference introduced or touched).

I wasn't able to run cargo test/cargo clippy in this sandbox session (the cargo invocation required approval that wasn't available here) — this review is based on manual code reading; the PR's reported local run should remain the source of truth for green CI.

Remaining lower-priority items from earlier reviews, still open (not addressed by this commit, not blocking):

• runtime.rs's propose_plan_update still calls preview_plan_diff once itself, then re-validates via apply_change(ProposePlanDiff) — duplicated validation, not a bug.
• The real-PTY end-to-end Stop-confirmation test removed in 2e056f8 is still not restored (intentionally deferred per that commit's message, pending lifecycle owner binding).
• work_graph/tests.rs still has no reducer-level test for WithdrawPlanDiff in isolation.

No new issues found in this commit.

[claude]

Claude finished @Hmbown's task in 34s —— View job

---

Reviewing new commit on PR #4567

• Gather context (PR body, prior review comments, custom instructions)
• Diff new commit 5016bde ("fix(work-surface): satisfy exact lint gate") against previously-reviewed state
• Check whether this commit touches any of the previously-flagged areas
• Post final review summary

Summary

5016bde touches only crates/tui/src/tui/work_surface/model.rs (5 lines), in the graph_rows waiting-count formatting introduced by the prior fix commit (6786816):

let waiting = (waiting > 0)
    .then(|| format!(" · {waiting} waiting"))
    .unwrap_or_default();

→

let waiting = if waiting > 0 {
    format!(" · {waiting} waiting")
} else {
    String::new()
};

This is behavior-preserving — both branches produce identical output for waiting > 0 and waiting == 0. It's a pure clippy-appeasement rewrite (this workspace's -D warnings gate is presumably flagging the bool::then + closure form here), not a logic change. No new code paths, no test impact needed.

None of the previously-flagged items (Waiting-count truthfulness, i18n Stop confirmation, accept_plan proposal-id, Error-state session gating) are touched by this commit — all four remain resolved as of 6786816, per the prior review pass.

No new issues found. CI (CI, Web Frontend, PR #4567) was still in progress at review time; DCO passed.