17 KiB
QUALITY-731 — Agent name round trip client spec
Context
QUALITY-731 is a shared-session viewer bug: the orchestrator's own client labels children with agent_run_configs[i].name, but a viewer reconstructs child conversations from server task records and currently does not have access to that short name. As a result, viewer-side pills, hover cards, breadcrumbs, status cards, and transcript participant labels fall back to title, which can be a long descriptive sentence or a truncated prompt.
The fix sources the orchestrator's short label from the existing agent_config_snapshot.name field instead of introducing a parallel top-level name field on the task or request types. This aligns with the paired warp-server spec, which uses AgentConfigSnapshot.Name as the canonical home for the orchestrator-supplied label.
Scope
This PR delivers the orchestrator → server → viewer round-trip plus the single highest-value display surface: the orchestration pill bar in OrchestrationViewerModel::apply_children_fetch. Other surfaces that still render task.title (or entry.display.title) directly are left for follow-up work, tracked under "Out of scope (follow-ups)" below. The wire contract and display_name() helper this PR introduces are the prerequisites those follow-ups will consume.
Relevant existing client surfaces:
app/src/server/server_api/ai.rs—SpawnAgentRequest.config: Option<AgentConfigSnapshot>already exists.CreateAgentTaskInput.agent_config_snapshot: Option<String>(serialized JSON) already exists. Both REST and GraphQL channels can carry anAgentConfigSnapshotpayload today.app/src/ai/ambient_agents/task.rs—AgentConfigSnapshot.name: Option<String>(#[serde(default, skip_serializing_if = "Option::is_none")]) already exists.AmbientAgentTask.agent_config_snapshot: Option<AgentConfigSnapshot>already deserializes from the server.app/src/ai/blocklist/action_model/execute/run_agents.rs—RunAgentsExecutorfans out eachRunAgentsAgentRunConfig;cfg.nameis the source of truth on the client side.app/src/pane_group/pane/terminal_pane.rs—launch_remote_childbuilds aSpawnAgentRequestwith anAgentConfigSnapshotforconfig.launch_local_no_harness_childandlaunch_local_harness_childbuild the local Oz / harness child task viaAIClient::create_agent_taskand the harness'slocal_child_task_config(harness).app/src/pane_group/pane/local_harness_launch.rs—prepare_local_harness_child_launchconstructs thelocal_child_task_configsnapshot.app/src/terminal/shared_session/viewer/orchestration_viewer_model.rs—apply_children_fetchcallstask.display_name()for the child conversation'sagent_name. That helper currently reads from the QUALITY-731 v1AmbientAgentTask.namefield.app/src/ai/blocklist/history_model.rs—start_new_child_conversationwrites itsnameargument directly intoAIConversation::agent_name.app/src/ai/agent/conversation.rs—agent_name()backs orchestration label surfaces;title()falls back tofallback_display_title.app/src/ai/conversation_details_panel.rs—ConversationDetailsData::from_taskreadstask.titlefor the side-pane header. Design options considered:- Parallel scalar fields on the task (the QUALITY-731 v1 approach:
AmbientAgentTask.name,SpawnAgentRequest.name,AIClient::create_agent_taskagent_name, GraphQLCreateAgentTaskInput.agentName). Required two name-like fields on the wire and in the model. Rejected per reviewer. - Reuse
agent_config_snapshot.name(selected). No new fields. Outbound paths stamp the orchestrator name inside the existingAgentConfigSnapshot { ... }builder. The viewer'sdisplay_name()reads fromagent_config_snapshot.name. Backward-compatible with any task that already populates the field through some other means.
Proposed changes
Outbound request wiring
Stamp the orchestrator-supplied short name into the existing AgentConfigSnapshot payload at request-construction boundaries. Trim whitespace at the construction site; treat empty/whitespace-only as absent.
launch_remote_childinapp/src/pane_group/pane/terminal_pane.rsbuilds theAgentConfigSnapshotit puts onSpawnAgentRequest.config. Addname: ...to that struct literal with the trimmedrequest.name, filtered for empty. No more top-levelrequest.nameclone orSpawnAgentRequest.namefield.launch_local_no_harness_child(local Oz path) interminal_pane.rscurrently passesNonefor the config snapshot tocreate_agent_task. Replace withSome(AgentConfigSnapshot { name: trimmed(request_name), ..Default::default() }).launch_local_harness_child/prepare_local_harness_child_launchinapp/src/pane_group/pane/local_harness_launch.rsbuild their snapshot vialocal_child_task_config(harness). Extendlocal_child_task_configto takeagent_name: Option<String>and stamp it inside the returned snapshot. Drop the QUALITY-731 v1agent_nameparameter onAIClient::create_agent_taskand the trim inside its impl; the construction site is now the single source.agent_sdk/ambient.rsCLIagent run-cloud(REST) already setsconfig.name = args.name. No change needed.build_handoff_spawn_request(handoff) andspawn_agent(standalone cloud-mode) don't supply a name today. No change.
Inbound response read
Rewrite AmbientAgentTask::display_name(&self) -> &str in app/src/ai/ambient_agents/task.rs. Lookup order:
- Trimmed
agent_config_snapshot.as_ref().and_then(|c| c.name.as_deref())when present and non-empty. - Trimmed
titlewhen non-empty. - The literal
"Agent".OrchestrationViewerModel::apply_children_fetchkeeps its existinglet name = task.display_name().to_string();line. The downstreamconversation.set_fallback_display_title(task.title.clone())call remains so the descriptive title stays available viaAIConversation::title()fallback.
Removals (QUALITY-731 v1 rollback)
Source code:
app/src/server/server_api/ai.rs: removeSpawnAgentRequest.namefield and its serde attrs. Remove theagent_name: Option<String>parameter from theAIClient::create_agent_tasktrait method and its impl, including the trim block and theagent_nameline insideCreateAgentTaskVariables.app/src/ai/ambient_agents/task.rs: remove theAmbientAgentTask.name: Option<String>field and its serde default. Keepdisplay_name()as a helper, but rewrite its body per the above section.app/src/pane_group/pane/terminal_pane.rs: remove therequest.name.clone()plumbing inlaunch_remote_child, theagent_name_for_create = Some(request_name.clone())line and the corresponding 5th positional arg inlaunch_local_no_harness_child, and theagent_name_for_task = Some(request_name.clone())line + 5th arg inlaunch_local_harness_child.app/src/pane_group/pane/local_harness_launch.rs: remove theagent_name: Option<String>parameter fromprepare_local_harness_child_launchand the 5th positional arg threaded intocreate_agent_task. The#[allow(clippy::too_many_arguments)]attribute on this function becomes unnecessary; remove it if so.app/src/ai/conversation_details_panel.rs: keep the deferral comment QUALITY-731 v1 added onfrom_task; the surface remains ontask.title(unchanged behavior).crates/warp_graphql_schema/api/schema.graphql: remove theagentName: Stringfield + docstring underCreateAgentTaskInput.- All
name: Noneliterals onSpawnAgentRequest { ... }builders added in QUALITY-731 v1 (inagent_sdk/ambient.rs,terminal/view/ambient_agent/model.rs,terminal/view_tests.rs,agent_sdk/mcp_config_tests.rs,ambient_agents/spawn_tests.rs,terminal/view/ambient_agent/model_tests.rs): remove. - All
name: Noneliterals onAmbientAgentTask { ... }test fixtures added in QUALITY-731 v1 (inagent_conversations_model_tests.rs,cloud_conversation_continuation_tests.rs,conversation_ended_tombstone_view_tests.rs,view_impl_tests.rs,spawn_tests.rstask_withhelper,pane_group/mod_tests.rs,conversation_details_panel_tests.rs,orchestration_event_streamer_tests.rs): remove. Tests: app/src/server/server_api/ai_tests.rs: removespawn_agent_request_serializes_name_when_present,spawn_agent_request_omits_name_when_none, and thename: Noneline in themake_spawn_agent_requestfixture.app/src/ai/ambient_agents/task_tests.rs: rewrite the fivedisplay_name_*tests to construct anAmbientAgentTaskwith anagent_config_snapshot.namevalue (orNone) instead of a top-levelname. Keep the same precedence-coverage shape (name+title, name=None falls back to title, name=whitespace falls back to title, empty title returns "Agent", trimming).app/src/terminal/shared_session/viewer/orchestration_viewer_model_tests.rs: rewrite the fourregisters_child_agent_name_*tests to populatetask.agent_config_snapshot.nameinstead oftask.name. Themake_task/make_task_with_namehelpers move the name into the config snapshot fixture.app/src/pane_group/pane/local_harness_launch_tests.rs: rewriteprepare_local_codex_child_forwards_agent_name_to_create_agent_taskandprepare_local_codex_child_passes_none_agent_name_when_unsetto assert thatlocal_child_task_config(or the constructed snapshot) carriesnamecorrectly. Or drop them in favor of a single test onlocal_child_task_configdirectly.
End-to-end flow
flowchart LR
A[run_agents cfg.name/cfg.title] --> B[RunAgentsExecutor]
B --> C[StartAgentRequest name + Remote title]
C --> D[SpawnAgentRequest config.name + title]
C --> E[createAgentTask agentConfigSnapshot.name for local harness]
D --> F[warp-server agent_config_snapshot.name + title]
E --> F
F --> G[GET /agent/runs returns agent_config_snapshot.name + title]
G --> H[OrchestrationViewerModel]
H --> I[display_name = agent_config_snapshot.name]
H --> J[fallback_display_title = title]
Testing and validation
Unit/client tests:
app/src/ai/ambient_agents/task_tests.rs:display_name()precedence (snapshot.name > title > "Agent") and trim behavior, including whitespace-only title.app/src/terminal/shared_session/viewer/orchestration_viewer_model_tests.rs: viewer registration of orchestrator name viaagent_config_snapshot.name, fallback to title (using distinct snapshot/title values so the two channels are distinguishable), "Agent" final fallback, whitespace-only title gating ofset_fallback_display_title.app/src/pane_group/pane/local_harness_launch_tests.rs:local_child_task_configcarries the orchestrator name in its snapshot, trims whitespace, returnsNonefor Oz/Unknown harnesses.normalize_orchestrator_agent_namecovers the trim/empty-vs-Some contract.- The construction-site wiring in
launch_remote_child(theSpawnAgentRequest.config.namefield is stamped with the result ofnormalize_orchestrator_agent_name(&request.name)) is covered indirectly by thenormalize_orchestrator_agent_nameunit tests + visible inspection of the struct literal interminal_pane.rs::launch_remote_child. A dedicated test for the assembledSpawnAgentRequestwould require factoring out abuild_spawn_requesthelper, which is deferred as out of scope here (the function takes a&mut PaneGroup+ViewContext<PaneGroup>and resolves runtime skills + snapshot-disabled flag through them, none of which are unit-testable as-is). Manual validation:
- Start or load an orchestrated shared session where the orchestrator's outbound spawn populates
agent_config_snapshot.name = "frontend-tests"and a long descriptivetitle. - Open the session as a viewer.
- Verify the pill label, hover card participant label, breadcrumb, child status card, and transcript participant all show
frontend-tests. - Verify the long title remains available as
AIConversation::title()fallback wherever the existing fallback path is used. - Verify a child whose orchestrator did not set a name still shows the skill-derived default (provided by the server).
- The conversation details side pane intentionally remains on
task.titleper the QUALITY-731 v1 deferral. Commands to run after implementation:
- Targeted Rust tests for any modules touched, for example:
cargo test -p warp -- ai::ambient_agents::taskcargo test -p warp -- terminal::shared_session::viewer::orchestration_viewer_model_testscargo test -p warp -- pane_group::pane::local_harness_launch_tests
cargo fmt./script/presubmitbefore pushing (skip thecommand-signatures-v2step locally only if the corepack/yarn-4 setup blocks it on this machine; CI is authoritative).- Manual UI verification against a local client connected to a server with the paired pivot changes.
Parallelization
Server agent: local, /Users/matthew/src/roundtrip-agent-name/warp-server, branch matthew/roundtrip-agent-name, base origin/matthew/restore-remote-orch-conversations, draft PR target #11223. Owns server-side rollback + helper + REST/GraphQL contract drops.
Client agent: local, /Users/matthew/src/roundtrip-agent-name/warp, branch matthew/roundtrip-agent-name, base origin/master, draft PR target #11090. Owns client-side rollback + outbound snapshot stamping + display_name() rewrite.
Sequencing:
- Both PRs can be force-pushed in parallel — the wire contract (use existing
agent_config_snapshotenvelope, drop QUALITY-731 v1 parallel fields) is fully agreed upfront. - End-to-end manual validation must wait for both branches to be on the pivoted contract. PR hygiene:
- Force-push removes the QUALITY-731 v1 commits from each PR. Rewrite the PR descriptions to call out the pivot and link to the paired PR.
- Mark the
Warp Agent Modecheckbox on the client PR template. - Keep both PRs in draft.
Out of scope (follow-ups)
The pivot delivers the wire contract and the orchestration viewer pill. The following surfaces still render task.title (or the denormalized entry.display.title) directly and would benefit from a follow-up that fans display_name() through them, but each requires an additional plumbing decision (the entry-based ones in particular don't have access to agent_config_snapshot today):
app/src/ai/agent_conversations_model/entry.rs—AgentConversationEntryis hydrated fromListConversationsItemand only carriesdisplay.titletoday. Routingdisplay_name()here means denormalizingagent_config_snapshot.nameonto the entry (or fetching the task) before render time.app/src/ai/conversation_details_panel.rs::from_agent_conversation_entry— same source as above; downstream of the entry.app/src/terminal/view/shared_session/conversation_ended_tombstone_view.rs— the tombstone reusestask.titlefor its header and reusesagent_config_snapshot.nameasskill_name; the latter is now mislabeled (an inlineQUALITY-731 follow-upcomment marks this). Splitting orchestrator-supplied agent name from skill-spec rendering belongs to a follow-up.app/src/ai/agent_sdk/ambient.rs(~line 845) — CLI/SDK-side surface that already reads task records; can adoptdisplay_name()once the helper is publicly reachable from that path.app/src/workspace/view/conversation_list/item.rsandapp/src/workspace/view.rs— workspace conversation list labels flow fromentry.display.title; same plumbing decision as the entry-based surfaces. TheConversationDetailsData::from_taskside-pane header is intentionally not in scope: product still evaluates whether to show both the short name and the descriptive title. The inline deferral comment remains in place.
Risks and mitigations
display_name()change: anywhere that readAmbientAgentTask.namedirectly (instead of going through the helper) must be moved to the helper to pick up the new source. Mitigation: deleting the field forces a compile error at every direct read; fix them at the call site.- Existing test fixtures with explicit
name: Nonewill no longer compile. Mitigation: blanket-revert thename: Nonelines as part of the same change. - A future caller that sets both
agent_config.nameand an orchestrator name via some other channel: server enforces the always-override precedence; client only stamps when an orchestrator name is provided. No client-side collision. - The details panel surface stays on
task.titleper the v1 deferral. Mitigation: the deferral comment inconversation_details_panel.rsremains. - Whitespace-only
task.titlewould desyncagent_name()(trimmed →"Agent") fromtitle()(untrimmed) if the viewer fallback gate were not also trimmed. Mitigation: the gate inOrchestrationViewerModel::apply_children_fetchcallstask.title.trim().to_string()before checking and before storing on the conversation. A dedicated test (registers_child_agent_name_does_not_set_fallback_for_whitespace_only_title) locks this in. - Force-push removes the v1 commits from the PR history; existing reviewer comments on those commits stay attached to the orphaned commits. Mitigation: PR description rewrite explains the pivot and links to the v1 review history.