use multi-harness cloud agent icons + status#9263
Conversation
harryalbert
changed the title
harryalbert
changed the title
|
I'm starting a first review of this pull request. I completed the review and posted feedback on this pull request. Comment You can view the conversation on Warp. I completed the review and posted feedback on this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR centralizes agent icon derivation and rendering across vertical tabs, pane headers, conversation-list rows, and notifications, adding cloud-lobe/status handling for multiple harnesses plus tests and specs for REMOTE-1458.
Concerns
- The new harness-to-CLI mapping is not gated by
FeatureFlag::AgentHarness, so existing or persisted non-Oz harness data can still display Claude/Gemini/OpenCode icons when the feature is disabled, contrary to the PR's flag-off validation. - Neutral/non-agent icons now reuse the agent-specific proportional renderer, shrinking plain terminal/code/file icons in vertical tabs even though non-agent surfaces are supposed to remain visually unchanged.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| IconWithStatusVariant::Neutral { icon, icon_color } => render_circle( | ||
| icon.to_warpui_icon(icon_color).finish(), | ||
| internal_colors::fg_overlay_2(theme), | ||
| total_size, | ||
| ), | ||
| IconWithStatusVariant::NeutralElement { icon_element } => render_circle( | ||
| icon_element, | ||
| internal_colors::fg_overlay_2(theme), | ||
| total_size, |
There was a problem hiding this comment.
0.76 * total_size / 0.43 * total_size. Preserve the old neutral sizing or apply these ratios only to agent variants so non-agent surfaces remain visually unchanged.
harryalbert
force-pushed
the
harry/remote-1458-add-claude-specific-icons-for-cloud-mode-in-vertical-tabs
branch
from
ad8dd0f to
aacac62
Compare
There was a problem hiding this comment.
Overview
This PR centralizes agent icon derivation and rendering across vertical tabs, pane headers, the conversation list, and notifications, while adding harness/name helpers and cross-surface tests.
Concerns
CLIAgent::from_harnessomitsHarness::Codex, so the new match is non-exhaustive and the app will not compile.- Unknown/future harnesses are filtered out and then fall through to the Oz ambient branch, which mislabels non-Oz harnesses as Oz despite the new harness parser documenting
Unknownas non-Oz. - The new proportional circle helper returns only the 76% inner circle for no-overlay variants, so neutral/statusless icons no longer occupy the requested vertical-tab/list/header footprint.
- Supplemental security pass found no additional security-specific issues.
Verdict
Found: 1 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| Harness::Oz => None, | ||
| Harness::Claude => Some(CLIAgent::Claude), | ||
| Harness::Gemini => Some(CLIAgent::Gemini), | ||
| Harness::OpenCode => Some(CLIAgent::OpenCode), | ||
| Harness::Unknown => Some(CLIAgent::Unknown), | ||
| } |
There was a problem hiding this comment.
🚨 [CRITICAL] This match omits Harness::Codex, so the new function is non-exhaustive and the crate will not compile.
| Harness::Oz => None, | |
| Harness::Claude => Some(CLIAgent::Claude), | |
| Harness::Gemini => Some(CLIAgent::Gemini), | |
| Harness::OpenCode => Some(CLIAgent::OpenCode), | |
| Harness::Unknown => Some(CLIAgent::Unknown), | |
| } | |
| Harness::Oz => None, | |
| Harness::Claude => Some(CLIAgent::Claude), | |
| Harness::Gemini => Some(CLIAgent::Gemini), | |
| Harness::OpenCode => Some(CLIAgent::OpenCode), | |
| Harness::Codex => Some(CLIAgent::Codex), | |
| Harness::Unknown => Some(CLIAgent::Unknown), |
| if inputs.is_ambient { | ||
| if let Some(agent) = inputs | ||
| .ambient_selected_third_party_cli_agent | ||
| .filter(|agent| !matches!(agent, CLIAgent::Unknown)) |
There was a problem hiding this comment.
CLIAgent::Unknown makes an ambient run with a future/unknown harness fall through to the Oz branch below, so non-Oz harnesses are mislabeled as Oz; preserve an explicit unknown/non-Oz variant instead.
| /// Builds the brand-circle container around `icon_element`. The circle's diameter is | ||
| /// `circle_size(total)` and the icon glyph is `icon_size(total)`, with the rest going | ||
| /// to symmetric padding around the glyph. | ||
| fn render_circle( |
There was a problem hiding this comment.
render_circle returns only circle_size(total_size) (76% of the requested size), and no-overlay callers return it directly, so neutral/statusless icons shrink instead of occupying the requested 24/32px footprint.
harryalbert
dismissed
oz-for-oss[bot]’s stale review
oz for oss shouldn't be requesting changes
| let name = String::deserialize(deserializer)?; | ||
| Ok(harness_from_name(&name)) | ||
| let harness = Harness::from_config_name(&name); | ||
| if matches!(harness, Harness::Unknown) && name != Harness::Unknown.config_name() { |
There was a problem hiding this comment.
i dont understand this name != Harness::Unknown.config_name()?
2 participants
Description
Given the fact that we're planning on accommodating multiple variants of cloud agents, we need to change the agent icons in the conversation list, notifications, pane header, and vertical tabs to support different harnesses. Peter drew up some mocks for this, and we'll use the new icon + status variants across all of these surfaces.
Testing
demo: https://www.loom.com/share/066e5842bf0541fa808925100cc6529e
Agent Mode