QUALITY-731: round-trip orchestrator agent short name through task records#11090
Merged
Conversation
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 threads the orchestrator-supplied child agent name through the task request/response paths and updates viewer registration to prefer the new display-name fallback. The REST and GraphQL payloads remain optional/backward-compatible, and the local/remote child paths both normalize empty names before persistence.
Concerns
- Non-blocking: the added client spec references a contributor-local absolute path; use a portable reference so the checked-in spec remains useful outside that workstation.
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
cephalonaut
force-pushed
the
matthew/roundtrip-agent-name
branch
2 times, most recently
from
d3315c5 to
9315ca3
Compare
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 |
Contributor
There was a problem hiding this comment.
Overview
This PR threads the orchestrator-supplied child-agent short name through agent_config_snapshot.name, updates viewer-side display-name fallback behavior, and adds focused unit coverage for trimming and precedence.
Concerns
- The remote child launch path now calls a helper that is only available from the non-wasm
local_harness_launchmodule, which breaks wasm compilation. - This is a user-facing display change, but the PR description has no screenshot or recording demonstrating the shared-session viewer surfaces end to end. For this user-facing change, please include screenshots or a screen recording demonstrating it working end to end.
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
| return None; | ||
| }; | ||
|
|
||
| let agent_name = normalize_orchestrator_agent_name(&request.name); |
Contributor
There was a problem hiding this comment.
normalize_orchestrator_agent_name comes from local_harness_launch, whose module/import is gated with #[cfg(not(target_family = "wasm"))], so wasm builds will fail here. Move the normalization helper into a shared non-gated module or avoid using the local-harness module from the remote path.
bnavetta
approved these changes
May 17, 2026
…pshot.name What: Stamp the orchestrator-supplied short name into the existing `AgentConfigSnapshot.name` field at the three outbound child-spawn sites (`launch_remote_child`, `launch_local_no_harness_child`, and the local-harness path via a new `agent_name` argument on `prepare_local_harness_child_launch` and `local_child_task_config`). Inbound, `AmbientAgentTask::display_name()` reads the short label from `agent_config_snapshot.name`, falling back to the descriptive `title` and then `"Agent"`, trimming whitespace at each layer. `OrchestrationViewerModel::apply_children_fetch` uses `display_name()` for the child pill label and stores `task.title` on the conversation via `set_fallback_display_title` so the descriptive title remains available through `AIConversation::title()`. The conversation details side pane intentionally keeps `task.title` per the v1 deferral comment. Why: The orchestrator's own client labels children with short names from `agent_run_configs[i].name`, but a viewer reconstructing children from server task records previously had no access to those labels and fell back to the descriptive `title`. Reusing the existing `agent_config_snapshot` envelope avoids introducing a parallel top-level `name` field on `AmbientAgentTask`/`SpawnAgentRequest`/`CreateAgentTaskInput`. Agent Mode: - [x] This PR was created by Warp Agent Mode Testing: - New tests cover `display_name()` precedence/trimming, viewer registration via `agent_config_snapshot.name`, and `local_child_task_config` stamping the orchestrator name. Co-Authored-By: Oz <oz-agent@warp.dev>
What: - Extract a single `normalize_orchestrator_agent_name(&str) -> Option<String>` helper in `local_harness_launch.rs` and call it from every site that produces the snapshot-safe `Option<String>` (Findings 5/6/15). - Trim `request.name` once at the top of each launcher and use the trimmed value for both the hidden child conversation's `agent_name` AND the task snapshot's `agent_config_snapshot.name`, so the two channels can no longer desync on whitespace (Finding 4). - Tighten the viewer's fallback gate in `OrchestrationViewerModel::apply_children_fetch` to trim `task.title` before checking AND before storing, matching the trim inside `display_name()` (Finding 3). - Add display_name test for whitespace-only title (Finding 12). - In the viewer 'no snapshot name' test, use distinct snapshot/title values so a regression that wires `fallback_display_title = display_name()` is observable (Finding 13). - Inline TODO in `conversation_ended_tombstone_view.rs` noting that `config.name` doubles as orchestrator name now, mislabeled as skill_name (Finding 11). - TECH.md: add an explicit Scope section, an Out-of-scope follow-ups section enumerating the other surfaces (entry-based labels, tombstone skill_name, agent_sdk surface, workspace list), document the whitespace risk + mitigation, and replace the deferred-but-unwritten `launch_remote_child` test with the indirect-coverage explanation (Findings 7/10). Why: Holistic review findings on the QUALITY-731 client pivot. Each finding is acknowledged in the commit body and the TECH.md so reviewers can map the change back to the comment. Agent Mode: - [x] This PR was created by Warp Agent Mode Testing: - Added `normalize_orchestrator_agent_name_trims_and_drops_empty` (helper contract). - Added `display_name_returns_literal_agent_for_whitespace_only_title`. - Added `registers_child_agent_name_does_not_set_fallback_for_whitespace_only_title`. - Tightened `registers_child_agent_name_falls_back_to_title_when_snapshot_name_is_missing` to use a long descriptive title distinct from any short name. Co-Authored-By: Oz <oz-agent@warp.dev>
Strip QUALITY-731 markers and pivot/finding references from source-level comments. Keep only what describes the contract or warns about a real hazard. PR description and spec carry the history. - task.rs::display_name: long doc reduced to one-line contract. - conversation_details_panel.rs::from_task: deferral note trimmed. - local_harness_launch.rs::normalize_orchestrator_agent_name: one-line. - local_harness_launch.rs::local_child_task_config: drop redundant\\n defensive-renormalize comment. - terminal_pane.rs: drop normalize-once narrative at all three launchers\\n (the helper name carries the contract). - orchestration_viewer_model.rs: short two-line trim rationale. - conversation_ended_tombstone_view.rs::enrich_from_task: one-line FIXME. - orchestration_viewer_model_tests.rs: drop QUALITY-731 / finding 3\\n framing; keep test-shape rationale. Agent Mode: - [x] This PR was created by Warp Agent Mode Co-Authored-By: Oz <oz-agent@warp.dev>
`launch_remote_child` in `terminal_pane.rs` compiles for both native and wasm targets (the shared-session viewer runs in the browser). It calls `normalize_orchestrator_agent_name`, which lived in `pane_group/pane/local_harness_launch.rs` \u2014 a module gated with `#[cfg(not(target_family = "wasm"))]`. Wasm builds therefore failed with E0425. Move the helper next to `AmbientAgentTask::display_name()` in `ai/ambient_agents/task.rs`, which is wasm-safe (it has no platform dependencies and is already imported from both targets). Update all three call sites (terminal_pane.rs, local_harness_launch.rs, the test file) to import from the new location. Validated with: - cargo check -p warp - CC_wasm32_unknown_unknown=/opt/homebrew/opt/llvm/bin/clang cargo check -p warp --target wasm32-unknown-unknown - cargo test -p warp --lib -- pane_group::pane::local_harness_launch ai::ambient_agents::task terminal::shared_session::viewer::orchestration_viewer_model (43 passed) Agent Mode: - [x] This PR was created by Warp Agent Mode Co-Authored-By: Oz <oz-agent@warp.dev>
cephalonaut
force-pushed
the
matthew/roundtrip-agent-name
branch
from
70f6ff6 to
e67e3f8
Compare
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 |
Contributor
There was a problem hiding this comment.
Overview
This PR routes orchestrator-supplied child agent names through agent_config_snapshot.name and updates the shared-session viewer to prefer that short display name while keeping the descriptive task title available as a fallback. I did not find code correctness or security issues in the annotated diff.
Concerns
- This is a user-facing change to shared-session child labels, hover/card/breadcrumb-related surfaces, but the PR description does not include screenshots or a short screen recording demonstrating the behavior end to end. For this user-facing change, please include screenshots or a screen recording demonstrating it working end to end.
Verdict
Found: 0 critical, 1 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
lawsmd
pushed a commit
to lawsmd/cortex
that referenced
this pull request
May 22, 2026
…cords (warpdotdev#11090) ## Description **Why.** In shared-session viewers, child agent pills, hover cards, and breadcrumbs were rendering the descriptive `task.title` (often a long sentence or a truncated prompt) instead of the short orchestrator-supplied agent name. The orchestrator's own client labels each child via `agent_run_configs[i].name`, but a viewer reconstructing children from server task records had no access to that label. **What.** The orchestrator-supplied short name now round-trips through the existing `agent_config_snapshot.name` field on `AgentConfigSnapshot`. Outbound: all three child-spawn sites stamp `agent_config_snapshot.name` via the shared `normalize_orchestrator_agent_name(&str) -> Option<String>` helper (trim, treat empty / whitespace-only as absent): - `launch_remote_child` (`SpawnAgentRequest.config.name`) - `launch_local_no_harness_child` (`create_agent_task`'s config snapshot) - `prepare_local_harness_child_launch` / `local_child_task_config` (new `agent_name` parameter) The same trimmed value also flows into the hidden child conversation's `agent_name` and any error-fallback conversation, so the two channels can't desync on whitespace. Inbound: `AmbientAgentTask::display_name(&self) -> &str` returns the trimmed `agent_config_snapshot.name`, falling back to trimmed `title`, then `"Agent"`. `OrchestrationViewerModel::apply_children_fetch` uses `display_name()` for the pill label and stores the trimmed `task.title` on the conversation via `set_fallback_display_title`, so `AIConversation::title()` keeps surfacing the descriptive title. The conversation details side pane intentionally still uses `task.title` pending product feedback on whether to show both surfaces; a one-line note marks the deferral inline. **Server side.** No code change required in warp-server: `agent_config_snapshot.name` already round-trips through both REST (`POST /agent/run`) and GraphQL (`createAgentTask`). The paired warp-server PR is warpdotdev/warp-server#11223. Full design trace is in `specs/QUALITY-731/TECH.md`. ## Linked Issue QUALITY-731 (Linear). - [x] The linked issue is labeled `ready-to-spec` or `ready-to-implement`. - [ ] Where appropriate, screenshots or a short video of the implementation are included below (especially for user-visible or UI changes). ## Testing - [ ] I have manually tested my changes locally with `./script/run` Automated coverage: - `display_name()` precedence and trim behavior (snapshot.name > title > "Agent"), including whitespace-only inputs. - Viewer registration via `agent_config_snapshot.name`: snapshot-name wins, falls back to title, "Agent" final fallback, trimming, and a guard that whitespace-only titles don't bleed into `AIConversation::title()`. - `local_child_task_config` carries the orchestrator name (stamp + trim + `None` for Oz/Unknown). `normalize_orchestrator_agent_name` covers the empty/whitespace contract directly. Manual end-to-end verification with a paired client+server build is pending warpdotdev/warp-server#11223. ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode <!-- CHANGELOG-NONE --> --------- 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.
Description
Why. In shared-session viewers, child agent pills, hover cards, and breadcrumbs were rendering the descriptive
task.title(often a long sentence or a truncated prompt) instead of the short orchestrator-supplied agent name. The orchestrator's own client labels each child viaagent_run_configs[i].name, but a viewer reconstructing children from server task records had no access to that label.What. The orchestrator-supplied short name now round-trips through the existing
agent_config_snapshot.namefield onAgentConfigSnapshot.Outbound: all three child-spawn sites stamp
agent_config_snapshot.namevia the sharednormalize_orchestrator_agent_name(&str) -> Option<String>helper (trim, treat empty / whitespace-only as absent):launch_remote_child(SpawnAgentRequest.config.name)launch_local_no_harness_child(create_agent_task's config snapshot)prepare_local_harness_child_launch/local_child_task_config(newagent_nameparameter)The same trimmed value also flows into the hidden child conversation's
agent_nameand any error-fallback conversation, so the two channels can't desync on whitespace.Inbound:
AmbientAgentTask::display_name(&self) -> &strreturns the trimmedagent_config_snapshot.name, falling back to trimmedtitle, then"Agent".OrchestrationViewerModel::apply_children_fetchusesdisplay_name()for the pill label and stores the trimmedtask.titleon the conversation viaset_fallback_display_title, soAIConversation::title()keeps surfacing the descriptive title.The conversation details side pane intentionally still uses
task.titlepending product feedback on whether to show both surfaces; a one-line note marks the deferral inline.Server side. No code change required in warp-server:
agent_config_snapshot.namealready round-trips through both REST (POST /agent/run) and GraphQL (createAgentTask). The paired warp-server PR is warpdotdev/warp-server#11223. Full design trace is inspecs/QUALITY-731/TECH.md.Linked Issue
QUALITY-731 (Linear).
ready-to-specorready-to-implement.Testing
./script/runAutomated coverage:
display_name()precedence and trim behavior (snapshot.name > title > "Agent"), including whitespace-only inputs.agent_config_snapshot.name: snapshot-name wins, falls back to title, "Agent" final fallback, trimming, and a guard that whitespace-only titles don't bleed intoAIConversation::title().local_child_task_configcarries the orchestrator name (stamp + trim +Nonefor Oz/Unknown).normalize_orchestrator_agent_namecovers the empty/whitespace contract directly.Manual end-to-end verification with a paired client+server build is pending warpdotdev/warp-server#11223.
Agent Mode