Prepare CodeWhale v0.8.68 release candidate

[Hmbown]

Summary

Completes the Codewhale v0.8.68 release candidate on one reviewable branch.

• finishes the underwater TUI while preserving compact-terminal behavior, keyboard and mouse parity, reduced-motion semantics, and theme reachability
• keeps permission posture changes coherent across live state, persistence, previews, and restart
• persists complete Worker and Open chat artifacts beyond the bounded resident tail
• restores CI portability across Rust targets and release workflows
• gives the TUI state machine an explicit 16 MiB async-owner stack, eliminating modal-event stack overflows on macOS
• retains the provider, Workflow, Fleet, tool, setup, URL, billing, and release-candidate work already consolidated in the original candidate

OpenHands runtime parity

This is runtime simplification, not benchmark setup.

The default model-facing prompt now centers the execution loop on inspecting the repository, editing only what the task requires, verifying the result, and finishing with evidence. UI tutorials and cache-management narration are no longer the default instructions. That makes normal Codewhale behavior closer to the lean OpenHands execution contract, so benchmark behavior can improve as a consequence without a benchmark-only mode or manifest.

Verification

• cargo fmt and git diff checks pass
• cargo clippy --workspace --all-targets --locked -- -D warnings passes
• full codewhale-tui suite passes: 6,681 unit tests plus all active integration targets
• qa_pty passes 13 of 13, including compact config, resize, wheel, and approval-click coverage
• integration_mock_llm passes 41 of 41
• release_runtime_qa passes 5 active tests; the explicitly ignored 32-worker storm remains ignored
• locked optimized build succeeds for codewhale and codewhale-tui
• both binaries report 0.8.68 at commit 2e368a0
• npm pack/install/download-wrapper smoke passes with zero audit vulnerabilities
• app-server stdio protocol smoke passes; all configured provider exec routes pass no-spend dry-run validation
• all 18 crates pass the publish dry-run/package-content gate

Release boundary

This PR does not tag a release, create a GitHub Release, or publish crates/npm artifacts. After merge, the merged commit will be rebuilt and installed only as local dogfood for final testing.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

[claude]

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

---

Reviewing PR #4361 (v0.8.68 release candidate)

Todo list

• Gather context (PR body, changed files, CLAUDE.md/AGENTS.md)
• Understand request (this is a code review request, not implementation)
• Review diff for correctness bugs (in progress — fanning out across subsystems, this is a 100-file/25k-line diff)
• Review provider/model/route safety (v0.8.65 EPIC v0.8.65 EPIC: Separate provider facts, model facts, offerings, and route resolution #2608 invariant)
• Review reuse/simplification opportunities
• Review test coverage
• Review security concerns
• Post final review summary

[claude]

Claude finished @Hmbown's task in 9m 19s —— View job

---

Reviewing PR #4361 (v0.8.68 release candidate)

Todo list

• Gather context (PR body, changed files, CLAUDE.md/AGENTS.md)
• Understand request (code review, not implementation)
• Review provider/model/route safety (v0.8.65 EPIC v0.8.65 EPIC: Separate provider facts, model facts, offerings, and route resolution #2608 invariant)
• Review correctness bugs across runtime_contract, engine, subagent, terminal_session, provider_readiness, route_billing
• Review TUI-layer changes (config, mouse_ui, model_picker, settings_picker, motion)
• Review reuse/simplification opportunities
• Review test coverage
• Review security concerns
• Post final review summary

This is a 100-file, ~25.7k-line diff, so I fanned review out across three areas: provider/routing/billing safety, engine/runtime-contract correctness, and the TUI layer (config persistence, motion, mouse/keyboard). Inline comments are posted on the 5 most concrete findings; summary below.

1. Correctness bugs

• crates/tui/src/main.rs:1264-1274 — the 16 MiB stack doesn't cover the engine. run_async_main (the #[tokio::main] fn) gets the widened codewhale-main OS thread, and only code directly .awaited there (tui::run_tui) benefits. Engine::run() (core/engine.rs:3956 spawn_engine) goes through spawn_supervised → plain tokio::spawn (utils.rs:356), landing on a default-stack Tokio worker — the #[tokio::main] macro never gets a thread_stack_size override. That's exactly the code this PR touches most (turn_loop.rs, dispatch.rs, new StuckGuard/subagent-coordination). The PR/CHANGELOG claim "gives the TUI state machine an explicit 16 MiB async-owner stack, eliminating modal-event stack overflows on macOS" only holds for the render/modal loop, not the engine half of that state machine. See inline comment for a concrete fix (build the runtime manually with thread_stack_size) or narrow the claim.
• crates/tui/src/commands/groups/config/config.rs:1515-1519 vs 1586 — approval_mode's session-only (persist == false) edit path requires control == ApprovalPolicyControl::Unset exactly, while the parallel allow_shell path uses control.editable_root() regardless of persist. A session-only allow_shell toggle is permitted under RootConfig; the equivalent approval_mode toggle is rejected in the same state. Likely unintentional drift between two permission-posture controls this PR touches — no test covers either session-only + RootConfig case.
• crates/tui/src/config_ui.rs:511-593 — apply_document's per-key loop bails on the first set_config_value error. Combined with the broadened approval_mode rejection above, a full settings save (theme, locale, etc.) while approval policy is environment/profile/managed/project-controlled now aborts after model (processed first) has already been applied to live state, with no rollback. Worth validating all keys up front or moving policy-gated keys to the end of the list.
• crates/tui/src/compaction.rs:1279 — minor: ApiProvider::parse(client.provider_name()) replaces a previously-typed client.api_provider(); a future rename/typo in provider_name() would silently misattribute costs to Custom rather than failing loudly. Untested fallback path.
• No unwrap/expect-on-fallible-path panics, no unbounded recursion, and no unguarded races were found in stuck_guard.rs, coord.rs, or the subagent spawn path — run_subagent_task is timeout-bounded and spawn depth is capped by max_spawn_depth.
• motion/mode.rs + frame_requester.rs: reduced-motion gating is sound — should_request_animation_frames() consistently blocks frame coalescing/requests outside MotionMode::Full, and the UI falls back to the calm needs_redraw path otherwise. No stack-overflow-adjacent width/height underflow issues found in phase_strip.rs/composer_chrome.rs/plan_todo_bridge.rs (saturating arithmetic used throughout). Mouse/keyboard parity in model_picker.rs/provider_picker.rs looks consistent.

2. Provider/model/route safety (EPIC #2608 invariant)

No violations found. This is the area I scrutinized hardest given the two large new files (provider_readiness.rs, route_billing.rs), and both are explicitly designed around the invariant:

• provider_readiness.rs scopes credential/readiness state by (provider, provider_id, endpoint, model, auth_class) tuples, never by string-matching a model prefix — tests explicitly assert cross-provider isolation for same-shaped model ids.
• route_billing.rs derives BillingPresentation from explicit provider + auth_mode/base_url config; a test (unknown_is_unknown_not_zero_dollars) specifically checks a DeepSeek-shaped model id routed through NvidiaNim doesn't inherit DeepSeek billing.
• fleet/profile.rs carries an explicit comment that provider is "never inferred by sniffing model for a provider-shaped substring (EPIC v0.8.65 EPIC: Separate provider facts, model facts, offerings, and route resolution #2608)" — provider is a validated, first-class field.
• Pre-existing prefix checks in pricing.rs are capability/pricing lookups keyed by model id, not provider-selection decisions, and are unchanged by this diff.

One edge case worth a look: route_billing.rs:284 (xiaomi_is_explicit_pay_as_you_go) has an untested inconsistent-signal case (payg mode set alongside a token-plan key) that could show metered dollars for a token-plan account — not a provider-inference bug, just an env-var-precedence gap.

3. Reuse / simplification

• crates/tui/src/core/mod.rs:17-27 — only model.rs and termination.rs from runtime_contract/ are wired in via #[path]; pub mod runtime_contract; is never declared, so budget.rs, context.rs, ledger.rs, manifest.rs, profile.rs, progress.rs, resources.rs, retry.rs, terminal.rs, work.rs (~1,576 lines) ship as intentionally uncompiled scaffolding (documented as staged per TUI-DOG-017). Reasonable as a rollout strategy, but means this PR adds ~1,576 lines with zero compiler or test verification — their mutual references (e.g. to RUNTIME_CONTRACT_SCHEMA_VERSION) could already be broken without CI catching it.
• fleet/profile.rs:296 (load_workspace_agent_profiles_tolerant) parses every profile TOML twice per roster load (once for identity, once for the full profile) — harmless, just wasted I/O.

4. Tests

• stuck_guard.rs has solid unit coverage, but no integration test drives its StuckSignal::Stop path through turn_loop.rs's new injection points.
• coord.rs/subagent/tests.rs cover per-tool interrupt/timeout cases well, but there's only one test (completion_claim_preserves_running_gate_and_excludes_late_cancel) for the manager-level race between concurrent agents/interrupt and natural completion — a partial gap given how central that race is to the new coordination logic.
• The approval_mode/allow_shell control-inconsistency and the apply_document partial-apply case above have no test coverage either direction.
• The dead runtime_contract/* files have no tests since they don't compile into the test binary.

5. Security

Nothing new stood out — no secret-handling, shell/exec-policy, or input-validation regressions found in the reviewed subsystems.

Good choices worth noting

• The provider/routing/billing redesign (provider_readiness.rs, route_billing.rs, fleet/profile.rs) is genuinely careful about the EPIC v0.8.65 EPIC: Separate provider facts, model facts, offerings, and route resolution #2608 invariant, with tests written specifically to catch prefix-based provider inference regressions.
• motion/mode.rs's reduced-motion gating is clean and consistently applied.
• Panic supervision (spawn_supervised) and depth/timeout bounding in the new subagent coordination code are solid safety nets even though the 16 MiB stack claim doesn't reach that code (finding Fix cargo fmt formatting issues #1).

[claude]

Correctness: approval_mode's session-only (persist == false) editability requires control to be exactly ApprovalPolicyControl::Unset (line 1517-1518), whereas the parallel allow_shell branch below (line 1586) calls control.editable_root() unconditionally, regardless of persist. That means a session-only toggle of allow_shell is permitted whenever editable_root() returns true (e.g. under RootConfig), but the equivalent session-only toggle of approval_mode is rejected in that same state. This is an unflagged behavioral inconsistency between two permission-posture controls added/touched in this PR, and no test in config/tests.rs exercises the RootConfig + session-only case for either key to confirm which behavior is intended.

[claude]

Minor: replacing the previous typed client.api_provider() with ApiProvider::parse(client.provider_name()) silently falls back to ApiProvider::Custom on any mismatch between a provider's provider_name() string and what ApiProvider::parse recognizes. A future rename/typo here would misattribute costs to "Custom" silently rather than failing loudly or falling back to the client's actual typed provider. No test covers this fallback path.

[claude]

Reuse/simplification note: per this comment, runtime_contract/budget.rs, context.rs, ledger.rs, manifest.rs, profile.rs, progress.rs, resources.rs, retry.rs, terminal.rs, work.rs (~1,576 lines total) are intentionally left uncompiled scaffolding — pub mod runtime_contract; is never declared, only model.rs and termination.rs are wired in via #[path]. That's a reasonable staged-rollout strategy per TUI-DOG-017, but it means this PR ships ~1,576 lines that get zero compiler checking and zero test execution (they don't even build under cfg(test)). Their cross-module references to each other (e.g. ledger.rs, manifest.rs, profile.rs importing super::RUNTIME_CONTRACT_SCHEMA_VERSION) could already be broken and CI would not catch it until a later PR wires them in. Worth a tracking issue/comment reference so this doesn't get lost.

[claude]

Correctness / claim mismatch: this gives the codewhale-main OS thread (which runs run_async_main, a #[tokio::main] fn) a 16 MiB stack. But run_async_main's #[tokio::main] macro builds its own multi-thread Tokio runtime with the default (un-overridden) worker stack size — there's no Builder::thread_stack_size call anywhere in the crate. Only code directly .awaited on this thread (i.e. tui::run_tui(...), main.rs:7339) benefits from the widened stack.

Engine::run() (crates/tui/src/core/engine.rs:3956 spawn_engine) is dispatched via spawn_supervised → plain tokio::spawn (utils.rs:356), so it runs on a different OS thread from Tokio's default worker pool — still the un-widened default stack. That's exactly the code this PR touches most (turn_loop.rs, dispatch.rs, new StuckGuard/subagent-coordination logic).

So the PR description's claim — "gives the TUI state machine an explicit 16 MiB async-owner stack, eliminating modal-event stack overflows on macOS" — only holds for the render/modal-dispatch loop, not the engine half of the state machine. If any of the stack-overflow reports motivating this change were actually inside engine/turn-loop code (not pure rendering), this change doesn't fix them.

[Fix this →](https://claude.ai/code?q=In+crates%2Ftui%2Fsrc%2Fmain.rs%2C+the+%23%5Btokio%3A%3Amain%5D+macro+on+run_async_main+builds+a+tokio+runtime+with+default+worker+thread+stack+size%2C+so+tokio%3A%3Aspawn+tasks+%28e.g.+Engine%3A%3Arun%28%29+via+spawn_engine+in+crates%2Ftui%2Fsrc%2Fcore%2Fengine.rs%3A3956%2C+which+goes+through+spawn_supervised+in+utils.rs%3A356%29+do+NOT+get+the+16+MiB+CODEWHALE_MAIN_STACK_BYTES+stack+-+that+only+applies+to+the+codewhale-main+OS+thread+itself.+Replace+%23%5Btokio%3A%3Amain%5D+with+a+manually+built+tokio%3A%3Aruntime%3A%3ABuilder%3A%3Anew_multi_thread%28%29.thread_stack_size%2816+*+1024+*+1024%29.build%28%29%3F.block_on%28...%29+so+worker+threads+running+the+engine+state+machine+also+get+the+larger+stack%2C+or+narrow+the+PR%2FCHANGELOG+claim+to+the+render+loop+only.&repo=Hmbown/CodeWhale

[claude]

Correctness (plausible): this loop (starting at apply_document, line ~511) applies each (key, value) pair via set_config_value and bail!s on the first error (line 593). approval_mode is processed right after model, before work_surface_placement and most other settings below. This PR broadened approval_mode's error condition in set_config_value (commands/groups/config/config.rs:1520) to reject whenever ApprovalPolicyControl is Environment/Profile/ManagedConfig/ProjectConfig/Requirements (previously narrower). So a full settings-editor save while approval policy is externally controlled now aborts the whole apply after model (and any earlier key) has already been applied to live app state — with no rollback of keys already processed and no test covering this interaction. Consider validating all keys up front, or moving policy-gated keys to the end of the list, so an approval-mode rejection can't leave other edits (like theme/locale in this same call) half-applied.

[claude]

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

---

Reviewing PR #4361 (v0.8.68 release candidate) — re-review after synchronize

Todo list

• Gather context (PR body, prior review comments, CLAUDE.md/AGENTS.md)
• Diff against previous review point to see what changed since last pass
• Check whether the 5 previously flagged findings were addressed
• Review new commits for fresh correctness/provider-safety issues
• Post updated review summary

What changed since the last review

The last full review (02:09–02:10 UTC) was against 2e368a0. Two commits landed after that:

• fd8254b — test(tui): isolate cross-platform release contracts
• 9cf5522 — test(tui): isolate Windows config migration path

Together they touch only crates/tui/src/settings.rs, crates/tui/src/tui/ui/tests.rs, and crates/tui/src/tui/views/mod.rs. I reviewed this incremental diff (git diff 2e368a0..9cf5522):

• settings.rs — extracts load_persisted_from_candidates(primary, legacy_home, legacy_config_dir) out of load_persisted() so tests can inject explicit paths instead of depending on dirs::config_dir() (which uses the Win32 known-folder API and can't be isolated via APPDATA/XDG_CONFIG_HOME env overrides in a parallel test runner). The extraction is a faithful behavior-preserving split — load_persisted() still calls it with the real settings_path_candidates(). Two migration tests were updated to use the new helper and a WT_SESSION marker was added so Windows test runs aren't misclassified as legacy ConHost by an unrelated heuristic. This is a legitimate test-isolation fix, no production-path behavior changes.
• tui/ui/tests.rs — pins app.low_motion = false; app.fancy_animations = true; in underwater_motion_keeps_its_smoother_cadence_during_live_status so the test's authored motion cadence doesn't inherit a host's real saved low-motion/legacy-console policy from create_test_app(). Reasonable, self-contained.
• views/mod.rs — replaces manual format!("...{}...", path.display()) with toml::Value::String(path...).to_string() when writing a test's managed config TOML. This is a real correctness improvement for the test itself: path.display() on Windows can contain backslashes that aren't valid inside an unescaped TOML string, so building the value through toml::Value guarantees correct escaping.

No new correctness, provider-safety, or security issues in this incremental diff — it's test-quality-only, matching both commit messages.

Status of the 5 previously-flagged findings

None of the files involved in this synchronize were touched, so all 5 remain open as reported in the prior review pass (unchanged, re-verified against current HEAD at 9cf5522):

1. crates/tui/src/main.rs:1273 — the 16 MiB stack (CODEWHALE_MAIN_STACK_BYTES) covers the codewhale-main OS thread only; #[tokio::main] (main.rs:1289) still builds its runtime with default worker stack size, so spawn_engine (core/engine.rs:3956) → spawn_supervised → tokio::spawn still lands on an un-widened stack.
2. crates/tui/src/commands/groups/config/config.rs:1516-1519 vs 1586 — approval_mode's session-only edit path still requires control == ApprovalPolicyControl::Unset exactly, while allow_shell's session-only path still uses control.editable_root() unconditionally — confirmed still present at current line numbers.
3. crates/tui/src/config_ui.rs apply_document — still bails on first set_config_value error with no rollback of already-applied keys.
4. crates/tui/src/compaction.rs:1279 — ApiProvider::parse(client.provider_name()) fallback-to-Custom behavior unchanged.
5. crates/tui/src/core/mod.rs:23-28 — runtime_contract/ still ships ~1,576 lines of uncompiled scaffolding (only model.rs/termination.rs wired via #[path], no pub mod runtime_contract;).

None of these block the test-isolation commits themselves, but they're still open items against the release-candidate claims in the PR description (particularly #1, which contradicts the "eliminates modal-event stack overflows" claim for engine-side stack overflows, and #2/#3, which have no test coverage either direction).