Skip to content

fix(tui): focus v0.9.1 chrome on todos and agents - #4711

Merged
Hmbown merged 4 commits into
mainfrom
codex/v091-finalization
Jul 22, 2026
Merged

fix(tui): focus v0.9.1 chrome on todos and agents#4711
Hmbown merged 4 commits into
mainfrom
codex/v091-finalization

Conversation

@Hmbown

@Hmbown Hmbown commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • keep the top bar, but render only active To-dos followed by Sub-agents; hide completed-only chrome and keep generic operation/coordination detail out of the live strip
  • make top/side dividers draggable with persisted bounded dimensions
  • use theme-native composer permission/mode rails and restore uwu config round-tripping
  • increase bounded inline reasoning visibility and clarify local vs full expansion
  • classify failed non-durable shell operations as transient instead of persistent Needs input
  • carry the release candidate cleanly through Rust 1.96 clippy
  • align real-PTY acceptance with the To-do/Sub-agent contract and silence desktop notifications inside sealed fixtures

Verification

  • cargo fmt --all -- --check
  • cargo check -p codewhale-tui
  • cargo clippy -p codewhale-tui --all-features --all-targets -- -D warnings
  • cargo test -p codewhale-tui --bin codewhale-tui --quiet -- --test-threads=1 (8064 passed, 4 ignored)
  • cargo test -p codewhale-tui --test qa_pty -- --test-threads=1 (25 passed, 1 ignored)
  • focused history, underwater, config UI, settings, composer-theme, and Work-surface suites

Closes #4699
Closes #4700
Closes #4701
Closes #4702
Closes #4712

Follow-up steer-vs-queue configuration remains tracked in #4703. Model-facing context-diet follow-ups remain tracked in #4704-#4710.

Copilot AI review requested due to automatic review settings July 22, 2026 21:47
@cursor

cursor Bot commented Jul 22, 2026

Copy link
Copy Markdown

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 AI 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.

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

@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Reviewing PR #4711

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md guidance)
  • Read diff against origin/main
  • Review correctness (logic/panics/error handling)
  • Review provider/model/route safety invariant
  • Review reuse/simplification opportunities
  • Review test coverage
  • Review security (secrets, shell/exec policy, input validation)
  • Post final review summary

View job run

@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

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


Review of PR #4711

  • Gather context (PR body, changed files, CLAUDE.md/AGENTS.md guidance)
  • Read diff against origin/main
  • Review correctness (logic/panics/error handling)
  • Review provider/model/route safety invariant
  • Review reuse/simplification opportunities
  • Review test coverage
  • Review security (secrets, shell/exec policy, input validation)
  • Post final review summary

Summary

Solid, well-tested change. I traced the full data flow for the new work-surface behavior (projectproject_visibleheight/split_chat/render in crates/tui/src/tui/work_surface/{model,render,input}.rs) and didn't find correctness bugs. A few things I checked closely because they looked suspicious at first but turned out fine:

  • latest_rows staleness: project_visible only overwrites latest_rows in the Top branch, but project() (called first, unconditionally) already sets latest_rows = rows.clone() for the full catalog — so split_chat's latest_rows.is_empty() check stays correct for Left/Right placements too (model.rs:327 vs model.rs:345-384). No staleness bug.
  • Divider drag math in input.rs (handle_mouse): sign conventions for Left vs Right placement deltas are correctly mirrored (resize_anchor_column - mouse.column for Right vs the reverse for Left), matching the divider positions rendered in render.rs::render_divider. Bounds are i32-safe (no overflow from u16 deltas).
  • height() in render.rs:33-58: moving effective_placement assignment before project_visible(app) is necessary and correct, since the projection now branches on effective_placement.
  • All 12 UiTheme const definitions in palette/themes.rs got the three new permission_* fields — no missed variant (would have failed to compile otherwise), consistent with the reported clean cargo check.

Provider/model/route safety (EPIC #2608): Not implicated — no code in this diff touches model/provider selection or prefix inference.

Bug actually fixed, worth calling out: config_ui.rs:929 adds the missing Some("uwu") => Ok(Self::Uwu) parse arm. Self::Uwu => "uwu" (serialize side) already existed, so uwu could be written to config but never read back — a real round-trip bug, correctly fixed here.

Minor findings

  1. Test coverage gap (low severity): there's no regression test that would have caught the uwu round-trip bug before it happened, and none added now to prevent a repeat for the next theme. Consider a loop test asserting UiThemeValue::from_str(&theme.as_setting()) succeeds for every entry in palette::SELECTABLE_THEMES (or the UiThemeValue enum), so a future theme addition can't silently repeat this. Fix this →
  2. Minor duplication (nit): the divider hit-test in input.rs::handle_mouse (on_divider match) and the divider's drawn position in render.rs::render_divider encode the same per-placement coordinate logic independently. Not a bug today, but a future placement tweak in one without the other would desync click target from visible divider. Not blocking.
  3. is_transient_failed_operation (model.rs) redundantly re-checks node.state == NodeState::Failed, which its only call site already guards via the NodeState::Failed if ... match arm. Harmless, just dead redundancy.

Security: no new shell/exec surface, no secret handling changes. The #[allow(clippy::too_many_arguments)] additions are cosmetic (Rust 1.96 clippy) and don't change behavior.

Tests: good additions — divider_drag_resizes_top_left_and_right_surfaces, top_bar_excludes_generic_operations, transient_failed_operation_is_recent_while_durable_failure_needs_input, and every_selectable_theme_keeps_permission_lanes_distinct all directly exercise the new behavior described in the PR summary.

Nothing here blocks merge from my read.

@claude

claude Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

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


Review of PR #4711 (updated after new commit)

  • Gather context (PR body, prior review, changed files)
  • Check what changed since last review
  • Read full diff against origin/main
  • Review correctness (logic/panics/error handling)
  • Review provider/model/route safety invariant
  • Review reuse/simplification opportunities
  • Review test coverage
  • Review security (secrets, shell/exec policy, input validation)
  • Post final review summary

What changed since my last pass

The new commit (2b53dd3, "test(tui): align PTY acceptance with todo chrome") is test-only — no production code changed (confirmed via git show --stat, touches only crates/tui/tests/qa_pty.rs and crates/tui/tests/support/qa_harness/harness.rs). My prior review of the production code (work_surface, themes.rs, config_ui.rs, settings.rs, subagent/coord.rs, etc.) still stands — no changes there.

This commit's changes

  1. qa_harness/harness.rs: make_sealed_workspace() now pre-seeds .codewhale/config.toml and .deepseek/config.toml with [notifications]\nmethod = "off"\ncompletion_sound = "off". This quiets notification side effects for every PTY test by default; individual tests that write their own config.toml simply overwrite it (confirmed several tests append the same [notifications] block to their custom configs), so no conflict.
  2. qa_pty.rs: renames assertions from the old ambient "Work" / "Work ·" labels to "To-do ·", matching the PR's chrome rename. Also strengthens real_coordination_details_use_typed_persisted_receipts_in_a_unix_pty:
    • First boot (default placement) now asserts ambient chrome contains neither "Coordination Work" nor the private marker — verifying coordination detail stays out of the live strip by default.
    • Shuts down, writes work_surface_placement = "right" to settings.toml, reboots reusing the same sealed workspace, and then asserts "Coordination Work" appears — verifying the detail is still reachable when the surface is explicitly placed to a side panel.
    • New assertion in work_surface_file_mutation_modes_are_truthful_in_real_pty_frames confirms a completed "Wrote 4 files" summary does not leak into ambient chrome after settling — directly exercises the "hide completed-only chrome" behavior from the PR summary.

Traced the ws/h ownership shuffle across the shutdown-and-rewrite-settings sequence (spawn_minimal_with_env consumes and returns ws, matching the established pattern used elsewhere in this file) — no use-after-move or shadowing issues.

Verification note: I don't have approval to run cargo check/clippy/tests in this environment, so I read the diff manually rather than compiling. If you'd like a compiled confirmation, --allowedTools would need to include the cargo invocations — everything else in this pass was static review.

No new findings. This is a solid tightening of test coverage for behavior already reviewed; nothing here blocks merge.

@Hmbown

Hmbown commented Jul 22, 2026

Copy link
Copy Markdown
Owner Author

v0.9.1 handoff receipt — 2026-07-22

Remote head is 2b53dd32611123297895e08c19f5997a2175bbb0 on codex/v091-finalization; local HEAD, origin/codex/v091-finalization, and GitHub PR head were verified identical with a clean worktree.

Local verification on this exact head:

  • cargo fmt --all -- --check — pass
  • cargo clippy -p codewhale-tui --all-features --all-targets -- -D warnings — pass
  • serialized TUI bin suite — 8064 passed, 4 ignored
  • serialized real PTY suite — 25 passed, 1 ignored
  • PTY fixtures now disable notifications and completion sounds, preventing macOS test notifications (PTY acceptance tests must not emit desktop notifications #4712)

Fresh CI run: https://github.com/Hmbown/CodeWhale/actions/runs/29962091776

At handoff, Ubuntu, npm smoke, version drift, web lint/typecheck, Buildkite, GitGuardian, and several CodeQL analyzers were green; macOS, Windows, Rust CodeQL, lint, and review were still running.

Security release gate: #4713. Remote baseline is 17 open Dependabot alerts (7 high, 10 moderate; axios, brace-expansion, js-yaml, protobufjs), 0 open code-scanning alerts, and 0 open secret-scanning alerts.

After all checks pass: merge this PR, fetch the exact merge commit from origin/main, run the repository-wide deep security gate and dependency disposition in #4713, then build/install/dogfood that exact clean merge and attach the receipt to #4650. Do not move the stale public v0.9.1 tag, create a GitHub Release, publish packages, or deploy the website without explicit approval.

@Hmbown

Hmbown commented Jul 22, 2026

Copy link
Copy Markdown
Owner Author

v0.9.1 takeover receipt — 2026-07-22

Confirmed local HEAD �b53dd32611123297895e08c19f5997a2175bbb0� matches remote PR head and origin/codex/v091-finalization; worktree clean.

All GitHub PR checks are now green for the exact handoff head, including Test (macos-latest), Test (windows-latest), Lint, Version drift, Change detection, npm wrapper smoke, Buildkite, GitGuardian, and CodeQL analyzers.

Local verification re-run on this exact head:

  • �cargo fmt --all -- --check� — pass
  • �cargo clippy -p codewhale-tui --all-features --all-targets -- -D warnings� — pass
  • �cargo test -p codewhale-tui --bins --locked� — 8064 passed; 0 failed; 4 ignored
  • �cargo test -p codewhale-tui --test qa_pty --locked -- --test-threads=1� — 25 passed; 0 failed; 1 ignored

No code changes were required. Merging PR #4711 now and will record the exact merge SHA before proceeding to the security gate (#4713) and release fan-in gate (#4650).

@Hmbown
Hmbown merged commit 88a158e into main Jul 22, 2026
22 checks passed
@Hmbown
Hmbown deleted the codex/v091-finalization branch July 24, 2026 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants