[QUALITY-726] Inherit-share + eager Oz task creation for run_agents local children#11042
Merged
Conversation
…r run_agents local children Local children of cloud orchestrators were not appearing in the shared-session viewer pill bar because their per-child terminals weren't asked to share, and their server task rows weren't created at dispatch time (only lazily on first request). This change unifies the Oz-local path with the harness-local path so both flow through the same share-handshake plumbing. * Drop the Oz-only `parent_run_id` exception in `start_agent.rs`; Oz local children now resolve their parent conversation's `run_id()` and thread it through `StartAgentRequest`, mirroring the harness and remote branches. * `launch_local_no_harness_child` is async now: it calls `AIClient::create_agent_task` at dispatch (mirroring `launch_local_harness_child`), then stamps the resulting `task_id` onto both the child `AIConversation` (via `conversation.set_task_id`) and the child controller (via `HiddenChildAgentTaskContext`). On failure the child surfaces through `create_error_child_agent_conversation` instead. * `IsSharedSessionCreator` is plumbed through `HiddenChildAgentConversationRequest` / `create_hidden_child_agent_conversation` / `insert_terminal_pane_hidden_for_child_agent` / `create_terminal_pane_data` so child panes inherit the host terminal's shared-session state when `FeatureFlag::OrchestrationViewerPillBar` is enabled. Both `launch_local_no_harness_child` and `launch_local_harness_child` compute the inherit-share value via the new `inherit_share_for_local_child` helper. * New accessor: `local_tty::TerminalManager::shared_session_source_type` returns the host's active or pending share source type. Reused by the dispatch helpers via a thin model-level helper that doesn't require trait downcasting. * No new subscriber: the existing per-`Network` share-reporter at `local_tty/terminal_manager.rs:1531-1563` now fires for child panes automatically once the three documented preconditions hold. * New tests in `app/src/pane_group/pane/terminal_pane_local_child_tests.rs` cover `create_agent_task` dispatch, task-id stamping, error path, and inherit-share plumbing. Co-Authored-By: Oz <oz-agent@warp.dev>
The new test module exercised the `launch_local_no_harness_child` / `launch_local_harness_child` dispatch path end-to-end, but it depends on infrastructure that is not wired up in the unit-test harness (several missing singleton models plus `AIClient` methods other than `create_agent_task` invoked from `controller.send_agent_query_in_conversation`, `HarnessAvailabilityModel`, ambient task reporters, etc.). The result was that the spawn callback panicked partway through `new_terminal_view.update`, permanently stripping the view from `window.views` and making any follow-up `as_ref(ctx)` (or even `try_as_ref(ctx)`) read return `None`. Per orchestrator guidance, removing the new test module and its supporting test-only hooks rather than wrestling with the infra gap. Coverage for B.2 / B.3 / B.4 falls back to the manual validation matrix in `specs/remote-local-orch-pill-ui/TECH.md`. This commit removes: - `app/src/pane_group/pane/terminal_pane_local_child_tests.rs` - The `#[cfg(test)] #[path = ..] mod tests;` wiring in `terminal_pane.rs` - `ServerApiProvider::set_ai_client_override_for_test` + the `AI_CLIENT_TEST_OVERRIDE` thread-local + the `#[cfg(test)]` branch in `get_ai_client` (added solely to support the deleted tests) - `pub(super)` visibility relaxations on `initialize_app`, `MockOptions` (and its fields), `mock_pane_group`, `get_newly_created_pane_id`, `new_ambient_agent_task_id`, `start_parent_conversation`, and `start_parent_conversation_for_terminal_view` in `mod_tests.rs`. Production code changes are unchanged. All 68 pre-existing pane_group unit tests still pass. 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 threads parent run IDs into local Oz child dispatch, eagerly creates server task rows for local Oz children, and plumbs shared-session creator state through hidden child pane creation so local children can surface in the orchestration viewer pill bar.
Concerns
- The local Oz launch path now references
ServerApiProviderfrom code that is still compiled for wasm, while the import is#[cfg(not(target_family = "wasm"))], so wasm builds will fail. - The share-inheritance helper treats any shared-session source as eligible, which can silently auto-share hidden local child panes from non-ambient shared sessions.
Security
- Hidden child terminals should only inherit sharing from ambient-agent orchestrator sessions; otherwise this broadens session sharing beyond the user's explicit shared terminal.
Verdict
Found: 1 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
Two review comments from oz-for-oss[bot]:
1. Wasm gating (#discussion_r3251313728): the new async `launch_local_no_harness_child` references `ServerApiProvider`, which is `cfg(not(target_family = "wasm"))`-only, but the function itself was compiled on every target. Gate `launch_local_no_harness_child` at the function level (matching the existing `launch_local_harness_child` pattern), gate the `harness_type: None` arm in `dispatch_start_agent_conversation`, and let the existing `#[cfg(target_family = "wasm")] Local { .. }` wildcard route the Oz path through `create_error_child_agent_conversation` on wasm. Also gates the now-wasm-dead helpers and imports (`register_legacy_local_lifecycle_subscription`, `host_terminal_shared_session_source_type`, `inherit_share_for_local_child`, `apply_child_model_id_override`, plus `HashMap`, `LifecycleEventType`, the child-agent struct cluster, `IsSharedSessionCreator`, `SessionSourceType`) so `./script/wasm/bundle --check-only` is now warning-clean.
2. Restrict inherit-share to AmbientAgent host (#discussion_r3251313731): `inherit_share_for_local_child` previously auto-shared any host source-type as `IsSharedSessionCreator::Yes`, including a user manually sharing their terminal. Tighten the predicate to match only on `SessionSourceType::AmbientAgent { .. }` — the only host source-type for which the viewer pill bar materializes at `shared_session/viewer/terminal_manager.rs:803`, so cascading manual shares would publish child transcripts with no UI affordance for the host to reach them. Function doc updated to reflect the narrower scope; spec `specs/remote-local-orch-pill-ui/TECH.md` (working-tree artifact, untracked) updated alongside.
Validation:
- `cargo nextest run -p warp pane_group::` — 68 passed.
- `cargo fmt --check` — clean.
- `cargo clippy --workspace --exclude warp_completer --exclude command-signatures-v2 --all-targets --tests -- -D warnings` — clean.
- `./script/wasm/bundle --check-only` — warning-free, builds.
Co-Authored-By: Oz <oz-agent@warp.dev>
- Delete unused TerminalManager::shared_session_source_type accessor. The dispatch helpers in terminal_pane.rs use a near-duplicate inline helper host_terminal_shared_session_source_type that reads the TerminalModel directly, bypassing the dyn TerminalManager downcast that would otherwise be required. Update the helper's doc comment to drop the stale 'mirrors the logic of' reference. - Bail out from the Oz Local arm of StartAgentExecutor when the parent conversation has no run_id yet. Previously the arm fell through with None, which is harmless for cloud orchestrators (they always have a run_id) but breaks parent linkage server-side for local-on-desktop orchestrators whose conversation hasn't yet acquired one — the eager CreateAgentTask path has no late-binding fallback, unlike the pre-change lazy flow which would have linked later via Request.metadata.parent_agent_id. Matches the bail-out symmetry of the harness and remote arms. Co-Authored-By: Oz <oz-agent@warp.dev>
The new bail-out added in 7f9f106 returns an immediate Sync error when an Oz local child is dispatched from a parent conversation that has not yet acquired a run_id. The existing test fixtures in start_agent_tests.rs created the parent via start_new_conversation() which leaves run_id unset, so the 6 tests that exercise the Oz local path (StartAgentExecutionMode::local_with_defaults()) started failing with 'expected async execution' panics on CI. Add a PARENT_RUN_ID constant and call assign_run_id_for_conversation on the parent right after start_new_conversation in each affected test so the Oz Local arm reaches its Async dispatch path. Co-Authored-By: Oz <oz-agent@warp.dev>
cephalonaut
changed the title
advait-m
approved these changes
May 17, 2026
- Remove three `file.rs:NNN` line references from doc/inline comments in terminal_pane.rs (in `inherit_share_for_local_child` and `launch_local_no_harness_child`). Line numbers go stale quickly; the file refs without line numbers still point readers at the right place. - Combine the two consecutive `BlocklistAIHistoryModel::handle(ctx).update(...)` calls in `launch_local_no_harness_child`'s success branch into a single update closure. The two calls had no semantic separation — both mutated the child conversation in immediate succession. Co-Authored-By: Oz <oz-agent@warp.dev>
lawsmd
pushed a commit
to lawsmd/cortex
that referenced
this pull request
May 22, 2026
…ocal children (warpdotdev#11042) ## Description ### What - Drop the Oz-only `parent_run_id` exception at `start_agent.rs:304-323`. Oz local children now resolve `parent_run_id` from the parent conversation's `run_id()` and thread it through `StartAgentRequest`, mirroring the third-party-harness and remote-child branches. - `launch_local_no_harness_child` is async now: it calls `AIClient::create_agent_task` eagerly at dispatch (mirroring `launch_local_harness_child`), stamps the resulting `task_id` onto both the child `AIConversation` (via `set_task_id`) and the child's `BlocklistAIController` (via `HiddenChildAgentTaskContext`). On failure the child surfaces as an error conversation. - `IsSharedSessionCreator` is plumbed through `HiddenChildAgentConversationRequest` → `create_hidden_child_agent_conversation` → `insert_terminal_pane_hidden_for_child_agent` → `create_terminal_pane_data` so child panes inherit the host terminal's shared-session state when `FeatureFlag::OrchestrationViewerPillBar` is enabled. Both `launch_local_no_harness_child` and `launch_local_harness_child` compute the value via the new `inherit_share_for_local_child` helper. - New accessor `local_tty::TerminalManager::shared_session_source_type()` exposes the host's active or pending share source type for child dispatch. - Existing per-`Network` share-reporter at `local_tty/terminal_manager.rs:1531-1563` now fires for child panes automatically once the three documented preconditions hold; no new subscriber is introduced. ### Why Local children of cloud orchestrators were not appearing in the shared-session viewer pill bar because their per-child terminals weren't asked to share, and their server task rows weren't created at dispatch time (only lazily on first request). This change unifies Oz-local with harness-local paths so both flow through the same share-handshake plumbing. ## Testing - The dispatch path is exercised end-to-end by the manual validation matrix in `specs/remote-local-orch-pill-ui/TECH.md` (cloud orchestrator + Oz local child, viewer pill-bar inherit-share, error fallback, harness-child inherit-share). A unit-test module was attempted but removed because the spawn-callback path depends on infrastructure (several singleton models and `AIClient` methods beyond `create_agent_task`) that isn't wired into the warp-crate unit-test harness; closing that gap is out of scope for this PR. - `cargo fmt`, `cargo clippy --workspace --exclude warp_completer --exclude command-signatures-v2 --all-targets --tests -- -D warnings`, and `cargo nextest run -p warp pane_group::` (68 tests, all passing) verify nothing pre-existing regressed. ## Agent Mode - [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode <!-- CHANGELOG-IMPROVEMENT: Local children of cloud orchestrators now appear in the shared-session viewer pill bar. --> --------- 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
3 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
What
parent_run_idexception atstart_agent.rs:304-323. Oz local children now resolveparent_run_idfrom the parent conversation'srun_id()and thread it throughStartAgentRequest, mirroring the third-party-harness and remote-child branches.launch_local_no_harness_childis async now: it callsAIClient::create_agent_taskeagerly at dispatch (mirroringlaunch_local_harness_child), stamps the resultingtask_idonto both the childAIConversation(viaset_task_id) and the child'sBlocklistAIController(viaHiddenChildAgentTaskContext). On failure the child surfaces as an error conversation.IsSharedSessionCreatoris plumbed throughHiddenChildAgentConversationRequest→create_hidden_child_agent_conversation→insert_terminal_pane_hidden_for_child_agent→create_terminal_pane_dataso child panes inherit the host terminal's shared-session state whenFeatureFlag::OrchestrationViewerPillBaris enabled. Bothlaunch_local_no_harness_childandlaunch_local_harness_childcompute the value via the newinherit_share_for_local_childhelper.local_tty::TerminalManager::shared_session_source_type()exposes the host's active or pending share source type for child dispatch.Networkshare-reporter atlocal_tty/terminal_manager.rs:1531-1563now fires for child panes automatically once the three documented preconditions hold; no new subscriber is introduced.Why
Local children of cloud orchestrators were not appearing in the shared-session viewer pill bar because their per-child terminals weren't asked to share, and their server task rows weren't created at dispatch time (only lazily on first request). This change unifies Oz-local with harness-local paths so both flow through the same share-handshake plumbing.
Testing
specs/remote-local-orch-pill-ui/TECH.md(cloud orchestrator + Oz local child, viewer pill-bar inherit-share, error fallback, harness-child inherit-share). A unit-test module was attempted but removed because the spawn-callback path depends on infrastructure (several singleton models andAIClientmethods beyondcreate_agent_task) that isn't wired into the warp-crate unit-test harness; closing that gap is out of scope for this PR.cargo fmt,cargo clippy --workspace --exclude warp_completer --exclude command-signatures-v2 --all-targets --tests -- -D warnings, andcargo nextest run -p warp pane_group::(68 tests, all passing) verify nothing pre-existing regressed.Agent Mode