feat(tui): prioritize the Fleet menu surface (#5888) - #5905
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6adc6744-d02f-4477-8511-78940eadd6d4) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @Hmbown's task in 2m 35s —— View job Review: PR #5905 — prioritize the Fleet menu surface
SummaryThis is a clean, minimal, presentation-only change. I traced every claim in the PR description against the code and found it accurate:
No correctness bugs, no dead code, no missing test coverage — the three new/updated tests ( Note: I wasn't able to run Nice work — this is exactly the kind of "fold advanced verbs one level deeper, dispatch untouched" change the ponytail method calls for. |
There was a problem hiding this comment.
🟢 Approval recommended
The changes appear confined to presentation/help-text and are covered by updated/new tests, with only a minor help-text capitalization typo to address.
Pull request overview
This PR implements issue #5888 by reducing the amount of Fleet UI/command surface presented at once in the TUI, while keeping all existing Fleet dispatch behavior reachable via /fleet help and preserving roster/alias compatibility.
Changes:
- Shortens
/fleet’s advertised usage to a prioritized core verb set and moves advanced verbs into grouped/fleet helptext. - Updates the Fleet roster view to hide the built-in legacy
generalalias (while keeping it dispatchable) so the default presentation is one row per posture. - Adds/updates tests to lock the new usage/help behavior and the roster presentation rules around
general.
File summaries
| File | Description |
|---|---|
| crates/tui/src/tui/views/fleet_roster/tests.rs | Updates canonical built-in roster expectations and adds tests for folding the built-in general alias while preserving dispatch compatibility. |
| crates/tui/src/tui/views/fleet_roster.rs | Filters the built-in general alias out of roster presentation while keeping engine roster semantics unchanged. |
| crates/tui/src/commands/groups/core/fleet.rs | Introduces primary verb/usage constants, revises help text to document advanced verbs, and updates tests to enforce the new surface. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| Durable runs — these act on the durable .codewhale/fleet.jsonl ledger for this \ | ||
| workspace, the same records `codewhale fleet` reads and writes. the ledger file, \ | ||
| saved rosters, and config tables keep the Fleet name:\n", |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b141b4c275
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| The fleet is who is working right now. The primary verbs cover the daily loop:\n\ | ||
| /fleet (or /fleet members) opens the roster — each member's role, model, and access; \ | ||
| Enter on a member row opens that member's editor. /fleet setup opens the authoring \ |
There was a problem hiding this comment.
When the UI locale is non-English, /fleet help still emits these newly added English paragraphs—including the literal Enter hint—because help_text() constructs them directly instead of using tr(locale, MessageId::...). Move the new prose into localized message IDs and compose command/key tokens in code so the prioritized Fleet surface follows the TUI localization contract.
AGENTS.md reference: crates/tui/AGENTS.md:L25-L26
Useful? React with 👍 / 👎.
| Durable runs — these act on the durable .codewhale/fleet.jsonl ledger for this \ | ||
| workspace, the same records `codewhale fleet` reads and writes. the ledger file, \ | ||
| saved rosters, and config tables keep the Fleet name:\n", |
There was a problem hiding this comment.
When a user opens /fleet help, this heading says all following durable-run entries act on the ledger, but operations_for_domain(ControlDomain::Fleet) also includes FleetRestart, whose descriptor is limited to CLI_ONLY; the generated list therefore presents /fleet restart <worker-id> like a usable slash verb even though it always returns an unavailable receipt. Filter operations not offered on the Slash surface or explicitly label restart as CLI-only.
Useful? React with 👍 / 👎.
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
PR simplifies the /fleet usage line to five primary verbs and hides the built-in general alias from the default roster presentation while preserving dispatch and help coverage. Tests were updated and added appropriately.
Findings
- [INFO] Built-in general filter does not trim id unlike operator filter (
crates/tui/src/tui/views/fleet_roster.rs:212)
The new presentation filter uses m.id.eq_ignore_ascii_case("general") without .trim(), while the operator row filter uses m.id.trim().eq_ignore_ascii_case("operator"). If a built-in member id ever contained surrounding whitespace, the alias would not be folded. Trimming keeps the alias handling consistent.
Suggestions
-
crates/tui/src/tui/views/fleet_roster.rs:212— Use the same trimmed id comparison as the operator filter so the built-in general alias is consistently folded out of presentation even if ids contain surrounding whitespace.&& !(m.id.trim().eq_ignore_ascii_case("general")
Assessment
Changes align with the PR intent and are well-covered by unit tests. Dispatch compatibility is preserved and help text keeps advanced verbs reachable. Only a minor consistency nit was found.
Advisory review by Codewhale (codewhale review --pr 5905 --post, head b141b4c2753a7cff13bcc966cd10626d82818d36). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| // origin, including saved-team members, which carry | ||
| // Personal/Workspace origin by construction) is the | ||
| // user's own member and stays visible. | ||
| && !(m.id.eq_ignore_ascii_case("general") |
There was a problem hiding this comment.
[INFO] Built-in general filter does not trim id unlike operator filter
The new presentation filter uses m.id.eq_ignore_ascii_case("general") without .trim(), while the operator row filter uses m.id.trim().eq_ignore_ascii_case("operator"). If a built-in member id ever contained surrounding whitespace, the alias would not be folded. Trimming keeps the alias handling consistent.
| // origin, including saved-team members, which carry | ||
| // Personal/Workspace origin by construction) is the | ||
| // user's own member and stays visible. | ||
| && !(m.id.eq_ignore_ascii_case("general") |
There was a problem hiding this comment.
Use the same trimmed id comparison as the operator filter so the built-in general alias is consistently folded out of presentation even if ids contain surrounding whitespace.
| && !(m.id.eq_ignore_ascii_case("general") | |
| && !(m.id.trim().eq_ignore_ascii_case("general") |
The /fleet usage line advertised 14 verbs at once, mixing member/model setup, saved teams, session workers, and durable run control. It now presents the prioritized core only — /fleet [members|setup|teams| workers|help] — and every other verb stays dispatchable, documented one level deeper in /fleet help under explicit groups: team model routes (models/add/remove), saved routes (save/save-as), and the durable run control plane (data-driven via operations_for_domain). The unknown-target error now points at the core set. The roster's default surface showed the legacy built-in general alias next to worker — the same posture twice. from_parts now folds only the untouched built-in alias out of presentation (12 -> 11 rows); a user-authored general (config/personal/project origin, incl. saved-team members) stays visible, and dispatch keeps resolving general through FleetRoster::get and the identity selector alias. Local gates, honestly: cargo check -p codewhale-tui CLEAN. The three directly modified command tests pass (fleet_aliases_are_registered_on_ command_info, fleet_usage_presents_a_prioritized_core_and_help_keeps_ the_rest_reachable, slash_command_and_cli_agree_on_fleet_verb_ids). This machine's 8MB hard stack limit aborts several pre-existing fleet tests that construct the live config/catalog (two proven byte-identical at baseline via stash; CI runs the same suite green on all three OSes), so hosted CI is the authoritative gate for this PR. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
b141b4c to
89b005b
Compare
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_542d39aa-2e1f-4561-be4c-ea40f643a8f6) |
|
Claude finished @Hmbown's task in 2m 15s —— View job Review: PR #5905 — prioritize the Fleet menu surface
SummaryThis is a scoped presentation/help-text change; dispatch and roster-resolution semantics are preserved as claimed. No provider/model routing is touched (not applicable to §2 of the review criteria). Findings1. Correctness — none found.
2. Pre-existing (not introduced by this diff), worth a follow-up:
3. Reuse/simplification — good: consolidating usage/help into 4. Tests — solid coverage of the new behavior, including the important negative case (
5. Security — no secret handling, shell/exec, or input-validation surface touched. AssessmentLow risk, matches the PR description's claims. No blocking issues. The one actionable item is the |
There was a problem hiding this comment.
Codewhale review
PR simplifies the Fleet command surface by reducing the usage string to five primary verbs and moving advanced verbs into /fleet help. It also hides the built-in 'general' alias from the default roster while preserving user-defined 'general' members. Tests were updated and added. The change is mostly presentational with low risk, but there is a potential shadowed-member gap and a minor trimming inconsistency.
Findings
- [WARNING] Built-in general alias may still be shown via shadowed list (
crates/tui/src/tui/views/fleet_roster.rs:217)
The filter only removes built-ingeneralfromroster.members(), not fromroster.shadowed(). If a user-definedgeneralshadows the built-in alias, the built-in entry could appear in the shadowed section, contradicting the goal of folding it out of presentation. Either filter shadowed members as well or verify that built-in aliases never appear inshadowed. - [INFO] Inconsistent trimming for id matching (
crates/tui/src/tui/views/fleet_roster.rs:212)
Operator id is checked withtrim(), but the general alias check does not trim. If a built-ingeneralid contains surrounding whitespace, it will not be hidden. Recommend usingtrim()for consistency. - [INFO] Missing test for unknown target error message (
crates/tui/src/commands/groups/core/fleet.rs:305)
The unknown target error message changed to usePRIMARY_VERBSand points to/fleet help, but no test covers this new string. Add a test that asserts the error message for an unknown verb includes the primary verbs and the help pointer.
Suggestions
-
crates/tui/src/tui/views/fleet_roster.rs:212— Align the id check with the operator check by trimming whitespace before comparing, so built-ingeneralis hidden even if ids carry stray whitespace.&& !(m.id.trim().eq_ignore_ascii_case("general") && m.origin == ProfileOrigin::BuiltIn)
Assessment
Low risk overall. The change is well-tested for the main paths, but the shadowed-member gap should be investigated and possibly addressed to fully meet the fold-out-of-presentation goal. The trimming inconsistency is minor and safe to fix. Consider adding a test for the new error message.
Advisory review by Codewhale (codewhale review --pr 5905 --post, head 89b005b4b33bcc0fc8fccaebfeed3be746a5071b). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
| }) | ||
| .cloned() | ||
| .collect(), | ||
| shadowed: roster.shadowed().to_vec(), |
There was a problem hiding this comment.
[WARNING] Built-in general alias may still be shown via shadowed list
The filter only removes built-in general from roster.members(), not from roster.shadowed(). If a user-defined general shadows the built-in alias, the built-in entry could appear in the shadowed section, contradicting the goal of folding it out of presentation. Either filter shadowed members as well or verify that built-in aliases never appear in shadowed.
| // origin, including saved-team members, which carry | ||
| // Personal/Workspace origin by construction) is the | ||
| // user's own member and stays visible. | ||
| && !(m.id.eq_ignore_ascii_case("general") |
There was a problem hiding this comment.
[INFO] Inconsistent trimming for id matching
Operator id is checked with trim(), but the general alias check does not trim. If a built-in general id contains surrounding whitespace, it will not be hidden. Recommend using trim() for consistency.
| None => CommandResult::error(format!( | ||
| "Unknown /fleet target '{other}'. Use members, setup, teams, list, status, \ | ||
| workers, interrupt <worker-id>, or resume <run-id>.." | ||
| "Unknown /fleet target '{other}'. Use {} — or /fleet help for the full \ |
There was a problem hiding this comment.
[INFO] Missing test for unknown target error message
The unknown target error message changed to use PRIMARY_VERBS and points to /fleet help, but no test covers this new string. Add a test that asserts the error message for an unknown verb includes the primary verbs and the help pointer.
| && !(m.id.eq_ignore_ascii_case("general") | ||
| && m.origin == ProfileOrigin::BuiltIn) |
There was a problem hiding this comment.
Align the id check with the operator check by trimming whitespace before comparing, so built-in general is hidden even if ids carry stray whitespace.
| && !(m.id.eq_ignore_ascii_case("general") | |
| && m.origin == ProfileOrigin::BuiltIn) | |
| && !(m.id.trim().eq_ignore_ascii_case("general") | |
| && m.origin == ProfileOrigin::BuiltIn) |
Closes #5888.
Before → after
/fleetusage line: 14 verbs → 5 (members|setup|teams|workers|help). The other nine stay fully dispatchable, documented one level deeper in/fleet helpunder explicit groups (team model routesmodels|add|remove, saved routessave|save-as, durable run control viaoperations_for_domain). No match arm was removed — nothing becomes undispatchable.generalalias (same posture asworker) folds out of presentation; a user-authoredgeneral(any non-built-in origin, incl. saved-team members) stays visible;FleetRoster::get("general")and the identity-selector alias keep engine dispatch compat.Tests: updated
built_in_party_lists_all_members_in_canonical_order,fleet_aliases_are_registered_on_command_info,slash_command_and_cli_agree_on_fleet_verb_ids; addedfleet_usage_presents_a_prioritized_core_and_help_keeps_the_rest_reachable,default_roster_folds_the_legacy_general_alias_out_of_presentation,user_defined_general_member_stays_visible.Gate honesty:
cargo check -p codewhale-tuiclean; the three directly modified command tests pass locally. Several pre-existing fleet tests that build the live config/catalog abort with stack overflow on the preparing machine (8MB hard stack limit) — two were proven byte-identical at baseline via stash; hosted CI runs this exact suite green on ubuntu/macos/windows, so the matrix here is the authoritative gate. Design grounded in the #5888 source-audit comment; prepared by an implement agent whose execution was blocked by a stale parked-agent write claim (see #5575 family), applied and gated by the operator.Note
Low Risk
Presentation and help copy only; no dispatch paths or roster engine semantics were removed.
Overview
/fleetsurface (#5888) — The command palette usage string shrinks from fourteen verbs to five:members,setup,teams,workers, andhelp. Model routes (models,add,remove), saved routes (save,save-as), and durable ledger verbs remain typed as before; they are grouped in/fleet helpinstead of the usage line. Unknown-verb errors now cite the primary set and point to help.Fleet roster — The default list hides only the built-in
generalalias (same posture asworker), so the UI shows eleven rows (operator plus ten members). Engine lookup and identity-selector alias behavior forgeneralare unchanged; a user-definedgeneral(non-built-in origin) still appears.Tests — Assertions lock the shortened usage, help coverage for advanced and control verbs, roster canonical order without built-in
general, and visibility rules for user-definedgeneral.Reviewed by Cursor Bugbot for commit 89b005b. Bugbot is set up for automated code reviews on this repo. Configure here.