Add pinning to orchestration pill bar#10777
Open
advait-m wants to merge 13 commits into
Open
Conversation
Lets users pin child agent pills in the orchestration pill bar so frequently-used children stay anchored to the leading section, with an outline pin glyph on hover (unpinned) and a solid pin glyph on hover (pinned). Pinned pills render between the orchestrator and a vertical divider, preserving spawn order within each section. Pin state lives in a shared singleton (OrchestrationPinModel) so pinning in one pane is reflected in every other pane's pill bar in real time; subscribers re-render on PinSetChanged, and the singleton subscribes to BlocklistAIHistoryModel to prune deleted conversations globally. Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
Previously, the inner Hoverable wrapping the leading slot of a child pill always carried the TogglePin click handler, even when the avatar was the visible content. Because the outer pill defers events to children, clicks on the avatar during the hover-in delay window were swallowed by the pin handler instead of bubbling up to navigate. Now the Hoverable wrapper is only attached when show_pin_glyph is true. When the avatar is showing, clicks bubble up to the outer pill body and navigate as expected. Also drops #[derive(Default)] on OrchestrationPinModel so the only way to construct it stays the new() path that wires up the history subscription. Co-Authored-By: Oz <oz-agent@warp.dev>
Adds pinned: bool to AgentConversationData (serde-default + skip on false, so legacy rows deserialize unchanged) and wires it through AIConversation (init, getter/setter, write event). Forks reset pin state to false. BlocklistAIHistoryModel grows set_conversation_pinned(id, pinned, ctx), modeled on update_event_sequence: mutate the in-memory conversation, then write the updated state to SQLite. OrchestrationPinModel::new now takes the initial pinned set (parsed from multi_agent_conversations at startup). toggle_pin pushes the change down to the history model, which persists it. Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
- set_conversation_pinned now log::warns when the conversation isn't loaded so dropped pin writes are visible instead of silent. - OrchestrationPinModel gains a reset() method; log_out clears the in-memory pin set so the next user doesn't inherit the previous account's pins. - Adds an e2e persistence test that toggles a pin and asserts both the in-memory AIConversation.pinned flag flips and an UpdateMultiAgentConversation event is emitted with pinned: true. A second toggle confirms the unpin path persists pinned: false. - Fixes a missed AgentConversationData literal in conversation_details_panel_tests.rs that was caught by the test build. Co-Authored-By: Oz <oz-agent@warp.dev>
Contributor
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Overview
This PR adds shared, persisted pin state for orchestration child-agent pills, wires the pill bar to render pinned and unpinned sections, and adds persistence/model coverage for the new AgentConversationData.pinned field.
Concerns
- No blocking correctness or security concerns found.
- One UI robustness suggestion is inline: make the new pin SVGs tintable so the existing
to_warpui_icon(text_color)call can preserve contrast across themes.
Verdict
Found: 0 critical, 0 important, 1 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| @@ -0,0 +1,4 @@ | |||
| <svg xmlns="http://www.w3.org/2000/svg" width="8" height="11" viewBox="0 0 8 11" fill="none"> | |||
| <path fill-rule="evenodd" clip-rule="evenodd" d="M1.58119 0.0137297C1.32819 0.0472298 1.13069 0.14373 0.970691 0.31123C0.779691 0.51123 0.682691 0.77823 0.705691 1.03673C0.723191 1.22923 0.783191 1.40373 1.14019 2.29773L1.45769 3.09423V3.70573V4.31773L0.920191 4.99023C0.174691 5.92223 0.0946913 6.03923 0.0266913 6.29523C-0.0508087 6.58623 0.0436912 6.92723 0.268191 7.16973C0.394191 7.30623 0.518691 7.37773 0.731191 7.43723C0.883191 7.47973 0.922691 7.48123 2.17219 7.48823L3.45669 7.49473L3.46219 9.08873C3.46769 10.6747 3.46769 10.6832 3.51069 10.7627C3.60019 10.9297 3.70519 10.9812 3.95769 10.9822C4.12369 10.9827 4.15869 10.9762 4.23669 10.9327C4.33519 10.8777 4.40669 10.7857 4.43719 10.6752C4.45019 10.6297 4.45769 10.0282 4.45769 9.04873V7.49473L5.74269 7.48823C6.99269 7.48123 7.03219 7.47973 7.18419 7.43723C7.39719 7.37773 7.52119 7.30623 7.64819 7.16873C7.87269 6.92623 7.96619 6.58673 7.88869 6.29523C7.82069 6.03923 7.74069 5.92223 6.99519 4.99023L6.45769 4.31773V3.70573V3.09423L6.77519 2.29773C7.13219 1.40373 7.19219 1.22923 7.20969 1.03673C7.23419 0.76223 7.12019 0.47423 6.90869 0.27723C6.77469 0.15223 6.67069 0.0937297 6.48769 0.0392298C6.37619 0.00622975 6.20569 0.00322971 4.03769 0.000229715C2.75619 -0.00127029 1.65069 0.00472975 1.58119 0.0137297ZM6.18769 1.01273C6.19469 1.02423 6.04519 1.42223 5.85519 1.89773C5.66569 2.37373 5.49869 2.81223 5.48419 2.87273C5.45069 3.01523 5.44669 4.35873 5.47969 4.51673C5.52319 4.72573 5.60469 4.85023 6.11069 5.48323C6.75119 6.28423 6.87969 6.45323 6.86619 6.47473C6.85169 6.49873 1.06369 6.49873 1.04869 6.47473C1.03619 6.45373 1.19219 6.24973 1.84469 5.43323C2.30969 4.85173 2.39269 4.72273 2.43569 4.51673C2.46869 4.35873 2.46469 3.01523 2.43119 2.87273C2.41669 2.81223 2.24969 2.37373 2.06019 1.89773C1.87019 1.42223 1.72069 1.02423 1.72769 1.01273C1.74469 0.98473 6.17069 0.98473 6.18769 1.01273Z" fill="#121212"/> | |||
Contributor
There was a problem hiding this comment.
💡 [SUGGESTION] The new pin SVGs hard-code fill colors, so Icon::to_warpui_icon(text_color) cannot tint them and the glyph can lose contrast across themes. Use currentColor for these paths, and apply the same treatment to pin-filled.svg.
Member
Author
There was a problem hiding this comment.
hmmm, light theme looks fine
Member
Author
There was a problem hiding this comment.
we tint it at paint time so this isn't relevant
cephalonaut
approved these changes
May 13, 2026
|
|
||
| /// Whether the user has pinned this child agent in the orchestration | ||
| /// pill bar. Persisted via `AgentConversationData.pinned`. | ||
| pinned: bool, |
Contributor
There was a problem hiding this comment.
There might be some structural grouping we could do of child agent specific properties: parent_agent_id, pinned, is_remote_child, parent_conversation_id, orchestration_harness_type evidently.
Member
Author
There was a problem hiding this comment.
agree, maybe smth like ChildAgentState or similar - tho will be a big surface area refactor, so can follow up on that
The pin model singleton is looked up unconditionally from the orchestration pill bar render path, so any test that goes through `initialize_app_for_terminal_view` and constructs a TerminalView panicked with: Cannot get singleton model of type "...OrchestrationPinModel" that was never registered Register it right after BlocklistAIHistoryModel since the pin model subscribes to history events on construction. Fixes terminal::view::tests and terminal::view::use_agent_footer::tests panics. Co-Authored-By: Oz <oz-agent@warp.dev>
Same root cause as the previous fix in test_util/terminal.rs — each test module that builds a TerminalView (directly or via PaneGroup) must register the OrchestrationPinModel singleton because the pill bar render path looks it up unconditionally. Fixes terminal::input::tests::* panics and preemptively fixes pane_group::tests::* which use the same construction path. Co-Authored-By: Oz <oz-agent@warp.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Loom: https://www.loom.com/share/240fef8cf25041f8ae312088d79e29ba
Fixes https://linear.app/warpdotdev/issue/QUALITY-672/add-pill-pinning-from-orchestration-pill-bar-mocks
Description
Adds pinning to the orchestration pill bar so frequently-used child agents stay anchored to the leading section of the bar instead of getting lost in a long row of pills. Pin state is shared across panes and persists across app restarts.
Icon::Pin(outline) +Icon::PinFilled(solid) variants, bundled from Figma SVGs underapp/assets/bundled/svg/.MouseStateHandleper pill, independent of the pill body and the 3-dot overflow button.Technical Design
Cross-pane state via a singleton model. Pin state lives in a new
OrchestrationPinModelsingleton, not onOrchestrationPillBar(which is per-TerminalView). Each pill bar reads from the singleton and subscribes toOrchestrationPinEvent::PinSetChanged, so toggling a pin in one pane immediately re-renders every other pane's bar. The singleton also subscribes toBlocklistAIHistoryModelevents (RemoveConversation/DeletedConversation) so deleted conversations are pruned from the pin set globally — keeping that cleanup in one place avoids each pill bar racing to clobber sibling panes' pin sets. A per-viewHashSet<AIConversationId>was considered first but rejected because it would have required broadcasting toggles across panes manually and would leak acrossTerminalViewlifecycle boundaries.Persistence piggybacks on the existing conversation write path. Rather than introducing a separate pin table or settings entry, pinned state is added as
pinned: boolon the existingAgentConversationData(which is the JSON blob already persisted per-conversation in SQLite). Toggling a pin callsBlocklistAIHistoryModel::set_conversation_pinned, which updates the in-memoryAIConversation.pinnedflag and reuseswrite_updated_conversation_stateto emit anUpdateMultiAgentConversationevent. The singleton is seeded at app boot from the samemulti_agent_conversationsvec already loaded forBlocklistAIHistoryModel, so there's no extra DB read. The field uses#[serde(default, skip_serializing_if = "is_false")]so existing rows deserialize cleanly and unpinned conversations don't bloat the persisted JSON.Click-handler scoping to avoid hover-delay races. The first pass wrapped the pill's leading element (avatar / pin glyph) in a
Hoverablethat always carried theTogglePinclick handler. That stole clicks during the 300mswith_hover_in_delaywindow — a user clicking the avatar to navigate could land on the toggle instead. The current code only wraps the inner element inHoverable(and only attaches the toggle handler) whenshow_pin_glyphis true; otherwise clicks bubble to the outer pill's navigate handler. This keeps the navigate-vs-toggle contract tied 1:1 to what the user actually sees rendered.Logout cleanup.
OrchestrationPinModel::reset()is invoked fromlog_outalongside the other*::reset()calls so a subsequent user doesn't inherit the previous account's pins. The persisted per-conversation flags are wiped by the existing sqlite reset that runs alongside logout, so this just clears the in-memory mirror.set_conversation_pinnedalsolog::warn!s when the conversation isn't loaded so silently-dropped writes are visible in logs rather than disappearing.Plan: https://staging.warp.dev/drive/notebook/DavRjsSq2TUxXeF8PBQsFc
Testing
./script/runUnit tests:
orchestration_pin_model_tests.rscoverstoggle_pin_in_set(flip-on-each-call, only-affects-target-id).toggle_pin_persists_pinned_state_to_sqlite_eventwires up settings + a mockGlobalResourceHandleschannel, restores a conversation, toggles a pin, and asserts (a) the in-memoryAIConversation.pinnedflips, (b) anUpdateMultiAgentConversationevent arrives withpinned: true, and (c) a follow-up unpin emits a second event withpinned: false.model.rsround-trip tests coverAgentConversationData.pinnedserde defaulting and skip-if-false behavior.orchestration_pill_bar_tests.rsdescendant-walking + navigation-action tests still pass.cargo fmt,cargo build -p warp,cargo nextest run -p warp orchestration_pin_model, andcargo clippy -p warp --tests --all-featuresall clean on this branch.